[1/3] drm/panel: make LVDS panel driver DPI capable
diff mbox series

Message ID 20200115123401.2264293-2-oleksandr.suvorov@toradex.com
State New
Headers show
Series
  • Generic DPI panel on Colibri iMX7 / Col.Eval.board
Related show

Commit Message

Oleksandr Suvorov Jan. 15, 2020, 12:34 p.m. UTC
From: Stefan Agner <stefan@agner.ch>

The LVDS panel driver has almost everything which is required to
describe a simple parallel RGB panel (also known as DPI, Display
Pixel Interface).

Signed-off-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 drivers/gpu/drm/panel/panel-lvds.c | 40 +++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 9 deletions(-)

Comments

Sam Ravnborg Jan. 18, 2020, 1:04 p.m. UTC | #1
Hi Oleksandr & Stafan.

Thanks for the update to panel-lvds.


On Wed, Jan 15, 2020 at 12:34:17PM +0000, Oleksandr Suvorov wrote:
> From: Stefan Agner <stefan@agner.ch>
> 
> The LVDS panel driver has almost everything which is required to
> describe a simple parallel RGB panel (also known as DPI, Display
> Pixel Interface).
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---

There are a few high-level things we need to have sorted out.

The driver, when this patch is added, assumes that certain properties
are now mandatory when using the panel-dpi compatible.
  - data-mapping
  - width-mm
  - height-mm
  - panel-timing

But this does not match the panel-dpi binding.
So we need the panel-dpi binding updated first.


The current driver specify the connector type in drm_panel_init().
But a DPI panel is assumed to use a DRM_MODE_CONNECTOR_DPI,
and not a DRM_MODE_CONNECTOR_LVDS.
So the drm_panel_init() call needs to take into account the type
of binding.


> @@ -257,7 +279,7 @@ static struct platform_driver panel_lvds_driver = {
>  	.probe		= panel_lvds_probe,
>  	.remove		= panel_lvds_remove,
>  	.driver		= {
> -		.name	= "panel-lvds",
> +		.name	= "panel-generic",

I think changing the name of the driver like this is an UAPI change,
which is not OK

>  		.of_match_table = panel_lvds_of_table,
>  	},
>  };

	Sam
Oleksandr Suvorov Jan. 20, 2020, 9:57 a.m. UTC | #2
Hi Sam,

On Sat, Jan 18, 2020 at 3:04 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > The LVDS panel driver has almost everything which is required to
> > describe a simple parallel RGB panel (also known as DPI, Display
> > Pixel Interface).
> >
> > ---
>
> There are a few high-level things we need to have sorted out.
>
> The driver, when this patch is added, assumes that certain properties
> are now mandatory when using the panel-dpi compatible.
>   - data-mapping
>   - width-mm
>   - height-mm
>   - panel-timing
>
> But this does not match the panel-dpi binding.
> So we need the panel-dpi binding updated first.
>
>
> The current driver specify the connector type in drm_panel_init().
> But a DPI panel is assumed to use a DRM_MODE_CONNECTOR_DPI,
> and not a DRM_MODE_CONNECTOR_LVDS.
> So the drm_panel_init() call needs to take into account the type
> of binding.
>
Thanks, I'll fix it in 2nd version.
>
> > @@ -257,7 +279,7 @@ static struct platform_driver panel_lvds_driver = {
> >       .probe          = panel_lvds_probe,
> >       .remove         = panel_lvds_remove,
> >       .driver         = {
> > -             .name   = "panel-lvds",
> > +             .name   = "panel-generic",
>
> I think changing the name of the driver like this is an UAPI change,
> which is not OK

I see 2 simple ways there:
- keep the original platform driver name;
- fork panel-lvds driver as panel-generic driver with dpi support.

What solution do you prefer?

> >               .of_match_table = panel_lvds_of_table,
> >       },
> >  };
>
>         Sam

--
Best regards
Oleksandr Suvorov

Toradex AG
Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500
4800 (main line)
Sam Ravnborg Jan. 20, 2020, 8:20 p.m. UTC | #3
Hi Oleksandr.

Thanks for the quick reply.

