Re: [Aarch64-laptops] [PATCH 0/4] drm+dt+efi: support devices with multiple possible panels

On Sun, Jun 30, 2019 at 1:36 PM Rob Clark robdclark@gmail.com wrote:

From: Rob Clark robdclark@chromium.org

Now that we can deal gracefully with bootloader (firmware) initialized display on aarch64 laptops[1], the next step is to deal with the fact that the same model of laptop can have one of multiple different panels. (For the yoga c630 that I have, I know of at least two possible panels, there might be a third.)

This is actually a scenario that comes up frequently in phones and tablets as well, so it is useful to have an upstream solution for this.

The basic idea is to add a 'panel-id' property in dt chosen node, and use that to pick the endpoint we look at when loading the panel driver, e.g.

/ { 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";
            power-supply = <&vlcm_3v3>;
            no-hpd;

            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@1 {
                            #address-cells = <1>;
                            #size-cells = <0>;
                            reg = <1>;

                            endpoint@c4 {
                                    reg = <0xc4>;
                                    remote-endpoint = <&boe_panel_in_edp>;
                            };

                            endpoint@c5 {
                                    reg = <0xc5>;
                                    remote-endpoint = <&ivo_panel_in_edp>;
                            };
                    };
            };
    }

};

Just to put out an alternative idea for how to handle this in DT (since Laurent seemed unhappy with the idea of using endpoints to describe multiple connections between ports that *might* exist.

This approach would require of_drm_find_panel() to check the device_node to see if it is a special "panel-select" thing. (I think we could use device_node::data to recover the actual selected panel.)

On the plus side, it would work for cases that aren't using of_graph to connect display/bridge to panel, so it would be pretty much transparent to drivers and bridges.

And I guess it could be extended to cases where gpio's are used to detect which panel is attached.. not sure how far down that road I want to go, as jhugo mentioned elsewhere on this patchset, in some cases dsi is used to select (which could be problematic to do from kernel if display is already active in video mode scanout), or efuses which aren't accessible from kernel.

chosen { panel-id = <0xc4>; };

panel_select { compatible = "linux,panel-select"; #address-cells = <1>; #size-cells = <0>;

boe_panel { compatible = "boe,nv133fhm-n61"; reg = <0xc4>; power-supply = <&vlcm_3v3>; no-hpd; };

ivo_panel { compatible = "ivo,m133nwf4-r0"; reg = <0xc5>; power-supply = <&vlcm_3v3>; no-hpd; };

ports { port { panel_in_edp: endpoint { remote-endpoint = <&sn65dsi86_out>; }; }; }; };

dsi_controller_or_bridge { ... ports { ... port@1 { reg = <1>; sn65dsi86_out: endpoint { remote-endpoint = <&panel_in_edp>; }; }; }; };

Personally I find my original proposal more natural (which is why I went with it, this second idea is more similar to what I initially had in mind before looking at of_graph). And it seems to be a bit weird to have a panel_select thing which isn't really hardware.

BR, -R