[RFC,02/16] dt-bindings: display: rockchip: Add EBC binding

Commit Message

Samuel Holland April 13, 2022, 10:19 p.m. UTC

The Rockchip E-Book Controller (EBC) is a controller for Electrophoretic
Displays (EPDs). It is self-contained; it does not interact directly
with the VOP or the RGA.

While two of the regulator consumers here actually power the display
panel, not the EBC hardware, they are consumed here because they are
only needed during display refreshes. They do not match the normal
panel prepare/enable lifecycle.

Signed-off-by: Samuel Holland <[e-mail]>
---

 .../display/rockchip/rockchip,rk3568-ebc.yaml | 106 ++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml

Comments

Andreas Kemnade April 14, 2022, 8:15 a.m. UTC | #1

Hi Samuel,

for comparison, here is my submission for the IMX EPDC bindings:

https://lore.kernel.org/linux-devicetree/[e-mail]/

On Wed, 13 Apr 2022 17:19:02 -0500
Samuel Holland <[e-mail]> wrote:

[...]
we have sy7636a driver in kernel which should be suitable for powering a EPD
and temperature measurement. So I would expect that to be 
> +  io-channels:
> +    maxItems: 1
> +    description: I/O channel for panel temperature measurement
> +
so how would I reference the hwmon/thermal(-zone) of the sy7636a here?

> +  panel-supply:
> +    description: Regulator supplying the panel's logic voltage
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  vcom-supply:
> +    description: Regulator supplying the panel's compensation voltage
> +
> +  vdrive-supply:
> +    description: Regulator supplying the panel's gate and source drivers
> +
SY7636a has only one logical regulator in kernel for for the latter two.

If we have a separate panel node, than maybe these regulators should go
there as they belong to the panel as they are powering the panel and
not the EBC.

> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: OF graph port for the attached display panel
> +
In my approach for the IMX EPDC, (I will send a better commented one
soon) I have no separate subnode to avoid messing with additional
display parameters. Not sure what is really better here.

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +  - panel-supply
> +  - vcom-supply
> +  - vdrive-supply

If things differ how the different phyiscally existing regulators are
mapped into logical ones (even the vdrive supply is still a bunch of
physical regulators mapped into one logical one), then not everything
can be required.

Regards,
Andreas

Samuel Holland April 15, 2022, 3 a.m. UTC | #2

Hi Andreas,

Thanks for the comments.

On 4/14/22 3:15 AM, Andreas Kemnade wrote:
> Hi Samuel,
> 
> for comparison, here is my submission for the IMX EPDC bindings:
> 
> https://lore.kernel.org/linux-devicetree/[e-mail]/
> 
> On Wed, 13 Apr 2022 17:19:02 -0500
> Samuel Holland <[e-mail]> wrote:
> 
> [...]
> we have sy7636a driver in kernel which should be suitable for powering a EPD
> and temperature measurement. So I would expect that to be 
>> +  io-channels:
>> +    maxItems: 1
>> +    description: I/O channel for panel temperature measurement
>> +
> so how would I reference the hwmon/thermal(-zone) of the sy7636a here?

It seems the consensus is to use a thermal zone for panel temperature, so I will
need to change this.

I think it's best to reference the thermal zone by phandle, not by name, even if
it requires extending the thermal zone API to support this.

>> +  panel-supply:
>> +    description: Regulator supplying the panel's logic voltage
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +  vcom-supply:
>> +    description: Regulator supplying the panel's compensation voltage
>> +
>> +  vdrive-supply:
>> +    description: Regulator supplying the panel's gate and source drivers
>> +
> SY7636a has only one logical regulator in kernel for for the latter two.

Both properties could point to the same regulator node if there are more
consumers than regulators. I don't know of a clean way to handle the opposite
situation.

The other benefit of separating out VCOM is that the controller or panel driver
can set a calibrated voltage from e.g. NVMEM or the panel's DT node.

> If we have a separate panel node, than maybe these regulators should go
> there as they belong to the panel as they are powering the panel and
> not the EBC.

I agree on this. It doesn't work with panel-simple, but as Maxime points out, we
have more flexibility with a custom panel driver.

>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description: OF graph port for the attached display panel
>> +
> In my approach for the IMX EPDC, (I will send a better commented one
> soon) I have no separate subnode to avoid messing with additional
> display parameters. Not sure what is really better here.

I tried to match the existing abstractions as much as possible, and I saw there
was already an "eink,vb3300-kca" display in panel-simple. I believe that one was
added for the reMarkable 2, where the existing LCD controller driver already
depends on the DRM panel code (although I have concerns about hooking that up to
a driver that doesn't understand EPDs).

My thought here is that the timings for a given panel should be the same across
controllers, both dedicated EPD controllers and LCD controllers. Or at least it
should be possible to derive the timings from some common set of parameters.

The panel node also usually hooks up to the backlight, although I am not sure
that is the right thing to do for EPDs. (And the PineNote has a separate issue
of having two backlights [warm/cool] for one display.)

Regards,
Samuel

Andreas Kemnade April 15, 2022, 11:45 a.m. UTC | #3

On Thu, 14 Apr 2022 22:00:09 -0500
Samuel Holland <[e-mail]> wrote:

> Hi Andreas,
> 
> Thanks for the comments.
> 
> On 4/14/22 3:15 AM, Andreas Kemnade wrote:
> > Hi Samuel,
> > 
> > for comparison, here is my submission for the IMX EPDC bindings:
> > 
> > https://lore.kernel.org/linux-devicetree/[e-mail]/
> > 
> > On Wed, 13 Apr 2022 17:19:02 -0500
> > Samuel Holland <[e-mail]> wrote:
> > 
> > [...]
> > we have sy7636a driver in kernel which should be suitable for powering a EPD
> > and temperature measurement. So I would expect that to be   
> >> +  io-channels:
> >> +    maxItems: 1
> >> +    description: I/O channel for panel temperature measurement
> >> +  
> > so how would I reference the hwmon/thermal(-zone) of the sy7636a here?  
> 
> It seems the consensus is to use a thermal zone for panel temperature, so I will
> need to change this.
> 
I am open to anything here as long as it fits together.

> I think it's best to reference the thermal zone by phandle, not by name, even if
> it requires extending the thermal zone API to support this.
> 
maybe referencing the hwmon might be interesting, or we add a hwmon_iio
adaptor. The other way round it is there. The thermal zone stuff is
only needed because hwmon cannot referenced directly.

Regards,
Andreas

Patch

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml
new file mode 100644
index 000000000000..957ca874ab02
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3568-ebc.yaml
@@ -0,0 +1,106 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/rockchip/rockchip,rk3568-ebc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip SoC E-Book Controller (EBC)
+
+description:
+  Rockchip EBC is a controller for Electrophoretic Displays (EPDs).
+
+maintainers:
+  - Samuel Holland <[e-mail]>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,rk3568-ebc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: AHB register clock
+      - description: Pixel clock
+
+  clock-names:
+    items:
+      - const: hclk
+      - const: dclk
+
+  resets:
+    items:
+      - description: hclk domain reset
+      - description: dclk domain reset
+
+  reset-names:
+    items:
+      - const: hclk
+      - const: dclk
+
+  io-channels:
+    maxItems: 1
+    description: I/O channel for panel temperature measurement
+
+  panel-supply:
+    description: Regulator supplying the panel's logic voltage
+
+  power-domains:
+    maxItems: 1
+
+  vcom-supply:
+    description: Regulator supplying the panel's compensation voltage
+
+  vdrive-supply:
+    description: Regulator supplying the panel's gate and source drivers
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description: OF graph port for the attached display panel
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - power-domains
+  - panel-supply
+  - vcom-supply
+  - vdrive-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/rk3568-power.h>
+
+    ebc: ebc@fdec0000 {
+      compatible = "rockchip,rk3568-ebc";
+      reg = <0x0 0xfdec0000 0x0 0x5000>;
+      interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&cru HCLK_EBC>, <&cru DCLK_EBC>;
+      clock-names = "hclk", "dclk";
+      resets = <&cru SRST_H_EBC>, <&cru SRST_D_EBC>;
+      reset-names = "hclk", "dclk";
+      power-domains = <&power RK3568_PD_RGA>;
+
+      panel-supply = <&v3p3>;
+      vcom-supply = <&vcom>;
+      vdrive-supply = <&vdrive>;
+
+      port {
+        ebc_out_panel: endpoint {
+          remote-endpoint = <&panel_in_ebc>;
+        };
+      };
+    };