| Message ID | 20200115123401.2264293-2-oleksandr.suvorov@toradex.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic DPI panel on Colibri iMX7 / Col.Eval.board | expand |
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
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)
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
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, }, };