On Wed, Feb 23, 2022 at 03:38:20PM +0100, Marek Vasut wrote:
On 2/23/22 15:37, Maxime Ripard wrote:
On Wed, Feb 23, 2022 at 03:09:08PM +0100, Marek Vasut wrote:
On 2/23/22 14:47, Maxime Ripard wrote:
On Wed, Feb 23, 2022 at 02:45:30PM +0100, Marek Vasut wrote:
On 2/23/22 14:41, Maxime Ripard wrote:
Hi,
On Tue, Feb 22, 2022 at 09:47:23AM +0100, Max Krummenacher wrote: > Use the new property bus-format to set the enum bus_format and bpc. > Completes: commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") > > Signed-off-by: Max Krummenacher max.krummenacher@toradex.com > > --- > > drivers/gpu/drm/panel/panel-simple.c | 32 ++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > Relates to the discussion: https://lore.kernel.org/all/20220201110717.3585-1-cniedermaier@dh-electronic... > > Max > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index c5f133667a2d..5c07260de71c 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev, > struct panel_desc *desc; > unsigned int bus_flags; > struct videomode vm; > + const char *format = ""; > int ret; > np = dev->of_node; > @@ -477,6 +478,37 @@ static int panel_dpi_probe(struct device *dev, > of_property_read_u32(np, "width-mm", &desc->size.width); > of_property_read_u32(np, "height-mm", &desc->size.height); > + of_property_read_string(np, "bus-format", &format); > + if (!strcmp(format, "BGR888_1X24")) { > + desc->bpc = 8; > + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; > + } else if (!strcmp(format, "GBR888_1X24")) { > + desc->bpc = 8; > + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; > + } else if (!strcmp(format, "RBG888_1X24")) { > + desc->bpc = 8; > + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; > + } else if (!strcmp(format, "RGB444_1X12")) { > + desc->bpc = 6; > + desc->bus_format = MEDIA_BUS_FMT_RGB444_1X12; > + } else if (!strcmp(format, "RGB565_1X16")) { > + desc->bpc = 6; > + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; > + } else if (!strcmp(format, "RGB666_1X18")) { > + desc->bpc = 6; > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; > + } else if (!strcmp(format, "RGB666_1X24_CPADHI")) { > + desc->bpc = 6; > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > + } else if (!strcmp(format, "RGB888_1X24")) { > + desc->bpc = 8; > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + } else { > + dev_err(dev, "%pOF: missing or unknown bus-format property\n", > + np); > + return -EINVAL; > + } > +
It doesn't seem right, really. We can't the bus format / bpc be inferred from the compatible? I'd expect two panels that don't have the same bus format to not be claimed as compatible.
Which compatible ?
Note that this is for panel-dpi compatible, i.e. the panel which has timings specified in DT (and needs bus format specified there too).
panel-dpi is supposed to have two compatibles, the panel-specific one and panel-dpi. This would obviously be tied to the panel-specific one.
This whole discussion is about the one which only has 'panel-dpi' compatible and describes the panel in DT completely. The specific compatible is not present in DT when this patch is needed.
From the panel-dpi DT binding:
properties: compatible: description: Shall contain a panel specific compatible and "panel-dpi" in that order. items: - {} - const: panel-dpi
The panel-specific compatible is mandatory, whether you like it or not.
It doesn't seem to me that's the intended use per panel-simple.c , so maybe the bindings need to be fixed too ?
It's not clear to me why this has anything to do with panel-simple, but the binding is correct, and that requirement is fairly standard. We have the same thing with panel-lvds for example.
Maxime