The mode parsing is currently implemented in three steps: of_get_display_timing() - DT panel-timing to struct display_timing videomode_from_timing() - struct display_timing to struct videomode drm_display_mode_from_videomode() - struct videomode to struct drm_display_mode
Replace all that with simple of_get_drm_panel_display_mode() call, which already populates struct drm_display_mode and then duplicate that mode in panel_lvds_get_modes() each time, since the mode does not change.
Nice bonus is the bus_flags parsed by of_get_drm_panel_display_mode() out of panel-timing DT node, which is used in subsequent patch to fix handling of 'de-active' DT property.
Tested-by: Christoph Niedermaier cniedermaier@dh-electronics.com Signed-off-by: Marek Vasut marex@denx.de Cc: Christoph Niedermaier cniedermaier@dh-electronics.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dmitry Osipenko digetx@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Robert Foss robert.foss@linaro.org Cc: Sam Ravnborg sam@ravnborg.org Cc: Thomas Zimmermann tzimmermann@suse.de To: dri-devel@lists.freedesktop.org --- V2: - Reinstate mandatory width-mm/height-mm DT property check - Collect TB from Christoph --- drivers/gpu/drm/panel/panel-lvds.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 27a1c9923b09..6422868c1089 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -30,7 +30,8 @@ struct panel_lvds { const char *label; unsigned int width; unsigned int height; - struct videomode video_mode; + struct drm_display_mode dmode; + u32 bus_flags; unsigned int bus_format; bool data_mirror;
@@ -87,16 +88,15 @@ static int panel_lvds_get_modes(struct drm_panel *panel, struct panel_lvds *lvds = to_panel_lvds(panel); struct drm_display_mode *mode;
- mode = drm_mode_create(connector->dev); + mode = drm_mode_duplicate(connector->dev, &lvds->dmode); if (!mode) return 0;
- drm_display_mode_from_videomode(&lvds->video_mode, mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode);
- connector->display_info.width_mm = lvds->width; - connector->display_info.height_mm = lvds->height; + connector->display_info.width_mm = lvds->dmode.width_mm; + connector->display_info.height_mm = lvds->dmode.height_mm; drm_display_info_set_bus_formats(&connector->display_info, &lvds->bus_format, 1); connector->display_info.bus_flags = lvds->data_mirror @@ -116,7 +116,6 @@ static const struct drm_panel_funcs panel_lvds_funcs = { static int panel_lvds_parse_dt(struct panel_lvds *lvds) { struct device_node *np = lvds->dev->of_node; - struct display_timing timing; int ret;
ret = of_drm_get_panel_orientation(np, &lvds->orientation); @@ -125,23 +124,20 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds) return ret; }
- ret = of_get_display_timing(np, "panel-timing", &timing); + ret = of_get_drm_panel_display_mode(np, &lvds->dmode, &lvds->bus_flags); if (ret < 0) { dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n", np, ret); return ret; }
- videomode_from_timing(&timing, &lvds->video_mode); - - ret = of_property_read_u32(np, "width-mm", &lvds->width); - if (ret < 0) { + if (lvds->dmode.width_mm == 0) { dev_err(lvds->dev, "%pOF: invalid or missing %s DT property\n", np, "width-mm"); return -ENODEV; } - ret = of_property_read_u32(np, "height-mm", &lvds->height); - if (ret < 0) { + + if (lvds->dmode.height_mm == 0) { dev_err(lvds->dev, "%pOF: invalid or missing %s DT property\n", np, "height-mm"); return -ENODEV;
This driver currently rewrites bus_flags based solely on the value of DT property 'data-mirror' and ignores bus_flags which might have been set in DT panel-timing node. Specificaly, the 'de-active' DT property sets DRM_BUS_FLAG_DE_ bus_flags.
Since of_get_drm_panel_display_mode() conveniently parses the bus_flags out of DT panel-timing property, just ORR them with bus_flags inferred from 'data-mirror' DT property and use the result as panel bus_flags.
This fixes handling of panels with 'panel-timing { de-active = <1>; };'.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Tested-by: Christoph Niedermaier cniedermaier@dh-electronics.com Signed-off-by: Marek Vasut marex@denx.de Cc: Christoph Niedermaier cniedermaier@dh-electronics.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dmitry Osipenko digetx@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Robert Foss robert.foss@linaro.org Cc: Sam Ravnborg sam@ravnborg.org Cc: Thomas Zimmermann tzimmermann@suse.de To: dri-devel@lists.freedesktop.org --- V2: - Collect RB from Laurent, TB from Christoph --- drivers/gpu/drm/panel/panel-lvds.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 6422868c1089..eca067e78579 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -33,7 +33,6 @@ struct panel_lvds { struct drm_display_mode dmode; u32 bus_flags; unsigned int bus_format; - bool data_mirror;
struct regulator *supply;
@@ -99,9 +98,7 @@ static int panel_lvds_get_modes(struct drm_panel *panel, connector->display_info.height_mm = lvds->dmode.height_mm; drm_display_info_set_bus_formats(&connector->display_info, &lvds->bus_format, 1); - connector->display_info.bus_flags = lvds->data_mirror - ? DRM_BUS_FLAG_DATA_LSB_TO_MSB - : DRM_BUS_FLAG_DATA_MSB_TO_LSB; + connector->display_info.bus_flags = lvds->bus_flags; drm_connector_set_panel_orientation(connector, lvds->orientation);
return 1; @@ -154,7 +151,9 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)
lvds->bus_format = ret;
- lvds->data_mirror = of_property_read_bool(np, "data-mirror"); + lvds->bus_flags |= of_property_read_bool(np, "data-mirror") ? + DRM_BUS_FLAG_DATA_LSB_TO_MSB : + DRM_BUS_FLAG_DATA_MSB_TO_LSB;
return 0; }
Hi Marek,
Thank you for the patch.
On Fri, Apr 01, 2022 at 06:21:53PM +0200, Marek Vasut wrote:
The mode parsing is currently implemented in three steps: of_get_display_timing() - DT panel-timing to struct display_timing videomode_from_timing() - struct display_timing to struct videomode drm_display_mode_from_videomode() - struct videomode to struct drm_display_mode
Replace all that with simple of_get_drm_panel_display_mode() call, which already populates struct drm_display_mode and then duplicate that mode in panel_lvds_get_modes() each time, since the mode does not change.
Nice bonus is the bus_flags parsed by of_get_drm_panel_display_mode() out of panel-timing DT node, which is used in subsequent patch to fix handling of 'de-active' DT property.
Tested-by: Christoph Niedermaier cniedermaier@dh-electronics.com Signed-off-by: Marek Vasut marex@denx.de Cc: Christoph Niedermaier cniedermaier@dh-electronics.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dmitry Osipenko digetx@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Robert Foss robert.foss@linaro.org Cc: Sam Ravnborg sam@ravnborg.org Cc: Thomas Zimmermann tzimmermann@suse.de To: dri-devel@lists.freedesktop.org
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
V2: - Reinstate mandatory width-mm/height-mm DT property check - Collect TB from Christoph
drivers/gpu/drm/panel/panel-lvds.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 27a1c9923b09..6422868c1089 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -30,7 +30,8 @@ struct panel_lvds { const char *label; unsigned int width; unsigned int height;
- struct videomode video_mode;
- struct drm_display_mode dmode;
- u32 bus_flags; unsigned int bus_format; bool data_mirror;
@@ -87,16 +88,15 @@ static int panel_lvds_get_modes(struct drm_panel *panel, struct panel_lvds *lvds = to_panel_lvds(panel); struct drm_display_mode *mode;
- mode = drm_mode_create(connector->dev);
- mode = drm_mode_duplicate(connector->dev, &lvds->dmode); if (!mode) return 0;
drm_display_mode_from_videomode(&lvds->video_mode, mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode);
connector->display_info.width_mm = lvds->width;
connector->display_info.height_mm = lvds->height;
- connector->display_info.width_mm = lvds->dmode.width_mm;
- connector->display_info.height_mm = lvds->dmode.height_mm; drm_display_info_set_bus_formats(&connector->display_info, &lvds->bus_format, 1); connector->display_info.bus_flags = lvds->data_mirror
@@ -116,7 +116,6 @@ static const struct drm_panel_funcs panel_lvds_funcs = { static int panel_lvds_parse_dt(struct panel_lvds *lvds) { struct device_node *np = lvds->dev->of_node;
struct display_timing timing; int ret;
ret = of_drm_get_panel_orientation(np, &lvds->orientation);
@@ -125,23 +124,20 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds) return ret; }
- ret = of_get_display_timing(np, "panel-timing", &timing);
- ret = of_get_drm_panel_display_mode(np, &lvds->dmode, &lvds->bus_flags); if (ret < 0) { dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n", np, ret); return ret; }
- videomode_from_timing(&timing, &lvds->video_mode);
- ret = of_property_read_u32(np, "width-mm", &lvds->width);
- if (ret < 0) {
- if (lvds->dmode.width_mm == 0) { dev_err(lvds->dev, "%pOF: invalid or missing %s DT property\n", np, "width-mm"); return -ENODEV; }
- ret = of_property_read_u32(np, "height-mm", &lvds->height);
- if (ret < 0) {
- if (lvds->dmode.height_mm == 0) { dev_err(lvds->dev, "%pOF: invalid or missing %s DT property\n", np, "height-mm"); return -ENODEV;
On 4/7/22 13:16, Laurent Pinchart wrote:
Hi Marek,
Thank you for the patch.
On Fri, Apr 01, 2022 at 06:21:53PM +0200, Marek Vasut wrote:
The mode parsing is currently implemented in three steps: of_get_display_timing() - DT panel-timing to struct display_timing videomode_from_timing() - struct display_timing to struct videomode drm_display_mode_from_videomode() - struct videomode to struct drm_display_mode
Replace all that with simple of_get_drm_panel_display_mode() call, which already populates struct drm_display_mode and then duplicate that mode in panel_lvds_get_modes() each time, since the mode does not change.
Nice bonus is the bus_flags parsed by of_get_drm_panel_display_mode() out of panel-timing DT node, which is used in subsequent patch to fix handling of 'de-active' DT property.
Tested-by: Christoph Niedermaier cniedermaier@dh-electronics.com Signed-off-by: Marek Vasut marex@denx.de Cc: Christoph Niedermaier cniedermaier@dh-electronics.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dmitry Osipenko digetx@gmail.com Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Robert Foss robert.foss@linaro.org Cc: Sam Ravnborg sam@ravnborg.org Cc: Thomas Zimmermann tzimmermann@suse.de To: dri-devel@lists.freedesktop.org
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
If that's OK with everyone, I will apply those soon.
dri-devel@lists.freedesktop.org