| Message ID | 20200216181513.28109-6-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: convert timing + panel-dpi to DT schema | expand |
On Sun, Feb 16, 2020 at 07:15:13PM +0100, Sam Ravnborg wrote: > RFC only - not tested yet! > > The panel-dpi compatible is a fallback that > allows the DT to specify the timing. > > When matching panel-dpi expect the device tree to include the > timing information for the display-panel. > > Background for this change: > There are a lot of panels and new models hits the market very often. > It is a lost cause trying to chase them all and users of new panels > will often find them in situations that the panel they ues are not > supported by the kernel. > On top of this a lot of panels are customized based on customer > specifications. > > Including the panel timing in the device tree allows for a simple > way to describe the actual HW and use this description in a generic > way in the kernel. > This allows uses of proprietary panels, or panels which are not > included in the kernel, to specify the timing in the device tree > together with all the other HW descriptions. > And thus, using the device tree it is then easy to add support > for an otherwise unknown panel. > > The current support expect panels that do not require any > delays for prepare/enable/disable/unprepare. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> I submitted a similar patch a year ago: https://lists.freedesktop.org/archives/dri-devel/2019-March/209682.html So I'm all for it :) Regardless of the outcome of this discussion, for all the patches: Acked-by: Maxime Ripard <mripard@kernel.org> Maxime
On Sun, Feb 16, 2020 at 8:15 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > RFC only - not tested yet! I've just tested this patch on Apalis iMX6Q and Colibri iMX7D using panel settings from the following patch: https://lore.kernel.org/linux-arm-kernel/20200115123401.2264293-4-oleksandr.suvorov@toradex.com/ It works for me, thanks! Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > > The panel-dpi compatible is a fallback that > allows the DT to specify the timing. > > When matching panel-dpi expect the device tree to include the > timing information for the display-panel. > > Background for this change: > There are a lot of panels and new models hits the market very often. > It is a lost cause trying to chase them all and users of new panels > will often find them in situations that the panel they ues are not > supported by the kernel. > On top of this a lot of panels are customized based on customer > specifications. > > Including the panel timing in the device tree allows for a simple > way to describe the actual HW and use this description in a generic > way in the kernel. > This allows uses of proprietary panels, or panels which are not > included in the kernel, to specify the timing in the device tree > together with all the other HW descriptions. > And thus, using the device tree it is then easy to add support > for an otherwise unknown panel. > > The current support expect panels that do not require any > delays for prepare/enable/disable/unprepare. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > --- > drivers/gpu/drm/panel/panel-simple.c | 74 +++++++++++++++++++++++++++- > 1 file changed, 72 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 82363d05bad4..188526637398 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -351,6 +351,65 @@ static const struct drm_panel_funcs panel_simple_funcs = { > .get_timings = panel_simple_get_timings, > }; > > +static struct panel_desc panel_dpi; > + > +static int panel_dpi_probe(struct device *dev, > + struct panel_simple *panel) > +{ > + struct display_timing *timing; > + const struct device_node *np; > + struct panel_desc *desc; > + unsigned int bus_flags; > + struct videomode vm; > + const char *mapping; > + int ret; > + > + np = dev->of_node; > + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); > + if (!desc) > + return -ENOMEM; > + > + timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL); > + if (!timing) > + return -ENOMEM; > + > + ret = of_get_display_timing(np, "panel-timing", timing); > + if (ret < 0) { > + dev_err(dev, "%pOF: no panel-timing node found for \"panel-dpi\" binding\n", > + np); > + return ret; > + } > + > + desc->timings = timing; > + desc->num_timings = 1; > + > + 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, "data-mapping", &mapping); > + if (!strcmp(mapping, "rgb24")) > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + else if (!strcmp(mapping, "rgb565")) > + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; > + else if (!strcmp(mapping, "bgr666")) > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; > + else if (!strcmp(mapping, "lvds666")) > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > + > + /* Extract bus_flags from display_timing */ > + bus_flags = 0; > + vm.flags = timing->flags; > + drm_bus_flags_from_videomode(&vm, &bus_flags); > + desc->bus_flags = bus_flags; > + > + /* We do not know the connector for the DT node, so guess it */ > + desc->connector_type = DRM_MODE_CONNECTOR_DPI; > + > + panel->desc = desc; > + > + return 0; > +} > + > #define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \ > (to_check->field.typ >= bounds->field.min && \ > to_check->field.typ <= bounds->field.max) > @@ -437,8 +496,15 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > return -EPROBE_DEFER; > } > > - if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) > - panel_simple_parse_panel_timing_node(dev, panel, &dt); > + if (desc == &panel_dpi) { > + /* Handle the generic panel-dpi binding */ > + err = panel_dpi_probe(dev, panel); > + if (err) > + goto free_ddc; > + } else { > + if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) > + panel_simple_parse_panel_timing_node(dev, panel, &dt); > + } > > drm_panel_init(&panel->base, dev, &panel_simple_funcs, > desc->connector_type); > @@ -3688,6 +3754,10 @@ static const struct of_device_id platform_of_match[] = { > }, { > .compatible = "winstar,wf35ltiacd", > .data = &winstar_wf35ltiacd, > + }, { > + /* Must be the last entry */ > + .compatible = "panel-dpi", > + .data = &panel_dpi, > }, { > /* sentinel */ > } > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Best regards Oleksandr Suvorov Toradex AG Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800 (main line)
Hi Sam, Thank you for the patch. On Sun, Feb 16, 2020 at 07:15:13PM +0100, Sam Ravnborg wrote: > RFC only - not tested yet! > > The panel-dpi compatible is a fallback that > allows the DT to specify the timing. > > When matching panel-dpi expect the device tree to include the > timing information for the display-panel. > > Background for this change: > There are a lot of panels and new models hits the market very often. > It is a lost cause trying to chase them all and users of new panels > will often find them in situations that the panel they ues are not s/ues/use/ > supported by the kernel. > On top of this a lot of panels are customized based on customer > specifications. > > Including the panel timing in the device tree allows for a simple > way to describe the actual HW and use this description in a generic > way in the kernel. > This allows uses of proprietary panels, or panels which are not > included in the kernel, to specify the timing in the device tree > together with all the other HW descriptions. > And thus, using the device tree it is then easy to add support > for an otherwise unknown panel. > > The current support expect panels that do not require any > delays for prepare/enable/disable/unprepare. I've proposed something similar a few times in the past, so I can't disagree :-) > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > --- > drivers/gpu/drm/panel/panel-simple.c | 74 +++++++++++++++++++++++++++- > 1 file changed, 72 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 82363d05bad4..188526637398 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -351,6 +351,65 @@ static const struct drm_panel_funcs panel_simple_funcs = { > .get_timings = panel_simple_get_timings, > }; > > +static struct panel_desc panel_dpi; Do you need this, can't you set data to 0 in the panel-dpi entry ? > + > +static int panel_dpi_probe(struct device *dev, > + struct panel_simple *panel) > +{ > + struct display_timing *timing; > + const struct device_node *np; > + struct panel_desc *desc; > + unsigned int bus_flags; > + struct videomode vm; > + const char *mapping; > + int ret; > + > + np = dev->of_node; > + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); > + if (!desc) > + return -ENOMEM; > + > + timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL); > + if (!timing) > + return -ENOMEM; > + > + ret = of_get_display_timing(np, "panel-timing", timing); > + if (ret < 0) { > + dev_err(dev, "%pOF: no panel-timing node found for \"panel-dpi\" binding\n", > + np); > + return ret; > + } > + > + desc->timings = timing; > + desc->num_timings = 1; > + > + 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, "data-mapping", &mapping); > + if (!strcmp(mapping, "rgb24")) > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + else if (!strcmp(mapping, "rgb565")) > + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; > + else if (!strcmp(mapping, "bgr666")) > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; > + else if (!strcmp(mapping, "lvds666")) > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; I think you've dropped lvds666 in the DT bindings, so this should be dropped too. Furthermore, I think the data-mapping property should be reconsidered (see my review of the bindings), so this would need to change too. > + > + /* Extract bus_flags from display_timing */ > + bus_flags = 0; > + vm.flags = timing->flags; > + drm_bus_flags_from_videomode(&vm, &bus_flags); > + desc->bus_flags = bus_flags; > + > + /* We do not know the connector for the DT node, so guess it */ > + desc->connector_type = DRM_MODE_CONNECTOR_DPI; > + > + panel->desc = desc; > + > + return 0; > +} > + > #define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \ > (to_check->field.typ >= bounds->field.min && \ > to_check->field.typ <= bounds->field.max) > @@ -437,8 +496,15 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > return -EPROBE_DEFER; > } > > - if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) > - panel_simple_parse_panel_timing_node(dev, panel, &dt); > + if (desc == &panel_dpi) { > + /* Handle the generic panel-dpi binding */ > + err = panel_dpi_probe(dev, panel); > + if (err) > + goto free_ddc; > + } else { > + if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) > + panel_simple_parse_panel_timing_node(dev, panel, &dt); > + } > > drm_panel_init(&panel->base, dev, &panel_simple_funcs, > desc->connector_type); > @@ -3688,6 +3754,10 @@ static const struct of_device_id platform_of_match[] = { > }, { > .compatible = "winstar,wf35ltiacd", > .data = &winstar_wf35ltiacd, > + }, { > + /* Must be the last entry */ > + .compatible = "panel-dpi", > + .data = &panel_dpi, > }, { > /* sentinel */ > }
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 82363d05bad4..188526637398 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -351,6 +351,65 @@ static const struct drm_panel_funcs panel_simple_funcs = { .get_timings = panel_simple_get_timings, }; +static struct panel_desc panel_dpi; + +static int panel_dpi_probe(struct device *dev, + struct panel_simple *panel) +{ + struct display_timing *timing; + const struct device_node *np; + struct panel_desc *desc; + unsigned int bus_flags; + struct videomode vm; + const char *mapping; + int ret; + + np = dev->of_node; + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); + if (!desc) + return -ENOMEM; + + timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL); + if (!timing) + return -ENOMEM; + + ret = of_get_display_timing(np, "panel-timing", timing); + if (ret < 0) { + dev_err(dev, "%pOF: no panel-timing node found for \"panel-dpi\" binding\n", + np); + return ret; + } + + desc->timings = timing; + desc->num_timings = 1; + + 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, "data-mapping", &mapping); + if (!strcmp(mapping, "rgb24")) + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; + else if (!strcmp(mapping, "rgb565")) + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; + else if (!strcmp(mapping, "bgr666")) + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; + else if (!strcmp(mapping, "lvds666")) + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; + + /* Extract bus_flags from display_timing */ + bus_flags = 0; + vm.flags = timing->flags; + drm_bus_flags_from_videomode(&vm, &bus_flags); + desc->bus_flags = bus_flags; + + /* We do not know the connector for the DT node, so guess it */ + desc->connector_type = DRM_MODE_CONNECTOR_DPI; + + panel->desc = desc; + + return 0; +} + #define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \ (to_check->field.typ >= bounds->field.min && \ to_check->field.typ <= bounds->field.max) @@ -437,8 +496,15 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) return -EPROBE_DEFER; } - if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) - panel_simple_parse_panel_timing_node(dev, panel, &dt); + if (desc == &panel_dpi) { + /* Handle the generic panel-dpi binding */ + err = panel_dpi_probe(dev, panel); + if (err) + goto free_ddc; + } else { + if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) + panel_simple_parse_panel_timing_node(dev, panel, &dt); + } drm_panel_init(&panel->base, dev, &panel_simple_funcs, desc->connector_type); @@ -3688,6 +3754,10 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "winstar,wf35ltiacd", .data = &winstar_wf35ltiacd, + }, { + /* Must be the last entry */ + .compatible = "panel-dpi", + .data = &panel_dpi, }, { /* sentinel */ }
RFC only - not tested yet! The panel-dpi compatible is a fallback that allows the DT to specify the timing. When matching panel-dpi expect the device tree to include the timing information for the display-panel. Background for this change: There are a lot of panels and new models hits the market very often. It is a lost cause trying to chase them all and users of new panels will often find them in situations that the panel they ues are not supported by the kernel. On top of this a lot of panels are customized based on customer specifications. Including the panel timing in the device tree allows for a simple way to describe the actual HW and use this description in a generic way in the kernel. This allows uses of proprietary panels, or panels which are not included in the kernel, to specify the timing in the device tree together with all the other HW descriptions. And thus, using the device tree it is then easy to add support for an otherwise unknown panel. The current support expect panels that do not require any delays for prepare/enable/disable/unprepare. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> --- drivers/gpu/drm/panel/panel-simple.c | 74 +++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-)