Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom

[Thierry Reding]

On Thu, Apr 22, 2021 at 03:08:48PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> > >
> > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote:
> > > > The sc7180-trogdor-pompom board might be attached to any number of a
> > > > pile of eDP panels. At the moment I'm told that the list might include:
> > > > - KD KD116N21-30NV-A010
> > > > - KD KD116N09-30NH-A016
> > > > - Starry 2081116HHD028001-51D
> > > > - Sharp LQ116M1JW10
> > > >
> > > > It should be noted that while the EDID programmed in the first 3
> > > > panels indicates that they should run with exactly the same timing (to
> > > > keep things simple), the 4th panel not only needs different timing but
> > > > has a different resolution.
> > > >
> > > > As is true in general with eDP panels, we can figure out which panel
> > > > we have and all the info needed to drive its pixel clock by reading
> > > > the EDID. However, we can do this only after we've powered the panel
> > > > on. Powering on the panels requires following the timing diagram in
> > > > each panel's datasheet which specifies delays between certain
> > > > actions. This means that, while we can be quite dynamic about handling
> > > > things we can't just totally skip out on describing the panel like we
> > > > could do if it was connected to an external-facing DP port.
> > >
> > > Is this a 'standard' eDP connector? AFAICT, there does seem to be
> > > such a thing.
> >
> > To answer this one: there's not any "standard" physical plug as far as
> > I can tell. There's a connector on the board side for the LCD that has
> > a whole hodgepodge of signals on it. Maybe USB for a camera. Some
> > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals.
> > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is
> > really a "panel ready" signal for eDP). The size / style of connector
> > and the exact set of signals (and their ordering) is board specific.
> > You then get a board-specific cable that splits things out. Some might
> > go to a camera/MIC sub board. Some go to the panel and hook onto a
> > panel-specific connector which has pin count and orderings defined by
> > that panel. :-P
> >
> >
> > > I've said in the past I'd be okay with a edp-connector
> > > node. If that needs just the "HPD absent delay" property, I think that
> > > would be okay. It's just a never ending stream of new properties with
> > > each new panel that I don't want to see.
> >
> > Thinking about this we'd need at least one other property right now
> > which is an enable delay. Specifically at least one panel I've
> > supported recently lied about HPD for a short period after bootup.
> > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD
> > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the
> > existing power supply / enable signals that "simple-panel" already
> > has.
> >
> > Also: if we weren't going to add the other delay properties in the
> > device tree, we'd have to add the code right away that used the EDID
> > to set other delays. That wouldn't be the end of the world, but it
> > would be code to write.
> >
> >
> > One last thought to add: I've looked at ~10 panels specs recently.
> > Though they are all a little different from each other, I will say
> > that almost every one of them seems to have the exact same timing
> > diagram in it just with different numbers filled in. To me that backs
> > up the idea that you can/should do the power sequence with a fairly
> > standard (parameterized) driver. I can't link the datasheets I have
> > but searching for "edp panel datasheet" finds me this random
> > datasheet:
> >
> > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf
> >
> > See "8.0 POWER SEQUENCE" in that document. All the panels have a
> > nearly identical diagram with different numbers filled in. You can
> > kinda tell it was copied from some other panel since some numbers
> > (like T4) aren't even defined.
> 
> So this thread has been quiet for a while, but the problem still exists.
> 
> Here's my current plan, but please yell if you disagree:
> 
> 1. See about adding a generic "eDP connector" node. Having stewed on
> this for a while I think I'm convinced that even though there's not
> really a single standard physical connector that is used everywhere
> that there are at least a set of signals that can be collectively
> thought about as the "eDP signals". Certainly I have a set of very
> different panels from very different manufacturers that I can
> "interchange" and they work fine assuming I have the right cable
> "adapting" them from the connector on my board to the connector on the
> panel. While different panels have different timings that they care
> are enforced, there is a way to express it in a relatively common way
> as evidenced by the fact that all panel datasheet timing diagrams look
> similar and the fact that panel-simple handles so many different
> panels (yes, we periodically add more timing constraints to handle
> there but mostly that's because the code wasn't able to handle every
> constraint that could be expressed in those standard-looking timing
> diagrams in the datasheets).
> 
> 
> 2. The "eDP connector" node will have all the same properties as
> today's "panel-simple.yaml" with the addition of:
> 
> enable-delay
> hpd-absent-delay
> 
> The idea is that you power on the panel, hardcode an enable-delay (to
> handle early HPD glitches), and then wait for HPD (or wait
> hpd-absent-delay if HPD isn't provided).
> 
> Note that "ddc-i2c-bus" will be a required node instead of optional.
> 
> 
> 3. Once we power the panel on then we will query the EDID and set the
> rest of the panel timings / modes based on the model specified in the
> EDID. Potentially it could update the "enable-delay" and
> "hpd-absent-delay" at this point too.

I think that sounds good. If ddc-i2c-bus is required, this basically
implies that EDID needs to be available for these panels, too. If that's
the case we can identify the panel based on information from the EDID.
That would make panels "discoverable", so that we can describe them with
a more generic compatible string that basically describes the interface
needed to get at the discoverable information, much like we would do for
a bus like PCI.

I don't know if the manufacturer ID and product code are enough to
uniquely identify every panel, but maybe something like the DisplayID
extension can be used to gather more identification data.

Thierry

Attachment: signature.asc
Description: PGP signature