Bjorn,, On Tue, Apr 26, 2022 at 12:41 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote: > > Add eDP support to herobrine boards, splitting up amongst the > different files as makes sense. Rationale for the current split of > things: > * The eDP connector itself is on qcard. However, not all devices with > a qcard will use an eDP panel. Some might use MIPI and, presumably, > someone could build a device with qcard that had no display at all. > * The qcard provides a PWM for backlight that goes to the eDP > connector. This PWM is also provided to the board and it's expected > that it would be used as the backlight PWM even for herobrine > devices with MIPI displays. > * It's currently assumed that all herobrine boards will have some sort > of display, either MIPI or eDP (but not both). > * We will assume herobrine-rev1 has eDP. The schematics allow for a > MIPI panel to be hooked up but, aside from some testing, nobody is > doing this and most boards don't have all the parts stuffed for > it. The two panels would also share a PWM for backlight, which is > weird. > * herobrine-villager and herobrine-hoglin (crd) also have eDP. > * herobrine-hoglin (crd) has slightly different regulator setup for > the backlight. It's expected that this is unique to this board. See > comments in the dts file. > * There are some regulators that are defined in the qcard schematic > but provided by the board like "vreg_edp_bl" and > "vreg_edp_3p3". While we could put references to these regulators > straight in the qcard.dtsi file, this would force someone using > qcard that didn't provide those regulators to provide a dummy or do > an ugly /delete-node/. Instead, we'll add references in > herobrine.dtsi. > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- > This patch most directly depends on ("arm64: dts: qcom: pm8350c: Add > pwm support") [1] and it won't even compile without that. To be > functional, of course, we also need the driver support for the PWM to > land. > > In order for this patch to function we also need Sankeerth's eDP > series [2] to land. > > To be schema happy, this patch also relies on my patch to add > regulators into the schema for the PHY an the eDP controller [3]. > Several other device tree files reference these supplies without being > in the schema, though, so we'll have to decide whether to block on > those bindings landing. It does sound as if the regulator names may > not be exactly correct right now, though. :( > > In theory, I could break this patch up into separate patches adding > the basic support to the qcard, then to herobrine.dtsi, and then one > patch each for herobrine-r1, villager, and crd. Doing so didn't seem > to make sense to me--I think it's easier to make sense of the change > as one patch. However, if someone feels strongly that it should be > broken up I'm happy to do so. > > I've managed to get the display on my herobrine-rev1 up and running on > today's linuxnext (next-20220422) with this series. For whatever > reason the eDP PHY wouldn't probe unless I hacked `fw_devlink=off` in > the config. I don't believe that problem is related to this patch, > though. > > [1] https://lore.kernel.org/r/1645509309-16142-4-git-send-email-quic_c_skakit@xxxxxxxxxxx > [2] https://lore.kernel.org/r/1650887072-16652-1-git-send-email-quic_sbillaka@xxxxxxxxxxx/ > [3] https://lore.kernel.org/r/20220425210643.2420919-1-dianders@xxxxxxxxxxxx > > Changes in v2: > - Commit message and comment cleanups from Stephen. > > .../boot/dts/qcom/sc7280-herobrine-crd.dts | 40 ++++++++++++++ > .../qcom/sc7280-herobrine-herobrine-r1.dts | 8 +++ > .../dts/qcom/sc7280-herobrine-villager-r0.dts | 8 +++ > .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 24 +++++++++ > arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 54 +++++++++++++++++++ > 5 files changed, 134 insertions(+) Since you're landing patches, I'm curious what you think about this one. I believe that the dependencies have all landed except for [3], but we the drivers are already expecting these regulators and other dts files provide them despite the fact that they're not in the bindings. Thanks! -Doug