2022-04-14 14:01:01

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding

Hi Samuel,

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

https://lore.kernel.org/linux-devicetree/[email protected]/

On Wed, 13 Apr 2022 17:19:02 -0500
Samuel Holland <[email protected]> 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


2022-04-16 01:03:31

by Samuel Holland

[permalink] [raw]
Subject: Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding

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/[email protected]/
>
> On Wed, 13 Apr 2022 17:19:02 -0500
> Samuel Holland <[email protected]> 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

2022-04-16 01:35:33

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding

On Thu, 14 Apr 2022 22:00:09 -0500
Samuel Holland <[email protected]> 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/[email protected]/
> >
> > On Wed, 13 Apr 2022 17:19:02 -0500
> > Samuel Holland <[email protected]> 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