[PATCH v2 1/2] drm/panel: lvds: Simplify mode parsing

[PATCH v2 1/2] drm/panel: lvds: Simplify mode parsing

[Marek Vasut]

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;
-- 
2.35.1

[PATCH v2 2/2] drm/panel: lvds: Use bus_flags from DT panel-timing property

[Marek Vasut]

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;
 }
-- 
2.35.1

Re: [PATCH v2 1/2] drm/panel: lvds: Simplify mode parsing

[Laurent Pinchart]

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;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/2] drm/panel: lvds: Simplify mode parsing

[Marek Vasut]

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.