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

Thierry Reding
2020-03-03T15:05:24Z

[-- Attachment #1.1: Type: text/plain, Size: 2268 bytes --]

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.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]