On Sat, Nov 30, 2019 at 10:37 AM Rob Clark robdclark@gmail.com wrote:
On Mon, Jul 1, 2019 at 7:03 AM Rob Herring robh+dt@kernel.org wrote:
On Sun, Jun 30, 2019 at 2:36 PM Rob Clark robdclark@gmail.com wrote:
From: Rob Clark robdclark@chromium.org
The panel-id property in chosen can be used to communicate which panel, of multiple possibilities, is installed.
Signed-off-by: Rob Clark robdclark@chromium.org
Documentation/devicetree/bindings/chosen.txt | 69 ++++++++++++++++++++ 1 file changed, 69 insertions(+)
I need to update this file to say it's moved to the schema repository...
But I don't think that will matter...
diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt index 45e79172a646..d502e6489b8b 100644 --- a/Documentation/devicetree/bindings/chosen.txt +++ b/Documentation/devicetree/bindings/chosen.txt @@ -68,6 +68,75 @@ on PowerPC "stdout" if "stdout-path" is not found. However, the "linux,stdout-path" and "stdout" properties are deprecated. New platforms should only use the "stdout-path" property.
+panel-id +--------
+For devices that have multiple possible display panels (multi-sourcing the +display panels is common on laptops, phones, tablets), this allows the +bootloader to communicate which panel is installed, e.g.
How does the bootloader figure out which panel? Why can't the kernel do the same thing?
+/ {
chosen {panel-id = <0xc4>;};ivo_panel {compatible = "ivo,m133nwf4-r0";power-supply = <&vlcm_3v3>;no-hpd;ports {port {ivo_panel_in_edp: endpoint {remote-endpoint = <&sn65dsi86_out_ivo>;};};};};boe_panel {compatible = "boe,nv133fhm-n61";Both panels are going to probe. So the bootloader needs to disable the not populated panel setting 'status' (or delete the node). If you do that, do you even need 'panel-id'?
So, I'm finally having some time to revisit this proposal.. I have a few updates:
- Updated DtbLoader.efi to read UEFIDisplayInfo and get the panel-id so I can drop the efi/libstub patch[1]
- I can drop drm_of_find_panel_id() and fold the logic to look at /chosen/panel-id into drm_of_find_panel_or_bridge() so that each driver or bridge doesn't need an update
This doesn't realy solve the issue that both panels will probe. On the other hand, I really don't want to make the DtbLoader know enough about the dt structure of each laptop to patch dt, since that is not going to be scalable at all. (Likewise, there is some interest for devices that use u-boot to take the panel-id from a uboot env var and patch dt, but again knowing enough to intelligently patch the dt is not going to be feasible.)
But, an alternate solution could be, instead of adding a 'panel-id' node in chosen, I could add it as an optional property in the panel node. So taking my original example of the yoga c630 laptops, with the two possible panel id's 0xc4 and 0xc5:
ivo_panel { compatible = "ivo,m133nwf4-r0"; panel-id = <0xc4>;
correction, the ivo panel should have 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>; }; }; }; };With this, the "firmware" (DtbLoader, u-boot, etc) could crawl the entire dt looking for a node with a panel-id that matches the one it's looking for, and change that node's status to enabled.
The advantage would be that the other panel(s) that is not installed will not be probed. The downsides are that (1) the drm drivers would have to loop over all the endpoints to find the active panel (some drivers do this already, most do not), and (2) the property name "panel-id" (or whatever we pick) will now be somehow special, you couldn't re-use that name elsewhere without potential to confuse the firmware. And it is more complexity in the firmware (although at least it can be done generically) compared to just adding a property in chosen.
Not sure if this is better, I thought my initial proposal was more elegant. I am open to other suggestions, anything other than teaching DtbLoader/u-boot about the specific dt of each different device that would use this.
BR, -R