Re: [PATCH 7/7] dt-bindings: display: add panel-timing property to sitronix,st7789v

[Michael Riesch]

Hi Maxime,

On 3/30/23 16:58, Maxime Ripard wrote:
> On Wed, Mar 29, 2023 at 12:08:50PM +0200, Michael Riesch wrote:
>> On 3/29/23 11:16, Maxime Ripard wrote:
>>> On Thu, Mar 16, 2023 at 11:29:53PM +0100, Michael Riesch wrote:
>>>> Hi Rob,
>>>>
>>>> On 3/16/23 22:57, Rob Herring wrote:
>>>>> On Tue, Mar 14, 2023 at 12:56:44PM +0100, Gerald Loacker wrote:
>>>>>> 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.
>>>>>
>>>>>> 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?
>>>
>>> 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?
> 
> 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/20230317232355.1554980-1-sre@xxxxxxxxxx/
We just need a way to resolve the conflicts between the two series.

Cc: Sebastian

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

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

Thanks and best regards,
Michael