On Mon, Jan 20, 2020 at 10:03:20AM +0000, Oleksandr Suvorov wrote:
> Hi Sam,
> 
> On Sat, Jan 18, 2020 at 3:04 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > >
> > > The LVDS panel driver has almost everything which is required to
> > > describe a simple parallel RGB panel (also known as DPI, Display
> > > Pixel Interface).
> > >
> > > ---
> >
> > There are a few high-level things we need to have sorted out.
> >
> > The driver, when this patch is added, assumes that certain properties
> > are now mandatory when using the panel-dpi compatible.
> >   - data-mapping
> >   - width-mm
> >   - height-mm
> >   - panel-timing
> >
> > But this does not match the panel-dpi binding.
> > So we need the panel-dpi binding updated first.
I just sent a patch-set converting this binding to DT schema.
Let's land this and you can make your changes on top of it.
Care to review it?

> >
> >
> > The current driver specify the connector type in drm_panel_init().
> > But a DPI panel is assumed to use a DRM_MODE_CONNECTOR_DPI,
> > and not a DRM_MODE_CONNECTOR_LVDS.
> > So the drm_panel_init() call needs to take into account the type
> > of binding.
> >
> Thanks, I'll fix it in 2nd version.
> >
> > > @@ -257,7 +279,7 @@ static struct platform_driver panel_lvds_driver = {
> > >       .probe          = panel_lvds_probe,
> > >       .remove         = panel_lvds_remove,
> > >       .driver         = {
> > > -             .name   = "panel-lvds",
> > > +             .name   = "panel-generic",
> >
> > I think changing the name of the driver like this is an UAPI change,
> > which is not OK
> 
> I see 2 simple ways there:
> - keep the original platform driver name;
Please keep the original platform driver name.
It is a bit confusing but this is the best option I see.

	Sam

Patch
diff mbox series

diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
index 5ce3f4a2b7a1..21a169aa3ae4 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -22,6 +22,11 @@ 
 #include <drm/drm_crtc.h>
 #include <drm/drm_panel.h>
 
+enum panel_type {
+	PANEL_LVDS,
+	PANEL_DPI
+};
+
 struct panel_lvds {
 	struct drm_panel panel;
 	struct device *dev;
@@ -115,6 +120,7 @@  static int panel_lvds_parse_dt(struct panel_lvds *lvds)
 	struct display_timing timing;
 	const char *mapping;
 	int ret;
+	enum panel_type type;
 
 	ret = of_get_display_timing(np, "panel-timing", &timing);
 	if (ret < 0) {
@@ -147,13 +153,28 @@  static int panel_lvds_parse_dt(struct panel_lvds *lvds)
 		return -ENODEV;
 	}
 
-	if (!strcmp(mapping, "jeida-18")) {
-		lvds->bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
-	} else if (!strcmp(mapping, "jeida-24")) {
-		lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
-	} else if (!strcmp(mapping, "vesa-24")) {
-		lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
-	} else {
+	type = (enum panel_type)of_device_get_match_data(lvds->dev);
+	switch (type) {
+	case PANEL_LVDS:
+		if (!strcmp(mapping, "jeida-18"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
+		else if (!strcmp(mapping, "jeida-24"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+		else if (!strcmp(mapping, "vesa-24"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
+		break;
+	case PANEL_DPI:
+		if (!strcmp(mapping, "rgb24"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+		else if (!strcmp(mapping, "rgb565"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
+		else if (!strcmp(mapping, "bgr666"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
+		else if (!strcmp(mapping, "lvds666"))
+			lvds->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
+	};
+
+	if (!lvds->bus_format) {
 		dev_err(lvds->dev, "%pOF: invalid or missing %s DT property\n",
 			np, "data-mapping");
 		return -EINVAL;
@@ -247,7 +268,8 @@  static int panel_lvds_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id panel_lvds_of_table[] = {
-	{ .compatible = "panel-lvds", },
+	{ .compatible = "panel-lvds", .data = (void *)PANEL_LVDS },
+	{ .compatible = "panel-dpi", .data = (void *)PANEL_DPI },
 	{ /* Sentinel */ },
 };
 
@@ -257,7 +279,7 @@  static struct platform_driver panel_lvds_driver = {
 	.probe		= panel_lvds_probe,
 	.remove		= panel_lvds_remove,
 	.driver		= {
-		.name	= "panel-lvds",
+		.name	= "panel-generic",
 		.of_match_table = panel_lvds_of_table,
 	},
 };