Re: [PATCH 4/5] Revert "drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested"

[Peter Rosin]

On 2019-12-10 14:24, Claudiu Beznea wrote:
> This reverts commit f6f7ad3234613f6f7f27c25036aaf078de07e9b0.
> ("drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested")
> because allowing selecting a higher pixel clock may overclock
> LCD devices, not all of them being capable of this.

Without this patch, there are panels that are *severly* underclocked (on the
magnitude of 40MHz instead of 65MHz or something like that, I don't remember
the exact figures). And they are of course not capable of that. All panels
have *some* slack as to what frequencies are supported, and the patch was
written under the assumption that the preferred frequency of the panel was
requested, which should leave at least a *little* headroom.

So, I'm curious as to what panel regressed. Or rather, what pixel-clock it needs
and what it gets with/without the patch?

Or is the revert based on some theory of a perceived risk of toasting a panel?

In short, this revert regresses my use case and I would like at least a hook to
re-enable the removed logic.

Cheers,
Peter

> Cc: Peter Rosin <peda@xxxxxxxxxx>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 721fa88bf71d..1a70dff1a417 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -117,18 +117,6 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  		div = DIV_ROUND_UP(prate, mode_rate);
>  		if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK)
>  			div = ATMEL_HLCDC_CLKDIV_MASK;
> -	} else {
> -		int div_low = prate / mode_rate;
> -
> -		if (div_low >= 2 &&
> -		    ((prate / div_low - mode_rate) <
> -		     10 * (mode_rate - prate / div)))
> -			/*
> -			 * At least 10 times better when using a higher
> -			 * frequency than requested, instead of a lower.
> -			 * So, go with that.
> -			 */
> -			div = div_low;
>  	}
>  
>  	cfg |= ATMEL_HLCDC_CLKDIV(div);
>