Re: [PATCH v10 12/24] drm/rockchip: dw_hdmi: drop mode_valid hook

[Sascha Hauer <s.hauer@pengutronix.de>]

On Sun, Apr 10, 2022 at 01:31:23PM +0200, Alex Bee wrote:
> Am 08.04.22 um 13:22 schrieb Sascha Hauer:
> > 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. The downstream Kernel also does not have
> > this check, so drop it to allow for more display resolutions.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > 
> You're correct: That frequency is meant to be greater or equal. But I'm
> not sure if it makes sense to completely drop it - what happens for
> clocks rates > 600 MHz which might be supported by later generation
> sinks (HDMI 2.1 or later)?
> As these are not supported by the IPs/PHYs currently supported by that
> driver a reason a simple
> 
>         int i;
> 
> 
> 
>         for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
> 
> -               if (pclk == mpll_cfg[i].mpixelclock) {
> 
> +               if (pclk >= mpll_cfg[i].mpixelclock) {

Should be <=

No other user currently in the tree has this check.
hdmi_phy_configure_dwc_hdmi_3d_tx() has this:

> 	/* PLL/MPLL Cfg - always match on final entry */
> 	for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
> 		if (mpixelclock <= mpll_config->mpixelclock)
> 			break;
> 
> 	for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
> 		if (mpixelclock <= curr_ctrl->mpixelclock)
> 			break;
> 
> 	for (; phy_config->mpixelclock != ~0UL; phy_config++)
> 		if (mpixelclock <= phy_config->mpixelclock)
> 			break;
> 
> 	if (mpll_config->mpixelclock == ~0UL ||
> 	    curr_ctrl->mpixelclock == ~0UL ||
> 	    phy_config->mpixelclock == ~0UL)
> 		return -EINVAL;

This means we already have this check and others already in the generic
part of the dw-hdmi driver. Should we maybe do the above in
dw_hdmi_bridge_mode_valid() instead of doing it in the users?

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 |