[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=UUTVZPzJvC-S6=p_xRpoW+7EtupDYobgu7aQPWvR6XmA@mail.gmail.com>
Date: Wed, 4 May 2022 10:32:04 -0700
From: Doug Anderson <dianders@...omium.org>
To: Bjorn Andersson <bjorn.andersson@...aro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>,
Rob Clark <robdclark@...omium.org>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
quic_kalyant <quic_kalyant@...cinc.com>,
Stephen Boyd <swboyd@...omium.org>,
Matthias Kaehlcke <mka@...omium.org>,
Andy Gross <agross@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] arm64: dts: qcom: sc7280: eDP for herobrine boards
Bjorn,,
On Tue, Apr 26, 2022 at 12:41 PM Douglas Anderson <dianders@...omium.org> 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@...omium.org>
> ---
> 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@quicinc.com
> [2] https://lore.kernel.org/r/1650887072-16652-1-git-send-email-quic_sbillaka@quicinc.com/
> [3] https://lore.kernel.org/r/20220425210643.2420919-1-dianders@chromium.org
>
> 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
Powered by blists - more mailing lists