Re: [PATCH 28/33] drm/panel-simple: Fix dotclock for Sharp LQ150X1LG11

Peter Rosin
2020-03-04T07:47:41Z

On 2020-03-03 16:05, Thierry Reding wrote:
> On Tue, Mar 03, 2020 at 02:57:45PM +0000, Peter Rosin wrote:
>>
>> On 2020-03-03 15:20, Thierry Reding wrote:
>>> On Mon, Mar 02, 2020 at 10:53:56PM +0000, Peter Rosin wrote:
>>>> On 2020-03-02 21:34, Ville Syrjala wrote:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> The currently listed dotclock disagrees with the currently
>>>>> listed vrefresh rate. Change the dotclock to match the vrefresh.
>>>>>
>>>>> Someone tell me which (if either) of the dotclock or vreresh is
>>>>> correct?
>>>>
>>>> TL/DR; I do not care if you change the refresh rate or the dotclock.
>>>>
>>>> The whole entry for that panel in simple-panel is dubious. The panel
>>>> is really an LVDS panel (capable of both VESA/Jeida RGB888, selectable
>>>> with the SELLVDS pin). With Jeida you can, as usual, omit the 4th
>>>> data channel and use the panel with RGB666. In either case, you need
>>>> an LVDS signal and nothing else...
>>>>
>>>> The panel can also rotate the picture 180 degrees using the RL/UD pin.
>>>>
>>>> These options are of course not expressed in the simple panel driver
>>>> (and we have always used fixed signals for those pins in our designs,
>>>> IIRC). As far as I'm concerned, the panel can be removed from
>>>> simple-panel. Our device trees are nowadays correctly expressing the
>>>> hardware with an LVDS encoder between the RGB output and the panel
>>>> and points to the panel-lvds driver for the panel.
>>>
>>> How do you make sure that you always bind against the correct driver? If
>>> it matches simple-panel and panel-lvds, it's not deterministically going
>>> to pick the right one. Well, it may actually be deterministic on Linux,
>>> but perhaps only by accident.
>>
>> You are probably right that it's fragile, but no problems so far. That
>> said, I did wonder why the panel-lvds driver "wins" over simple-panel for
>>
>> compatible = "sharp,lq150x1lg11", "panel-lvds";
>>
>> I figured it was by design and didn't spend too much time thinking about
>> it. Maybe I should have?
>
> It's most likely because panel-lvds.o is linked into the kernel before
> panel-simple.o and the first match wins. You may want to move the entry
> to panel-lvds to make this a little more robust.

Ok, or because I dropped the simple-panel driver when we no longer
depended on it. Either way, what do you mean "move to [..] panel-lvds"?
It has no list of panels, you have to end with "panel-lvds" in the
compatible for the driver to bind.

Cheers,
Peter