Skip to contentSkip to conversation

drm/omap: Add virtual-planes support

PATCH Superseded Benoit Parrot 4 participants 19 messages Mar 26, 2018 – Apr 27, 2018

Subsystems (4)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover drm/omap: Add virtual-planes support

This patch series adds virtual-plane support to omapdrm driver to allow the use of display wider than 2048 pixels.

The DT bindings are also cleaned up to remove duplication when properties are common to all implementations.

This patch series depends on Peter Ujfalusi's normalized zpos patch set which can be found starting here:
https://patchwork.kernel.org/patch/10299041/

Changes since v1:
- Added a check to filter out unsupportable display mode based on size
- Added Rob reviewed-by for patch #2
- Improve description of patch #3 replace term to remove DRM references and use terminology consistent with the Technical Reference Manual
- Fix the reported zpos issue by reusing and locally extending the normalized zpos handling.
- Addressed Tomi's review comments

Benoit Parrot (6):
drm/omap: Add ability to filter out modes which can't be supported
dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt
dt-bindings: display/ti: Add plane binding to dispc node drm/omap: Add virtual plane DT parsing support
drm/omap: Add virtual plane support to omap_plane drm/omap: Allow wider display when a virtual plane is available

 .../devicetree/bindings/display/ti/ti,dra7-dss.txt |   5 -
 .../devicetree/bindings/display/ti/ti,omap-dss.txt |  72 +++++++++
 .../bindings/display/ti/ti,omap2-dss.txt           |   4 -
 .../bindings/display/ti/ti,omap3-dss.txt           |   4 -
 .../bindings/display/ti/ti,omap4-dss.txt           |   4 -
 .../bindings/display/ti/ti,omap5-dss.txt           |   4 -
 drivers/gpu/drm/omapdrm/dss/dispc.c                | 137 +++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h              |  12 ++
 drivers/gpu/drm/omapdrm/omap_connector.c           |  11 ++
 drivers/gpu/drm/omapdrm/omap_drv.c                 | 127 +++++++++++++++-
 drivers/gpu/drm/omapdrm/omap_fb.c                  |  66 ++++++---
 drivers/gpu/drm/omapdrm/omap_fb.h                  |   4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c               | 163 ++++++++++++++++-----
 drivers/gpu/drm/omapdrm/omap_plane.h               |   5 +-
 14 files changed, 528 insertions(+), 90 deletions(-)
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw

PATCH v2 1/6 drm/omap: Add ability to filter out modes which can't be supported

Currently available display mode from a connector are filtered out based only on pixel clock capability. However we also need to filter out wider mode if we cannot handle them based on available pipeline capabilities.

---
 drivers/gpu/drm/omapdrm/dss/dispc.c      | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  1 +
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++++++++
 3 files changed, 38 insertions(+)
drivers/gpu/drm/omapdrm/dss/dispc.c +27 -0
index 624dee22f46b..35541d4441df 100644
@@ -100,6 +100,8 @@ struct dispc_features {
 	u8 mgr_height_start;
 	u16 mgr_width_max;
 	u16 mgr_height_max;
+	u16 ovl_width_max;
+	u16 ovl_height_max;
 	unsigned long max_lcd_pclk;
 	unsigned long max_tv_pclk;
 	unsigned int max_downscale;
@@ -2465,6 +2467,12 @@ static int dispc_ovl_calc_scaling(unsigned long pclk, unsigned long lclk,
 	return 0;
 }
 
+static void dispc_ovl_get_max_size(u16 *width, u16 *height)
+{
+	*width = dispc.feat->ovl_width_max;
+	*height = dispc.feat->ovl_height_max;
+}
+
 static int dispc_ovl_setup_common(enum omap_plane_id plane,
 		enum omap_overlay_caps caps, u32 paddr, u32 p_uv_addr,
 		u16 screen_width, int pos_x, int pos_y, u16 width, u16 height,
@@ -2500,6 +2508,10 @@ static int dispc_ovl_setup_common(enum omap_plane_id plane,
 	out_width = out_width == 0 ? width : out_width;
 	out_height = out_height == 0 ? height : out_height;
 
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported (%d).\n",
+	     out_width, dispc.feat->ovl_width_max);
+
 	if (ilace && height == out_height)
 		fieldmode = true;
 
@@ -4043,6 +4055,8 @@ static const struct dispc_features omap24xx_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	66500000,
 	.max_downscale		=	2,
 	/*
@@ -4080,6 +4094,8 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	173000000,
 	.max_tv_pclk		=	59000000,
 	.max_downscale		=	4,
@@ -4114,6 +4130,8 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	173000000,
 	.max_tv_pclk		=	59000000,
 	.max_downscale		=	4,
@@ -4148,6 +4166,8 @@ static const struct dispc_features omap36xx_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	173000000,
 	.max_tv_pclk		=	59000000,
 	.max_downscale		=	4,
@@ -4182,6 +4202,8 @@ static const struct dispc_features am43xx_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	173000000,
 	.max_tv_pclk		=	59000000,
 	.max_downscale		=	4,
@@ -4216,6 +4238,8 @@ static const struct dispc_features omap44xx_dispc_feats = {
 	.mgr_height_start	=	26,
 	.mgr_width_max		=	2048,
 	.mgr_height_max		=	2048,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	2048,
 	.max_lcd_pclk		=	170000000,
 	.max_tv_pclk		=	185625000,
 	.max_downscale		=	4,
@@ -4255,6 +4279,8 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.mgr_height_start	=	27,
 	.mgr_width_max		=	4096,
 	.mgr_height_max		=	4096,
+	.ovl_width_max		=	2048,
+	.ovl_height_max		=	4096,
 	.max_lcd_pclk		=	170000000,
 	.max_tv_pclk		=	186000000,
 	.max_downscale		=	4,
@@ -4525,6 +4551,7 @@ static const struct dispc_ops dispc_ops = {
 	.ovl_enable = dispc_ovl_enable,
 	.ovl_setup = dispc_ovl_setup,
 	.ovl_get_color_modes = dispc_ovl_get_color_modes,
+	.ovl_get_max_size = dispc_ovl_get_max_size,
 };
 
 /* DISPC HW IP initialisation */
drivers/gpu/drm/omapdrm/dss/omapdss.h +1 -0
index f8f83e826a56..c58c75292182 100644
@@ -719,6 +719,7 @@ struct dispc_ops {
 			enum omap_channel channel);
 
 	const u32 *(*ovl_get_color_modes)(enum omap_plane_id plane);
+	void (*ovl_get_max_size)(u16 *width, u16 *height);
 };
 
 void dispc_set_ops(const struct dispc_ops *o);
drivers/gpu/drm/omapdrm/omap_connector.c +10 -0
index a0d7b1d905e8..d5e059abb555 100644
@@ -197,6 +197,16 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 			r = 0;
 	}
 
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;
+	}
+
 	if (!r) {
 		/* check if vrefresh is still valid */
 		new_mode = drm_mode_duplicate(dev, mode);
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#2 lore raw

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

Quoted 39 lines · Benoit Parrot

Currently available display mode from a connector are filtered out based only on pixel clock capability. However we also need to filter out wider mode if we cannot handle them based on available pipeline capabilities.

---
 drivers/gpu/drm/omapdrm/dss/dispc.c      | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  1 +
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++++++++
 3 files changed, 38 insertions(+)
drivers/gpu/drm/omapdrm/dss/dispc.c +8 -0
index 624dee22f46b..35541d4441df 100644
@@ -100,6 +100,8 @@ struct dispc_features {
 	u8 mgr_height_start;
 	u16 mgr_width_max;
 	u16 mgr_height_max;
+	u16 ovl_width_max;
+	u16 ovl_height_max;
 	unsigned long max_lcd_pclk;
 	unsigned long max_tv_pclk;
 	unsigned int max_downscale;
@@ -2465,6 +2467,12 @@ static int dispc_ovl_calc_scaling(unsigned long pclk, unsigned long lclk,
 	return 0;
 }
 
+static void dispc_ovl_get_max_size(u16 *width, u16 *height)
+{
+	*width = dispc.feat->ovl_width_max;
+	*height = dispc.feat->ovl_height_max;
+}
+
 static int dispc_ovl_setup_common(enum omap_plane_id plane,
 		enum omap_overlay_caps caps, u32 paddr, u32 p_uv_addr,
 		u16 screen_width, int pos_x, int pos_y, u16 width, u16 height,

drivers/gpu/drm/omapdrm/dss/dispc.c static int dispc_ovl_setup_common(enum omap_plane_id plane, · line 2513

Quoted 6 lines
drivers/gpu/drm/omapdrm/dss/dispc.c +3 -0
@@ -2500,6 +2508,10 @@ static int dispc_ovl_setup_common(enum omap_plane_id plane,
 	out_width = out_width == 0 ? width : out_width;
 	out_height = out_height == 0 ? height : out_height;
 
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported (%d).\n",
+	     out_width, dispc.feat->ovl_width_max);

Why don't you return an error here? I don't see a need for WARN here.

Quoted 7 lines
drivers/gpu/drm/omapdrm/dss/dispc.c +0 -0
 void dispc_set_ops(const struct dispc_ops *o);
drivers/gpu/drm/omapdrm/omap_connector.c +0 -0
index a0d7b1d905e8..d5e059abb555 100644
@@ -197,6 +197,16 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 			r = 0;

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 10 lines
drivers/gpu/drm/omapdrm/omap_connector.c +8 -0
 	}
 
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;

You should check the height also.

Tomi

Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#3 lore raw

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

Tomi Valkeinen <[email protected]> wrote on Wed [2018-Apr-04 14:12:13 +0300]:

Quoted 43 lines · Benoit Parrot
Quoted 42 lines

Currently available display mode from a connector are filtered out based only on pixel clock capability. However we also need to filter out wider mode if we cannot handle them based on available pipeline capabilities.

---
 drivers/gpu/drm/omapdrm/dss/dispc.c      | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  1 +
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++++++++
 3 files changed, 38 insertions(+)
drivers/gpu/drm/omapdrm/dss/dispc.c +8 -0
index 624dee22f46b..35541d4441df 100644
@@ -100,6 +100,8 @@ struct dispc_features {
 	u8 mgr_height_start;
 	u16 mgr_width_max;
 	u16 mgr_height_max;
+	u16 ovl_width_max;
+	u16 ovl_height_max;
 	unsigned long max_lcd_pclk;
 	unsigned long max_tv_pclk;
 	unsigned int max_downscale;
@@ -2465,6 +2467,12 @@ static int dispc_ovl_calc_scaling(unsigned long pclk, unsigned long lclk,
 	return 0;
 }
 
+static void dispc_ovl_get_max_size(u16 *width, u16 *height)
+{
+	*width = dispc.feat->ovl_width_max;
+	*height = dispc.feat->ovl_height_max;
+}
+
 static int dispc_ovl_setup_common(enum omap_plane_id plane,
 		enum omap_overlay_caps caps, u32 paddr, u32 p_uv_addr,
 		u16 screen_width, int pos_x, int pos_y, u16 width, u16 height,
@@ -2500,6 +2508,10 @@ static int dispc_ovl_setup_common(enum omap_plane_id plane,
 	out_width = out_width == 0 ? width : out_width;
 	out_height = out_height == 0 ? height : out_height;
 
Quoted 5 lines

drivers/gpu/drm/omapdrm/dss/dispc.c static int dispc_ovl_setup_common(enum omap_plane_id plane, · line 2513

Quoted 3 lines
drivers/gpu/drm/omapdrm/dss/dispc.c +3 -0
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported (%d).\n",
+	     out_width, dispc.feat->ovl_width_max);

Why don't you return an error here? I don't see a need for WARN here.

So here you mean replace the WARN with something like this:

	if (out_width > dispc.feat->ovl_width_max) {
		DSSERR("Requested OVL width (%d) is larger than can be supported (%d).\n",
		       out_width, dispc.feat->ovl_width_max);
                return -EINVAL;
	}
Quoted 10 lines
Quoted 10 lines
drivers/gpu/drm/omapdrm/dss/dispc.c +0 -0
 void dispc_set_ops(const struct dispc_ops *o);
drivers/gpu/drm/omapdrm/omap_connector.c +0 -0
index a0d7b1d905e8..d5e059abb555 100644
@@ -197,6 +197,16 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 			r = 0;
 	}
 
Quoted 10 lines

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 8 lines
drivers/gpu/drm/omapdrm/omap_connector.c +8 -0
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;

You should check the height also.

Yeah, I'll fix that.

Benoit

Quoted 6 lines

Tomi

Signature · 2 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#4 lore raw

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

Hi Benoit,

Quoted 55 lines · On Wednesday, 4 April 2018 16:15:11 EEST Benoit Parrot wrote:

Tomi Valkeinen wrote on Wed [2018-Apr-04 14:12:13 +0300]:

Quoted 47 lines · Benoit Parrot
Quoted 8 lines

Currently available display mode from a connector are filtered out based only on pixel clock capability. However we also need to filter out wider mode if we cannot handle them based on available pipeline capabilities.

---

Quoted 8 lines
 drivers/gpu/drm/omapdrm/dss/dispc.c      | 27 +++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  1 +
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++++++++++
 3 files changed, 38 insertions(+)
(quoted diff) +0 -0

b/drivers/gpu/drm/omapdrm/dss/dispc.c index 624dee22f46b..35541d4441df 100644

--- a/drivers/gpu/drm/omapdrm/dss/dispc.c

Quoted 29 lines

+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c

drivers/gpu/drm/omapdrm/dss/dispc.c +8 -0
@@ -100,6 +100,8 @@ struct dispc_features {
 	u8 mgr_height_start;
 	u16 mgr_width_max;
 	u16 mgr_height_max;
+	u16 ovl_width_max;
+	u16 ovl_height_max;
 	unsigned long max_lcd_pclk;
 	unsigned long max_tv_pclk;
 	unsigned int max_downscale;
@@ -2465,6 +2467,12 @@ static int dispc_ovl_calc_scaling(unsigned long
 pclk, unsigned long lclk,
 	return 0;
 }
 
+static void dispc_ovl_get_max_size(u16 *width, u16 *height)
+{
+	*width = dispc.feat->ovl_width_max;
+	*height = dispc.feat->ovl_height_max;
+}
+
 static int dispc_ovl_setup_common(enum omap_plane_id plane,
 		enum omap_overlay_caps caps, u32 paddr, u32 p_uv_addr,
drivers/gpu/drm/omapdrm/dss/dispc.c +0 -0
 		u16 screen_width, int pos_x, int pos_y, u16 width, u16 height,
@@ -2500,6 +2508,10 @@ static int dispc_ovl_setup_common(enum
 omap_plane_id plane,
 	out_width = out_width == 0 ? width : out_width;
 	out_height = out_height == 0 ? height : out_height;
 
Quoted 6 lines

drivers/gpu/drm/omapdrm/dss/dispc.c static int dispc_ovl_setup_common(enum omap_plane_id plane, · line 2513

Quoted 4 lines
drivers/gpu/drm/omapdrm/dss/dispc.c +3 -0
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported
 (%d).\n",
+	     out_width, dispc.feat->ovl_width_max);

Why don't you return an error here? I don't see a need for WARN here.

Quoted 4 lines

So here you mean replace the WARN with something like this:

	if (out_width > dispc.feat->ovl_width_max) {
		DSSERR("Requested OVL width (%d) is larger than can be supported (%d).

\n",

Quoted 3 lines
out_width, dispc.feat->ovl_width_max);
                return -EINVAL;
	}

Can this happen ? If we reject invalid settings in omapdrm we should never get them here.

Quoted 23 lines
Quoted 12 lines
Quoted 12 lines
drivers/gpu/drm/omapdrm/dss/omapdss.h +0 -0
 void dispc_set_ops(const struct dispc_ops *o);
(quoted diff) +0 -0

b/drivers/gpu/drm/omapdrm/omap_connector.c index
a0d7b1d905e8..d5e059abb555 100644

drivers/gpu/drm/omapdrm/omap_connector.c +0 -0
@@ -197,6 +197,16 @@ static int omap_connector_mode_valid(struct
 drm_connector *connector,
 			r = 0;
 	}
 
Quoted 10 lines

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 8 lines
drivers/gpu/drm/omapdrm/omap_connector.c +8 -0
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;

You should check the height also.

Quoted 1 line

Yeah, I'll fix that.

Unless I'm mistaken the restriction doesn't come from the output side of the display controller but from the overlays (planes), right ? Shouldn't it then be implemented in the drm_plane_helper_funcs.atomic_check operation ?

Signature · 10 lines
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#5 lore raw

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

Quoted 11 lines · Laurent Pinchart
Quoted 11 lines
Quoted 6 lines

drivers/gpu/drm/omapdrm/dss/dispc.c static int dispc_ovl_setup_common(enum omap_plane_id plane, · line 2513

Quoted 4 lines
(quoted diff) +2 -0
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported

(%d).\n",

drivers/gpu/drm/omapdrm/dss/dispc.c +1 -0
+	     out_width, dispc.feat->ovl_width_max);

Why don't you return an error here? I don't see a need for WARN here.

So here you mean replace the WARN with something like this:

	if (out_width > dispc.feat->ovl_width_max) {
		DSSERR("Requested OVL width (%d) is larger than can be supported (%d).
Quoted 7 lines

\n",

Quoted 3 lines
out_width, dispc.feat->ovl_width_max);
                return -EINVAL;
	}

Can this happen ? If we reject invalid settings in omapdrm we should never get them here.

That's true. And we should check them in the plane atomic check (but do we?).

In that case I don't mind a warn there, but you should still return an error if it happens, instead of continuing with bad config.

Quoted 11 lines
Quoted 11 lines
Quoted 10 lines

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 8 lines
drivers/gpu/drm/omapdrm/omap_connector.c +8 -0
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;

You should check the height also.

Quoted 5 lines
Quoted 1 line

Yeah, I'll fix that.

Unless I'm mistaken the restriction doesn't come from the output side of the display controller but from the overlays (planes), right ? Shouldn't it then be implemented in the drm_plane_helper_funcs.atomic_check operation ?

Yes, but I don't so. If our planes can support up to, say, 1000. Then we plug in a monitor with native width of 1100, which omapdrm would accept happily and try to use it by default. But we can't show fbdev or any normal setup there, because the planes won't support it. How would we manage that?

While not strictly correct, I think it's fine to reject videomodes which can't be shown with a normal full-screen plane.

Tomi

Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

Hi Tomi,

Quoted 16 lines · On Thursday, 5 April 2018 13:21:30 EEST Tomi Valkeinen wrote:
Quoted 15 lines · Laurent Pinchart
Quoted 14 lines
Quoted 6 lines

drivers/gpu/drm/omapdrm/dss/dispc.c static int dispc_ovl_setup_common(enum omap_plane_id plane, · line 2513

Quoted 4 lines
(quoted diff) +2 -0
+	WARN(out_width > dispc.feat->ovl_width_max,
+	     "Requested OVL width (%d) is larger than can be supported

(%d).\n",

drivers/gpu/drm/omapdrm/dss/dispc.c +1 -0
+	     out_width, dispc.feat->ovl_width_max);

Why don't you return an error here? I don't see a need for WARN here.

So here you mean replace the WARN with something like this:

	if (out_width > dispc.feat->ovl_width_max) {
		DSSERR("Requested OVL width (%d) is larger than can be supported
		(%d).\n",
out_width, dispc.feat->ovl_width_max);
                return -EINVAL;
	}
Quoted 5 lines
Quoted 2 lines

Can this happen ? If we reject invalid settings in omapdrm we should never get them here.

That's true. And we should check them in the plane atomic check (but do we?).

We don't, that should be added.

Quoted 2 lines

In that case I don't mind a warn there, but you should still return an error if it happens, instead of continuing with bad config.

But this should really not happen if we add a check to the CRTC atomic_check() handler. Do you distrust the DRM core that much ? :-)

Quoted 18 lines
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 8 lines
drivers/gpu/drm/omapdrm/omap_connector.c +8 -0
+	/* Check if the advertised width exceed what the pipeline can do */
+	if (!r) {
+		struct omap_drm_private *priv = dev->dev_private;
+		u16 width, height;
+
+		priv->dispc_ops->ovl_get_max_size(&width, &height);
+		if (mode->hdisplay > width)
+			r = -EINVAL;

You should check the height also.

Quoted 6 lines
Quoted 1 line

Yeah, I'll fix that.

Unless I'm mistaken the restriction doesn't come from the output side of the display controller but from the overlays (planes), right ? Shouldn't it then be implemented in the drm_plane_helper_funcs.atomic_check operation ?

Quoted 8 lines

Yes, but I don't so. If our planes can support up to, say, 1000. Then we plug in a monitor with native width of 1100, which omapdrm would accept happily and try to use it by default. But we can't show fbdev or any normal setup there, because the planes won't support it. How would we manage that?

While not strictly correct, I think it's fine to reject videomodes which can't be shown with a normal full-screen plane.

It could be argued that such modes would still be useful even if planes can't be shown full-screen, or that two planes could be used side by side to achieve a larger full-screen display than what would be possible with a single plane. I'll leave it up to you to decide whether we should support such use cases.

Signature · 10 lines
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#7 lore raw

PATCH v2 2/6 dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt

In reply to Benoit Parrot

Add common DISPC bindings into the top level bindings file. Move common bindings here instead of having multiple copies of the same information in all of the variant specific files.

---
 Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt  | 5 -----
 Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt  | 7 +++++++
 Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt | 4 ----
 6 files changed, 7 insertions(+), 21 deletions(-)
Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt +0 -5
index 91279f1060fe..c30f9ec189ed 100644
@@ -47,11 +47,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
-			in bytes per second
-
-
 HDMI
 ----
 
Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +7 -0
index e1ef29569338..249e588d7865 100644
@@ -21,6 +21,13 @@ a RGB pixel stream to encoders.
 The encoder modules encode the received RGB pixel stream to a video output like
 HDMI, MIPI DPI, etc.
 
+DISPC
+-----
+
+Optional properties:
+- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
+			in bytes per second
+
 Video Ports
 -----------
 
Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt +0 -4
index ee867c4d1152..afcd5a86c6a4 100644
@@ -28,10 +28,6 @@ Required properties:
 - ti,hwmods: "dss_dispc"
 - interrupts: the DISPC interrupt
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
-			in bytes per second
-
 
 RFBI
 ----
Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt +0 -4
index cd02516a40b6..dc66e1447c31 100644
@@ -37,10 +37,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
-			in bytes per second
-
 
 RFBI
 ----
Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt +0 -4
index 0f85f6b3a5a8..bc624db8888d 100644
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
-			in bytes per second
-
 
 RFBI
 ----
Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt +0 -4
index 20861218649f..118a486c47bb 100644
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
-			in bytes per second
-
 
 RFBI
 ----
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#8 lore raw

Re: [Patch v2 2/6] dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt

Hi Benoit,

Thank you for the patch.

Quoted 47 lines · On Monday, 26 March 2018 19:21:24 EEST Benoit Parrot wrote:

Add common DISPC bindings into the top level bindings file. Move common bindings here instead of having multiple copies of the same information in all of the variant specific files.

---
 Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt  | 5 -----
 Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt  | 7 +++++++
 Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt | 4 ----
 6 files changed, 7 insertions(+), 21 deletions(-)
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt index 91279f1060fe..c30f9ec189ed 100644

Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt +0 -2
@@ -47,11 +47,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second

Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt +0 -2
-
-
 HDMI
 ----
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index e1ef29569338..249e588d7865 100644

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +0 -0
@@ -21,6 +21,13 @@ a RGB pixel stream to encoders.
 The encoder modules encode the received RGB pixel stream to a video output

like HDMI, MIPI DPI, etc.

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +5 -0
+DISPC
+-----
+
+Optional properties:
+- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt a RGB pixel stream to encoders. · line 30

Quoted 2 lines
Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +2 -0
+			in bytes per second
+

I like the idea, but I think you need to expand the description a bit. ti,omap-dss.txt should explain how each module should be represented by a child DT node in the DSS DT node, and this section should briefly explain what the DISPC is (you can simply move the 4th paragraph of the file here) and that it describes common properties. You could also move the description of other DISPC properties here.

Quoted 67 lines

Video Ports
-----------

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt index ee867c4d1152..afcd5a86c6a4 100644

Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt +0 -2
@@ -28,10 +28,6 @@ Required properties:
 - ti,hwmods: "dss_dispc"
 - interrupts: the DISPC interrupt
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second

Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt +0 -1
-
 
 RFBI
 ----
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt index cd02516a40b6..dc66e1447c31 100644

Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt +0 -2
@@ -37,10 +37,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second

Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt +0 -1
-
 
 RFBI
 ----
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt index 0f85f6b3a5a8..bc624db8888d 100644

Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt +0 -2
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second

Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt +0 -1
-
 
 RFBI
 ----
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt index 20861218649f..118a486c47bb 100644

Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt +0 -2
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second
-

RFBI
----

Signature · 10 lines
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#9 lore raw

Re: [Patch v2 2/6] dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt

Laurent Pinchart <[email protected]> wrote on Wed [2018-Apr-04 17:29:59 +0300]:

Quoted 46 lines

Hi Benoit,

Thank you for the patch.

Quoted 41 lines · On Monday, 26 March 2018 19:21:24 EEST Benoit Parrot wrote:

Add common DISPC bindings into the top level bindings file. Move common bindings here instead of having multiple copies of the same information in all of the variant specific files.

---
 Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt  | 5 -----
 Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt  | 7 +++++++
 Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt | 4 ----
 Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt | 4 ----
 6 files changed, 7 insertions(+), 21 deletions(-)
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt index 91279f1060fe..c30f9ec189ed 100644

Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt +0 -2
@@ -47,11 +47,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit - in bytes per second

Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt +0 -2
-
-
 HDMI
 ----
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index e1ef29569338..249e588d7865 100644

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +0 -0
@@ -21,6 +21,13 @@ a RGB pixel stream to encoders.
 The encoder modules encode the received RGB pixel stream to a video output

like HDMI, MIPI DPI, etc.

Quoted 14 lines

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt a RGB pixel stream to encoders. · line 30

Quoted 8 lines
Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +5 -0
+DISPC
+-----
+
+Optional properties:
+- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +2 -0
+			in bytes per second
+

I like the idea, but I think you need to expand the description a bit. ti,omap-dss.txt should explain how each module should be represented by a child DT node in the DSS DT node, and this section should briefly explain what the DISPC is (you can simply move the 4th paragraph of the file here) and that it describes common properties.

Yes, I'll do that in the next revision.

Quoted 2 lines

You could also move the description of other
DISPC properties here.

I had done that originally but Tomi pointed out that most of the other properties are not common with all of the platform variants, some would have multiple clocks or registers block or no hwmod components. I thought it would more confusing to have some of those variant specific "required" properties be shown here as optional.

Benoit
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#10 lore raw

PATCH v2 3/6 dt-bindings: display/ti: Add plane binding to dispc node

In reply to Benoit Parrot

Currently all available display pipelines (i.e. plane) and output port resources are exposed to user-space. In some cases it is needed to be able to restrict which resources are actually visible from user-space. Also in cases where a display wider than 2048 pixels is to be supported more than one video pipeline is needed. In this case the 2nd hardware pipeline needed is not visible to user space applications.

These video pipeline definitions must be statically defined so that the number of visible pipelines does not change from the user-space perspective.

In order to allow this we are adding an optional 'plane' sub-node to the generic DISPC node.

---
 .../devicetree/bindings/display/ti/ti,omap-dss.txt | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +65 -0
index 249e588d7865..2dd411cb5a83 100644
@@ -28,6 +28,36 @@ Optional properties:
 - max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
 			in bytes per second
 
+Optional Subnode:
+- plane: Child node(s) which defines which video planes are available to
+	the system. If at least one plane child node is defined then
+	only planes defined by these nodes will be available to the system.
+	Plane nodes must be sequential starting with reg = <0> as DT parsing
+	will stop on the first missing numbered node.
+	This means if plane #1 is defined but plane #0 is not then it will
+	be as if none of the plane nodes were defined.
+
+	Each plane node contains the following properties:
+	Required properties:
+	- reg:       Used to identify the plane
+	- video-pipelines: One or two HW pipeline number(s).
+		When 2 numbers are present this indicates a virtual wide
+		plane composed of two physical planes intended to be used
+		when the display is larger then the capacity of a
+		single plane i.e. wider than 2048 pixels.
+		The first number in the pair will dictate the capabilities
+		of the plane. This means that for proper
+		operation the virtual plane should be composed of HW
+		planes of the same capabilities.
+		If GFX pipeline is used in a virtual plane it should be
+		specified first, otherwise unexpected behavior would
+		be encountered.
+	Optional property:
+	- video-outputs:  One or more HW output number(s).
+		Describe the list of video output on which this plane
+		is available. If this node is not present then the
+		plane will be available on all available video output.
+
 Video Ports
 -----------
 
@@ -216,3 +246,38 @@ OMAP HDMI --(HDMI)--> TPD12S015 --(HDMI)--> HDMI Connector
 		};
 	};
 };
+
+A short example on how to define a virtual plane configuration
+to enable wide display support.
+Here we define:
+- plane#0 to be the HW pipeline #0 (i.e. GFX pipeline)
+	  only available on video output #0
+- plane#1 to be a virtual wide plane composed of HW pipeline #1 and #2
+	  (i.e. VID1 & VID2) available on video output #0 & #1
+- plane#2 to be the HW pipeline #3 (i.e. VID3 pipeline)
+	  only available on video output #0
+
+&dss {
+        dispc@58001000 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                plane@0 {
+                        reg = <0>;
+                        video-pipelines = <0>;
+                        video-outputs = <0>;
+                };
+
+                plane@1 {
+                        reg = <1>;
+                        video-pipelines = <1 2>;
+                        video-outputs = <0 1>;
+                };
+
+                plane@2 {
+                        reg = <2>;
+                        video-pipelines = <3>;
+                        video-outputs = <0>;
+                };
+        };
+};
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#11 lore raw

Re: [Patch v2 3/6] dt-bindings: display/ti: Add plane binding to dispc node

Hi Benoit,

Thank you for the patch.

Quoted 7 lines · On Monday, 26 March 2018 19:21:25 EEST Benoit Parrot wrote:

Currently all available display pipelines (i.e. plane) and output port resources are exposed to user-space. In some cases it is needed to be able to restrict which resources are actually visible from user-space. Also in cases where a display wider than 2048 pixels is to be supported more than one video pipeline is needed. In this case the 2nd hardware pipeline needed is not visible to user space applications.

Quoted 6 lines

These video pipeline definitions must be statically defined so that the number of visible pipelines does not change from the user-space perspective.

In order to allow this we are adding an optional 'plane' sub-node to the generic DISPC node.

I'm sorry but this is really configuration data, it doesn't describe the hardware. I don't think these properties belong to DT.

Quoted 86 lines
---
 .../devicetree/bindings/display/ti/ti,omap-dss.txt | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index 249e588d7865..2dd411cb5a83 100644

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +0 -0
@@ -28,6 +28,36 @@ Optional properties:
 - max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth

limit in bytes per second

Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt +65 -0
+Optional Subnode:
+- plane: Child node(s) which defines which video planes are available to
+	the system. If at least one plane child node is defined then
+	only planes defined by these nodes will be available to the system.
+	Plane nodes must be sequential starting with reg = <0> as DT parsing
+	will stop on the first missing numbered node.
+	This means if plane #1 is defined but plane #0 is not then it will
+	be as if none of the plane nodes were defined.
+
+	Each plane node contains the following properties:
+	Required properties:
+	- reg:       Used to identify the plane
+	- video-pipelines: One or two HW pipeline number(s).
+		When 2 numbers are present this indicates a virtual wide
+		plane composed of two physical planes intended to be used
+		when the display is larger then the capacity of a
+		single plane i.e. wider than 2048 pixels.
+		The first number in the pair will dictate the capabilities
+		of the plane. This means that for proper
+		operation the virtual plane should be composed of HW
+		planes of the same capabilities.
+		If GFX pipeline is used in a virtual plane it should be
+		specified first, otherwise unexpected behavior would
+		be encountered.
+	Optional property:
+	- video-outputs:  One or more HW output number(s).
+		Describe the list of video output on which this plane
+		is available. If this node is not present then the
+		plane will be available on all available video output.
+
 Video Ports
 -----------
@@ -216,3 +246,38 @@ OMAP HDMI --(HDMI)--> TPD12S015 --(HDMI)--> HDMI
 Connector };
 	};
 };
+
+A short example on how to define a virtual plane configuration
+to enable wide display support.
+Here we define:
+- plane#0 to be the HW pipeline #0 (i.e. GFX pipeline)
+	  only available on video output #0
+- plane#1 to be a virtual wide plane composed of HW pipeline #1 and #2
+	  (i.e. VID1 & VID2) available on video output #0 & #1
+- plane#2 to be the HW pipeline #3 (i.e. VID3 pipeline)
+	  only available on video output #0
+
+&dss {
+        dispc@58001000 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                plane@0 {
+                        reg = <0>;
+                        video-pipelines = <0>;
+                        video-outputs = <0>;
+                };
+
+                plane@1 {
+                        reg = <1>;
+                        video-pipelines = <1 2>;
+                        video-outputs = <0 1>;
+                };
+
+                plane@2 {
+                        reg = <2>;
+                        video-pipelines = <3>;
+                        video-outputs = <0>;
+                };
+        };
+};
Signature · 10 lines
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#12 lore raw

Re: [Patch v2 3/6] dt-bindings: display/ti: Add plane binding to dispc node

Quoted 16 lines · Laurent Pinchart

Hi Benoit,

Thank you for the patch.

Quoted 11 lines · On Monday, 26 March 2018 19:21:25 EEST Benoit Parrot wrote:

Currently all available display pipelines (i.e. plane) and output port resources are exposed to user-space. In some cases it is needed to be able to restrict which resources are actually visible from user-space. Also in cases where a display wider than 2048 pixels is to be supported more than one video pipeline is needed. In this case the 2nd hardware pipeline needed is not visible to user space applications.

These video pipeline definitions must be statically defined so that the number of visible pipelines does not change from the user-space perspective.

Quoted 5 lines
Quoted 2 lines

In order to allow this we are adding an optional 'plane' sub-node to the generic DISPC node.

I'm sorry but this is really configuration data, it doesn't describe the hardware. I don't think these properties belong to DT.

I agree, but the question then is: where should it be? There was a discussion in the v1 thread about this, and as far as I see, there are no other usable alternatives.

Tomi

Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#13 lore raw

Re: [Patch v2 3/6] dt-bindings: display/ti: Add plane binding to dispc node

Quoted 20 lines · Tomi Valkeinen
Quoted 19 lines · Laurent Pinchart

Hi Benoit,

Thank you for the patch.

Quoted 13 lines · On Monday, 26 March 2018 19:21:25 EEST Benoit Parrot wrote:

Currently all available display pipelines (i.e. plane) and output port resources are exposed to user-space. In some cases it is needed to be able to restrict which resources are actually visible from user-space. Also in cases where a display wider than 2048 pixels is to be supported more than one video pipeline is needed. In this case the 2nd hardware pipeline needed is not visible to user space applications.

These video pipeline definitions must be statically defined so that the number of visible pipelines does not change from the user-space perspective.

In order to allow this we are adding an optional 'plane' sub-node to the generic DISPC node.

Quoted 6 lines
Quoted 2 lines

I'm sorry but this is really configuration data, it doesn't describe the hardware. I don't think these properties belong to DT.

I agree, but the question then is: where should it be? There was a discussion in the v1 thread about this, and as far as I see, there are no other usable alternatives.

Runtime configuration and atomic_check not an option? See my reply in v1. -Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#14 lore raw

PATCH v2 4/6 drm/omap: Add virtual plane DT parsing support

In reply to Benoit Parrot

Virtual planes are used to extend display size capability for display larger than 2048 pixels by splitting the frame buffer equally between two physical video-pipelines.

Here we are adding DT support to parse 'plane' child nodes which describe how logical planes are mapped to physical video-pipeline(s) and which video-outputs they are available on.

---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 110 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  11 ++++
 2 files changed, 121 insertions(+)
drivers/gpu/drm/omapdrm/dss/dispc.c +110 -0
index 35541d4441df..06a2e894175e 100644
@@ -4360,6 +4360,115 @@ static u32 dispc_get_memory_bandwidth_limit(void)
 	return limit;
 }
 
+static struct device_node *dispc_of_get_plane_by_id(struct device_node *node,
+						    u32 id)
+{
+	struct device_node *plane;
+
+	for_each_child_of_node(node, plane) {
+		u32 plane_id = 0;
+
+		if (of_node_cmp(plane->name, "plane") != 0)
+			continue;
+		of_property_read_u32(plane, "reg", &plane_id);
+		if (id == plane_id)
+			return plane;
+	}
+
+	return NULL;
+}
+
+static int dispc_parse_dt_plane_data(struct dispc_plane_mappings *map)
+{
+	struct platform_device *pdev = dispc.pdev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *ep;
+	struct property *prop;
+	const __be32 *cur;
+	u32 v;
+	u32 num_ovls = dispc_get_num_ovls();
+	u32 hw_plane_mask = 0;
+	u32 num_planes;
+	int i, index;
+
+	if (!np)
+		return 0;
+
+	for (i = 0; i < num_ovls; i++) {
+		ep = dispc_of_get_plane_by_id(np, i);
+		if (!ep)
+			break;
+		if (!of_property_read_bool(ep, "video-pipelines")) {
+			dev_err(&pdev->dev,
+				"malformed plane node: video-pipelines missing.\n");
+			return -EINVAL;
+		}
+
+		index = 0;
+		of_property_for_each_u32(ep, "video-pipelines", prop, cur, v) {
+			if (v >= num_ovls) {
+				dev_err(&pdev->dev,
+					"video-pipelines property: '%d' out-of-range.\n",
+					v);
+				return -EINVAL;
+			}
+			if (hw_plane_mask & BIT_MASK(v)) {
+				dev_err(&pdev->dev,
+					"video-pipelines property: '%d' used more than once.\n",
+					v);
+				return -EINVAL;
+			}
+			hw_plane_mask |= BIT(v);
+
+			if (index == 0) {
+				map->plane[i].main_id = v;
+			} else if (index == 1) {
+				map->plane[i].aux_id = v;
+				map->plane[i].is_virtual = true;
+			} else if (index > 1) {
+				dev_err(&pdev->dev,
+					"video-pipelines property: more than 2 values specified.\n");
+				return -EINVAL;
+			}
+			index++;
+		}
+
+		of_property_for_each_u32(ep, "video-outputs", prop, cur, v) {
+			if (v >= dispc_get_num_mgrs()) {
+				dev_err(&pdev->dev,
+					"video-outputs property: '%d' out-of-range.\n",
+					v);
+				return -EINVAL;
+			}
+			map->plane[i].crtc_mask |= BIT(v);
+		}
+	}
+
+	num_planes = i;
+
+	if (num_planes) {
+		dev_dbg(&pdev->dev, "Plane definitions found from DT:");
+		for (i = 0; i < num_planes; i++) {
+			if (map->plane[i].is_virtual) {
+				dev_dbg(&pdev->dev,
+					"plane%d: virtual video-pipelines: %d, %d video-output mask: 0x%04x",
+					i, map->plane[i].main_id,
+					map->plane[i].aux_id,
+					map->plane[i].crtc_mask);
+			} else {
+				dev_dbg(&pdev->dev,
+					"plane%d: video-pipelines: %d video-output mask: 0x%04x",
+					i, map->plane[i].main_id,
+					map->plane[i].crtc_mask);
+			}
+		}
+	}
+
+	map->num_planes = num_planes;
+
+	return 0;
+}
+
 /*
  * Workaround for errata i734 in DSS dispc
  *  - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled
@@ -4552,6 +4661,7 @@ static const struct dispc_ops dispc_ops = {
 	.ovl_setup = dispc_ovl_setup,
 	.ovl_get_color_modes = dispc_ovl_get_color_modes,
 	.ovl_get_max_size = dispc_ovl_get_max_size,
+	.get_plane_mapping = dispc_parse_dt_plane_data,
 };
 
 /* DISPC HW IP initialisation */
drivers/gpu/drm/omapdrm/dss/omapdss.h +11 -0
index c58c75292182..ad0f751ec047 100644
@@ -637,6 +637,16 @@ void omapdss_set_is_initialized(bool set);
 struct device_node *dss_of_port_get_parent_device(struct device_node *port);
 u32 dss_of_port_get_port_number(struct device_node *port);
 
+struct dispc_plane_mappings {
+	struct {
+		u32 main_id;
+		u32 aux_id;
+		u32 crtc_mask;
+		bool is_virtual;
+	} plane[4];
+	u32 num_planes;
+};
+
 struct dss_mgr_ops {
 	int (*connect)(enum omap_channel channel,
 		struct omap_dss_device *dst);
@@ -720,6 +730,7 @@ struct dispc_ops {
 
 	const u32 *(*ovl_get_color_modes)(enum omap_plane_id plane);
 	void (*ovl_get_max_size)(u16 *width, u16 *height);
+	int (*get_plane_mapping)(struct dispc_plane_mappings *plane);
 };
 
 void dispc_set_ops(const struct dispc_ops *o);
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#15 lore raw

PATCH v2 5/6 drm/omap: Add virtual plane support to omap_plane

In reply to Benoit Parrot

Add virtual wide plane support by adding an secondary plane_id so that an "omap_plane" can be composed of up to two physical planes.

When at least one 'plane' child node is present in DT then omap_plane_init will only use the plane described in DT. Some of these nodes may be a virtual wide plane if they are defined as two physical planes.
Planes can also be associated with various crtcs independently. Therefore we can restrict the use of virtual plane to specific CRTC/video port if need be, if crtc_mask is not specified then the plane will be available to all available crtcs. Physical planes which are not described will essentially be hidden from the driver.

If no 'plane' child nodes exist then the normal plane allocation will take place.

---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 127 +++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_fb.c    |  66 ++++++++++-----
 drivers/gpu/drm/omapdrm/omap_fb.h    |   4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c | 151 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/omapdrm/omap_plane.h |   4 +-
 5 files changed, 283 insertions(+), 69 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.c +121 -6
index 37ee20c773c7..4c43ef239136 100644
@@ -16,6 +16,7 @@
  */
 
 #include <linux/sys_soc.h>
+#include <linux/sort.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -116,6 +117,112 @@ static void omap_atomic_commit_tail(struct drm_atomic_state *old_state)
 	priv->dispc_ops->runtime_put();
 }
 
