| Message ID | 20220801131901.183090-1-marex@denx.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: display: bridge: icn6211: Add support for RGB/BGR swap | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: > The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, > document a DT property to select this swap in DT. This can be useful on > hardware where such swap happens. We should ensure this series[1] works for you instead. > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Rob Herring <robh@kernel.org> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: dri-devel@lists.freedesktop.org > Cc: devicetree@vger.kernel.org > --- > .../devicetree/bindings/display/bridge/chipone,icn6211.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > index 18563ebed1a96..e721dd76e6640 100644 > --- a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml > @@ -33,6 +33,12 @@ properties: > Optional external clock connected to REF_CLK input. > The clock rate must be in 10..154 MHz range. > > + blue-and-red-swap: 'swap' assumes I know the default order and I don't. Better to make the property name describe the end result. ('invert' properties have the same issue) > + description: > + If defined, the DPI output blue and red channels are swapped, > + i.e. RGB becomes BGR. > + type: boolean > + > enable-gpios: > description: Bridge EN pin, chip is reset when EN is low. Rob [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com
On 8/1/22 18:32, Rob Herring wrote: > On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: >> The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, >> document a DT property to select this swap in DT. This can be useful on >> hardware where such swap happens. > > We should ensure this series[1] works for you instead. [...] > Rob > > [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com I'm still not convinced that we should encode this pixel format value directly into the DT instead of trying to describe the DPI bus color channel width/order/shift in the DT instead. I think I mentioned that before in one of the previous versions of that series.
On Tue, Aug 2, 2022 at 5:33 AM Marek Vasut <marex@denx.de> wrote: > > On 8/1/22 18:32, Rob Herring wrote: > > On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: > >> The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, > >> document a DT property to select this swap in DT. This can be useful on > >> hardware where such swap happens. > > > > We should ensure this series[1] works for you instead. > > [...] > > > Rob > > > > [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com > > I'm still not convinced that we should encode this pixel format value > directly into the DT instead of trying to describe the DPI bus color > channel width/order/shift in the DT instead. I think I mentioned that > before in one of the previous versions of that series. I worry that it gets pretty verbose, but worth having the discussion. In any case, let's have that discussion and not add yet another one off property. Rob
On 8/4/22 00:41, Rob Herring wrote: > On Tue, Aug 2, 2022 at 5:33 AM Marek Vasut <marex@denx.de> wrote: >> >> On 8/1/22 18:32, Rob Herring wrote: >>> On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: >>>> The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, >>>> document a DT property to select this swap in DT. This can be useful on >>>> hardware where such swap happens. >>> >>> We should ensure this series[1] works for you instead. >> >> [...] >> >>> Rob >>> >>> [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com >> >> I'm still not convinced that we should encode this pixel format value >> directly into the DT instead of trying to describe the DPI bus color >> channel width/order/shift in the DT instead. I think I mentioned that >> before in one of the previous versions of that series. > > I worry that it gets pretty verbose, but worth having the discussion. > In any case, let's have that discussion and not add yet another one > off property. Done, I replied
On Mon, Aug 8, 2022 at 3:57 AM Marek Vasut <marex@denx.de> wrote: > On 8/4/22 00:41, Rob Herring wrote: > > On Tue, Aug 2, 2022 at 5:33 AM Marek Vasut <marex@denx.de> wrote: > >> > >> On 8/1/22 18:32, Rob Herring wrote: > >>> On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: > >>>> The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, > >>>> document a DT property to select this swap in DT. This can be useful on > >>>> hardware where such swap happens. > >>> > >>> We should ensure this series[1] works for you instead. > >> > >> [...] > >> > >>> Rob > >>> > >>> [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com > >> > >> I'm still not convinced that we should encode this pixel format value > >> directly into the DT instead of trying to describe the DPI bus color > >> channel width/order/shift in the DT instead. I think I mentioned that > >> before in one of the previous versions of that series. > > > > I worry that it gets pretty verbose, but worth having the discussion. > > In any case, let's have that discussion and not add yet another one > > off property. > > Done, I replied So what is the verdict? Is this patch set dropped or shall it be applied? Yours, Linus Walleij
On 8/26/22 13:52, Linus Walleij wrote: > On Mon, Aug 8, 2022 at 3:57 AM Marek Vasut <marex@denx.de> wrote: >> On 8/4/22 00:41, Rob Herring wrote: >>> On Tue, Aug 2, 2022 at 5:33 AM Marek Vasut <marex@denx.de> wrote: >>>> >>>> On 8/1/22 18:32, Rob Herring wrote: >>>>> On Mon, Aug 01, 2022 at 03:19:00PM +0200, Marek Vasut wrote: >>>>>> The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, >>>>>> document a DT property to select this swap in DT. This can be useful on >>>>>> hardware where such swap happens. >>>>> >>>>> We should ensure this series[1] works for you instead. >>>> >>>> [...] >>>> >>>>> Rob >>>>> >>>>> [1] https://lore.kernel.org/r/20220628181838.2031-3-max.oss.09@gmail.com >>>> >>>> I'm still not convinced that we should encode this pixel format value >>>> directly into the DT instead of trying to describe the DPI bus color >>>> channel width/order/shift in the DT instead. I think I mentioned that >>>> before in one of the previous versions of that series. >>> >>> I worry that it gets pretty verbose, but worth having the discussion. >>> In any case, let's have that discussion and not add yet another one >>> off property. >> >> Done, I replied > > So what is the verdict? Is this patch set dropped or shall it be > applied? Drop Let's wait for the DPI pixel format description discussion in DT gets sorted out.
diff --git a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml index 18563ebed1a96..e721dd76e6640 100644 --- a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml +++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml @@ -33,6 +33,12 @@ properties: Optional external clock connected to REF_CLK input. The clock rate must be in 10..154 MHz range. + blue-and-red-swap: + description: + If defined, the DPI output blue and red channels are swapped, + i.e. RGB becomes BGR. + type: boolean + enable-gpios: description: Bridge EN pin, chip is reset when EN is low.
The ICN6211 is capable of swapping the output DPI RGB/BGR color channels, document a DT property to select this swap in DT. This can be useful on hardware where such swap happens. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Jagan Teki <jagan@amarulasolutions.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Rob Herring <robh@kernel.org> Cc: Robert Foss <robert.foss@linaro.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: dri-devel@lists.freedesktop.org Cc: devicetree@vger.kernel.org --- .../devicetree/bindings/display/bridge/chipone,icn6211.yaml | 6 ++++++ 1 file changed, 6 insertions(+)