From: Rob Clark <robdclark@gmail.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
aarch64-laptops@lists.linaro.org,
Vasily Khoruzhick <anarsoul@gmail.com>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Jeffrey Hugo <jhugo@codeaurora.org>,
Rob Clark <robdclark@chromium.org>,
Thierry Reding <thierry.reding@gmail.com>,
Sam Ravnborg <sam@ravnborg.org>, David Airlie <airlied@linux.ie>,
Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/4] dt-bindings: display: panel: document panel-id
Date: Sun, 8 Dec 2019 13:23:59 -0800 [thread overview]
Message-ID: <CAF6AEGsYa0p_6MgO+=gaok5GKkTDeUJYZw0MqiFc7+qUXuNS9A@mail.gmail.com> (raw)
In-Reply-To: <20191208182757.GE14311@pendragon.ideasonboard.com>
On Sun, Dec 8, 2019 at 10:28 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> On Sun, Dec 08, 2019 at 08:50:32AM -0800, Rob Clark wrote:
> > On Sun, Dec 8, 2019 at 6:45 AM Laurent Pinchart wrote:
> > > On Sat, Dec 07, 2019 at 12:35:50PM -0800, Rob Clark wrote:
> > > > From: Rob Clark <robdclark@chromium.org>
> > > >
> > > > For devices that have one of several possible panels installed, the
> > > > panel-id property gives firmware a generic way to locate and enable the
> > > > panel node corresponding to the installed panel. Example of how to use
> > > > this property:
> > > >
> > > > ivo_panel {
> > > > compatible = "ivo,m133nwf4-r0";
> > > > panel-id = <0xc5>;
> > > > status = "disabled";
> > > >
> > > > ports {
> > > > port {
> > > > ivo_panel_in_edp: endpoint {
> > > > remote-endpoint = <&sn65dsi86_out_ivo>;
> > > > };
> > > > };
> > > > };
> > > > };
> > > >
> > > > boe_panel {
> > > > compatible = "boe,nv133fhm-n61";
> > > > panel-id = <0xc4>;
> > > > status = "disabled";
> > > >
> > > > ports {
> > > > port {
> > > > boe_panel_in_edp: endpoint {
> > > > remote-endpoint = <&sn65dsi86_out_boe>;
> > > > };
> > > > };
> > > > };
> > > > };
> > > >
> > > > sn65dsi86: bridge@2c {
> > > > compatible = "ti,sn65dsi86";
> > > >
> > > > ports {
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > port@0 {
> > > > reg = <0>;
> > > > sn65dsi86_in_a: endpoint {
> > > > remote-endpoint = <&dsi0_out>;
> > > > };
> > > > };
> > > >
> > > > port@1 {
> > > > reg = <1>;
> > > >
> > > > sn65dsi86_out_boe: endpoint@c4 {
> > > > remote-endpoint = <&boe_panel_in_edp>;
> > > > };
> > > >
> > > > sn65dsi86_out_ivo: endpoint@c5 {
> > > > remote-endpoint = <&ivo_panel_in_edp>;
> > > > };
> > > > };
> > > > };
> > > > };
> > > >
> > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > ---
> > > > .../bindings/display/panel/panel-common.yaml | 26 +++++++++++++++++++
> > > > 1 file changed, 26 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.yaml b/Documentation/devicetree/bindings/display/panel/panel-common.yaml
> > > > index ef8d8cdfcede..6113319b91dd 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/panel-common.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.yaml
> > > > @@ -75,6 +75,32 @@ properties:
> > > > in the device graph bindings defined in
> > > > Documentation/devicetree/bindings/graph.txt.
> > > >
> > > > + panel-id:
> > > > + description:
> > > > + To support the case where one of several different panels can be installed
> > > > + on a device, the panel-id property can be used by the firmware to identify
> > > > + which panel should have it's status changed to "ok". This property is not
> > > > + used by the HLOS itself.
> > >
> > > If your firmware can modify the status property of a panel, it can also
> > > add DT nodes. As discussed before, I don't think this belongs to DT.
> > > Even if panel-id isn't used by the operating system, you have Linux
> > > kernel patches in this series that show that this isn't transparent.
> >
> > I've already explained several times why this is not feasible. It
> > would require DtbLoader to be familiar with each individual device,
> > and be rev'd every time a new device appears. That is not practical
> > at all.
> >
> > (And fwiw, the ACPI tables describe each panel.. with an ACPI method
> > that is passed the the panel-id and returns the appropriate table..
> > since DT doesn't have methods, this is the solution.)
> >
> > I stand by this patch, we can't keep running away from this problem
> > and wave the magic firmware wand.
>
> I believe in firmware solutions more than firmware magic wands :-)
>
and with that in mind, I think I've come up with a firmware solution,
in the form of dtb overlays :-)
I've managed to get DtbLoader to find and load a panel overlay based
on the panel-id it reads, which drops all patches in the patchset
except the last one, which now has this delta:
---------
diff --git a/arch/arm64/boot/dts/qcom/Makefile
b/arch/arm64/boot/dts/qcom/Makefile
index 6498a1ec893f..1a61e8da2521 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+subdir-y += panels
dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb
dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb
dtb-$(CONFIG_ARCH_QCOM) += ipq8074-hk01.dtb
diff --git a/arch/arm64/boot/dts/qcom/panels/Makefile
b/arch/arm64/boot/dts/qcom/panels/Makefile
new file mode 100644
index 000000000000..dbf55f423555
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/panels/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_ARCH_QCOM) += panel-c4.dtb
+dtb-$(CONFIG_ARCH_QCOM) += panel-c5.dtb
diff --git a/arch/arm64/boot/dts/qcom/panels/panel-c4.dts
b/arch/arm64/boot/dts/qcom/panels/panel-c4.dts
new file mode 100644
index 000000000000..ebcf65419dad
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/panels/panel-c4.dts
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Panel overlay for panel-id 0xc4
+ *
+ * Copyright (c) 2019, Linaro Ltd.
+ */
+
+/dts-v1/;
+/plugin/;
+/ {
+ fragment@0 {
+ target-path = "/panel";
+ __overlay__ {
+ compatible = "boe,nv133fhm-n61";
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/panels/panel-c5.dts
b/arch/arm64/boot/dts/qcom/panels/panel-c5.dts
new file mode 100644
index 000000000000..0ad5bb6003e3
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/panels/panel-c5.dts
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Panel overlay for panel-id 0xc5
+ *
+ * Copyright (c) 2019, Linaro Ltd.
+ */
+
+/dts-v1/;
+/plugin/;
+/ {
+ fragment@0 {
+ target-path = "/panel";
+ __overlay__ {
+ compatible = "ivo,m133nwf4-r0";
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
index c35d8099d8eb..92c76afb721c 100644
--- a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
+++ b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
@@ -22,11 +22,13 @@
hsuart0 = &uart6;
};
+ /*
+ * stub node which defines how panel is connected to bridge, which
+ * will be updated by panel specific overlay
+ */
panel {
- compatible = "ivo,m133nwf4-r0";
power-supply = <&vlcm_3v3>;
no-hpd;
-
ports {
port {
panel_in_edp: endpoint {
---------
Side note, try as I might, I couldn't get the 'target = <&phandle>'
approach to work in the overlays, so I ended up going with target-path
instead. From digging thru the fdt_overlay code, I *think* it is
because I end up w/ an overlay dtb without symbols. In the end, I
guess target-path works just as well.
BR,
-R
next prev parent reply other threads:[~2019-12-08 21:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-07 20:35 [PATCH 0/4] drm+dt: multi panel selection and yoga c630 display Rob Clark
2019-12-07 20:35 ` [PATCH 1/4] dt-bindings: display: panel: document panel-id Rob Clark
2019-12-07 21:13 ` Sam Ravnborg
2019-12-07 21:34 ` Rob Clark
2019-12-08 9:39 ` Sam Ravnborg
2019-12-08 14:45 ` Laurent Pinchart
2019-12-08 16:50 ` Rob Clark
2019-12-08 18:27 ` Laurent Pinchart
2019-12-08 21:23 ` Rob Clark [this message]
2019-12-09 11:05 ` Laurent Pinchart
2019-12-09 15:31 ` Rob Herring
2019-12-09 15:57 ` Rob Clark
2019-12-07 20:35 ` [PATCH 4/4] arm64: dts: qcom: c630: Enable display Rob Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAF6AEGsYa0p_6MgO+=gaok5GKkTDeUJYZw0MqiFc7+qUXuNS9A@mail.gmail.com' \
--to=robdclark@gmail.com \
--cc=aarch64-laptops@lists.linaro.org \
--cc=airlied@linux.ie \
--cc=anarsoul@gmail.com \
--cc=bjorn.andersson@linaro.org \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jhugo@codeaurora.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robdclark@chromium.org \
--cc=robh+dt@kernel.org \
--cc=sam@ravnborg.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).