+static int omap_atomic_state_zpos_cmp(const void *a, const void *b)
+{
+	const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
+	const struct drm_plane_state *sb = *(struct drm_plane_state **)b;
+
+	if (sa->zpos != sb->zpos)
+		return sa->zpos - sb->zpos;
+	else
+		return sa->plane->base.id - sb->plane->base.id;
+}
+
+static int omap_atomic_crtc_normalize_zpos(struct drm_crtc *crtc,
+					   struct drm_crtc_state *crtc_state)
+{
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_device *dev = crtc->dev;
+	int total_planes = dev->mode_config.num_total_plane;
+	struct drm_plane_state **states;
+	struct drm_plane *plane;
+	int i, inc, n = 0;
+	int ret = 0;
+
+	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+			 crtc->base.id, crtc->name);
+
+	states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
+	if (!states)
+		return -ENOMEM;
+
+	/*
+	 * Normalization process might create new states for planes which
+	 * normalized_zpos has to be recalculated.
+	 */
+	drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
+		struct drm_plane_state *plane_state =
+			drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto done;
+		}
+		states[n++] = plane_state;
+		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
+				 plane->base.id, plane->name,
+				 plane_state->zpos);
+	}
+
+	sort(states, n, sizeof(*states), omap_atomic_state_zpos_cmp, NULL);
+
+	for (inc = 0, i = 0; i < n; i++) {
+		plane = states[i]->plane;
+
+		states[i]->normalized_zpos = i + inc;
+		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+				 plane->base.id, plane->name, i + inc);
+		/*
+		 * If the current plane is virtual it uses 2 hw planes
+		 * therefore the very next zpos is used by the secondary/aux
+		 * plane so we need to skip one zpos from this point on.
+		 */
+		if (is_omap_plane_virtual(plane))
+			inc++;
+	}
+	crtc_state->zpos_changed = true;
+
+done:
+	kfree(states);
+	return ret;
+}
+
+static int omap_atomic_normalize_zpos(struct drm_device *dev,
+				      struct drm_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	int i, ret = 0;
+
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
+		    new_crtc_state->zpos_changed) {
+			ret = omap_atomic_crtc_normalize_zpos(crtc,
+							      new_crtc_state);
+			if (ret)
+				return ret;
+		}
+	}
+	return 0;
+}
+
+static int omap_atomic_check(struct drm_device *dev,
+			     struct drm_atomic_state *state)
+{
+	int ret;
+
+	ret = drm_atomic_helper_check(dev, state);
+	if (ret)
+		return ret;
+
+	if (dev->mode_config.normalize_zpos) {
+		ret = omap_atomic_normalize_zpos(dev, state);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs = {
 	.atomic_commit_tail = omap_atomic_commit_tail,
 };
@@ -123,7 +230,7 @@ static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs =
 static const struct drm_mode_config_funcs omap_mode_config_funcs = {
 	.fb_create = omap_framebuffer_create,
 	.output_poll_changed = drm_fb_helper_output_poll_changed,
-	.atomic_check = drm_atomic_helper_check,
+	.atomic_check = omap_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
@@ -188,10 +295,9 @@ static int omap_connect_dssdevs(void)
 	return r;
 }
 
-static int omap_modeset_init_properties(struct drm_device *dev)
+static int omap_modeset_init_properties(struct drm_device *dev, u32 num_planes)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	unsigned int num_planes = priv->dispc_ops->get_num_ovls();
 
 	priv->zorder_prop = drm_property_create_range(dev, 0, "zorder", 0,
 						      num_planes - 1);
@@ -210,10 +316,19 @@ static int omap_modeset_init(struct drm_device *dev)
 	int num_crtcs, crtc_idx, plane_idx;
 	int ret;
 	u32 plane_crtc_mask;
+	struct dispc_plane_mappings plane_mappings = {0};
 
 	drm_mode_config_init(dev);
 
-	ret = omap_modeset_init_properties(dev);
+	ret = priv->dispc_ops->get_plane_mapping(&plane_mappings);
+	if (ret < 0)
+		return ret;
+
+	/* use plane mappings info */
+	if (plane_mappings.num_planes)
+		num_ovls = plane_mappings.num_planes;
+
+	ret = omap_modeset_init_properties(dev, num_ovls);
 	if (ret < 0)
 		return ret;
 
@@ -266,7 +381,7 @@ static int omap_modeset_init(struct drm_device *dev)
 			return -ENOMEM;
 
 		plane = omap_plane_init(dev, plane_idx, DRM_PLANE_TYPE_PRIMARY,
-					plane_crtc_mask);
+					plane_crtc_mask, &plane_mappings);
 		if (IS_ERR(plane))
 			return PTR_ERR(plane);
 
@@ -296,7 +411,7 @@ static int omap_modeset_init(struct drm_device *dev)
 			return -EINVAL;
 
 		plane = omap_plane_init(dev, plane_idx, DRM_PLANE_TYPE_OVERLAY,
-			plane_crtc_mask);
+					plane_crtc_mask, &plane_mappings);
 		if (IS_ERR(plane))
 			return PTR_ERR(plane);
 
