On Mon, Dec 9, 2019 at 7:31 AM Rob Herring robh+dt@kernel.org wrote:
On Sun, Dec 8, 2019 at 3:24 PM Rob Clark robdclark@gmail.com wrote:
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 installedon a device, the panel-id property can be used by the firmware to identifywhich panel should have it's status changed to "ok". This property is notused 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:
This looks good to me. The only slight concern I have with it is making the overlay filename an ABI. I don't have a better suggestion though. How would this work for other vendors or the same panel ID (for different panels) used on different platforms? For different vendors at least, I guess dtbloader gets the base dtb path somehow and the overlay's are relative to that?
Not sure if "different vendors" in this context means different OEMs/ODMs, or different SoC's? This solution is snapdragon specific.. and in this case the panel id seems to be a flat namespace (I don't see re-use for different panels). But in DtbLoader I attempt loading a device specific path first, just in case. In particular the paths DtbLoader uses (for dtb and panel overlay) are (where $SysVendor/$ProductName/$BoardName come from SMBIOS tables and $PanelId comes from a qcom specific UEFIDisplayInfo variable) described below:
It tries to load dtb from (in order, paths relative to EFI system partition where DtbLoader is installed):
1) \dtb$SysVendor$ProductName-$BoardName.dtb 2) \dtb$SysVendor$ProductName.dtb
and panel overlay dtb from:
1) \dtb$SysVendor$ProductName-$BoardName-panel-$PanelId.dtb 2) \dtb$SysVendor$ProductName-panel-$PanelId.dtb 3) \dtb\panels\panel-$PanelId.dtb
We are already using different dtb names for the main dtb compared to what the kernel uses. Which isn't great. At some point we might want to add SysVendor/ProductName/BoardName fields in the dtb so we could automate renaming and stuffing the dtb's into the correct layout.
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*/
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.
It's the base dtb that needs the symbols I think.
BTW, to answer the question on #dri-devel, if you wanted to put the full panel into an overlay, the way to solve the problem of having bridge specific knowledge is defining a connector node. That should provide enough abstraction. Presumably the connector is actually the same across panels in this situation, so that should match up with the actual h/w. It could be possible to have a different physical connector populated for each possible panel, but hopefully that's not the common case.
ok, I'm not too familiar with this connector node thing. I think in the end, it is really just the compatible string that differs (ie. power-supply, etc would all be the same for each possible panel). But it might be worth trying this connector node thing
BR, -R