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

Maxime Ripard
2025-03-11T07:52:07Z

[-- Attachment #1: Type: text/plain, Size: 5226 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]