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
WARNING: multiple messages have this Message-ID (diff)
From: Rob Clark <robdclark@gmail.com> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Rob Clark <robdclark@chromium.org>, aarch64-laptops@lists.linaro.org, Rob Herring <robh+dt@kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Jeffrey Hugo <jhugo@codeaurora.org>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Vasily Khoruzhick <anarsoul@gmail.com>, David Airlie <airlied@linux.ie>, Thierry Reding <thierry.reding@gmail.com>, Mark Rutland <mark.rutland@arm.com>, Sam Ravnborg <sam@ravnborg.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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-12-08 21:24 UTC|newest] Thread overview: 32+ 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 ` Rob Clark 2019-12-07 20:35 ` [PATCH 1/4] dt-bindings: display: panel: document panel-id Rob Clark 2019-12-07 20:35 ` Rob Clark 2019-12-07 21:13 ` Sam Ravnborg 2019-12-07 21:13 ` Sam Ravnborg 2019-12-07 21:34 ` Rob Clark 2019-12-07 21:34 ` Rob Clark 2019-12-08 9:39 ` Sam Ravnborg 2019-12-08 9:39 ` Sam Ravnborg 2019-12-08 14:45 ` Laurent Pinchart 2019-12-08 14:45 ` Laurent Pinchart 2019-12-08 16:50 ` Rob Clark 2019-12-08 16:50 ` Rob Clark 2019-12-08 18:27 ` Laurent Pinchart 2019-12-08 18:27 ` Laurent Pinchart 2019-12-08 21:23 ` Rob Clark [this message] 2019-12-08 21:23 ` Rob Clark 2019-12-09 11:05 ` Laurent Pinchart 2019-12-09 11:05 ` Laurent Pinchart 2019-12-09 15:31 ` Rob Herring 2019-12-09 15:31 ` Rob Herring 2019-12-09 15:57 ` Rob Clark 2019-12-09 15:57 ` Rob Clark 2019-12-07 20:35 ` [PATCH 2/4] drm/of: add support to find any enabled endpoint Rob Clark 2019-12-07 20:35 ` Rob Clark 2019-12-08 9:31 ` Sam Ravnborg 2019-12-08 9:31 ` Sam Ravnborg 2019-12-07 20:35 ` [PATCH 3/4] drm/bridge: ti-sn65dsi86: " Rob Clark 2019-12-07 20:35 ` Rob Clark 2019-12-07 20:35 ` [PATCH 4/4] arm64: dts: qcom: c630: Enable display Rob Clark 2019-12-07 20:35 ` 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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.