Re: [RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3664 bytes --]

On Mon, Jul 21, 2014 at 02:33:21PM +0200, Boris BREZILLON wrote:
> On Mon, 21 Jul 2014 14:15:16 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote:
> > > Hi Thierry,
> > > 
> > > Oops, I missed this reply.
> > > 
> > > On Tue, 15 Jul 2014 12:31:37 +0200
> > > Thierry Reding <thierry.reding@gmail.com> wrote:
> > > 
> > > > On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote:
> > > > > On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding <thierry.reding@gmail.com> wrote:
> > > > > > On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote:
> > > > [...]
> > > > > > > diff --git a/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt
> > > > > > [...]
> > > > > > > +The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
> > > > > > > +See Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt for more details.
> > > > > > 
> > > > > > I think it's better to refer to these using relative filenames. When the
> > > > > > device tree bindings are moved out of the kernel tree, they may no
> > > > > > longer use the same hierarchy.
> > > > > 
> > > > > Sure.
> > > > > By relative path you mean ../../mfd/atmel-hlcdc.txt or just
> > > > > mfd/atmel-hlcdc.txt ?
> > > > 
> > > > I think the former is more explicit.
> > > 
> > > Okay.
> > > 
> > > > 
> > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters.
> > > > > > > +   The first cell is a phandle to a DRM panel device
> > > > > > > +   The second cell encodes the RGB mode, which can take the following values:
> > > > > > > +   * 0: RGB444
> > > > > > > +   * 1: RGB565
> > > > > > > +   * 2: RGB666
> > > > > > > +   * 3: RGB888
> > > > > > 
> > > > > > These are properties of the panel and should be obtained from the panel
> > > > > > directly rather than an additional cell in this specifier.
> > > > > 
> > > > > Okay.
> > > > > What's the preferred way of doing this ?
> > > > > What about defining an rgb-mode property in the panel node.
> > > > 
> > > > There's .bpc in struct drm_display_info, I suspect that it could be used
> > > > for this. Alternatively, maybe we could extend the list of color formats
> > > > that go into drm_display_info.color_formats? RGB444 is already covered.
> > > 
> > > I don't think this color_formats field is intended to represent data
> > > stream format going through the bus.
> > > Moreover, AFAIU, RGB444 in this definition represent RGB 4:4:4 (chroma
> > > subsampling rate) and not 12 bits signals (4 bits for each color).
> > > 
> > > Anyway I'll propose a patch series adding a new field to
> > > drm_display_info to encode the mediabus format (as discussed with
> > > Laurent and you).
> > > 
> > > > 
> > > > Also, like Laurent said, this shouldn't go into the device tree, since
> > > > it's already implied by the panel's compatible value, so we'd be
> > > > duplicating information.
> > > 
> > > Again, this is not necessarily true (depending on your board design).
> > > One can decide to connect an RGB888 panel on an RGB666 bus and connect
> > > the missing pins to ground.
> > 
> > I think in that case the board design itself could be considered as an
> > RGB888 to RGB666 bridge, and I think that's what the device tree should
> > be describing rather than a panel with a variable number of input
> > formats.
> 
> So, you're suggesting to add an RGB to RGB drm_bridge driver (and
> the appropriate DT bindings) to handle this case, right ?

Yes, exactly.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel