Re: [PATCH 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

Re: [PATCH 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

[Michael Riesch]

Hi Sascha,

Can you Cc: linux-rockchip list to get more feedback?

On 8/22/22 17:20, Sascha Hauer wrote:
> This series adds support for 4k@30 to the rockchip HDMI controller. This
> has been tested on a rk3568 rock3a board. It should be possible to add
> 4k@60 support the same way, but it doesn't work for me, so let's add
> 4k@30 as a first step.
> 
> Sascha
> 
> Sascha Hauer (2):
>   drm/rockchip: dw_hdmi: relax mode_valid hook
>   drm/rockchip: dw_hdmi: Add support for 4k@30 resolution

Great stuff, thanks!

On a Radxa ROCK3 Model A with a HP 27f 4k monitor

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Best regards,
Michael

> 
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

[Sascha Hauer]

On Wed, Aug 24, 2022 at 07:43:03AM +0200, Michael Riesch wrote:
> Hi Sascha,
> 
> Can you Cc: linux-rockchip list to get more feedback?

Yes, will do that next time.

> 
> On 8/22/22 17:20, Sascha Hauer wrote:
> > This series adds support for 4k@30 to the rockchip HDMI controller. This
> > has been tested on a rk3568 rock3a board. It should be possible to add
> > 4k@60 support the same way, but it doesn't work for me, so let's add
> > 4k@30 as a first step.
> > 
> > Sascha
> > 
> > Sascha Hauer (2):
> >   drm/rockchip: dw_hdmi: relax mode_valid hook
> >   drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
> 
> Great stuff, thanks!
> 
> On a Radxa ROCK3 Model A with a HP 27f 4k monitor
> 
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

[Dan Johansen]

Hi Sasha.

Great to see work being done to enable HiDPI support in rockchip-drm.

Would it be possible to add 2k/1440p too or is it included in this patch?

Den 24.08.2022 kl. 09.10 skrev Sascha Hauer:
> On Wed, Aug 24, 2022 at 07:43:03AM +0200, Michael Riesch wrote:
>> Hi Sascha,
>>
>> Can you Cc: linux-rockchip list to get more feedback?
> Yes, will do that next time.
>
>> On 8/22/22 17:20, Sascha Hauer wrote:
>>> This series adds support for 4k@30 to the rockchip HDMI controller. This
>>> has been tested on a rk3568 rock3a board. It should be possible to add
>>> 4k@60 support the same way, but it doesn't work for me, so let's add
>>> 4k@30 as a first step.
>>>
>>> Sascha
>>>
>>> Sascha Hauer (2):
>>>    drm/rockchip: dw_hdmi: relax mode_valid hook
>>>    drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
>> Great stuff, thanks!
>>
>> On a Radxa ROCK3 Model A with a HP 27f 4k monitor
>>
>> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> Thanks
>
> Sascha
>
-- 
Kind regards
*Dan Johansen*
Project lead of the *Manjaro ARM* project
Manjaro-ARM <https://manjaro.org>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/2] drm/rockchip: dw_hdmi: relax mode_valid hook

[Robin Murphy]

On 2022-08-22 16:20, Sascha Hauer wrote:
> The driver checks if the pixel clock of the given mode matches an entry
> in the mpll config table. The frequencies in the mpll table are meant as
> a frequency range up to which the entry works, not as a frequency that
> must match the pixel clock. Return MODE_OK when the pixelclock is
> smaller than one of the mpll frequencies to allow for more display
> resolutions.

Has the issue been fixed that this table is also used to validate modes 
on RK3328, which doesn't even *have* the Synopsys phy? Last time I 
looked, that tended to lead to complete display breakage when the proper 
phy driver later decides it doesn't like a pixel clock that mode_valid 
already said was OK.

The more general concern is that these known-good clock rates are good, 
but others may not be even when nominally supported, which I suspect is 
the dirty secret of why it was implemented this way to begin with. I 
would really really love this patch so my RK3399 board can drive my 
1920x1200 monitor at native resolution, but on the other hand my RK3288 
box generates such a crap 154MHz clock for that mode that - unless 
that's been improved in the meantime too - patch #2 might be almost be 
considered a regression if it means such a setup would start defaulting 
to an unusably glitchy display instead of falling back to 1920x1080 
which does at least work perfectly (even if the slightly squished aspect 
ratio is ugly).

Thanks,
Robin.

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   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 c14f888938688..b6b662dabedc6 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -251,7 +251,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;
>   		}

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/2] drm/rockchip: dw_hdmi: relax mode_valid hook

[Sascha Hauer]

On Wed, Aug 24, 2022 at 05:07:50PM +0100, Robin Murphy wrote:
> On 2022-08-22 16:20, Sascha Hauer wrote:
> > The driver checks if the pixel clock of the given mode matches an entry
> > in the mpll config table. The frequencies in the mpll table are meant as
> > a frequency range up to which the entry works, not as a frequency that
> > must match the pixel clock. Return MODE_OK when the pixelclock is
> > smaller than one of the mpll frequencies to allow for more display
> > resolutions.
> 
> Has the issue been fixed that this table is also used to validate modes on
> RK3328, which doesn't even *have* the Synopsys phy? Last time I looked, that
> tended to lead to complete display breakage when the proper phy driver later
> decides it doesn't like a pixel clock that mode_valid already said was OK.
> 
> The more general concern is that these known-good clock rates are good, but
> others may not be even when nominally supported, which I suspect is the
> dirty secret of why it was implemented this way to begin with. I would
> really really love this patch so my RK3399 board can drive my 1920x1200
> monitor at native resolution, but on the other hand my RK3288 box generates
> such a crap 154MHz clock for that mode that - unless that's been improved in
> the meantime too - patch #2 might be almost be considered a regression if it
> means such a setup would start defaulting to an unusably glitchy display
> instead of falling back to 1920x1080 which does at least work perfectly
> (even if the slightly squished aspect ratio is ugly).

I could limit the change to rk3568 only. Would that be an option?
Not sure if I should rk3399 as well then as this would work, at least in
your setup.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/2] drm/rockchip: dw_hdmi: relax mode_valid hook

[Robin Murphy]

On 25/08/2022 12:40 pm, Sascha Hauer wrote:
> On Wed, Aug 24, 2022 at 05:07:50PM +0100, Robin Murphy wrote:
>> On 2022-08-22 16:20, Sascha Hauer wrote:
>>> The driver checks if the pixel clock of the given mode matches an entry
>>> in the mpll config table. The frequencies in the mpll table are meant as
>>> a frequency range up to which the entry works, not as a frequency that
>>> must match the pixel clock. Return MODE_OK when the pixelclock is
>>> smaller than one of the mpll frequencies to allow for more display
>>> resolutions.
>>
>> Has the issue been fixed that this table is also used to validate modes on
>> RK3328, which doesn't even *have* the Synopsys phy? Last time I looked, that
>> tended to lead to complete display breakage when the proper phy driver later
>> decides it doesn't like a pixel clock that mode_valid already said was OK.
>>
>> The more general concern is that these known-good clock rates are good, but
>> others may not be even when nominally supported, which I suspect is the
>> dirty secret of why it was implemented this way to begin with. I would
>> really really love this patch so my RK3399 board can drive my 1920x1200
>> monitor at native resolution, but on the other hand my RK3288 box generates
>> such a crap 154MHz clock for that mode that - unless that's been improved in
>> the meantime too - patch #2 might be almost be considered a regression if it
>> means such a setup would start defaulting to an unusably glitchy display
>> instead of falling back to 1920x1080 which does at least work perfectly
>> (even if the slightly squished aspect ratio is ugly).
> 
> I could limit the change to rk3568 only. Would that be an option?
> Not sure if I should rk3399 as well then as this would work, at least in
> your setup.

I think for now it might be enough to force an exact match if 
hdmi->plat_data.phy_force_vendor is set, with a big fat comment that 
it's to preserve the previous behaviour until vendor phy support can be 
sorted out properly. Beyond that, given that RK3288 and RK3399 do 
nominally support 4K as well, I don't think we actually have to leave 
them out, I just wanted to flag up that untested non-standard clock 
rates are a known source of potential issues once we open the door to them.

Cheers,
Robin.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip