Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/23/22 17:39, Maxime Ripard wrote:
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@xxxxxxxxxxx>

---

     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@xxxxxxxxxxxxxxxxxx/

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.

I think this patch is related to this patch, which was not mentioned in the commit message:

[RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property"

(unless I am confused)

With LVDS the situation is simpler, you have three formats and that's it (18bpp and 2 24bpp), with DPI it is more complex, since you need to deal with color channel width (888, 666 and even 565 and other oddities), then with mapping (RGB, BGR, etc), and then you can have panels with only 18bpp inputs wired to 24bpp DPI bus and vice versa which you also have to somehow describe.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux