[PATCH v2 0/3] drm: rockchip: hdmi: enable higher resolutions than FHD

[PATCH v2 0/3] drm: rockchip: hdmi: enable higher resolutions than FHD

[Vicente Bergas]

This patch series enable a QHD HDMI monitor to work at native resolution.
Tested on a Sapphire board with RK3399 connected to a Q27q-10 monitor at 2560x1440@60

Changes since v1:
Use alternative clock rounding code proposed by Doug Anderson

Vicente Bergas (3):
  drm: rockchip: hdmi: fix clock rounding code
  drm: rockchip: hdmi: allow any clock that is within the range
  drm: rockchip: hdmi: add higher pixel clock frequencies

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8 +++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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);
 
 	VOP_REG_SET(vop, common, standby, 0);
 	mutex_unlock(&vop->vop_lock);
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[crj]

在 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);


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Doug Anderson]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Doug Anderson]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm: rockchip: hdmi: allow any clock that is within the range

[Vicente Bergas]

For a video mode to work it suffices that the available bandwidth is
large enough. There is no need to have an exact match.

This greatly expands the list of supported monitors.

Signed-off-by: Vicente Bergas <vicencb@gmail.com>
Tested-by: Vicente Bergas <vicencb@gmail.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 23de359a1dec..87a9198f7494 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -230,7 +230,7 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *hdmi, void *data,
 	int i;
 
 	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
-		if (pclk == mpll_cfg[i].mpixelclock) {
+		if (pclk <= mpll_cfg[i].mpixelclock) {
 			valid = true;
 			break;
 		}
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm: rockchip: hdmi: add higher pixel clock frequencies

[Vicente Bergas]

In order to support video resolutions beyond FHD more bandwidth is needed.
The new entry values have been taken from u-boot:
https://gitlab.denx.de/u-boot/u-boot/-/blob/ba2a0cbb053951ed6d36161989d38da724696b4d/drivers/video/rockchip/rk_hdmi.c#L63

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 87a9198f7494..db4a946f92aa 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -148,6 +148,10 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 			{ 0x214c, 0x0003},
 			{ 0x4064, 0x0003}
 		},
+	}, {
+		272000000, {
+			{ 0x0040, 0x0003},
+		},
 	}, {
 		~0UL, {
 			{ 0x00a0, 0x000a },
@@ -173,6 +177,8 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 		146250000, { 0x0038, 0x0038, 0x0038 },
 	}, {
 		148500000, { 0x0000, 0x0038, 0x0038 },
+	}, {
+		272000000, { 0x0000,                },
 	}, {
 		~0UL,      { 0x0000, 0x0000, 0x0000},
 	}
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/3] drm: rockchip: hdmi: enable higher resolutions than FHD

[Robin Murphy]

On 2020-09-22 21:31, Vicente Bergas wrote:
> This patch series enable a QHD HDMI monitor to work at native resolution.
> Tested on a Sapphire board with RK3399 connected to a Q27q-10 monitor at 2560x1440@60

Indeed for RK3399 it also allows my 1920x1200 monitor (Dell U2415) to be 
driven at its native resolution with a 154MHz pixel clock. However, as 
predicted, it also breaks RK3328 for the same monitor - instead of 
rejecting the native mode and falling back to "standard" 1920x1080, it 
now tries to use it, which results in no signal and a spam of CRTC 
timeout warnings in dmesg :(

I'll try to test RK3288 as well soon - I tried hacking a specific entry 
for 154MHz into the tables a while ago, and while it worked perfectly on 
RK3399, RK3288 gave a fairly glitchy picture as if the clock signal was 
unstable or slightly out of spec. I'm interested to see if patch #1 
makes any difference there.

Thanks,
Robin.

> 
> Changes since v1:
> Use alternative clock rounding code proposed by Doug Anderson
> 
> Vicente Bergas (3):
>    drm: rockchip: hdmi: fix clock rounding code
>    drm: rockchip: hdmi: allow any clock that is within the range
>    drm: rockchip: hdmi: add higher pixel clock frequencies
> 
>   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8 +++++++-
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
>   2 files changed, 9 insertions(+), 6 deletions(-)
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/3] drm: rockchip: hdmi: enable higher resolutions than FHD

[Doug Anderson]

Hi,

On Tue, Sep 22, 2020 at 1:31 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> This patch series enable a QHD HDMI monitor to work at native resolution.
> Tested on a Sapphire board with RK3399 connected to a Q27q-10 monitor at 2560x1440@60
>
> Changes since v1:
> Use alternative clock rounding code proposed by Doug Anderson
>
> Vicente Bergas (3):
>   drm: rockchip: hdmi: fix clock rounding code
>   drm: rockchip: hdmi: allow any clock that is within the range
>   drm: rockchip: hdmi: add higher pixel clock frequencies
>
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8 +++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ++-----
>  2 files changed, 9 insertions(+), 6 deletions(-)

So I haven't looked at all this code in over a year and that was
mostly just a quick re-hash of the work I did years and years ago...
...but:

1. On rk3288, which also uses dw-hdmi, things get super complicated
because of the PLL sharing on the Rockchip platform.  I don't _think_
this has all been worked out, has it?  Specifically there is only one
"free" PLL but two VOPs, so if you have two displays (like eDP and
HDMI), one of those two has to make due with a very limited set of
pixel clocks that it can make.  On downstream Chrome OS tree we made
the simplifying assumption that the internal panel was going to be the
one to suffer (so we had to futz around w/ non-standard pixels clocks
for all our panels) and added some not-so-generic code to then give
the HDMI VOP full control of one of the PLLs.  Without these tricks
then you really shouldn't be advertising a lot of clock rates because
the PLL won't be guaranteed to make them.  I've never had time to find
a wonderful solution myself and I've tried (at least 3 times) to
convince others to solve this but failed.  :(

2. In order to get all the clock rates working reliably across a large
number of monitors on rk3288, I spent a lot of time tweaking all these
parameters, and it mattered.  You can see what's in the downstream
tree at:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c

Those numbers came from me staring at pages and pages or hardcoded
numbers for different display modes until my eyes bled and then coming
up with a script:

https://chromium.googlesource.com/chromiumos/platform/drm-tests/+/f806cb7d7f14dd17087ae8ebed67bd3589708812/rk3288_hdmitables.py

You are certainly free to totally ignore all that, but I figured I'd
at last point you at it in case it helped make your rates more
reliable.  I spent many hours in a lab tweaking settings of dw_hdmi
and various PLL settings for rk3288 to make sure we could actually
produce a low enough jitter signal that plugging into a random monitor
would actually work.

NOTE: I only enabled this on rk3288.  All rk3399 Chromebooks only
supported DP out (through Type C), so I have zero idea about HDMI
support on rk3399 or any other Rockchip SoCs and whether any of my
work would actually be relevant there...


Though I would have liked to get everything resolved with upstream
last year, I just wasn't able to find the time to do it.  I at least
(hopefully) documented this in:

https://crbug.com/970892

In any case, I'll probably step aside now and let upstream folks
decide how they want to move forward.  I don't think I have enough
skin ihe game these days to really make stink about whatever you guys
decide to do.  ;-)


-Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel