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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2272669c-38ee-1928-9563-46755574897c@axentia.se>
Date:   Tue, 10 Dec 2019 17:22:59 +0000
From:   Peter Rosin <peda@...ntia.se>
To:     "Claudiu.Beznea@...rochip.com" <Claudiu.Beznea@...rochip.com>,
        "sam@...nborg.org" <sam@...nborg.org>,
        "bbrezillon@...nel.org" <bbrezillon@...nel.org>,
        "airlied@...ux.ie" <airlied@...ux.ie>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "Nicolas.Ferre@...rochip.com" <Nicolas.Ferre@...rochip.com>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "Ludovic.Desroches@...rochip.com" <Ludovic.Desroches@...rochip.com>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>
CC:     "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/5] Revert "drm/atmel-hlcdc: allow selecting a higher
 pixel-clock than requested"

On 2019-12-10 15:59, Claudiu.Beznea@...rochip.com wrote:
> 
> 
> On 10.12.2019 16:11, Peter Rosin wrote:
>> 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). 
> 
> With patch that switches by default to 2xsystem clock for pixel clock, if
> using 133MHz system clock (as you specified in the patch I proposed for
> revert here) that would go, without this patch at 53MHz if 65MHz is
> requested. Correct me if I'm wrong.

It might have been 53MHz, whatever it was it was too low for things to work.

>> 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.
> 
> I see, but from my point of view, the upper layers should decide what
> frequency settings should be done on the LCD controller and not let this at
>  the driver's latitude.

Right, but the upper layers do not support negotiating a frequency from
ranges. At least the didn't when the patch was written, and implementing
*that* seemed like a huge undertaking.

>>
>> So, I'm curious as to what panel regressed. Or rather, what pixel-clock it needs
>> and what it gets with/without the patch?
> 
> I have 2 use cases:
> 1/ system clock = 200MHz and requested pixel clock (mode_rate) ~71MHz. With
> the reverted patch the resulted computed pixel clock would be 80MHz.
> Previously it was at 66MHz

I don't see how that's possible.

[doing some calculation by hand]

Arrgh. *blush*

The code does not do what I intended for it to do.
Can you please try this instead of reverting?

Cheers,
Peter

From b3e86d55b8d107a5c07e98f879c67f67120c87a6 Mon Sep 17 00:00:00 2001
From: Peter Rosin <peda@...ntia.se>
Date: Tue, 10 Dec 2019 18:11:28 +0100
Subject: [PATCH] drm/atmel-hlcdc: prefer a lower pixel-clock than requested

The intention was to only select a higher pixel-clock rate than the
requested, if a slight overclocking would result in a rate significantly
closer to the requested rate than if the conservative lower pixel-clock
rate is selected. The fixed patch has the logic the other way around and
actually prefers the higher frequency. Fix that.

Fixes: f6f7ad323461 ("drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested")
Reported-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
Signed-off-by: Peter Rosin <peda@...ntia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 9e34bce089d0..03691845d37a 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -120,8 +120,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 		int div_low = prate / mode_rate;
 
 		if (div_low >= 2 &&
-		    ((prate / div_low - mode_rate) <
-		     10 * (mode_rate - prate / div)))
+		    (10 * (prate / div_low - mode_rate) <
+		     (mode_rate - prate / div)))
 			/*
 			 * At least 10 times better when using a higher
 			 * frequency than requested, instead of a lower.
-- 
2.20.1