| Message ID | 20210730212625.3071831-1-dianders@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | eDP: Support probing eDP panels dynamically instead of hardcoding | expand |
Hi, On Tue, Aug 3, 2021 at 1:41 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Douglas, > > On Fri, Jul 30, 2021 at 02:26:19PM -0700, Douglas Anderson wrote: > > The goal of this patch series is to move away from hardcoding exact > > eDP panels in device tree files. As discussed in the various patches > > in this series (I'm not repeating everything here), most eDP panels > > are 99% probable and we can get that last 1% by allowing two "power > > up" delays to be specified in the device tree file and then using the > > panel ID (found in the EDID) to look up additional power sequencing > > delays for the panel. > > Have you considered a new driver for edp panels? > panel-edp.c? > > There will be some duplicate code from pnale-simple - but the same can > be said by the other panel drivers too. > In the end I think it is better to separate them so we end up with two > less complex panel drivers rather than one do-it-all panel driver. > > I have not looked in detail how this would look like, but my first > impression is that we should split it out. I certainly could, but my argument against it is that really it's the exact same set of eDP panels that would be supported by both drivers. By definition the "simple" eDP panels are the ones that just have a regulator/enable GPIO and some timings to turn them off. Those are the exact same set of panels that can be probed if we just provide the panel ID that's hardcoded in the EDID. As you can see from the implementation patch I'm actually sharing the private data structures (the ones containing the timing) for panels that are supported both as "probable" and as hardcoded. If we split into two drivers we'd either need to duplicate the timings for all panels supported by both drivers or we'd have to somehow export them (maybe hard if things are in modules). Also, since it's the same set of eDP panels we'd need to exactly duplicate all the code handling delays / HPD. It just doesn't feel right to me. -Doug
Hi, On Thu, Aug 12, 2021 at 2:38 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Doug, > On Mon, Aug 09, 2021 at 03:18:03PM -0700, Doug Anderson wrote: > > Hi, > > > > On Tue, Aug 3, 2021 at 1:41 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > > > > > Hi Douglas, > > > > > > On Fri, Jul 30, 2021 at 02:26:19PM -0700, Douglas Anderson wrote: > > > > The goal of this patch series is to move away from hardcoding exact > > > > eDP panels in device tree files. As discussed in the various patches > > > > in this series (I'm not repeating everything here), most eDP panels > > > > are 99% probable and we can get that last 1% by allowing two "power > > > > up" delays to be specified in the device tree file and then using the > > > > panel ID (found in the EDID) to look up additional power sequencing > > > > delays for the panel. > > > > > > Have you considered a new driver for edp panels? > > > panel-edp.c? > > > > > > There will be some duplicate code from pnale-simple - but the same can > > > be said by the other panel drivers too. > > > In the end I think it is better to separate them so we end up with two > > > less complex panel drivers rather than one do-it-all panel driver. > > > > > > I have not looked in detail how this would look like, but my first > > > impression is that we should split it out. > > > > I certainly could, but my argument against it is that really it's the > > exact same set of eDP panels that would be supported by both drivers. > > The idea was to move all eDP panels to the new driver. > > My hope it that we can make panel-simple handle a more more narrow set > of panels. eDP capable displays are IMO not simple panels. Ah! OK, this makes sense. I can work on this, though it might be a short while before I can. I think moving all eDP panels out of panel-simple.c to something like panel-simple-edp.c makes sense. It will be a patch that will be very hard to cherry-pick anywhere since it will conflict with everything but it should be doable. > Likewise DSI capable panels could also be pulled out of panel-simple. At the moment I haven't done much with DSI panels so I might leave them in panel-simple for now. I'll evaluate to see how nasty it would be for me to try this. > This would continue to duplicate some code - but we have a lot of > duplicated code across the various panels and the best way forward > would be to implement more helpers that can be used by the drivers. > > Sam - who is trying to recover form the deadly man flu... Feel better! -Doug
The goal of this patch series is to move away from hardcoding exact eDP panels in device tree files. As discussed in the various patches in this series (I'm not repeating everything here), most eDP panels are 99% probable and we can get that last 1% by allowing two "power up" delays to be specified in the device tree file and then using the panel ID (found in the EDID) to look up additional power sequencing delays for the panel. This patch series is the logical contiunation of a previous patch series where I proposed solving this problem by adding a board-specific compatible string [1]. In the discussion that followed it sounded like people were open to something like the solution proposed in this new series. In version 2 I got rid of the idea that we could have a "fallback" compatible string that we'd use if we didn't recognize the ID in the EDID. This simplifies the bindings a lot and the implementation somewhat. As a result of not having a "fallback", though, I'm not confident in transitioning any existing boards over to this since the panel will totally fail to work if we don't recognize the ID from the EDID and I can't guarantee that I've seen every panel that might have shipped on an existing product. The plan is to use "edp-panel" only on new boards or new revisions of old boards where we can guarantee that every EDID that ships out of the factory has an ID in the table. Version 2 of this series is also rebased upon my other series for the Samsung ATNA33XC20 panel [2] since they both touch the "delay" structure and it seems likely that the Samsung panel series will land first. [1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/ [2] https://lore.kernel.org/r/20210730154605.2843418-1-dianders@chromium.org/ Changes in v2: - No longer allow fallback to panel-simple. - Add "-ms" suffix to delays. - Rebased atop revert of delays between GPIO & regulator - Don't support a "fallback" panel. Probed panels must be probed. - Not based on patch to copy "desc"--just allocate for probed panels. - Add "-ms" suffix to delays. Douglas Anderson (6): dt-bindings: drm/panel-simple: Introduce generic eDP panels drm/edid: Break out reading block 0 of the EDID drm/edid: Allow the querying/working with the panel ID from the EDID drm/panel-simple: Don't re-read the EDID every time we power off the panel drm/panel-simple: Split the delay structure out of the panel description drm/panel-simple: Implement generic "edp-panel"s probed by EDID .../bindings/display/panel/panel-edp.yaml | 188 ++++++++++ drivers/gpu/drm/drm_edid.c | 113 +++++- drivers/gpu/drm/panel/panel-simple.c | 352 +++++++++++++----- include/drm/drm_edid.h | 47 +++ 4 files changed, 586 insertions(+), 114 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-edp.yaml