drivers/gpu/drm/omapdrm/omap_fb.c +44 -22

107 lines not on this page · raw message

drivers/gpu/drm/omapdrm/omap_fb.h +3 -1
index 94ad5f9e4404..f68e81353288 100644
@@ -37,7 +37,9 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 int omap_framebuffer_pin(struct drm_framebuffer *fb);
 void omap_framebuffer_unpin(struct drm_framebuffer *fb);
 void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
-		struct drm_plane_state *state, struct omap_overlay_info *info);
+		struct drm_plane_state *state,
+		struct omap_overlay_info *main_info,
+		struct omap_overlay_info *aux_info);
 struct drm_connector *omap_framebuffer_get_next_connector(
 		struct drm_framebuffer *fb, struct drm_connector *from);
 bool omap_framebuffer_supports_rotation(struct drm_framebuffer *fb);
drivers/gpu/drm/omapdrm/omap_plane.c +112 -39

235 lines not on this page · raw message

drivers/gpu/drm/omapdrm/omap_plane.h +3 -1
index dc5e82ad061d..48815a05f4fe 100644
@@ -30,8 +30,10 @@ struct drm_plane;
 
 struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int idx, enum drm_plane_type type,
-		u32 possible_crtcs);
+		u32 possible_crtcs,
+		struct dispc_plane_mappings *plane_mappings);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
+bool is_omap_plane_virtual(struct drm_plane *plane);
 
 #endif /* __OMAPDRM_PLANE_H__ */
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#16 lore raw

Re: [Patch v2 5/6] drm/omap: Add virtual plane support to omap_plane

Quoted 15 lines · Benoit Parrot

Add virtual wide plane support by adding an secondary plane_id so that an "omap_plane" can be composed of up to two physical planes.

When at least one 'plane' child node is present in DT then omap_plane_init will only use the plane described in DT. Some of these nodes may be a virtual wide plane if they are defined as two physical planes.
Planes can also be associated with various crtcs independently. Therefore we can restrict the use of virtual plane to specific CRTC/video port if need be, if crtc_mask is not specified then the plane will be available to all available crtcs. Physical planes which are not described will essentially be hidden from the driver.

Quoted 2 lines

If no 'plane' child nodes exist then the normal plane allocation will take place.

The descs in many of the patches are a bit messy. How do you format them? At least vim's autoformat gives a more consistent wrapping.

Quoted 135 lines
---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 127 +++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_fb.c    |  66 ++++++++++-----
 drivers/gpu/drm/omapdrm/omap_fb.h    |   4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c | 151 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/omapdrm/omap_plane.h |   4 +-
 5 files changed, 283 insertions(+), 69 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.c +107 -0
index 37ee20c773c7..4c43ef239136 100644
@@ -16,6 +16,7 @@
  */
 
 #include <linux/sys_soc.h>
+#include <linux/sort.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -116,6 +117,112 @@ static void omap_atomic_commit_tail(struct drm_atomic_state *old_state)
 	priv->dispc_ops->runtime_put();
 }
 
+static int omap_atomic_state_zpos_cmp(const void *a, const void *b)
+{
+	const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
+	const struct drm_plane_state *sb = *(struct drm_plane_state **)b;
+
+	if (sa->zpos != sb->zpos)
+		return sa->zpos - sb->zpos;
+	else
+		return sa->plane->base.id - sb->plane->base.id;
+}
+
+static int omap_atomic_crtc_normalize_zpos(struct drm_crtc *crtc,
+					   struct drm_crtc_state *crtc_state)
+{
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_device *dev = crtc->dev;
+	int total_planes = dev->mode_config.num_total_plane;
+	struct drm_plane_state **states;
+	struct drm_plane *plane;
+	int i, inc, n = 0;
+	int ret = 0;
+
+	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+			 crtc->base.id, crtc->name);
+
+	states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
+	if (!states)
+		return -ENOMEM;
+
+	/*
+	 * Normalization process might create new states for planes which
+	 * normalized_zpos has to be recalculated.
+	 */
+	drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
+		struct drm_plane_state *plane_state =
+			drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto done;
+		}
+		states[n++] = plane_state;
+		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
+				 plane->base.id, plane->name,
+				 plane_state->zpos);
+	}
+
+	sort(states, n, sizeof(*states), omap_atomic_state_zpos_cmp, NULL);
+
+	for (inc = 0, i = 0; i < n; i++) {
+		plane = states[i]->plane;
+
+		states[i]->normalized_zpos = i + inc;
+		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+				 plane->base.id, plane->name, i + inc);
+		/*
+		 * If the current plane is virtual it uses 2 hw planes
+		 * therefore the very next zpos is used by the secondary/aux
+		 * plane so we need to skip one zpos from this point on.
+		 */
+		if (is_omap_plane_virtual(plane))
+			inc++;
+	}
+	crtc_state->zpos_changed = true;
+
+done:
+	kfree(states);
+	return ret;
+}
+
+static int omap_atomic_normalize_zpos(struct drm_device *dev,
+				      struct drm_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	int i, ret = 0;
+
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
+		    new_crtc_state->zpos_changed) {
+			ret = omap_atomic_crtc_normalize_zpos(crtc,
+							      new_crtc_state);
+			if (ret)
+				return ret;
+		}
+	}
+	return 0;
+}
+
+static int omap_atomic_check(struct drm_device *dev,
+			     struct drm_atomic_state *state)
+{
+	int ret;
+
+	ret = drm_atomic_helper_check(dev, state);
+	if (ret)
+		return ret;
+
+	if (dev->mode_config.normalize_zpos) {
+		ret = omap_atomic_normalize_zpos(dev, state);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs = {
 	.atomic_commit_tail = omap_atomic_commit_tail,
 };

drivers/gpu/drm/omapdrm/omap_drv.c static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs = · line 235

Quoted 7 lines
drivers/gpu/drm/omapdrm/omap_drv.c +1 -1
@@ -123,7 +230,7 @@ static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs =
 static const struct drm_mode_config_funcs omap_mode_config_funcs = {
 	.fb_create = omap_framebuffer_create,
 	.output_poll_changed = drm_fb_helper_output_poll_changed,
-	.atomic_check = drm_atomic_helper_check,
+	.atomic_check = omap_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };

I don't think it's a good idea to copypaste drm_atomic_helper_crtc_normalize_zpos from the drm core. With Peter's patches, the drm core does the normalizing, doesn't it? So they are already sorted properly, and you can just go through them and adjust the zpos if there are virtual planes.

Tomi

Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#17 lore raw

PATCH v2 6/6 drm/omap: Allow wider display when a virtual plane is available

In reply to Benoit Parrot

Add an exception case when filtering out display mode so that if a virtual wide plane is available then display wider than 2048 can be supported as long as the required timing parameters can also be met.

---
 drivers/gpu/drm/omapdrm/omap_connector.c |  3 ++-
 drivers/gpu/drm/omapdrm/omap_plane.c     | 12 ++++++++++++
 drivers/gpu/drm/omapdrm/omap_plane.h     |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)
drivers/gpu/drm/omapdrm/omap_connector.c +2 -1
index d5e059abb555..517f7fa80ce1 100644
@@ -203,7 +203,8 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 		u16 width, height;
 
 		priv->dispc_ops->ovl_get_max_size(&width, &height);
-		if (mode->hdisplay > width)
+		if (mode->hdisplay > width &&
+		    !omap_have_any_virtual_plane(dev))
 			r = -EINVAL;
 	}
 
drivers/gpu/drm/omapdrm/omap_plane.c +12 -0
index 21c927bbf5a7..8529abdcdeb8 100644
@@ -209,6 +209,18 @@ bool is_omap_plane_virtual(struct drm_plane *plane)
 	return omap_plane->virtual_plane;
 }
 
+bool omap_have_any_virtual_plane(struct drm_device *dev)
+{
+	struct drm_plane *plane;
+
+	drm_for_each_plane(plane, dev) {
+		if (is_omap_plane_virtual(plane))
+			return true;
+	}
+
+	return false;
+}
+
 /* helper to install properties which are common to planes and crtcs */
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj)
drivers/gpu/drm/omapdrm/omap_plane.h +1 -0
index 48815a05f4fe..86b1c2022231 100644
@@ -35,5 +35,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
 bool is_omap_plane_virtual(struct drm_plane *plane);
+bool omap_have_any_virtual_plane(struct drm_device *dev);
 
 #endif /* __OMAPDRM_PLANE_H__ */
Signature · 6 lines
2.9.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#18 lore raw

Re: [Patch v2 6/6] drm/omap: Allow wider display when a virtual plane is available

Quoted 16 lines · Benoit Parrot

Add an exception case when filtering out display mode so that if a virtual wide plane is available then display wider than 2048 can be supported as long as the required timing parameters can also be met.

---
 drivers/gpu/drm/omapdrm/omap_connector.c |  3 ++-
 drivers/gpu/drm/omapdrm/omap_plane.c     | 12 ++++++++++++
 drivers/gpu/drm/omapdrm/omap_plane.h     |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)
drivers/gpu/drm/omapdrm/omap_connector.c +0 -0
index d5e059abb555..517f7fa80ce1 100644

drivers/gpu/drm/omapdrm/omap_connector.c static int omap_connector_mode_valid(struct drm_connector *connector, · line 207

Quoted 6 lines
drivers/gpu/drm/omapdrm/omap_connector.c +2 -1
@@ -203,7 +203,8 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
 		u16 width, height;
 
 		priv->dispc_ops->ovl_get_max_size(&width, &height);
-		if (mode->hdisplay > width)
+		if (mode->hdisplay > width &&
+		    !omap_have_any_virtual_plane(dev))

This doesn't sound correct. It's not whether we have any virtual planes, but whether we have virtual planes for this display.

Tomi

Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel