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.

[1] https://learn.adafruit.com/adafruit-dpi-display-kippah-ttl-tft/downloads

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../display/bridge/simple-bridge.yaml         | 89 ++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
index 43cf4df9811a..c1747c033040 100644
--- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
@@ -27,6 +27,7 @@ properties:
           - const: adi,adv7123
       - enum:
           - adi,adv7123
+          - dpi-color-encoder
           - dumb-vga-dac
           - ti,opa362
           - ti,ths8134
@@ -37,13 +38,31 @@ properties:
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: The bridge input
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              dpi-color-coding: true
+
       port@1:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
         description: The bridge output
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              dpi-color-coding: true
+
     required:
       - port@0
       - port@1
@@ -61,6 +80,44 @@ required:
 
 additionalProperties: false
 
+allOf:
+  - $ref: /schemas/display/dpi-color-coding.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: dpi-color-encoder
+    then:
+      properties:
+        ports:
+          properties:
+            port@0:
+              properties:
+                endpoint:
+                  required:
+                    - dpi-color-coding
+
+            port@1:
+              properties:
+                endpoint:
+                  required:
+                    - dpi-color-coding
+    else:
+      properties:
+        ports:
+          properties:
+            port@0:
+              properties:
+                endpoint:
+                  properties:
+                    dpi-color-coding: false
+
+            port@1:
+              properties:
+                endpoint:
+                  properties:
+                    dpi-color-coding: false
+
 examples:
   - |
     bridge {
@@ -88,4 +145,32 @@ examples:
         };
     };
 
+  - |
+    bridge {
+        compatible = "dpi-color-enoder";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+
+                dpi_in: endpoint {
+                    remote-endpoint = <&dc_out>;
+                    dpi-color-coding = "18bit-configuration1";
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+
+                dpi_out: endpoint {
+                    remote-endpoint = <&panel_in>;
+                    dpi-color-coding = "24bit";
+                };
+            };
+        };
+    };
+
 ...
-- 
2.34.1

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?

Rob

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. I personally don't like number 2 as this
feels like adding quirks to displays, which they don't have.
Number 3 actually describe the hardware connection. The only impact for
software is to know which bus format it should use.

Best regards
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

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. 

> Number 3 actually describe the hardware connection. The only impact for
> software is to know which bus format it should use.

I'm not opposed to this, but only if it provides *something* that option 
2 does not. I'm not seeing what that is.

Node or not, either case needs a format property. We already have a 
variety of bus/pixel format related properties. I've rejected new ones 
because we need something common here that's flexible enough to handle 
any situation. That's either something that can describe any bit layout 
or something enumerating the formats (as MEDIA_BUS_FMT_* does). The 
former is hard to get right and there's always something else you can't 
handle. I'm not opposed to just reusing MEDIA_BUS_FMT_ if that works.

Rob

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?

> 
>> Number 3 actually describe the hardware connection. The only impact for
>> software is to know which bus format it should use.
> 
> I'm not opposed to this, but only if it provides *something* that option 
> 2 does not. I'm not seeing what that is.

Option 3 provides an intermediate bridge for both DT and OSes.  It makes
the DPI color coding manipulation transparent to source side and sink side.

> 
> Node or not, either case needs a format property. We already have a 
> variety of bus/pixel format related properties. I've rejected new ones 
> because we need something common here that's flexible enough to handle 
> any situation. That's either something that can describe any bit layout 
> or something enumerating the formats (as MEDIA_BUS_FMT_* does). The 
> former is hard to get right and there's always something else you can't 
> handle. I'm not opposed to just reusing MEDIA_BUS_FMT_ if that works.

Agree that the former is hard to get right.

Since this is all about DPI and we have that MIPI DPI standard document
which specifies color codings, maybe it's fine to start with those limited
color codings.

BTW, I'd admit that the artificial paddings are not described by this patch
set, though maybe no one cares about that.  The paddings could be floating/
low/high/other component bits.

> 
> Rob

-- 
Regards,
Liu Ying

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.

Maxime

On 03/06/2025, 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.

I don't see lane-mapping anywhere. Do you mean data-mapping instead?
data-mapping is not defined in dtschema. Only lvds-codec.yaml defines
data-mapping in endpoint.

With option 4, I guess you meant display sink drivers, i.e., panel and
bridge drivers, wouldn't be affected. Then, display source drivers, i.e.,
display controller and bridge drivers, would be affected. This adds
burdens for driver developers/maintainers(though almost no effort from
DT's PoV), doesn't it?

Moreover, why it has to be the display sink drivers which are not affected?
DT writers might choose to set the format at the sink endpoint, e.g., setting
RGB666 at the sink endpoint of a RGB888 DPI panel or bridge.

> 
> Maxime

-- 
Regards,
Liu Ying

On Fri, Mar 07, 2025 at 11:10:00AM +0800, Liu Ying wrote:
> On 03/06/2025, 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.
> 
> I don't see lane-mapping anywhere. Do you mean data-mapping instead?
> data-mapping is not defined in dtschema. Only lvds-codec.yaml defines
> data-mapping in endpoint.

I meant as a general concept. The properties are data-lanes and
clock-lanes in
Documentation/devicetree/bindings/media/video-interfaces.yaml

> With option 4, I guess you meant display sink drivers, i.e., panel and
> bridge drivers, wouldn't be affected. Then, display source drivers, i.e.,
> display controller and bridge drivers, would be affected. This adds
> burdens for driver developers/maintainers(though almost no effort from
> DT's PoV), doesn't it?

Not necessarily, panels have a phandle to the parent endpoint too so
they can do that walk and configure their format if it's any easier.

> Moreover, why it has to be the display sink drivers which are not affected?
> DT writers might choose to set the format at the sink endpoint, e.g., setting
> RGB666 at the sink endpoint of a RGB888 DPI panel or bridge.

Why wouldn't you run the panel at the highest bpc possible?

Maxime

On 03/10/2025, Maxime Ripard wrote:
> On Fri, Mar 07, 2025 at 11:10:00AM +0800, Liu Ying wrote:
>> On 03/06/2025, 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.
>>
>> I don't see lane-mapping anywhere. Do you mean data-mapping instead?
>> data-mapping is not defined in dtschema. Only lvds-codec.yaml defines
>> data-mapping in endpoint.
> 
> I meant as a general concept. The properties are data-lanes and
> clock-lanes in
> Documentation/devicetree/bindings/media/video-interfaces.yaml

This requires referenceing video-interfaces.yaml in existing DT binding docs
and driver modifictions, which adds burdens.

> 
>> With option 4, I guess you meant display sink drivers, i.e., panel and
>> bridge drivers, wouldn't be affected. Then, display source drivers, i.e.,
>> display controller and bridge drivers, would be affected. This adds
>> burdens for driver developers/maintainers(though almost no effort from
>> DT's PoV), doesn't it?
> 
> Not necessarily, panels have a phandle to the parent endpoint too so
> they can do that walk and configure their format if it's any easier.

I'm sorry, I don't get your meaning here.  I have no idea how to support
this new property in endpoint-base(graph.yaml) or video-interfaces.yaml
_without_ changing existing display source drivers.

> 
>> Moreover, why it has to be the display sink drivers which are not affected?
>> DT writers might choose to set the format at the sink endpoint, e.g., setting
>> RGB666 at the sink endpoint of a RGB888 DPI panel or bridge.
> 
> Why wouldn't you run the panel at the highest bpc possible?

Because hardware designers route less data signals than regular ones to the
DPI panel or bridge.

> 
> Maxime

-- 
Regards,
Liu Ying

On Tue, Mar 11, 2025 at 10:38:37AM +0800, Liu Ying wrote:
> On 03/10/2025, Maxime Ripard wrote:
> > On Fri, Mar 07, 2025 at 11:10:00AM +0800, Liu Ying wrote:
> >> On 03/06/2025, 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.
> >>
> >> I don't see lane-mapping anywhere. Do you mean data-mapping instead?
> >> data-mapping is not defined in dtschema. Only lvds-codec.yaml defines
> >> data-mapping in endpoint.
> > 
> > I meant as a general concept. The properties are data-lanes and
> > clock-lanes in
> > Documentation/devicetree/bindings/media/video-interfaces.yaml
> 
> This requires referenceing video-interfaces.yaml in existing DT binding docs
> and driver modifictions, which adds burdens.

If burden is an issue, any new driver also adds burden, thus we
shouldn't merge anything anymore and we can stop there?

It's not black and white, it should be managed, and here it's
manageable and / or mitigated.

> >> With option 4, I guess you meant display sink drivers, i.e., panel and
> >> bridge drivers, wouldn't be affected. Then, display source drivers, i.e.,
> >> display controller and bridge drivers, would be affected. This adds
> >> burdens for driver developers/maintainers(though almost no effort from
> >> DT's PoV), doesn't it?
> > 
> > Not necessarily, panels have a phandle to the parent endpoint too so
> > they can do that walk and configure their format if it's any easier.
> 
> I'm sorry, I don't get your meaning here.  I have no idea how to support
> this new property in endpoint-base(graph.yaml) or video-interfaces.yaml
> _without_ changing existing display source drivers.

Make a helper, and drivers can always call that helper. It will be a
single function call, and panel-simple will cover most of the existing
user-base already.

Otherwise, we have the option to use coccinelle if you want to mass
convert it.

> >> Moreover, why it has to be the display sink drivers which are not affected?
> >> DT writers might choose to set the format at the sink endpoint, e.g., setting
> >> RGB666 at the sink endpoint of a RGB888 DPI panel or bridge.
> > 
> > Why wouldn't you run the panel at the highest bpc possible?
> 
> Because hardware designers route less data signals than regular ones to the
> DPI panel or bridge.

Sure, and that's a hardware property, that's what your bridge or that
property would cover. It wouldn't cover why someone would want to change
*that*, or I didn't underdstand what you meant?

Maxime

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

Rob

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?
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. If it's required, then we would have to create
something like dpi-endpoint-base, but existing display device DT nodes are
based on endpoint-base.

> 
> Rob

-- 
Regards,
Liu Ying

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

> 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
been optional as well, right?

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

Maxime

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

Hi,

On Tue, Mar 11, 2025 at 10:29:28AM +0800, Liu Ying wrote:
> 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

I don't think it should be put in the graph binding document, but either
in video-interfaces, or, since it's mostly used by v4l2, in a similar
but separate document for DRM.

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

If the property is generic, and the support for it done well enough,
it's probably going to be a single function call in drivers. I wouldn't
call that a burden.

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

Not that welcomed by whom?

> and it is supposed to be required.

Not supposed to be required by whom?

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

I mean, you can turn that any way you like, but we've supported the
setups your bridge needs to cover fine so far. Your work would
definitely improve the situation, but it's definitely not going to be
mandatory. Just like the color encoding property wouldn't be.

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

Both Rob and I are maintainers of the affected code, let us worry about
the maintenance burden.

Maxime

On Tue, 04 Mar 2025 18:15:28 +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.
> 
> [1] https://learn.adafruit.com/adafruit-dpi-display-kippah-ttl-tft/downloads
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  .../display/bridge/simple-bridge.yaml         | 89 ++++++++++++++++++-
>  1 file changed, 87 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/display/bridge/simple-bridge.example.dtb: /example-1/bridge: failed to match any schema with compatible: ['dpi-color-enoder']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250304101530.969920-4-victor.liu@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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.
> 
> [1] https://learn.adafruit.com/adafruit-dpi-display-kippah-ttl-tft/downloads
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

So it's just a bunch of signals that aren't wired / set to the ground? I
assume to free a few GPIOs?

I guess that makes sense.

> ---
>  .../display/bridge/simple-bridge.yaml         | 89 ++++++++++++++++++-
>  1 file changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> index 43cf4df9811a..c1747c033040 100644
> --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
> @@ -27,6 +27,7 @@ properties:
>            - const: adi,adv7123
>        - enum:
>            - adi,adv7123
> +          - dpi-color-encoder

I don't think we can claim that there's a generic DPI color encoder.
Maybe we can add the prefix dumb or transparent?

>            - dumb-vga-dac
>            - ti,opa362
>            - ti,ths8134
> @@ -37,13 +38,31 @@ properties:
>  
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/properties/port
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
>          description: The bridge input
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              dpi-color-coding: true
> +
>        port@1:
> -        $ref: /schemas/graph.yaml#/properties/port
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
>          description: The bridge output
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              dpi-color-coding: true
> +
>      required:
>        - port@0
>        - port@1
> @@ -61,6 +80,44 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - $ref: /schemas/display/dpi-color-coding.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: dpi-color-encoder
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              properties:
> +                endpoint:
> +                  required:
> +                    - dpi-color-coding
> +
> +            port@1:
> +              properties:
> +                endpoint:
> +                  required:
> +                    - dpi-color-coding
> +    else:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              properties:
> +                endpoint:
> +                  properties:
> +                    dpi-color-coding: false
> +
> +            port@1:
> +              properties:
> +                endpoint:
> +                  properties:
> +                    dpi-color-coding: false
> +

Also it complicates the binding enough to warrant for another binding for
that specific "component".

>  examples:
>    - |
>      bridge {
> @@ -88,4 +145,32 @@ examples:
>          };
>      };
>  
> +  - |
> +    bridge {
> +        compatible = "dpi-color-enoder";

You have a typo in the compatible.

Maxime

On 03/04/2025, Maxime Ripard wrote:
> 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.
>>
>> [1] https://learn.adafruit.com/adafruit-dpi-display-kippah-ttl-tft/downloads
>>
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> 
> So it's just a bunch of signals that aren't wired / set to the ground? I
> assume to free a few GPIOs?

Yes. Freed pins could work as GPIOs or other functions.

> 
> I guess that makes sense.
> 
>> ---
>>  .../display/bridge/simple-bridge.yaml         | 89 ++++++++++++++++++-
>>  1 file changed, 87 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
>> index 43cf4df9811a..c1747c033040 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
>> @@ -27,6 +27,7 @@ properties:
>>            - const: adi,adv7123
>>        - enum:
>>            - adi,adv7123
>> +          - dpi-color-encoder
> 
> I don't think we can claim that there's a generic DPI color encoder.
> Maybe we can add the prefix dumb or transparent?

TBH, I don't have a best compatible string and open to suggestions.
"dpi-color-encoder" came up to my mind just because the MIPI DPI standard
documentation mentions *color coding* and this bridge changes the coding,
hence *encoder*.

If folks think "dumb-dpi-color-encoder" or "transparent-dpi-color-encoder"
are good enough, I'd use it.  Just let me know which one to use.

> 
>>            - dumb-vga-dac
>>            - ti,opa362
>>            - ti,ths8134
>> @@ -37,13 +38,31 @@ properties:
>>  
>>      properties:
>>        port@0:
>> -        $ref: /schemas/graph.yaml#/properties/port
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>>          description: The bridge input
>>  
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              dpi-color-coding: true
>> +
>>        port@1:
>> -        $ref: /schemas/graph.yaml#/properties/port
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>>          description: The bridge output
>>  
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              dpi-color-coding: true
>> +
>>      required:
>>        - port@0
>>        - port@1
>> @@ -61,6 +80,44 @@ required:
>>  
>>  additionalProperties: false
>>  
>> +allOf:
>> +  - $ref: /schemas/display/dpi-color-coding.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: dpi-color-encoder
>> +    then:
>> +      properties:
>> +        ports:
>> +          properties:
>> +            port@0:
>> +              properties:
>> +                endpoint:
>> +                  required:
>> +                    - dpi-color-coding
>> +
>> +            port@1:
>> +              properties:
>> +                endpoint:
>> +                  required:
>> +                    - dpi-color-coding
>> +    else:
>> +      properties:
>> +        ports:
>> +          properties:
>> +            port@0:
>> +              properties:
>> +                endpoint:
>> +                  properties:
>> +                    dpi-color-coding: false
>> +
>> +            port@1:
>> +              properties:
>> +                endpoint:
>> +                  properties:
>> +                    dpi-color-coding: false
>> +
> 
> Also it complicates the binding enough to warrant for another binding for
> that specific "component".

Do you suggest to write a separate/new DT binding for this bridge?

> 
>>  examples:
>>    - |
>>      bridge {
>> @@ -88,4 +145,32 @@ examples:
>>          };
>>      };
>>  
>> +  - |
>> +    bridge {
>> +        compatible = "dpi-color-enoder";
> 
> You have a typo in the compatible.

Hmm, good catch!  I did upgrade dtschema to version 2025.2 with pip3 and
didn't see any warnings/errors reported by dt_binding_check, weird...

> 
> Maxime

-- 
Regards,
Liu Ying