diff mbox series

[v2,1/3] drm: rockchip: hdmi: fix clock rounding code

Message ID 20200922203107.2932-2-vicencb@gmail.com
State New
Headers show
Series drm: rockchip: hdmi: enable higher resolutions than FHD | expand

Commit Message

Vicente Bergas Sept. 22, 2020, 8:31 p.m. UTC
Under certain conditions vop_crtc_mode_fixup rounds the clock
148500000 to 148501000 which leads to the following error:
dwhdmi-rockchip ff940000.hdmi: PHY configuration failed (clock 148501000)

The issue was found on RK3399 booting with u-boot. U-boot configures the
display at 2560x1440 and then linux comes up with a black screen.
A workaround was to un-plug and re-plug the HDMI display.

Signed-off-by: Vicente Bergas <vicencb@gmail.com>
Tested-by: Vicente Bergas <vicencb@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

crj Sept. 23, 2020, 1:37 a.m. UTC | #1
在 2020/9/23 4:31, Vicente Bergas 写道:
> Under certain conditions vop_crtc_mode_fixup rounds the clock
> 148500000 to 148501000 which leads to the following error:
> dwhdmi-rockchip ff940000.hdmi: PHY configuration failed (clock 148501000)
>
> The issue was found on RK3399 booting with u-boot. U-boot configures the
> display at 2560x1440 and then linux comes up with a black screen.
> A workaround was to un-plug and re-plug the HDMI display.
>
> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> Tested-by: Vicente Bergas <vicencb@gmail.com>
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index c80f7d9fd13f..92efbd899dee 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1176,12 +1176,9 @@ static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>   	 *
>   	 * 2. Get the clock framework to round the rate for us to tell us
>   	 *    what it will actually make.
> -	 *
> -	 * 3. Store the rounded up rate so that we don't need to worry about
> -	 *    this in the actual clk_set_rate().
>   	 */
>   	rate = clk_round_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);
> -	adjusted_mode->clock = DIV_ROUND_UP(rate, 1000);
> +	adjusted_mode->clock = rate / 1000;
>   
>   	return true;
>   }
> @@ -1380,7 +1377,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
>   
>   	VOP_REG_SET(vop, intr, line_flag_num[0], vact_end);
>   
> -	clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
> +	clk_set_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);


In some RK platform, such as RK3328, dclk is generated by the INNO HDMI 
PHY PLL, which support

frac divider can support any dclk frequency. And The PLL must use the 
exact clock to determine

the PLL configuration. So adjusted_mode->clock * 1000 + 999 may cause 
INNO HDMI PHY PLL

couldn't find the right configuration. INNO HDMI PHY's driver path is:

drivers/phy/rockchip/phy-rockchip-inno-hdmi.c


>   
>   	VOP_REG_SET(vop, common, standby, 0);
>   	mutex_unlock(&vop->vop_lock);
Douglas Anderson Sept. 23, 2020, 10:55 p.m. UTC | #2
Hi,

On Tue, Sep 22, 2020 at 1:31 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> Under certain conditions vop_crtc_mode_fixup rounds the clock
> 148500000 to 148501000 which leads to the following error:
> dwhdmi-rockchip ff940000.hdmi: PHY configuration failed (clock 148501000)
>
> The issue was found on RK3399 booting with u-boot. U-boot configures the
> display at 2560x1440 and then linux comes up with a black screen.
> A workaround was to un-plug and re-plug the HDMI display.
>
> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> Tested-by: Vicente Bergas <vicencb@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

My rk3288-veyron-minnie is stuck at work (but not plugged in) and it's
Covid times, so I can't easily test this.  ...but it looks fine to me
and makes things more symmetric / clean.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

-Doug
Douglas Anderson Sept. 23, 2020, 10:58 p.m. UTC | #3
Hi,

On Tue, Sep 22, 2020 at 6:38 PM crj <algea.cao@rock-chips.com> wrote:
>
>
> 在 2020/9/23 4:31, Vicente Bergas 写道:
> > Under certain conditions vop_crtc_mode_fixup rounds the clock
> > 148500000 to 148501000 which leads to the following error:
> > dwhdmi-rockchip ff940000.hdmi: PHY configuration failed (clock 148501000)
> >
> > The issue was found on RK3399 booting with u-boot. U-boot configures the
> > display at 2560x1440 and then linux comes up with a black screen.
> > A workaround was to un-plug and re-plug the HDMI display.
> >
> > Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> > Tested-by: Vicente Bergas <vicencb@gmail.com>
> > ---
> >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index c80f7d9fd13f..92efbd899dee 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -1176,12 +1176,9 @@ static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
> >        *
> >        * 2. Get the clock framework to round the rate for us to tell us
> >        *    what it will actually make.
> > -      *
> > -      * 3. Store the rounded up rate so that we don't need to worry about
> > -      *    this in the actual clk_set_rate().
> >        */
> >       rate = clk_round_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);
> > -     adjusted_mode->clock = DIV_ROUND_UP(rate, 1000);
> > +     adjusted_mode->clock = rate / 1000;
> >
> >       return true;
> >   }
> > @@ -1380,7 +1377,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
> >
> >       VOP_REG_SET(vop, intr, line_flag_num[0], vact_end);
> >
> > -     clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
> > +     clk_set_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);
>
>
> In some RK platform, such as RK3328, dclk is generated by the INNO HDMI
> PHY PLL, which support
>
> frac divider can support any dclk frequency. And The PLL must use the
> exact clock to determine
>
> the PLL configuration. So adjusted_mode->clock * 1000 + 999 may cause
> INNO HDMI PHY PLL
>
> couldn't find the right configuration. INNO HDMI PHY's driver path is:
>
> drivers/phy/rockchip/phy-rockchip-inno-hdmi.c

I don't think you'll have a problem.  Assuming I'm looking at the
correct code, in the function inno_hdmi_phy_rk3228_clk_round_rate()
you've got "rate = (rate / 1000) * 1000".  I believe the clock
framework automatically calls round rate when someone tries to set the
rate, so this should clean the clock back up.

If I'm misunderstanding, please yell.

-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c80f7d9fd13f..92efbd899dee 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1176,12 +1176,9 @@  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
 	 *
 	 * 2. Get the clock framework to round the rate for us to tell us
 	 *    what it will actually make.
-	 *
-	 * 3. Store the rounded up rate so that we don't need to worry about
-	 *    this in the actual clk_set_rate().
 	 */
 	rate = clk_round_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);
-	adjusted_mode->clock = DIV_ROUND_UP(rate, 1000);
+	adjusted_mode->clock = rate / 1000;
 
 	return true;
 }
@@ -1380,7 +1377,7 @@  static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	VOP_REG_SET(vop, intr, line_flag_num[0], vact_end);
 
-	clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
+	clk_set_rate(vop->dclk, adjusted_mode->clock * 1000 + 999);
 
 	VOP_REG_SET(vop, common, standby, 0);
 	mutex_unlock(&vop->vop_lock);