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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> 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:
pgpiCqeUwfltT.pgp
Description: PGP signature
