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

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

[Rob Clark]

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>;
				};
			};
		};
	}
};

Note that the panel-id is potentially a sparse-int.  The values I've
seen so far on aarch64 laptops are:

  * 0xc2
  * 0xc3
  * 0xc4
  * 0xc5
  * 0x8011
  * 0x8012
  * 0x8055
  * 0x8056

At least on snapdragon aarch64 laptops, they can be any u32 value.

However, on these laptops, the bootloader/firmware is not populating the
chosen node, but instead providing an "UEFIDisplayInfo" variable, which
contains the panel id.  Unfortunately EFI variables are only available
before ExitBootServices, so the second patch checks for this variable
before EBS and populates the /chosen/panel-id variable.

[1] https://patchwork.freedesktop.org/series/63001/

Rob Clark (4):
  dt-bindings: chosen: document panel-id binding
  efi/libstub: detect panel-id
  drm: add helper to lookup panel-id
  drm/bridge: ti-sn65dsi86: use helper to lookup panel-id

 Documentation/devicetree/bindings/chosen.txt | 69 ++++++++++++++++++++
 drivers/firmware/efi/libstub/arm-stub.c      | 49 ++++++++++++++
 drivers/firmware/efi/libstub/efistub.h       |  2 +
 drivers/firmware/efi/libstub/fdt.c           |  9 +++
 drivers/gpu/drm/bridge/ti-sn65dsi86.c        |  5 +-
 drivers/gpu/drm/drm_of.c                     | 21 ++++++
 include/drm/drm_of.h                         |  7 ++
 7 files changed, 160 insertions(+), 2 deletions(-)

-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rob Clark]

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(+)

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.
+
+/ {
+	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>;
+				};
+			};
+		};
+	};
+
+	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>;
+					remote-endpoint = <&boe_panel_in_edp>;
+				};
+
+				endpoint@c5 {
+					reg = <0xc5>;
+					remote-endpoint = <&ivo_panel_in_edp>;
+				};
+			};
+		};
+	}
+};
+
+Note that panel-id values can be sparse (ie. not just integers 0..n).
+
 linux,booted-from-kexec
 -----------------------
 
-- 
2.20.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

[Rob Herring]

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'?

> +               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.

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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@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?

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.
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

[Rob Clark]

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?

see jhugo's response, he has I guess more access to the bootloader than I

> > +
> > +/ {
> > +       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'?

I don't think we can rely on bootloader knowing a thing about DT on
these devices..

OTOH I don't really think it is a big problem for both panels to
probe.  But I suppose it might be possible to have something somewhere
in the kernel that realizes and disables the unused panels.

> > +               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.

it matches /chosen/panel-id.. in this case I'm not sure how the
panel-id's are assigned, but for our purposes all that matters is that
they are assigned.

> 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.

In the non-bridge case (panel hooked directly to dsi controller), the
dsi controller could use the same ports {} mechanism.

For multiple displays, we could extend, I suppose, /chosen/panel-id to
be an array of id's indexed by display.  I think this is the type of
extension we could do later when the use-case comes up.  Just having
this solved upstream for single display would already be a huge
advancement.  (You don't want to look at how this is solved downstream
for android phones.)

Btw, if you are curious how this works on windows/ACPI, the ACPI
tables have entries for each of the panels.  The kernel is expected to
take the panel-id from that EFI variable that jhugo mentioned, and
pass it to a _ROM method which returns the appropriate panel table.
(Not entirely sure how the orchestrate reading the EFI variable early,
since it does not appear to be available after ExitBootServices)

BR,
-R

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

[Rob Herring]

On Mon, Jul 1, 2019 at 8:42 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?
>
> see jhugo's response, he has I guess more access to the bootloader than I
>
> > > +
> > > +/ {
> > > +       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'?
>
> I don't think we can rely on bootloader knowing a thing about DT on
> these devices..

But libstub 

> OTOH I don't really think it is a big problem for both panels to
> probe.  But I suppose it might be possible to have something somewhere
> in the kernel that realizes and disables the unused panels.

For starters, the refcount for regulator(s) would be off.

What does DRM do when multiple panels show up?

> > > +               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.
>
> it matches /chosen/panel-id.. in this case I'm not sure how the
> panel-id's are assigned, but for our purposes all that matters is that
> they are assigned.
>
> > 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.
>
> In the non-bridge case (panel hooked directly to dsi controller), the
> dsi controller could use the same ports {} mechanism.

Perhaps, but that's not what we do today and I don't think that's necessary. We can have multiple panels as child nodes. However, we'd probably have colliding addresses (reg) and only 1 should be enabled.

> For multiple displays, we could extend, I suppose, /chosen/panel-id to
> be an array of id's indexed by display.  I think this is the type of
> extension we could do later when the use-case comes up.  Just having
> this solved upstream for single display would already be a huge
> advancement.  (You don't want to look at how this is solved downstream
> for android phones.)

Then you need some way to define which display is which index. Given this touches 

> Btw, if you are curious how this works on windows/ACPI, the ACPI
> tables have entries for each of the panels.  The kernel is expected to
> take the panel-id from that EFI variable that jhugo mentioned, and
> pass it to a _ROM method which returns the appropriate panel table.
> (Not entirely sure how the orchestrate reading the EFI variable early,
> since it does not appear to be available after ExitBootServices)

I do think the entire picture needs to be spelled out including how any display handoff is handled. I guess EFI FB is used here? My initial thought is just add a panel id property to each panel node and then the kernel w

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

[Rob Clark]

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>;
        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

[1] https://github.com/robclark/edk2/commits/dtbloader

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

[Rob Clark]

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
>
> [1] https://github.com/robclark/edk2/commits/dtbloader

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

[Laurent Pinchart]

Hi Rob,

Thank you for the patch.

On Sun, Jun 30, 2019 at 01:36:04PM -0700, Rob Clark 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.)

I have to ask the obvious question: why doesn't the boot loader just
pass a correct DT to Linux ? There's no point in passing a list of
panels that are not there, this seems quite a big hack to me. A proper
boot loader should construct the DT based on hardware detection.

> 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>;
> 				};
> 			};
> 		};
> 	}
> };
> 
> Note that the panel-id is potentially a sparse-int.  The values I've
> seen so far on aarch64 laptops are:
> 
>   * 0xc2
>   * 0xc3
>   * 0xc4
>   * 0xc5
>   * 0x8011
>   * 0x8012
>   * 0x8055
>   * 0x8056
> 
> At least on snapdragon aarch64 laptops, they can be any u32 value.
> 
> However, on these laptops, the bootloader/firmware is not populating the
> chosen node, but instead providing an "UEFIDisplayInfo" variable, which
> contains the panel id.  Unfortunately EFI variables are only available
> before ExitBootServices, so the second patch checks for this variable
> before EBS and populates the /chosen/panel-id variable.
> 
> [1] https://patchwork.freedesktop.org/series/63001/
> 
> Rob Clark (4):
>   dt-bindings: chosen: document panel-id binding
>   efi/libstub: detect panel-id
>   drm: add helper to lookup panel-id
>   drm/bridge: ti-sn65dsi86: use helper to lookup panel-id
> 
>  Documentation/devicetree/bindings/chosen.txt | 69 ++++++++++++++++++++
>  drivers/firmware/efi/libstub/arm-stub.c      | 49 ++++++++++++++
>  drivers/firmware/efi/libstub/efistub.h       |  2 +
>  drivers/firmware/efi/libstub/fdt.c           |  9 +++
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c        |  5 +-
>  drivers/gpu/drm/drm_of.c                     | 21 ++++++
>  include/drm/drm_of.h                         |  7 ++
>  7 files changed, 160 insertions(+), 2 deletions(-)

-- 
Regards,

Laurent Pinchart
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

[Rob Clark]

On Sun, Jun 30, 2019 at 1:47 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> Thank you for the patch.
>
> On Sun, Jun 30, 2019 at 01:36:04PM -0700, Rob Clark 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.)
>
> I have to ask the obvious question: why doesn't the boot loader just
> pass a correct DT to Linux ? There's no point in passing a list of
> panels that are not there, this seems quite a big hack to me. A proper
> boot loader should construct the DT based on hardware detection.

Hi Laurent,

Actually the bootloader on these devices is passing *no* dt (they boot
ACPI, we are loading dtb from grub currently)

I think normally a device built w/ dt in mind would populate
/chosen/panel-id directly (rather than the way it is currently
populated based on reading an efi variable prior to ExitBootServices).
But that is considerably easier ask than having it re-write of_graph
bindings.  Either way, we aren't in control of the bootloader on these
devices, so it is a matter of coming up with something that works on
actual hw that we don't like rather than idealized hw that we don't
have ;-)

BR,
-R

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

[Laurent Pinchart]

Hi Rob,

On Sun, Jun 30, 2019 at 02:05:21PM -0700, Rob Clark wrote:
> On Sun, Jun 30, 2019 at 1:47 PM Laurent Pinchart wrote:
> > On Sun, Jun 30, 2019 at 01:36:04PM -0700, Rob Clark 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.)
> >
> > I have to ask the obvious question: why doesn't the boot loader just
> > pass a correct DT to Linux ? There's no point in passing a list of
> > panels that are not there, this seems quite a big hack to me. A proper
> > boot loader should construct the DT based on hardware detection.
> 
> Hi Laurent,
> 
> Actually the bootloader on these devices is passing *no* dt (they boot
> ACPI, we are loading dtb from grub currently)

Ah, the broken promises of ACPI on ARM64. I wonder how long it will take
before a public acknowledgement that it was a bad idea. Bad ideas happen
and can be forgiven, but stubborness in claiming it was the right
decision is another story.

(Not that you can be blamed for this of course :-))

> I think normally a device built w/ dt in mind would populate
> /chosen/panel-id directly (rather than the way it is currently
> populated based on reading an efi variable prior to ExitBootServices).
> But that is considerably easier ask than having it re-write of_graph
> bindings. Either way, we aren't in control of the bootloader on these
> devices,

If you can't control the initial boot loader, then I see two options,
none of which you will like I'm afraid.

- As you pass the DT to Linux from grub, there's your intermediate boot
  loader where you can construct a valid DT.

- If the ACPI cult needs to be venerated, then drivers should be
  converted to support ACPI without the need for DT.

A possible a middleground could be a platform driver (in
drivers/firmware/efi/ ? in drivers/platform/ ?) that will patch the DT
to instantiate the right panel based on the information retrieved from
the boot loader. We will need something similar for the Intel IPU3
camera driver, as Intel decided to come up with two different ACPI
"bindings", one for Windows and one for Chrome OS, leaving Windows
machine impossible to handle from a kernel driver due to required
information being hardcoded in Windows drivers shipped by Intel. This is
thus an option that may (unfortunately) need to become more widespread
for ACPI-based systems.

> so it is a matter of coming up with something that works on actual hw
> that we don't like rather than idealized hw that we don't have ;-)

That doesn't however justify not going for the best solution we can
achieve. What do you like best in the above ? :-)

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rob Clark]

On Sun, Jun 30, 2019 at 2:15 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> On Sun, Jun 30, 2019 at 02:05:21PM -0700, Rob Clark wrote:
> > On Sun, Jun 30, 2019 at 1:47 PM Laurent Pinchart wrote:
> > > On Sun, Jun 30, 2019 at 01:36:04PM -0700, Rob Clark 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.)
> > >
> > > I have to ask the obvious question: why doesn't the boot loader just
> > > pass a correct DT to Linux ? There's no point in passing a list of
> > > panels that are not there, this seems quite a big hack to me. A proper
> > > boot loader should construct the DT based on hardware detection.
> >
> > Hi Laurent,
> >
> > Actually the bootloader on these devices is passing *no* dt (they boot
> > ACPI, we are loading dtb from grub currently)
>
> Ah, the broken promises of ACPI on ARM64. I wonder how long it will take
> before a public acknowledgement that it was a bad idea. Bad ideas happen
> and can be forgiven, but stubborness in claiming it was the right
> decision is another story.
>
> (Not that you can be blamed for this of course :-))

To be fair, I think the only blame here is that MS let qcom get away
with some things in their ACPI and UEFI implementation..  I think
we'll need to shift to ACPI eventually for these laptops, in order to
keep up.  DT isn't a thing that would scale with the volume of x86
laptops that exist, and if aarch64 laptops get there too, we'll need
ACPI.  Lets face it, the # of different dt devices supported upstream
is a drop in the bucket compared to number of *actually physically
different* x86 devices supported by upstream.  (And I don't mean
individual models of laptops, but different production runs where they
picked a different panel or trackpad or whatever.)

But we have a lot of upstream work to get there to support how ACPI
works on these things:

 * The new Platform Extension Plugin (PEP) model for device power
   control
 * untangling drm bridge hookup from DT
 * untangling drm panel hook from DT
 * figuring out how to deal with mis-matches between dt device
   model and ACPI device model

There is some early work for ACPI support for these devices, but
realistically I think it is going to take a better part of a year to
get there.  Until then we rely on DT.

That isn't to say my proposal doesn't make a ton of sense.  We also
need to solve this problem for DT based devices, and I think
/chosen/panel-id makes a *ton* of sense for those devices.

> > I think normally a device built w/ dt in mind would populate
> > /chosen/panel-id directly (rather than the way it is currently
> > populated based on reading an efi variable prior to ExitBootServices).
> > But that is considerably easier ask than having it re-write of_graph
> > bindings. Either way, we aren't in control of the bootloader on these
> > devices,
>
> If you can't control the initial boot loader, then I see two options,
> none of which you will like I'm afraid.
>
> - As you pass the DT to Linux from grub, there's your intermediate boot
>   loader where you can construct a valid DT.

not really a solution that is going to scale

> - If the ACPI cult needs to be venerated, then drivers should be
>   converted to support ACPI without the need for DT.

we're working on it

> A possible a middleground could be a platform driver (in
> drivers/firmware/efi/ ? in drivers/platform/ ?) that will patch the DT
> to instantiate the right panel based on the information retrieved from
> the boot loader. We will need something similar for the Intel IPU3
> camera driver, as Intel decided to come up with two different ACPI
> "bindings", one for Windows and one for Chrome OS, leaving Windows
> machine impossible to handle from a kernel driver due to required
> information being hardcoded in Windows drivers shipped by Intel. This is
> thus an option that may (unfortunately) need to become more widespread
> for ACPI-based systems.

again, a kernel (or bootloader) side massively intrusive re-write the
dt approach isn't going to scale.  If you keep it simple, ie.
/chosen/panel-id I can see a possibility to move my patch from
drivers/firmware/efi into an earlier stage.  But if it has to re-write
graph, that falls apart as soon as a new device comes along with a
different bridge, or perhaps some vendor decides to use dsi directly
and forego the bridge.

usually (from what I've seen so far) there are a few gpios to probe to
decide which panel you have.  So after a few lines of gpio banging you
can either ask fw engineers to set appropriate node in chosen.. or
re-write of_graph bindings.  I think the former has a chance of
gaining traction on android devices.. latter not so much.  You are
really making too big of an ask for fw engineers ;-)

> > so it is a matter of coming up with something that works on actual hw
> > that we don't like rather than idealized hw that we don't have ;-)
>
> That doesn't however justify not going for the best solution we can
> achieve. What do you like best in the above ? :-)

I want a solution that is achievable ;-)

BR,
-R
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

[Rob Clark]

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
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno