Re: [RFC][PATCH] drm: kirin: Restrict modes to known good mode clocks

From: John Stultz <john.stultz@linaro.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>,
	Chen Feng <puck.chen@hisilicon.com>,
	lkml <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Xinliang Liu <z.liuxinliang@hisilicon.com>,
	Xinwei Kong <kong.kongxinwei@hisilicon.com>,
	Rongrong Zou <zourongrong@gmail.com>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [RFC][PATCH] drm: kirin: Restrict modes to known good mode clocks
Date: Tue, 11 Jul 2017 08:44:16 -0700	[thread overview]
Message-ID: <CALAqxLWhDxKPmch9PFk9MgTDFmjowwDcjoK_0gG273YcYQDj5Q@mail.gmail.com> (raw)
In-Reply-To: <CAKMK7uFGvM56kYuPL9zSE=mDq-zXXbTSdetFs9+eiJbUvJYkjw@mail.gmail.com>

On Tue, Jul 11, 2017 at 8:12 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jul 11, 2017 at 5:05 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> > be even better if you could calculate whether the mode is valid, but I didn't
>>>> > spend enough time to figure out if this is possible.
>>>>
>>>> Theoretically that might be possible, checking if the requested freq
>>>> matches the calculated freq, and I've tried that but so far I've not
>>>> been able to get it to work, as in some cases the freq on the current
>>>> whitelist don't exactly match but do work on the large majority of
>>>> monitors tested (while other freq have a similar error but don't work
>>>> on most monitors).
>>>>
>>>> I'd like to spend some more time to try to refine and tune this, but
>>>> having the current whitelist works fairly well, so I'm not sure its
>>>> worth sitting on (this is basically the last functional patch
>>>> outstanding for HiKey to fully work upstream - except the mali gpu of
>>>> course), while I try to tinker and tune it.
>>>>
>>>> Thanks so much for the feedback!
>>>
>>> Yeah the proper approach is to compute your pll/clock settings and bail
>>> out if those don't work. That's generally a magic spreadsheet supplied by
>>> the hw validation engineers, and I honestly don't want to know how they
>>> create it. Explicit modelist in the kernel sounds like a very bad hack.
>>
>> So without such a magic spreadsheet, how would you suggest I move this forward?
>> Not having the whitelist hack and picking modes the device can't
>> generate is a fairly major usability issue.
>
> I guess if the whitelist is the only thing I'd do 2 things differently:
> - Whitelist the clocks, not modes, since that's what seems to matter here.
> - Put it as close as possible to the code that comuptes the clock
> settings (yet if you use the clock subsystem that's a bit hard, but
> for an atomic driver this should be where this is done ...).
>
> Whitelist of modes looks super-hacky.

Sure. The whitelist modes were easiest to use initially dealing with
problem reports since the EDID numbers were what users could report
working or not.  But this feedback sounds reasonable, as I can map
those to the underlying pixel clocks and generate a whitelist of
those.

I really appreciate the feedback here!

thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel