Re: [PATCH 1/4] dt-bindings: chosen: document panel-id binding

[Jeffrey Hugo]

On 7/1/2019 8:03 AM, Rob Herring wrote:
> On Sun, Jun 30, 2019 at 2:36 PM Rob Clark <robdclark@xxxxxxxxx> wrote:
> > From: Rob Clark <robdclark@xxxxxxxxxxxx>
> >
> > The panel-id property in chosen can be used to communicate which panel,
> > of multiple possibilities, is installed.
> >
> > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> > ---
> >   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?

Its platform specific.  In the devices that Rob Clark seems interested
in, there are multiple mechanisms in place - read a gpio, enable the
DSI and send a specific command to the panel controller asking for its
panel id, or read some efuses.

The efuses may not be accessible by Linux.

The DSI solution is problematic because it causes a chicken and egg
situation where linux needs the DT to probe the DSI driver to query
the panel, in order to edit the DT to probe DSI/panel.

In the systems Rob Clark is interested in, the FW already provides a
specific EFI variable with the panel id encoded in it for HLOS to use
(although this is broken on some of the devices), but this is a
specific vendor's solution.

The FW/bootloader has probably already figured out the panel details
and brought up the display for a boot splash, bios menu, etc.  I'm not
sure it makes a lot of sense to define N mechanisms for linux to
figure out the same across every platform/vendor.

> > +
> > +/ {
> > +       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'?
>
> > +               power-supply = <&vlcm_3v3>;
> > +               no-hpd;
> > +
> > +               ports {
> > +                       port {
> > +                               boe_panel_in_edp: endpoint {
> > +                                       remote-endpoint = <&sn65dsi86_out_boe>;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       display_or_bridge_device {
> > +
> > +               ports {
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +
> > +                       ...
> > +
> > +                       port@0 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               reg = <0>;
> > +
> > +                               endpoint@c4 {
> > +                                       reg = <0xc4>;
>
> What does this number represent? It is supposed to be defined by the
> display_or_bridge_device, not a specific panel.

Its the specific FW/bootloader defined panel id, that matches the
above defined panel-id property.

> We also need to consider how the DSI case with panels as children of
> the DSI controller would work and how this would work with multiple
> displays each having multiple panel options.
>
> Rob


--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.