[PATCH 0/3] drm/omap: fix am4 evm lcd

[Adam Ford]

On Mon, Dec 2, 2019 at 7:13 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Tomi,
>
> On Thu, Nov 14, 2019 at 11:39:47AM +0200, Tomi Valkeinen wrote:
> > Hi Tony, Thierry, Laurent,
> >
> > After the recent change of moving from omapdrm specific panel-dpi driver
> > to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> > right. This series has fixes for it, but I'm not sure if these are the
> > right ways to fix the issues, so comments welcome.
> >
> > 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> > happened with "panel-dpi" compatible string. Now with panel-simple, the
> > match should happen with the panel model compatible string, which is
> > "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> > compatible exists in panel-simple.
>
> I've also noticed that we have a few other omap-based platforms that
> got broken, for the same reason :-( We're missing driver support for
> innolux,at070tn83, samsung,lte430wq-f0c and startek,startek-kd050c, and
> we have a few nodes that use panel-dpi without any more precise
> compatible string.
>
> > Interestingly, the actual panel at least on my EVMs and ePOSes is not
> > osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> > information about osd057T0559-34ts. I don't know the history with this,
> > so it is possible that the early versions of the boards did have
> > osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> >
> > As osd070t1718-19ts is supported by panel-simple, changing the
> > compatible string to osd070t1718-19ts in the DT file solves this one.
> >
> > 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> > video timings from the DT file, so they are present in all the DT files.
> > panel-simple uses timings from a table in the panel-simple driver, but
> > gives a kernel warning if the DT file contains timings.
> >
> > This can be solved by removing the timings from the DT file.
> >
> > 3) Sync drive edge is not right. This one might have been present also
> > with panel-dpi, I didn't verify. The problem is that the panel-simple
> > data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> > and pixdata edge, but not for sync edge.
> >
> > The datasheet for the panel does not give any hint on what the edge
> > should be.  Omapdrm defaults to driving syncs on falling edge, which
> > caused the image to be shifted one pixel to the right.
> >
> > Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> > EVM also has the same panel with the same behavior.
> >
> > ====
> >
> > The reason I'm not sure if the 1) and 2) fixes are correct is that
> > they're breaking DT compatibility. Should we instead make changes to
> > panel-simple to keep the same DT files working?
>
> That's tempting, as breaking DT is quite bad, but that would require
> adding a match on panel-dpi, and parsing timings in the panel-simple
> driver. Thierry has always been opposed to that as far as I can tell,
> and even if I don't share his point of view, I don't want to move in
> this direction without a consensus.

What about a generic driver separate from the simple panel driver that
does just the panel-dpi parsing?

If we exported the necessary functions from simple-panel, we could
call them from the panel-dpi parser and not have to re-invent the
functions to enable power, gpio or back light and/or fix them each
time they get updated.


adam
>
> Your series is fine in my opinion, as even if we decide to handle
> backward compatibility in this case, changing the DT files in mainline
> is still the right way to go (if only to avoid giving wrong examples).
>
> > This would mean adding a new entry for the osd057T0559-34ts panel, but
> > as we don't have datasheet for it, I think we could just append the
> > compatible string to osd070t1718-19t's data.
> >
> > It would also mean doing some change to the panel-simple code that gives
> > the warning about timings in DT data. This might make sense, as I think
> > we have other DT files with video timings too.
> >
> > For 3), I think the patch is fine, but I'm not sure if the display
> > controller driver should be able to deduce the sync drive edge from the
> > pixdata drive edge. Are they usually the same? I have no idea...
> >
> > Tomi Valkeinen (3):
> >   ARM: dts: am437x-gp/epos-evm: fix panel compatible
> >   ARM: dts: am437x-gp/epos-evm: drop unused panel timings
> >   drm/panel: simple: fix osd070t1718_19ts sync drive edge
> >
> >  arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
> >  arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
> >  drivers/gpu/drm/panel/panel-simple.c |  3 ++-
> >  3 files changed, 4 insertions(+), 35 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel