| Message ID | 20200726203324.3722593-2-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: support chained bridges + panel updates | expand |
Hi Sam, Thank you for the patch. On Sun, Jul 26, 2020 at 10:33:10PM +0200, Sam Ravnborg wrote: > Warn if we detect a panel with incomplete/wrong description. > This is inspired by a similar patch by Laurent that introduced checks > for LVDS panels - this extends the checks to the remaining type of > connectors. > > This is known to warn for some of the existing panels but added > despite this as we need help from people using the panels to > add the missing info. > The checks are not complete but will catch the most common mistakes. > > The checks at the same time serves as documentation for the minimum s/serves/serve/ > required description for a panel. > > The checks uses dev_warn() as we know this will hit. WARN() was > too noisy at the moment for anything else than LVDS. > > v2: > - Use dev_warn (Laurent) > - Check for empty bus_flags > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/panel/panel-simple.c | 41 ++++++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 54323515ca2c..a8d68102931e 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -500,6 +500,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > struct panel_simple *panel; > struct display_timing dt; > struct device_node *ddc; > + u32 bus_flags; > int err; > > panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL); > @@ -549,8 +550,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > panel_simple_parse_panel_timing_node(dev, panel, &dt); > } > > - if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) { > - /* Catch common mistakes for LVDS panels. */ > + /* Catch common mistakes for panels. */ > + switch (desc->connector_type) { > + case 0: > + dev_warn(dev, "Specify missing connector_type\n"); > + break; > + case DRM_MODE_CONNECTOR_LVDS: > WARN_ON(desc->bus_flags & > ~(DRM_BUS_FLAG_DE_LOW | > DRM_BUS_FLAG_DE_HIGH | > @@ -564,6 +569,38 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > WARN_ON((desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG || > desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) && > desc->bpc != 8); > + break; > + case DRM_MODE_CONNECTOR_eDP: > + if (desc->bus_format == 0) > + dev_warn(dev, "Specify missing bus_format\n"); > + if (desc->bpc != 6 && desc->bpc != 8) > + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); bpc is unsigned, so s/%d/%u/ > + break; > + case DRM_MODE_CONNECTOR_DSI: > + if (desc->bpc != 6 && desc->bpc != 8) > + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); Same here. > + break; > + case DRM_MODE_CONNECTOR_DPI: > + bus_flags = DRM_BUS_FLAG_DE_LOW | > + DRM_BUS_FLAG_DE_HIGH | > + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE | > + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE | > + DRM_BUS_FLAG_DATA_MSB_TO_LSB | > + DRM_BUS_FLAG_DATA_LSB_TO_MSB | > + DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE | > + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; > + if (desc->bus_flags & ~bus_flags) > + dev_warn(dev, "Unexpected bus_flags(%d)\n", desc->bus_flags & ~bus_flags); > + if (!(desc->bus_flags & bus_flags)) > + dev_warn(dev, "Specify missing bus_flags\n"); > + if (desc->bus_format == 0) > + dev_warn(dev, "Specify missing bus_format\n"); > + if (desc->bpc != 6 && desc->bpc != 8) > + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); And here too. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + break; > + default: > + dev_warn(dev, "Specify a valid connector_type: %d\n", desc->connector_type); > + break; > } > > drm_panel_init(&panel->base, dev, &panel_simple_funcs,
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 54323515ca2c..a8d68102931e 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -500,6 +500,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) struct panel_simple *panel; struct display_timing dt; struct device_node *ddc; + u32 bus_flags; int err; panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL); @@ -549,8 +550,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) panel_simple_parse_panel_timing_node(dev, panel, &dt); } - if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) { - /* Catch common mistakes for LVDS panels. */ + /* Catch common mistakes for panels. */ + switch (desc->connector_type) { + case 0: + dev_warn(dev, "Specify missing connector_type\n"); + break; + case DRM_MODE_CONNECTOR_LVDS: WARN_ON(desc->bus_flags & ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH | @@ -564,6 +569,38 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) WARN_ON((desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG || desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) && desc->bpc != 8); + break; + case DRM_MODE_CONNECTOR_eDP: + if (desc->bus_format == 0) + dev_warn(dev, "Specify missing bus_format\n"); + if (desc->bpc != 6 && desc->bpc != 8) + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); + break; + case DRM_MODE_CONNECTOR_DSI: + if (desc->bpc != 6 && desc->bpc != 8) + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); + break; + case DRM_MODE_CONNECTOR_DPI: + bus_flags = DRM_BUS_FLAG_DE_LOW | + DRM_BUS_FLAG_DE_HIGH | + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE | + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE | + DRM_BUS_FLAG_DATA_MSB_TO_LSB | + DRM_BUS_FLAG_DATA_LSB_TO_MSB | + DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE | + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; + if (desc->bus_flags & ~bus_flags) + dev_warn(dev, "Unexpected bus_flags(%d)\n", desc->bus_flags & ~bus_flags); + if (!(desc->bus_flags & bus_flags)) + dev_warn(dev, "Specify missing bus_flags\n"); + if (desc->bus_format == 0) + dev_warn(dev, "Specify missing bus_format\n"); + if (desc->bpc != 6 && desc->bpc != 8) + dev_warn(dev, "Expected bpc in {6,8} but got: %d\n", desc->bpc); + break; + default: + dev_warn(dev, "Specify a valid connector_type: %d\n", desc->connector_type); + break; } drm_panel_init(&panel->base, dev, &panel_simple_funcs,
Warn if we detect a panel with incomplete/wrong description. This is inspired by a similar patch by Laurent that introduced checks for LVDS panels - this extends the checks to the remaining type of connectors. This is known to warn for some of the existing panels but added despite this as we need help from people using the panels to add the missing info. The checks are not complete but will catch the most common mistakes. The checks at the same time serves as documentation for the minimum required description for a panel. The checks uses dev_warn() as we know this will hit. WARN() was too noisy at the moment for anything else than LVDS. v2: - Use dev_warn (Laurent) - Check for empty bus_flags Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> --- drivers/gpu/drm/panel/panel-simple.c | 41 ++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-)