Re: [PATCH] ARM: dts: bcm283x: Use firmware PM driver for V3D

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Eric Anholt <eric@anholt.net>,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	Maxime Ripard <maxime@cerno.tech>
Subject: Re: [PATCH] ARM: dts: bcm283x: Use firmware PM driver for V3D
Date: Mon, 16 Mar 2020 13:40:17 +0100	[thread overview]
Message-ID: <a25b759292901eee3eab4dbf8002d2050edda6d3.camel@suse.de> (raw)
In-Reply-To: <90e92438-00df-520f-c8b1-2e2077934592@i2se.com>

[-- Attachment #1.1: Type: text/plain, Size: 2997 bytes --]

Hi Stefan,
thanks for taking the time with this. That was a hard to find one, specially
given the race in X11.

On Sun, 2020-03-15 at 20:16 +0100, Stefan Wahren wrote:
> Hi Nicolas,
> 
> [adjust audience]
>
> i've narrowed down the issue. From kernel 4.19 until 5.1 the DRM
> emulated driver was responsible for a working X on my Raspberry Pi 3
> with HP ZR2440w. Starting with 5.2 the vc4drmfb took over and with 5.3 X
> didn't start anymore (display freeze).
> 
> So i start bisecting and this was the commit where the freezing started:
> 
> e08ab74bd4 drm/modes: Rewrite the command line parser
> 
> After this i enabled drm debug and saw that suggest mode 1920x1200 by
> the firmware is rejected by the driver because the pixel clock would be
> too high (154 MHz, max = 148.5). This wasn't a problem before since the
> firmware provided video cmdline parameter wasn't parseable:
> 
> [drm] parse error at position 69 in video mode
> '1920x1200M@60,margin_left=0,margin_right=0,margin_top=0,margin_bottom=0'
> 
> After mentioned commit the display just freezes (no try to use
> 1920x1080, no error message).
> 
> For comparison i switched to the vendor tree with firmware kms driver
> and noticed that the driver switches to 1920x1200 with a pixel at 154 MHz.
> 
> So this patch works for me:
> 
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index cea18dc..647803e 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -681,11 +681,12 @@ static enum drm_mode_status
>  vc4_hdmi_encoder_mode_valid(struct drm_encoder *crtc,
>                  const struct drm_display_mode *mode)
>  {
> -    /* HSM clock must be 108% of the pixel clock.

I think it'd be nice to understand how Eric came by this number. Maybe just
empirically with 1080p60? That said, I think your change is pretty much
harmless.

I'll add a reminder to Maxime's series for him to update RPi[0-3]'s max
frequency to 1920x1200@60's.

> -     * the AXI clock needs to be at least 25% of pixel clock, but
> -     * HSM ends up being the limiting factor.
> +    /* According to spec the HSM clock must be 108% of the pixel clock.
> +     * Additionally, the AXI clock needs to be at least 25% of pixel clock,
> +     * but HSM ends up being the limiting factor.
> +     * It seems that operating the pixel clock at 154 MHz still works.
>       */
> -    if (mode->clock > HSM_CLOCK_FREQ / (1000 * 108 / 100))
> +    if (mode->clock > HSM_CLOCK_FREQ / (1000 * 106 / 100))

Isn't hard-coding the HSM clock kind of limited, one could overclock it, isn't
it? I remember reading someone did it to support wider resolutions.

Regards,
Nicolas

[1] https://www.raspberrypi.org/forums/viewtopic.php?f=29&t=24679#p333810

>          return MODE_CLOCK_HIGH;
>
>      return MODE_OK;

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel