[Patch v2 0/6] drm/omap: Add virtual-planes support

[Patch v2 0/6] drm/omap: Add virtual-planes support

[Benoit Parrot]

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(-)

-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 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(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 624dee22f46b..35541d4441df 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -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 */
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index f8f83e826a56..c58c75292182 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -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);
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index a0d7b1d905e8..d5e059abb555 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -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);
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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(-)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
index 91279f1060fe..c30f9ec189ed 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
@@ -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
 ----
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
index e1ef29569338..249e588d7865 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
@@ -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
 -----------
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
index ee867c4d1152..afcd5a86c6a4 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
@@ -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
 ----
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
index cd02516a40b6..dc66e1447c31 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
@@ -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
 ----
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
index 0f85f6b3a5a8..bc624db8888d 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
@@ -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
 ----
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
index 20861218649f..118a486c47bb 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
@@ -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
 ----
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 .../devicetree/bindings/display/ti/ti,omap-dss.txt | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
index 249e588d7865..2dd411cb5a83 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
@@ -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>;
+                };
+        };
+};
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Patch v2 4/6] drm/omap: Add virtual plane DT parsing support

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 110 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  11 ++++
 2 files changed, 121 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 35541d4441df..06a2e894175e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -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 */
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index c58c75292182..ad0f751ec047 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -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);
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 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(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 37ee20c773c7..4c43ef239136 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -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);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index b2539a90e1a4..80b29b7f5696 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -153,25 +153,27 @@ static uint32_t drm_rotation_to_tiler(unsigned int drm_rot)
 /* update ovl info for scanout, handles cases of multi-planar fb's, etc.
  */
 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 omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
 	const struct drm_format_info *format = omap_fb->format;
 	struct plane *plane = &omap_fb->planes[0];
 	uint32_t x, y, orient = 0;
 
-	info->fourcc = fb->format->format;
+	main_info->fourcc = fb->format->format;
 
-	info->pos_x      = state->crtc_x;
-	info->pos_y      = state->crtc_y;
-	info->out_width  = state->crtc_w;
-	info->out_height = state->crtc_h;
-	info->width      = state->src_w >> 16;
-	info->height     = state->src_h >> 16;
+	main_info->pos_x      = state->crtc_x;
+	main_info->pos_y      = state->crtc_y;
+	main_info->out_width  = state->crtc_w;
+	main_info->out_height = state->crtc_h;
+	main_info->width      = state->src_w >> 16;
+	main_info->height     = state->src_h >> 16;
 
 	/* DSS driver wants the w & h in rotated orientation */
 	if (drm_rotation_90_or_270(state->rotation))
-		swap(info->width, info->height);
+		swap(main_info->width, main_info->height);
 
 	x = state->src_x >> 16;
 	y = state->src_y >> 16;
@@ -202,11 +204,12 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 
 		/* Note: x and y are in TILER units, not pixels */
 		omap_gem_rotated_dma_addr(plane->bo, orient, x, y,
-					  &info->paddr);
-		info->rotation_type = OMAP_DSS_ROT_TILER;
-		info->rotation = state->rotation ?: DRM_MODE_ROTATE_0;
+					  &main_info->paddr);
+		main_info->rotation_type = OMAP_DSS_ROT_TILER;
+		main_info->rotation = state->rotation ?: DRM_MODE_ROTATE_0;
 		/* Note: stride in TILER units, not pixels */
-		info->screen_width  = omap_gem_tiled_stride(plane->bo, orient);
+		main_info->screen_width  =
+				omap_gem_tiled_stride(plane->bo, orient);
 	} else {
 		switch (state->rotation & DRM_MODE_ROTATE_MASK) {
 		case 0:
@@ -221,27 +224,46 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 			break;
 		}
 
-		info->paddr         = get_linear_addr(plane, format, 0, x, y);
-		info->rotation_type = OMAP_DSS_ROT_NONE;
-		info->rotation      = DRM_MODE_ROTATE_0;
-		info->screen_width  = plane->pitch;
+		main_info->paddr = get_linear_addr(plane, format, 0, x, y);
+		main_info->rotation_type = OMAP_DSS_ROT_NONE;
+		main_info->rotation = DRM_MODE_ROTATE_0;
+		main_info->screen_width = plane->pitch;
 	}
 
 	/* convert to pixels: */
-	info->screen_width /= format->cpp[0];
+	main_info->screen_width /= format->cpp[0];
 
 	if (fb->format->format == DRM_FORMAT_NV12) {
 		plane = &omap_fb->planes[1];
 
-		if (info->rotation_type == OMAP_DSS_ROT_TILER) {
+		if (main_info->rotation_type == OMAP_DSS_ROT_TILER) {
 			WARN_ON(!(omap_gem_flags(plane->bo) & OMAP_BO_TILED));
 			omap_gem_rotated_dma_addr(plane->bo, orient, x/2, y/2,
-						  &info->p_uv_addr);
+						  &main_info->p_uv_addr);
 		} else {
-			info->p_uv_addr = get_linear_addr(plane, format, 1, x, y);
+			main_info->p_uv_addr =
+				get_linear_addr(plane, format, 1, x, y);
 		}
 	} else {
-		info->p_uv_addr = 0;
+		main_info->p_uv_addr = 0;
+	}
+
+	if (aux_info) {
+		main_info->width /= 2;
+		main_info->out_width /= 2;
+
+		*aux_info = *main_info;
+
+		aux_info->pos_x = main_info->pos_x + main_info->out_width;
+
+		aux_info->paddr =
+				get_linear_addr(&omap_fb->planes[0], format, 0,
+						x + main_info->width, y);
+		if (fb->format->format == DRM_FORMAT_NV12) {
+			aux_info->p_uv_addr =
+				get_linear_addr(&omap_fb->planes[1], format, 1,
+						x + main_info->width, y);
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.h b/drivers/gpu/drm/omapdrm/omap_fb.h
index 94ad5f9e4404..f68e81353288 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.h
+++ b/drivers/gpu/drm/omapdrm/omap_fb.h
@@ -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);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 2c19d2239bc5..21c927bbf5a7 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -30,10 +30,14 @@
 
 struct omap_plane {
 	struct drm_plane base;
-	enum omap_plane_id id;
+	enum omap_plane_id main_id;
+	enum omap_plane_id aux_id;
 	const char *name;
+	bool virtual_plane;
 };
 
+static const char *plane_id_to_name[];
+
 static int omap_plane_prepare_fb(struct drm_plane *plane,
 				 struct drm_plane_state *new_state)
 {
@@ -56,38 +60,75 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	struct omap_drm_private *priv = plane->dev->dev_private;
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct drm_plane_state *state = plane->state;
-	struct omap_overlay_info info;
+	struct omap_overlay_info main_info, aux_info;
 	int ret;
+	bool dual_plane = omap_plane->virtual_plane;
 
 	DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
 
-	memset(&info, 0, sizeof(info));
-	info.rotation_type = OMAP_DSS_ROT_NONE;
-	info.rotation = DRM_MODE_ROTATE_0;
-	info.global_alpha = 0xff;
-	info.zorder = state->normalized_zpos;
+	memset(&main_info, 0, sizeof(main_info));
+	main_info.rotation_type = OMAP_DSS_ROT_NONE;
+	main_info.rotation = DRM_MODE_ROTATE_0;
+	main_info.global_alpha = 0xff;
+	main_info.zorder = state->normalized_zpos;
 
-	/* update scanout: */
-	omap_framebuffer_update_scanout(state->fb, state, &info);
+	aux_info = main_info;
 
-	DBG("%dx%d -> %dx%d (%d)", info.width, info.height,
-			info.out_width, info.out_height,
-			info.screen_width);
-	DBG("%d,%d %pad %pad", info.pos_x, info.pos_y,
-			&info.paddr, &info.p_uv_addr);
+	/* update scanout: */
+	omap_framebuffer_update_scanout(state->fb, state, &main_info,
+					dual_plane ? &aux_info : NULL);
+
+	DBG("%s: %dx%d -> %dx%d (%d)",
+	    plane_id_to_name[omap_plane->main_id],
+	    main_info.width, main_info.height,
+	    main_info.out_width, main_info.out_height,
+	    main_info.screen_width);
+	DBG("%d,%d %pad %pad", main_info.pos_x, main_info.pos_y,
+	    &main_info.paddr, &main_info.p_uv_addr);
+
+	if (dual_plane) {
+		/*
+		 * If the current plane is virtual it uses 2 hw planes
+		 * therefore the very next zorder is used by the aux_id
+		 * plane so we just use the main_id zorder + 1
+		 */
+		aux_info.zorder = main_info.zorder + 1;
+
+		DBG("%s: %dx%d -> %dx%d (%d)",
+		    plane_id_to_name[omap_plane->aux_id],
+		    aux_info.width, aux_info.height,
+		    aux_info.out_width, aux_info.out_height,
+		    aux_info.screen_width);
+		DBG("%d,%d %pad %pad", aux_info.pos_x, aux_info.pos_y,
+		    &aux_info.paddr, &aux_info.p_uv_addr);
+	}
 
-	/* and finally, update omapdss: */
-	ret = priv->dispc_ops->ovl_setup(omap_plane->id, &info,
+	ret = priv->dispc_ops->ovl_setup(omap_plane->main_id, &main_info,
 			      omap_crtc_timings(state->crtc), false,
 			      omap_crtc_channel(state->crtc));
 	if (ret) {
-		dev_err(plane->dev->dev, "Failed to setup plane %s\n",
+		dev_err(plane->dev->dev, "Failed to setup plane1 %s\n",
 			omap_plane->name);
-		priv->dispc_ops->ovl_enable(omap_plane->id, false);
+		priv->dispc_ops->ovl_enable(omap_plane->main_id, false);
 		return;
 	}
 
-	priv->dispc_ops->ovl_enable(omap_plane->id, true);
+	priv->dispc_ops->ovl_enable(omap_plane->main_id, true);
+
+	if (dual_plane) {
+		ret = priv->dispc_ops->ovl_setup(omap_plane->aux_id, &aux_info,
+				      omap_crtc_timings(state->crtc), false,
+				      omap_crtc_channel(state->crtc));
+		if (ret) {
+			dev_err(plane->dev->dev, "Failed to setup plane2 %s\n",
+				omap_plane->name);
+			priv->dispc_ops->ovl_enable(omap_plane->aux_id, false);
+			priv->dispc_ops->ovl_enable(omap_plane->main_id, false);
+			return;
+		}
+
+		priv->dispc_ops->ovl_enable(omap_plane->aux_id, true);
+	}
 }
 
 static void omap_plane_atomic_disable(struct drm_plane *plane,
@@ -95,12 +136,15 @@ static void omap_plane_atomic_disable(struct drm_plane *plane,
 {
 	struct omap_drm_private *priv = plane->dev->dev_private;
 	struct omap_plane *omap_plane = to_omap_plane(plane);
+	bool dual_plane = omap_plane->virtual_plane;
 
 	plane->state->rotation = DRM_MODE_ROTATE_0;
 	plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
-			   ? 0 : omap_plane->id;
+			   ? 0 : omap_plane->main_id;
 
-	priv->dispc_ops->ovl_enable(omap_plane->id, false);
+	priv->dispc_ops->ovl_enable(omap_plane->main_id, false);
+	if (dual_plane)
+		priv->dispc_ops->ovl_enable(omap_plane->aux_id, false);
 }
 
 static int omap_plane_atomic_check(struct drm_plane *plane,
@@ -158,6 +202,13 @@ static void omap_plane_destroy(struct drm_plane *plane)
 	kfree(omap_plane);
 }
 
+bool is_omap_plane_virtual(struct drm_plane *plane)
+{
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+
+	return omap_plane->virtual_plane;
+}
+
 /* 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)
@@ -195,7 +246,7 @@ static void omap_plane_reset(struct drm_plane *plane)
 	 * plane.
 	 */
 	plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
-			   ? 0 : omap_plane->id;
+			   ? 0 : omap_plane->main_id;
 }
 
 static int omap_plane_atomic_set_property(struct drm_plane *plane,
@@ -246,43 +297,65 @@ static const char *plane_id_to_name[] = {
 	[OMAP_DSS_VIDEO3] = "vid3",
 };
 
-static const enum omap_plane_id plane_idx_to_id[] = {
-	OMAP_DSS_GFX,
-	OMAP_DSS_VIDEO1,
-	OMAP_DSS_VIDEO2,
-	OMAP_DSS_VIDEO3,
+static const char * const virtual_plane_id_to_name[] = {
+	[OMAP_DSS_GFX] = "virt-gfx",
+	[OMAP_DSS_VIDEO1] = "virt-vid1",
+	[OMAP_DSS_VIDEO2] = "virt-vid2",
+	[OMAP_DSS_VIDEO3] = "virt-vid3",
 };
 
-/* initialize 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)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	unsigned int num_planes = priv->dispc_ops->get_num_ovls();
 	struct drm_plane *plane;
 	struct omap_plane *omap_plane;
-	enum omap_plane_id id;
 	int ret;
 	u32 nformats;
 	const u32 *formats;
 
-	if (WARN_ON(idx >= ARRAY_SIZE(plane_idx_to_id)))
-		return ERR_PTR(-EINVAL);
-
-	id = plane_idx_to_id[idx];
+	if (plane_mappings->num_planes)
+		num_planes = plane_mappings->num_planes;
 
-	DBG("%s: type=%d", plane_id_to_name[id], type);
+	if (WARN_ON(idx >= num_planes))
+		return ERR_PTR(-EINVAL);
 
 	omap_plane = kzalloc(sizeof(*omap_plane), GFP_KERNEL);
 	if (!omap_plane)
 		return ERR_PTR(-ENOMEM);
 
-	formats = priv->dispc_ops->ovl_get_color_modes(id);
+	if (plane_mappings->num_planes) {
+		/* Use plane data from DT */
+		omap_plane->main_id = plane_mappings->plane[idx].main_id;
+		if (plane_mappings->plane[idx].is_virtual) {
+			omap_plane->name =
+				virtual_plane_id_to_name[omap_plane->main_id];
+			omap_plane->aux_id = plane_mappings->plane[idx].aux_id;
+			omap_plane->virtual_plane = true;
+		} else {
+			omap_plane->name =
+				plane_id_to_name[omap_plane->main_id];
+		}
+		if (plane_mappings->plane[idx].crtc_mask)
+			possible_crtcs = plane_mappings->plane[idx].crtc_mask;
+	} else {
+		/* Use legacy plane allocation */
+		omap_plane->main_id = idx;
+		omap_plane->name = plane_id_to_name[idx];
+	}
+
+	DBG("%s: type=%d", omap_plane->name, type);
+	DBG("	omap_plane->main_id: %d", omap_plane->main_id);
+	if (omap_plane->virtual_plane)
+		DBG("	omap_plane->aux_id: %d", omap_plane->aux_id);
+	DBG("	crtc_mask: 0x%04x", possible_crtcs);
+
+	formats = priv->dispc_ops->ovl_get_color_modes(omap_plane->main_id);
 	for (nformats = 0; formats[nformats]; ++nformats)
 		;
-	omap_plane->id = id;
-	omap_plane->name = plane_id_to_name[id];
 
 	plane = &omap_plane->base;
 
@@ -301,7 +374,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 error:
 	dev_err(dev->dev, "%s(): could not create plane: %s\n",
-		__func__, plane_id_to_name[id]);
+		__func__, omap_plane->name);
 
 	kfree(omap_plane);
 	return NULL;
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.h b/drivers/gpu/drm/omapdrm/omap_plane.h
index dc5e82ad061d..48815a05f4fe 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.h
+++ b/drivers/gpu/drm/omapdrm/omap_plane.h
@@ -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__ */
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 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(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index d5e059abb555..517f7fa80ce1 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -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;
 	}
 
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 21c927bbf5a7..8529abdcdeb8 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -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)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.h b/drivers/gpu/drm/omapdrm/omap_plane.h
index 48815a05f4fe..86b1c2022231 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.h
+++ b/drivers/gpu/drm/omapdrm/omap_plane.h
@@ -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__ */
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> 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.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  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(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 624dee22f46b..35541d4441df 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -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);

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

>  void dispc_set_ops(const struct dispc_ops *o);
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index a0d7b1d905e8..d5e059abb555 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -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;

You should check the height also.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Benoit Parrot]

Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2018-Apr-04 14:12:13 +0300]:
> On 26/03/18 19:21, Benoit Parrot wrote:
> > 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.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> >  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(+)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> > index 624dee22f46b..35541d4441df 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> > @@ -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);
> 
> 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;
	}

> 
> >  void dispc_set_ops(const struct dispc_ops *o);
> > diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> > index a0d7b1d905e8..d5e059abb555 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> > @@ -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;
> 
> You should check the height also.

Yeah, I'll fix that.

Benoit

> 
>  Tomi
> 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Laurent Pinchart]

Hi Benoit,

On Wednesday, 4 April 2018 16:15:11 EEST Benoit Parrot wrote:
> Tomi Valkeinen wrote on Wed [2018-Apr-04 14:12:13 +0300]:
> > On 26/03/18 19:21, Benoit Parrot wrote:
> >> 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.
> >> 
> >> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> >> ---
> >> 
> >>  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(+)
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> >> b/drivers/gpu/drm/omapdrm/dss/dispc.c index 624dee22f46b..35541d4441df
> >> 100644
> >--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> >> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> >> @@ -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);
> > 
> > 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;
> 	}

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

> >>  void dispc_set_ops(const struct dispc_ops *o);
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> >> b/drivers/gpu/drm/omapdrm/omap_connector.c index
> >> a0d7b1d905e8..d5e059abb555 100644
> >> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> >> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> >> @@ -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;
> > 
> > You should check the height also.
> 
> 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 ?

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Laurent Pinchart]

Hi Benoit,

Thank you for the patch.

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.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  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(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt index
> 91279f1060fe..c30f9ec189ed 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> @@ -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
>  ----
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index
> e1ef29569338..249e588d7865 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> @@ -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
> +

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.

>  Video Ports
>  -----------
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt index
> ee867c4d1152..afcd5a86c6a4 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
> @@ -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
>  ----
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt index
> cd02516a40b6..dc66e1447c31 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> @@ -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
>  ----
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt index
> 0f85f6b3a5a8..bc624db8888d 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
> @@ -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
>  ----
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt index
> 20861218649f..118a486c47bb 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
> @@ -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
>  ----

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Laurent Pinchart]

Hi Benoit,

Thank you for the patch.

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.

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

> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  .../devicetree/bindings/display/ti/ti,omap-dss.txt | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index
> 249e588d7865..2dd411cb5a83 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> @@ -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>;
> +                };
> +        };
> +};


-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tomi Valkeinen]

On 04/04/18 17:36, Laurent Pinchart wrote:
> Hi Benoit,
> 
> Thank you for the patch.
> 
> 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.
> 
> 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

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tomi Valkeinen]

On 04/04/18 17:23, Laurent Pinchart wrote:

>>>> +	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;
>> 	}
> 
> 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.

>>>> +	/* 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.
> 
> 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

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> 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.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index d5e059abb555..517f7fa80ce1 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -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

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> 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.

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.

> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 37ee20c773c7..4c43ef239136 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -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,
>  };

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

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Apr 04, 2018 at 05:56:46PM +0300, Tomi Valkeinen wrote:
> On 04/04/18 17:36, Laurent Pinchart wrote:
> > Hi Benoit,
> > 
> > Thank you for the patch.
> > 
> > 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.
> > 
> > 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Laurent Pinchart]

Hi Tomi,

On Thursday, 5 April 2018 13:21:30 EEST Tomi Valkeinen wrote:
> On 04/04/18 17:23, Laurent Pinchart wrote:
> >>>> +	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;
> >> 	}
> > 
> > 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.

> 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 ? :-)

> >>>> +	/* 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.
> > 
> > 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.

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.

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Benoit Parrot]

Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote on Wed [2018-Apr-04 17:29:59 +0300]:
> Hi Benoit,
> 
> Thank you for the patch.
> 
> 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.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  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(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt index
> > 91279f1060fe..c30f9ec189ed 100644
> > --- a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > +++ b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > @@ -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
> >  ----
> > 
> > diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index
> > e1ef29569338..249e588d7865 100644
> > --- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > +++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > @@ -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
> > +
> 
> 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.
  
> 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
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel