Add timing override to sitronix,st7789v

From: Michael Riesch <[email protected]>

The sitronix-st7789v driver now considers the rotation property. Add the property to the documentation.

Signed-off-by Michael Riesch <[email protected]>

Signed-off-by Gerald Loacker <[email protected]>

---

The sitronix-st7789v driver now considers the panel-timing property. Add the property to the documentation.

Signed-off-by Gerald Loacker <[email protected]>

The sitronix-st7789v driver now considers the panel-timing property.

Add the property to the documentation.

Signed-off-by Gerald Loacker <[email protected]>

---
 .../display/panel/sitronix,st7789v.yaml         | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml +17 -0

index ed942cd3620f..8810f123dedf 100644
@@ -21,6 +21,7 @@ properties:
   reset-gpios: true
   power-supply: true
   backlight: true
+  panel-timing: true
   port: true
   rotation: true
 

@@ -54,6 +55,22 @@ examples:
             spi-cpol;
             spi-cpha;
 
+            panel-timing {
+                clock-frequency = <7000000>;
+                hactive = <240>;
+                vactive = <320>;
+                hfront-porch = <38>;
+                hback-porch = <10>;
+                hsync-len = <10>;
+                vfront-porch = <8>;
+                vback-porch = <4>;
+                vsync-len = <4>;
+                hsync-active = <1>;
+                vsync-active = <1>;
+                de-active = <1>;
+                pixelclk-active = <1>;
+            };
+
             port {
                 panel_input: endpoint {
                     remote-endpoint = <&tcon0_out_panel>;

Signature · 1 line

2.37.2

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

Quoted 45 lines

Signed-off-by Gerald Loacker <[email protected]>

---
 .../display/panel/sitronix,st7789v.yaml         | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml +17 -0

index ed942cd3620f..8810f123dedf 100644
@@ -21,6 +21,7 @@ properties:
   reset-gpios: true
   power-supply: true
   backlight: true
+  panel-timing: true
   port: true
   rotation: true
 

@@ -54,6 +55,22 @@ examples:
             spi-cpol;
             spi-cpha;
 
+            panel-timing {
+                clock-frequency = <7000000>;
+                hactive = <240>;
+                vactive = <320>;
+                hfront-porch = <38>;
+                hback-porch = <10>;
+                hsync-len = <10>;
+                vfront-porch = <8>;
+                vback-porch = <4>;
+                vsync-len = <4>;
+                hsync-active = <1>;
+                vsync-active = <1>;
+                de-active = <1>;
+                pixelclk-active = <1>;
+            };
+
             port {
                 panel_input: endpoint {
                     remote-endpoint = <&tcon0_out_panel>;

Signature · 1 line

2.37.2

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

What do you think about that?

Quoted 26 lines · Michael Riesch

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

Quoted 1 line

What do you think about that?

I don't see why we would need the device tree to support that. What you described is essentially what overscan is for HDMI/analog output, and we already have everything to deal with overscan properly in KMS.

Quoted 27 lines · Maxime Ripard

Quoted 26 lines · Michael Riesch

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

Quoted 5 lines

Quoted 1 line

What do you think about that?

I don't see why we would need the device tree to support that. What you described is essentially what overscan is for HDMI/analog output, and we already have everything to deal with overscan properly in KMS.

Thanks for your response, but I am afraid I don't quite follow.

How are we supposed to expose control over the hsync/vsync/data enable polarity? I only know that the display controller and the panel need to agree on a setting that works for both. What is the canonical way to do this?

A different question is the partial mode, for which (IIUC) you suggest the overscan feature. As I have never heard of this before, it would be very nice if you could point me to some examples. Where would the effective resolution be set in this case?

Quoted 34 lines · Michael Riesch

Quoted 27 lines · Maxime Ripard

Quoted 26 lines · Michael Riesch

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

Quoted 5 lines

Quoted 1 line

What do you think about that?

I don't see why we would need the device tree to support that. What you described is essentially what overscan is for HDMI/analog output, and we already have everything to deal with overscan properly in KMS.

Quoted 6 lines

Thanks for your response, but I am afraid I don't quite follow.

How are we supposed to expose control over the hsync/vsync/data enable polarity? I only know that the display controller and the panel need to agree on a setting that works for both. What is the canonical way to do this?

So typically, it would come from the panel datasheet and would thus be exposed by the panel driver. st7789v is not a panel itself but a (pretty flexible) panel controller so it's not fixed and I don't think we have a good answer to that (yet).

Quoted 4 lines

A different question is the partial mode, for which (IIUC) you suggest the overscan feature. As I have never heard of this before, it would be very nice if you could point me to some examples. Where would the effective resolution be set in this case?

So, back when CRT were a thing the edges of the tube were masked by the plastic case. HDMI inherited from that and that's why you still have some UI on some devices (like consoles) to setup the active area of the display.

The underlying issue is exactly what you describe: the active area is larger than what the plastic case allows to see. I don't think anyone ever had the usecase you have, but it would be the right solution to me to solve essentially the same issue the same way we do on other output types.

Quoted 42 lines · Maxime Ripard

Quoted 40 lines · Michael Riesch

Quoted 27 lines · Maxime Ripard

Quoted 26 lines · Michael Riesch

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

Quoted 5 lines

Quoted 1 line

What do you think about that?

I don't see why we would need the device tree to support that. What you described is essentially what overscan is for HDMI/analog output, and we already have everything to deal with overscan properly in KMS.

Thanks for your response, but I am afraid I don't quite follow.

How are we supposed to expose control over the hsync/vsync/data enable polarity? I only know that the display controller and the panel need to agree on a setting that works for both. What is the canonical way to do this?

Quoted 4 lines

So typically, it would come from the panel datasheet and would thus be exposed by the panel driver. st7789v is not a panel itself but a (pretty flexible) panel controller so it's not fixed and I don't think we have a good answer to that (yet).

Then it seems to me that creating a panel driver (= st8879v panel controller driver with a new compatible) would make sense.

By coincidence Sebastian Reichel has come up with this approach recently, see
https://lore.kernel.org/dri-devel/[email protected]/
We just need a way to resolve the conflicts between the two series.

Cc Sebastian

Quoted 10 lines

Quoted 4 lines

A different question is the partial mode, for which (IIUC) you suggest the overscan feature. As I have never heard of this before, it would be very nice if you could point me to some examples. Where would the effective resolution be set in this case?

So, back when CRT were a thing the edges of the tube were masked by the plastic case. HDMI inherited from that and that's why you still have some UI on some devices (like consoles) to setup the active area of the display.

Quoted 5 lines

The underlying issue is exactly what you describe: the active area is larger than what the plastic case allows to see. I don't think anyone ever had the usecase you have, but it would be the right solution to me to solve essentially the same issue the same way we do on other output types.

OK, we'll look into the overscan feature. But still the information about the active area should come from the driver, right?

Quoted 43 lines · Michael Riesch

Quoted 42 lines · Maxime Ripard

Quoted 40 lines · Michael Riesch

Quoted 27 lines · Maxime Ripard

Quoted 26 lines · Michael Riesch

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 1 line · Gerald Loacker

The sitronix-st7789v driver now considers the panel-timing property.

I read the patch for that and still don't know 'why'. Commit messages should answer why.

Quoted 1 line

Add the property to the documentation.

We generally don't put timings in DT for panels. Why is this one special?

For now, having the timings in the device tree allows for setting the hsync/vsync/de polarity.

As a next step, we aim to implement the partial mode feature of this panel. It is possible to use only a certain region of the panel, which is helpful e.g., when a part of the panel is occluded and should not be considered by DRM. We thought that this could be specified as timing in DT.

(The hactive and vactive properties serve as dimensions of this certain region, of course. We still need to specify somehow the position of the region. Maybe with additional properties hactive-start and vactive-start?)

Quoted 5 lines

Quoted 1 line

What do you think about that?

I don't see why we would need the device tree to support that. What you described is essentially what overscan is for HDMI/analog output, and we already have everything to deal with overscan properly in KMS.

Thanks for your response, but I am afraid I don't quite follow.

How are we supposed to expose control over the hsync/vsync/data enable polarity? I only know that the display controller and the panel need to agree on a setting that works for both. What is the canonical way to do this?

Quoted 7 lines

Quoted 4 lines

So typically, it would come from the panel datasheet and would thus be exposed by the panel driver. st7789v is not a panel itself but a (pretty flexible) panel controller so it's not fixed and I don't think we have a good answer to that (yet).

Then it seems to me that creating a panel driver (= st8879v panel controller driver with a new compatible) would make sense.

I don't see why? The entire controller is the same except (maybe) for some initialization data. Doing a new driver for it seems like taking the easy way out?

Quoted 6 lines

By coincidence Sebastian Reichel has come up with this approach recently, see
https://lore.kernel.org/dri-devel/[email protected]/
We just need a way to resolve the conflicts between the two series.

Cc Sebastian

That's not a new driver though? That approach looks sane to me.

Quoted 16 lines

Quoted 15 lines

Quoted 4 lines

A different question is the partial mode, for which (IIUC) you suggest the overscan feature. As I have never heard of this before, it would be very nice if you could point me to some examples. Where would the effective resolution be set in this case?

So, back when CRT were a thing the edges of the tube were masked by the plastic case. HDMI inherited from that and that's why you still have some UI on some devices (like consoles) to setup the active area of the display.

The underlying issue is exactly what you describe: the active area is larger than what the plastic case allows to see. I don't think anyone ever had the usecase you have, but it would be the right solution to me to solve essentially the same issue the same way we do on other output types.

Quoted 2 lines

OK, we'll look into the overscan feature. But still the information about the active area should come from the driver, right?

No, the userspace is in charge there.

Quoted 16 lines

Quoted 16 lines

Quoted 15 lines

Quoted 4 lines

A different question is the partial mode, for which (IIUC) you suggest the overscan feature. As I have never heard of this before, it would be very nice if you could point me to some examples. Where would the effective resolution be set in this case?

So, back when CRT were a thing the edges of the tube were masked by the plastic case. HDMI inherited from that and that's why you still have some UI on some devices (like consoles) to setup the active area of the display.

The underlying issue is exactly what you describe: the active area is larger than what the plastic case allows to see. I don't think anyone ever had the usecase you have, but it would be the right solution to me to solve essentially the same issue the same way we do on other output types.

Quoted 4 lines

Quoted 2 lines

OK, we'll look into the overscan feature. But still the information about the active area should come from the driver, right?

No, the userspace is in charge there.

I'd prefer not to have the hardware description in user space. But we can continue this discussing once our v2 is out.

This patch set adds additional functionality to the sitronix,st7789v driver.

Patches 1,3 and 4 propagate useful flags to the drm subsystem. Patch 2 adds the orientation property.
Patch 5 parses the device tree for a panel-timing and makes it possible to
override the default timing.
Patches 6 and 7 add the new properties to the dt-bindings.

Gerald Loacker (4):
drm/panel: sitronix-st7789v: propagate h/v-sync polarity drm/panel: sitronix-st7789v: add bus_flags to connector drm/panel: sitronix-st7789v: parse device tree to override timing mode
dt-bindings: display: add panel-timing property to sitronix,st7789v

Michael Riesch (3):
drm/panel: sitronix-st7789v: propagate RGB666 format drm/panel: sitronix-st7789v: add panel orientation support
dt-bindings: display: add rotation property to sitronix,st7789v

 .../display/panel/sitronix,st7789v.yaml       |  19 ++
 .../gpu/drm/panel/panel-sitronix-st7789v.c    | 204 +++++++++++++++---
 2 files changed, 191 insertions(+), 32 deletions(-)

This patch set adds additional functionality to the sitronix,st7789v driver.

Patches 1,3 and 4 propagate useful flags to the drm subsystem. Patch 2 adds the orientation property.

Patch 5 parses the device tree for a panel-timing and makes it possible to
override the default timing.
Patches 6 and 7 add the new properties to the dt-bindings.

Gerald Loacker (4):
drm/panel: sitronix-st7789v: propagate h/v-sync polarity drm/panel: sitronix-st7789v: add bus_flags to connector drm/panel: sitronix-st7789v: parse device tree to override timing mode
dt-bindings: display: add panel-timing property to sitronix,st7789v

Michael Riesch (3):
drm/panel: sitronix-st7789v: propagate RGB666 format drm/panel: sitronix-st7789v: add panel orientation support
dt-bindings: display: add rotation property to sitronix,st7789v

 .../display/panel/sitronix,st7789v.yaml       |  19 ++
 .../gpu/drm/panel/panel-sitronix-st7789v.c    | 204 +++++++++++++++---
 2 files changed, 191 insertions(+), 32 deletions(-)