| Message ID | 20190326103146.24795-7-tomi.valkeinen@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
On 26.03.2019 11:31, Tomi Valkeinen wrote: > Modify aux_link_setup so that it does not use tc->link, and thus makes > aux setup independent of the link probing. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/bridge/tc358767.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 7ef8d754b4ff..f5c232a9064e 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -542,7 +542,6 @@ static int tc_aux_link_setup(struct tc_data *tc) > unsigned long rate; > u32 value; > int ret; > - u32 dp_phy_ctrl; > > rate = clk_get_rate(tc->refclk); > switch (rate) { > @@ -567,10 +566,7 @@ static int tc_aux_link_setup(struct tc_data *tc) > value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2; > tc_write(SYS_PLLPARAM, value); > > - dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN; > - if (tc->link.base.num_lanes == 2) > - dp_phy_ctrl |= PHY_2LANE; > - tc_write(DP_PHY_CTRL, dp_phy_ctrl); > + tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_A0_EN); Description does not explain why removal of PHY_2LANE is OK. I guess because it is set anyway before training but that needs lurking into the code. With description fixed: Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej > > /* > * Initially PLLs are in bypass. Force PLL parameter update, > @@ -587,8 +583,9 @@ static int tc_aux_link_setup(struct tc_data *tc) > if (ret == -ETIMEDOUT) { > dev_err(tc->dev, "Timeout waiting for PHY to become ready"); > return ret; > - } else if (ret) > + } else if (ret) { > goto err; > + } > > /* Setup AUX link */ > tc_write(DP0_AUXCFG1, AUX_RX_FILTER_EN |
On 15/04/2019 10:38, Andrzej Hajda wrote: > On 26.03.2019 11:31, Tomi Valkeinen wrote: >> Modify aux_link_setup so that it does not use tc->link, and thus makes >> aux setup independent of the link probing. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/bridge/tc358767.c | 9 +++------ >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c >> index 7ef8d754b4ff..f5c232a9064e 100644 >> --- a/drivers/gpu/drm/bridge/tc358767.c >> +++ b/drivers/gpu/drm/bridge/tc358767.c >> @@ -542,7 +542,6 @@ static int tc_aux_link_setup(struct tc_data *tc) >> unsigned long rate; >> u32 value; >> int ret; >> - u32 dp_phy_ctrl; >> >> rate = clk_get_rate(tc->refclk); >> switch (rate) { >> @@ -567,10 +566,7 @@ static int tc_aux_link_setup(struct tc_data *tc) >> value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2; >> tc_write(SYS_PLLPARAM, value); >> >> - dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN; >> - if (tc->link.base.num_lanes == 2) >> - dp_phy_ctrl |= PHY_2LANE; >> - tc_write(DP_PHY_CTRL, dp_phy_ctrl); >> + tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_A0_EN); > > > Description does not explain why removal of PHY_2LANE is OK. > > I guess because it is set anyway before training but that needs lurking > into the code. Good point, the desc is a bit unclear. The reason is that we're setting up only AUX here, so anything related to the data-lanes is don't-care. Tomi
Hi Tomi, Thank you for the patch. On Mon, Apr 15, 2019 at 10:52:38AM +0300, Tomi Valkeinen wrote: > On 15/04/2019 10:38, Andrzej Hajda wrote: > > On 26.03.2019 11:31, Tomi Valkeinen wrote: > >> Modify aux_link_setup so that it does not use tc->link, and thus makes > >> aux setup independent of the link probing. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > >> --- > >> drivers/gpu/drm/bridge/tc358767.c | 9 +++------ > >> 1 file changed, 3 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > >> index 7ef8d754b4ff..f5c232a9064e 100644 > >> --- a/drivers/gpu/drm/bridge/tc358767.c > >> +++ b/drivers/gpu/drm/bridge/tc358767.c > >> @@ -542,7 +542,6 @@ static int tc_aux_link_setup(struct tc_data *tc) > >> unsigned long rate; > >> u32 value; > >> int ret; > >> - u32 dp_phy_ctrl; > >> > >> rate = clk_get_rate(tc->refclk); > >> switch (rate) { > >> @@ -567,10 +566,7 @@ static int tc_aux_link_setup(struct tc_data *tc) > >> value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2; > >> tc_write(SYS_PLLPARAM, value); > >> > >> - dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN; > >> - if (tc->link.base.num_lanes == 2) > >> - dp_phy_ctrl |= PHY_2LANE; > >> - tc_write(DP_PHY_CTRL, dp_phy_ctrl); > >> + tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_A0_EN); > > > > > > Description does not explain why removal of PHY_2LANE is OK. > > > > I guess because it is set anyway before training but that needs lurking > > into the code. > > Good point, the desc is a bit unclear. The reason is that we're setting > up only AUX here, so anything related to the data-lanes is don't-care. And also because the tc_aux_link_setup() function is called at probe time, while the tc_main_link_setup() function is called later and will overwrite the DP_PHY_CTRL register with bits related to the main link. With the description fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 7ef8d754b4ff..f5c232a9064e 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -542,7 +542,6 @@ static int tc_aux_link_setup(struct tc_data *tc) unsigned long rate; u32 value; int ret; - u32 dp_phy_ctrl; rate = clk_get_rate(tc->refclk); switch (rate) { @@ -567,10 +566,7 @@ static int tc_aux_link_setup(struct tc_data *tc) value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2; tc_write(SYS_PLLPARAM, value); - dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN; - if (tc->link.base.num_lanes == 2) - dp_phy_ctrl |= PHY_2LANE; - tc_write(DP_PHY_CTRL, dp_phy_ctrl); + tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_A0_EN); /* * Initially PLLs are in bypass. Force PLL parameter update, @@ -587,8 +583,9 @@ static int tc_aux_link_setup(struct tc_data *tc) if (ret == -ETIMEDOUT) { dev_err(tc->dev, "Timeout waiting for PHY to become ready"); return ret; - } else if (ret) + } else if (ret) { goto err; + } /* Setup AUX link */ tc_write(DP0_AUXCFG1, AUX_RX_FILTER_EN |
Modify aux_link_setup so that it does not use tc->link, and thus makes aux setup independent of the link probing. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/bridge/tc358767.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)