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

From: Peter Rosin
Date: Tue Dec 10 2019 - 09:11:48 EST


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