Re: [PATCH 3/5] dt-bindings: display: simple-bridge: Document DPI color encoder

Liu Ying
2025-03-11T02:28:17Z

On 03/10/2025, Maxime Ripard wrote:
> On Fri, Mar 07, 2025 at 11:25:40AM +0800, Liu Ying wrote:
>> On 03/07/2025, Rob Herring wrote:
>>> On Thu, Mar 06, 2025 at 12:35:49PM +0100, Maxime Ripard wrote:
>>>> On Thu, Mar 06, 2025 at 03:02:41PM +0800, Liu Ying wrote:
>>>>> On 03/06/2025, Rob Herring wrote:
>>>>>> On Wed, Mar 05, 2025 at 10:35:26AM +0100, Alexander Stein wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Am Dienstag, 4. März 2025, 16:23:20 CET schrieb Rob Herring:
>>>>>>>> On Tue, Mar 04, 2025 at 06:15:28PM +0800, Liu Ying wrote:
>>>>>>>>> A DPI color encoder, as a simple display bridge, converts input DPI color
>>>>>>>>> coding to output DPI color coding, like Adafruit Kippah DPI hat[1] which
>>>>>>>>> converts input 18-bit pixel data to 24-bit pixel data(with 2 low padding
>>>>>>>>> bits in every color component though). Document the DPI color encoder.
>>>>>>>>
>>>>>>>> Why do we need a node for this? Isn't this just wired how it is wired
>>>>>>>> and there's nothing for s/w to see or do? I suppose if you are trying to
>>>>>>>> resolve the mode with 24-bit on one end and 18-bit on the other end, you
>>>>>>>> need to allow that and not require an exact match. You still might need
>>>>>>>> to figure out which pins the 18-bit data comes out on, but you have that
>>>>>>>> problem with an 18-bit panel too. IOW, how is this any different if you
>>>>>>>> have an 18-bit panel versus 24-bit panel?
>>>>>>>
>>>>>>> Especially panel-simple.c has a fixed configuration for each display, such as:
>>>>>>>> .bus_format = MEDIA_BUS_FMT_RGB666_1X18
>>>>>>>
>>>>>>> How would you allow or even know it should be addressed as
>>>>>>> MEDIA_BUS_FMT_RGB888_1X24 instead? I see different ways:
>>>>>>> 1. Create a new display setting/compatible
>>>>>>> 2. Add an overwrite property to the displays
>>>>>>> 3. Use a (transparent) bridge (this series)
>>>>>>>
>>>>>>> Number 1 is IMHO out of question.
>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>>> I personally don't like number 2 as this
>>>>>>> feels like adding quirks to displays, which they don't have.
>>>>>>
>>>>>> This is what I would do except apply it to the controller side. We know
>>>>>> the panel side already. This is a board variation, so a property makes
>>>>>> sense. I don't think you need any more than knowing what's on each end.
>>>>>
>>>>> With option 2, no matter putting a property in source side or sink side,
>>>>> impacted display drivers and DT bindings need to be changed, once a board
>>>>> manipulates the DPI color coding. This adds burdens and introduces new
>>>>> versions of those DT bindings. Is this what we want?
>>>>
>>>> There's an option 4: make it a property of the OF graph endpoints. In
>>>> essence, it's similar to properties that are already there like
>>>> lane-mapping, and it wouldn't affect the panel drivers, or create an
>>>> intermediate bridge.
>>>
>>> Yes, that's actually where I meant to put the property(ies).
>>
>> Put optional dpi-color-coding or something else in endpoint-base?
>
> I'm not sure what you mean by endpoint base, but it would be just like

I meant the endpoint-base definition in graph.yaml.

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/graph.yaml#L37

If optional dpi-color-coding property or something else is put there, then
any existing DT binding doc which references it starts to contain the
additional property automatically, which means those existing DT binding docs
don't need any change. I'm not saying that this is ok to do(due to burdens
added by driver modification and maintainence) - I still think option 3 is the
right thing to do.

> data-lanes, on the endpoint itself, right next to remote-endpoint. Given
> the nomenclature we have, something like "color-format" or
> "color-encoding", and taking the media format bus as value.

This requires to change drivers and maintain the change, which adds burdens.

>
>> Assuming it's optional, then it implies that it will overwrite OS's
>> setting, which sounds kinda awkward, because it is supposed to be
>> required to describe the actual color coding.
>
> I'm sorry, I don't understand what you mean here. Your bridge would have

I meant an optional new property is not that welcomed and it is supposed
to be required.

> been optional as well, right?

No, if _no_ additional property is added to endpoint-base in graph.yaml or
to video-interfaces.yaml, then my bridge would be required, not optional,
because that would be the only way to support DPI color encoding.

>
> Worst case scenario, your driver could make that property mandatory on
> its endpoints. Plenty of drivers are doing it.

This adds a new property to existing DT binding docs(yet another new version
of DT binding doc) and requires modifications on existing drivers, which again
adds burdens. It's also a burden for developers to support that property in
new DT binding docs and new drivers. For existing DT binding docs and drivers
which are using that property, that's fine and good to them.

>
> Maxime

--
Regards,
Liu Ying