devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Add support for AM62L DSS
@ 2025-03-26 14:57 Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: " Devarsh Thakkar
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Devarsh Thakkar @ 2025-03-26 14:57 UTC (permalink / raw)
  To: jyri.sarha, tomi.valkeinen, airlied, maarten.lankhorst, mripard,
	tzimmermann, dri-devel, simona, linux-kernel, devicetree, robh,
	krzk+dt, conor+dt
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, devarsht

This adds support for DSS subsystem present in TI's AM62L SoC
which supports single display pipeline with DPI output which
is also routed to DSI Tx controller within the SoC.

Change Log:
V4:
- Update vid_info struct to keep hw_id and instantiate
  only for actually existing pipes

V3:
- Make generic infra to support truncated K3 DSS IP's
- Remove AM62A updates from AM62L DT binding updates

V2:
- Fix incorrect format of compatible string (comma instead of
  hyphen) for AM62L SoC
- Use separate register space and helper functions for AM62L
  due to minor differences in register offset/bit position differences
  for first plane

Rangediff:
V3->V4:
- https://gist.github.com/devarsht/1e75c9e1ac0cdfc01703a0776e31e782

V2->V3:
- https://gist.github.com/devarsht/24fa8dd2986861efa431352d19ebbb41

V1->V2
- https://gist.github.com/devarsht/11d47f25ca9fea6976e6284330ddf443

Links to previous versions:
V3: https://lore.kernel.org/all/20250306132914.1469387-1-devarsht@ti.com/
V2: https://lore.kernel.org/all/20250204061552.3720261-1-devarsht@ti.com/
V1: https://lore.kernel.org/all/20241231090432.3649158-1-devarsht@ti.com/

Test logs:
https://gist.github.com/devarsht/16fe796b8fd3ea8abf5df8e2327d2311

Devarsh Thakkar (3):
  dt-bindings: display: ti,am65x-dss: Add support for AM62L DSS
  drm/tidss: Update infrastructure to support K3 DSS cut-down versions
  drm/tidss: Add support for AM62L display subsystem

 .../bindings/display/ti/ti,am65x-dss.yaml     |  21 ++-
 drivers/gpu/drm/tidss/tidss_crtc.c            |   8 +-
 drivers/gpu/drm/tidss/tidss_dispc.c           | 176 ++++++++++++++----
 drivers/gpu/drm/tidss/tidss_dispc.h           |  13 +-
 drivers/gpu/drm/tidss/tidss_drv.c             |   1 +
 drivers/gpu/drm/tidss/tidss_kms.c             |   2 +-
 drivers/gpu/drm/tidss/tidss_plane.c           |   2 +-
 7 files changed, 178 insertions(+), 45 deletions(-)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Add support for AM62L DSS
  2025-03-26 14:57 [PATCH v4 0/3] Add support for AM62L DSS Devarsh Thakkar
@ 2025-03-26 14:57 ` Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 3/3] drm/tidss: Add support for AM62L display subsystem Devarsh Thakkar
  2 siblings, 0 replies; 6+ messages in thread
From: Devarsh Thakkar @ 2025-03-26 14:57 UTC (permalink / raw)
  To: jyri.sarha, tomi.valkeinen, airlied, maarten.lankhorst, mripard,
	tzimmermann, dri-devel, simona, linux-kernel, devicetree, robh,
	krzk+dt, conor+dt
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, devarsht

The DSS controller on TI's AM62L SoC is an update from that on TI's
AM625/AM65x/AM62A7 SoC. The AM62L DSS [1] only supports a single display
pipeline using a single overlay manager, single video port and a single
video lite pipeline which does not support scaling.

The output of video port is routed to SoC boundary via DPI interface and
the DPI signals from the video port are also routed to DSI Tx controller
present within the SoC.

[1]: Section 11.7 (Display Subsystem and Peripherals)
Link : https://www.ti.com/lit/pdf/sprujb4

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Jayesh Choudhary <j-choudhary@ti.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V4:
- No change

V3:
- Remove AM62A references as suggested
- Add Reviewed-by

V2: 
- Add Reviewed-by
- s/ti,am62l,dss/ti,am62l-dss

 .../bindings/display/ti/ti,am65x-dss.yaml     | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 31c4ffcb599c..a5b13cb7bc73 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -12,18 +12,25 @@ maintainers:
   - Tomi Valkeinen <tomi.valkeinen@ti.com>
 
 description: |
-  The AM625 and AM65x TI Keystone Display SubSystem with two output
+  The AM625 and AM65x TI Keystone Display SubSystem has two output
   ports and two video planes. In AM65x DSS, the first video port
   supports 1 OLDI TX and in AM625 DSS, the first video port output is
   internally routed to 2 OLDI TXes. The second video port supports DPI
   format. The first plane is full video plane with all features and the
   second is a "lite plane" without scaling support.
+  The AM62L display subsystem has a single output port which supports DPI
+  format but it only supports single video "lite plane" which does not support
+  scaling. The output port is routed to SoC boundary via DPI interface and same
+  DPI signals are also routed internally to DSI Tx controller present within the
+  SoC. Due to clocking limitations only one of the interface i.e. either DSI or
+  DPI can be used at once.
 
 properties:
   compatible:
     enum:
       - ti,am625-dss
       - ti,am62a7-dss
+      - ti,am62l-dss
       - ti,am65x-dss
 
   reg:
@@ -91,6 +98,8 @@ properties:
           For AM625 DSS, the internal DPI output port node from video
           port 1.
           For AM62A7 DSS, the port is tied off inside the SoC.
+          For AM62L DSS, the DSS DPI output port node from video port 1
+          or DSI Tx controller node connected to video port 1.
 
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
@@ -123,6 +132,16 @@ allOf:
         ports:
           properties:
             port@0: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,am62l-dss
+    then:
+      properties:
+        ports:
+          properties:
+            port@1: false
 
 required:
   - compatible
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions
  2025-03-26 14:57 [PATCH v4 0/3] Add support for AM62L DSS Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: " Devarsh Thakkar
@ 2025-03-26 14:57 ` Devarsh Thakkar
  2025-03-27 11:18   ` Tomi Valkeinen
  2025-03-26 14:57 ` [PATCH v4 3/3] drm/tidss: Add support for AM62L display subsystem Devarsh Thakkar
  2 siblings, 1 reply; 6+ messages in thread
From: Devarsh Thakkar @ 2025-03-26 14:57 UTC (permalink / raw)
  To: jyri.sarha, tomi.valkeinen, airlied, maarten.lankhorst, mripard,
	tzimmermann, dri-devel, simona, linux-kernel, devicetree, robh,
	krzk+dt, conor+dt
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, devarsht

SoCs like AM62Lx support cut-down version of K3 DSS where although same
register space is supported as in other K3 DSS supported SoCs such as
AM65x, AM62x, AM62Ax but some of the resources such as planes and
corresponding register spaces are truncated.

For e.g. AM62Lx has only single VIDL pipeline supported, so corresponding
register spaces for other video pipelines need to be skipped.

To add a generic support for future SoCs where one or more video pipelines
can get truncated from the parent register space, move the video plane
related information to vid_info struct which will also have a field to
indicate hardware index of each of the available video planes, so that
driver only maps and programs those video pipes and skips the unavailable
ones.

While at it, also change the num_planes field in the features structure to
num_vid so that all places in code which use vid_info structure are
highlighted in the code.

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V4:
- Create vid_info struct only for instantiated planes
- s/num_planes/num_vids
- s/vid_lite/is_lite
- Add hw_id member in vid_info struct and remove is_present

V2->V3:
- No change (patch introduced in V3)

 drivers/gpu/drm/tidss/tidss_crtc.c  |   8 +-
 drivers/gpu/drm/tidss/tidss_dispc.c | 135 ++++++++++++++++++++--------
 drivers/gpu/drm/tidss/tidss_dispc.h |  11 ++-
 drivers/gpu/drm/tidss/tidss_kms.c   |   2 +-
 drivers/gpu/drm/tidss/tidss_plane.c |   2 +-
 5 files changed, 114 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index 94f8e3178df5..6db100b81482 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -130,7 +130,7 @@ static void tidss_crtc_position_planes(struct tidss_device *tidss,
 	    !to_tidss_crtc_state(cstate)->plane_pos_changed)
 		return;
 
-	for (layer = 0; layer < tidss->feat->num_planes; layer++) {
+	for (layer = 0; layer < tidss->feat->num_vids ; layer++) {
 		struct drm_plane_state *pstate;
 		struct drm_plane *plane;
 		bool layer_active = false;
@@ -271,9 +271,9 @@ static void tidss_crtc_atomic_disable(struct drm_crtc *crtc,
 	 * another videoport, the DSS will report sync lost issues. Disable all
 	 * the layers here as a work-around.
 	 */
-	for (u32 layer = 0; layer < tidss->feat->num_planes; layer++)
-		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, layer,
-				       false);
+	for (u32 layer = 0; layer < tidss->feat->num_vids; layer++)
+		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport,
+				       tidss->feat->vid_info[layer].hw_id, false);
 
 	dispc_vp_disable(tidss->dispc, tcrtc->hw_videoport);
 
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index cacb5f3d8085..6f0255d65a06 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -103,9 +103,16 @@ const struct dispc_features dispc_k2g_feats = {
 		},
 	},
 
-	.num_planes = 1,
-	.vid_name = { "vid1" },
-	.vid_lite = { false },
+	.num_vids = 1,
+
+	.vid_info = {
+		{
+			.name = "vid1",
+			.is_lite = false,
+			.hw_id = 0,
+		},
+	},
+
 	.vid_order = { 0 },
 };
 
@@ -178,11 +185,22 @@ const struct dispc_features dispc_am65x_feats = {
 		},
 	},
 
-	.num_planes = 2,
+	.num_vids = 2,
 	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
-	.vid_name = { "vid", "vidl1" },
-	.vid_lite = { false, true, },
-	.vid_order = { 1, 0 },
+	.vid_info = {
+		{
+			.name = "vid",
+			.is_lite = false,
+			.hw_id = 0,
+		},
+		{
+			.name = "vidl1",
+			.is_lite = true,
+			.hw_id = 1,
+		},
+	},
+
+	.vid_order = {1, 0},
 };
 
 static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
@@ -267,9 +285,32 @@ const struct dispc_features dispc_j721e_feats = {
 			.gamma_type = TIDSS_GAMMA_10BIT,
 		},
 	},
-	.num_planes = 4,
-	.vid_name = { "vid1", "vidl1", "vid2", "vidl2" },
-	.vid_lite = { 0, 1, 0, 1, },
+
+	.num_vids = 4,
+
+	.vid_info = {
+		{
+			.name = "vid1",
+			.is_lite = false,
+			.hw_id = 0,
+		},
+		{
+			.name = "vidl1",
+			.is_lite = true,
+			.hw_id = 1,
+		},
+		{
+			.name = "vid2",
+			.is_lite = false,
+			.hw_id = 2,
+		},
+		{
+			.name = "vidl2",
+			.is_lite = true,
+			.hw_id = 3,
+		},
+	},
+
 	.vid_order = { 1, 3, 0, 2 },
 };
 
@@ -315,11 +356,23 @@ const struct dispc_features dispc_am625_feats = {
 		},
 	},
 
-	.num_planes = 2,
+	.num_vids = 2,
+
 	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
-	.vid_name = { "vid", "vidl1" },
-	.vid_lite = { false, true, },
-	.vid_order = { 1, 0 },
+	.vid_info = {
+		{
+			.name = "vid",
+			.is_lite = false,
+			.hw_id = 0,
+		},
+		{
+			.name = "vidl1",
+			.is_lite = true,
+			.hw_id = 1,
+		}
+	},
+
+	.vid_order = {1, 0},
 };
 
 const struct dispc_features dispc_am62a7_feats = {
@@ -369,11 +422,22 @@ const struct dispc_features dispc_am62a7_feats = {
 		},
 	},
 
-	.num_planes = 2,
-	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
-	.vid_name = { "vid", "vidl1" },
-	.vid_lite = { false, true, },
-	.vid_order = { 1, 0 },
+	.num_vids = 2,
+
+	.vid_info = {
+		{
+			.name = "vid",
+			.is_lite = false,
+			.hw_id = 0,
+		},
+		{
+			.name = "vidl1",
+			.is_lite = true,
+			.hw_id = 1,
+		}
+	},
+
+	.vid_order = {1, 0},
 };
 
 static const u16 *dispc_common_regmap;
@@ -788,7 +852,8 @@ void dispc_k3_clear_irqstatus(struct dispc_device *dispc, dispc_irq_t clearmask)
 		if (clearmask & DSS_IRQ_VP_MASK(i))
 			dispc_k3_vp_write_irqstatus(dispc, i, clearmask);
 	}
-	for (i = 0; i < dispc->feat->num_planes; ++i) {
+
+	for (i = 0; i < dispc->feat->num_vids; ++i) {
 		if (clearmask & DSS_IRQ_PLANE_MASK(i))
 			dispc_k3_vid_write_irqstatus(dispc, i, clearmask);
 	}
@@ -809,8 +874,8 @@ dispc_irq_t dispc_k3_read_and_clear_irqstatus(struct dispc_device *dispc)
 	for (i = 0; i < dispc->feat->num_vps; ++i)
 		status |= dispc_k3_vp_read_irqstatus(dispc, i);
 
-	for (i = 0; i < dispc->feat->num_planes; ++i)
-		status |= dispc_k3_vid_read_irqstatus(dispc, i);
+	for (i = 0; i < dispc->feat->num_vids; ++i)
+		status |= dispc_k3_vid_read_irqstatus(dispc, dispc->feat->vid_info[i].hw_id);
 
 	dispc_k3_clear_irqstatus(dispc, status);
 
@@ -825,8 +890,8 @@ static dispc_irq_t dispc_k3_read_irqenable(struct dispc_device *dispc)
 	for (i = 0; i < dispc->feat->num_vps; ++i)
 		enable |= dispc_k3_vp_read_irqenable(dispc, i);
 
-	for (i = 0; i < dispc->feat->num_planes; ++i)
-		enable |= dispc_k3_vid_read_irqenable(dispc, i);
+	for (i = 0; i < dispc->feat->num_vids; ++i)
+		enable |= dispc_k3_vid_read_irqenable(dispc, dispc->feat->vid_info[i].hw_id);
 
 	return enable;
 }
@@ -849,10 +914,11 @@ static void dispc_k3_set_irqenable(struct dispc_device *dispc,
 			main_enable |= BIT(i);		/* VP IRQ */
 		else
 			main_disable |= BIT(i);		/* VP IRQ */
+
 	}
 
-	for (i = 0; i < dispc->feat->num_planes; ++i) {
-		dispc_k3_vid_set_irqenable(dispc, i, mask);
+	for (i = 0; i < dispc->feat->num_vids; ++i) {
+		dispc_k3_vid_set_irqenable(dispc, dispc->feat->vid_info[i].hw_id, mask);
 		if (mask & DSS_IRQ_PLANE_MASK(i))
 			main_enable |= BIT(i + 4);	/* VID IRQ */
 		else
@@ -861,7 +927,6 @@ static void dispc_k3_set_irqenable(struct dispc_device *dispc,
 
 	if (main_enable)
 		dispc_write(dispc, DISPC_IRQENABLE_SET, main_enable);
-
 	if (main_disable)
 		dispc_write(dispc, DISPC_IRQENABLE_CLR, main_disable);
 
@@ -2025,7 +2090,7 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
 		      const struct drm_plane_state *state,
 		      u32 hw_videoport)
 {
-	bool lite = dispc->feat->vid_lite[hw_plane];
+	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
 	u32 fourcc = state->fb->format->format;
 	bool need_scaling = state->src_w >> 16 != state->crtc_w ||
 		state->src_h >> 16 != state->crtc_h;
@@ -2096,7 +2161,7 @@ void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
 		       const struct drm_plane_state *state,
 		       u32 hw_videoport)
 {
-	bool lite = dispc->feat->vid_lite[hw_plane];
+	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
 	u32 fourcc = state->fb->format->format;
 	u16 cpp = state->fb->format->cpp[0];
 	u32 fb_width = state->fb->pitches[0] / cpp;
@@ -2210,7 +2275,7 @@ static void dispc_k2g_plane_init(struct dispc_device *dispc)
 	/* MFLAG_START = MFLAGNORMALSTARTMODE */
 	REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 6, 6);
 
-	for (hw_plane = 0; hw_plane < dispc->feat->num_planes; hw_plane++) {
+	for (hw_plane = 0; hw_plane < dispc->feat->num_vids; hw_plane++) {
 		u32 size = dispc_vid_get_fifo_size(dispc, hw_plane);
 		u32 thr_low, thr_high;
 		u32 mflag_low, mflag_high;
@@ -2226,7 +2291,7 @@ static void dispc_k2g_plane_init(struct dispc_device *dispc)
 
 		dev_dbg(dispc->dev,
 			"%s: bufsize %u, buf_threshold %u/%u, mflag threshold %u/%u preload %u\n",
-			dispc->feat->vid_name[hw_plane],
+			dispc->feat->vid_info[hw_plane].name,
 			size,
 			thr_high, thr_low,
 			mflag_high, mflag_low,
@@ -2265,7 +2330,7 @@ static void dispc_k3_plane_init(struct dispc_device *dispc)
 	/* MFLAG_START = MFLAGNORMALSTARTMODE */
 	REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 6, 6);
 
-	for (hw_plane = 0; hw_plane < dispc->feat->num_planes; hw_plane++) {
+	for (hw_plane = 0; hw_plane < dispc->feat->num_vids; hw_plane++) {
 		u32 size = dispc_vid_get_fifo_size(dispc, hw_plane);
 		u32 thr_low, thr_high;
 		u32 mflag_low, mflag_high;
@@ -2281,7 +2346,7 @@ static void dispc_k3_plane_init(struct dispc_device *dispc)
 
 		dev_dbg(dispc->dev,
 			"%s: bufsize %u, buf_threshold %u/%u, mflag threshold %u/%u preload %u\n",
-			dispc->feat->vid_name[hw_plane],
+			dispc->feat->vid_info[hw_plane].name,
 			size,
 			thr_high, thr_low,
 			mflag_high, mflag_low,
@@ -2898,8 +2963,8 @@ int dispc_init(struct tidss_device *tidss)
 	if (r)
 		return r;
 
-	for (i = 0; i < dispc->feat->num_planes; i++) {
-		r = dispc_iomap_resource(pdev, dispc->feat->vid_name[i],
+	for (i = 0; i < dispc->feat->num_vids; i++) {
+		r = dispc_iomap_resource(pdev, dispc->feat->vid_info[i].name,
 					 &dispc->base_vid[i]);
 		if (r)
 			return r;
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index 086327d51a90..72a0146e57d5 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -46,6 +46,12 @@ struct dispc_features_scaling {
 	u32 xinc_max;
 };
 
+struct dispc_vid_info {
+	const char *name; /* Should match dt reg names */
+	u32 hw_id;
+	bool is_lite;
+};
+
 struct dispc_errata {
 	bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
 };
@@ -82,9 +88,8 @@ struct dispc_features {
 	const char *vpclk_name[TIDSS_MAX_PORTS]; /* Should match dt clk names */
 	const enum dispc_vp_bus_type vp_bus_type[TIDSS_MAX_PORTS];
 	struct tidss_vp_feat vp_feat;
-	u32 num_planes;
-	const char *vid_name[TIDSS_MAX_PLANES]; /* Should match dt reg names */
-	bool vid_lite[TIDSS_MAX_PLANES];
+	u32 num_vids;
+	struct dispc_vid_info vid_info[TIDSS_MAX_PLANES];
 	u32 vid_order[TIDSS_MAX_PLANES];
 };
 
diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
index f371518f8697..19432c08ec6b 100644
--- a/drivers/gpu/drm/tidss/tidss_kms.c
+++ b/drivers/gpu/drm/tidss/tidss_kms.c
@@ -115,7 +115,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
 
 	const struct dispc_features *feat = tidss->feat;
 	u32 max_vps = feat->num_vps;
-	u32 max_planes = feat->num_planes;
+	u32 max_planes = feat->num_vids;
 
 	struct pipe pipes[TIDSS_MAX_PORTS];
 	u32 num_pipes = 0;
diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
index 116de124bddb..ff71370cad8b 100644
--- a/drivers/gpu/drm/tidss/tidss_plane.c
+++ b/drivers/gpu/drm/tidss/tidss_plane.c
@@ -200,7 +200,7 @@ struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
 	struct tidss_plane *tplane;
 	enum drm_plane_type type;
 	u32 possible_crtcs;
-	u32 num_planes = tidss->feat->num_planes;
+	u32 num_planes = tidss->feat->num_vids;
 	u32 color_encodings = (BIT(DRM_COLOR_YCBCR_BT601) |
 			       BIT(DRM_COLOR_YCBCR_BT709));
 	u32 color_ranges = (BIT(DRM_COLOR_YCBCR_FULL_RANGE) |
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v4 3/3] drm/tidss: Add support for AM62L display subsystem
  2025-03-26 14:57 [PATCH v4 0/3] Add support for AM62L DSS Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: " Devarsh Thakkar
  2025-03-26 14:57 ` [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions Devarsh Thakkar
@ 2025-03-26 14:57 ` Devarsh Thakkar
  2 siblings, 0 replies; 6+ messages in thread
From: Devarsh Thakkar @ 2025-03-26 14:57 UTC (permalink / raw)
  To: jyri.sarha, tomi.valkeinen, airlied, maarten.lankhorst, mripard,
	tzimmermann, dri-devel, simona, linux-kernel, devicetree, robh,
	krzk+dt, conor+dt
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, devarsht

Enable display for AM62L DSS [1] which supports only a single display
pipeline using a single overlay manager, single video port and a single
video lite pipeline which does not support scaling.

The output of video port is routed to SoC boundary via DPI interface and
the DPI signals from the video port are also routed to DSI Tx controller
present within the SoC.

[1]: Section 11.7 (Display Subsystem and Peripherals)
Link : https://www.ti.com/lit/pdf/sprujb4

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V4:
- Rebase on top of previous patch to use vid_info structure

V3: 
- Rebase on top of
  0002-drm-tidss-Update-infra-to-support-DSS7-cut-down-vers.patch
- Use the generic "tidss_am65x_common_regs" as common reg space
  instead of creating a new one

V2: 
- Add separate common reg space for AM62L
- Add separate irq enable/disable/read/clear helpers for AM62L
- Use separate helper function for setting overlay attributes
- Drop Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
  due to additional changes made in V2.
 
 drivers/gpu/drm/tidss/tidss_dispc.c | 41 +++++++++++++++++++++++++++++
 drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
 drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
 3 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index 6f0255d65a06..4420eb15e8ed 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -440,6 +440,42 @@ const struct dispc_features dispc_am62a7_feats = {
 	.vid_order = {1, 0},
 };
 
+const struct dispc_features dispc_am62l_feats = {
+	.max_pclk_khz = {
+		[DISPC_VP_DPI] = 165000,
+	},
+
+	.subrev = DISPC_AM62L,
+
+	.common = "common",
+	.common_regs = tidss_am65x_common_regs,
+
+	.num_vps = 1,
+	.vp_name = { "vp1" },
+	.ovr_name = { "ovr1" },
+	.vpclk_name =  { "vp1" },
+	.vp_bus_type = { DISPC_VP_DPI },
+
+	.vp_feat = { .color = {
+			.has_ctm = true,
+			.gamma_size = 256,
+			.gamma_type = TIDSS_GAMMA_8BIT,
+		},
+	},
+
+	.num_vids = 1,
+
+	.vid_info = {
+		{
+			.name = "vidl1",
+			.is_lite = true,
+			.hw_id = 1,
+		}
+	},
+
+	.vid_order = {0},
+};
+
 static const u16 *dispc_common_regmap;
 
 struct dss_vp_data {
@@ -944,6 +980,7 @@ dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc)
 		return dispc_k2g_read_and_clear_irqstatus(dispc);
 	case DISPC_AM625:
 	case DISPC_AM62A7:
+	case DISPC_AM62L:
 	case DISPC_AM65X:
 	case DISPC_J721E:
 		return dispc_k3_read_and_clear_irqstatus(dispc);
@@ -961,6 +998,7 @@ void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask)
 		break;
 	case DISPC_AM625:
 	case DISPC_AM62A7:
+	case DISPC_AM62L:
 	case DISPC_AM65X:
 	case DISPC_J721E:
 		dispc_k3_set_irqenable(dispc, mask);
@@ -1453,6 +1491,7 @@ void dispc_ovr_set_plane(struct dispc_device *dispc, u32 hw_plane,
 		break;
 	case DISPC_AM625:
 	case DISPC_AM62A7:
+	case DISPC_AM62L:
 	case DISPC_AM65X:
 		dispc_am65x_ovr_set_plane(dispc, hw_plane, hw_videoport,
 					  x, y, layer);
@@ -2373,6 +2412,7 @@ static void dispc_plane_init(struct dispc_device *dispc)
 		break;
 	case DISPC_AM625:
 	case DISPC_AM62A7:
+	case DISPC_AM62L:
 	case DISPC_AM65X:
 	case DISPC_J721E:
 		dispc_k3_plane_init(dispc);
@@ -2481,6 +2521,7 @@ static void dispc_vp_write_gamma_table(struct dispc_device *dispc,
 		break;
 	case DISPC_AM625:
 	case DISPC_AM62A7:
+	case DISPC_AM62L:
 	case DISPC_AM65X:
 		dispc_am65x_vp_write_gamma_table(dispc, hw_videoport);
 		break;
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index 72a0146e57d5..28958514b8f5 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -67,6 +67,7 @@ enum dispc_vp_bus_type {
 enum dispc_dss_subrevision {
 	DISPC_K2G,
 	DISPC_AM625,
+	DISPC_AM62L,
 	DISPC_AM62A7,
 	DISPC_AM65X,
 	DISPC_J721E,
@@ -96,6 +97,7 @@ struct dispc_features {
 extern const struct dispc_features dispc_k2g_feats;
 extern const struct dispc_features dispc_am625_feats;
 extern const struct dispc_features dispc_am62a7_feats;
+extern const struct dispc_features dispc_am62l_feats;
 extern const struct dispc_features dispc_am65x_feats;
 extern const struct dispc_features dispc_j721e_feats;
 
diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index d4652e8cc28c..f2a4f659f574 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -242,6 +242,7 @@ static const struct of_device_id tidss_of_table[] = {
 	{ .compatible = "ti,k2g-dss", .data = &dispc_k2g_feats, },
 	{ .compatible = "ti,am625-dss", .data = &dispc_am625_feats, },
 	{ .compatible = "ti,am62a7-dss", .data = &dispc_am62a7_feats, },
+	{ .compatible = "ti,am62l-dss", .data = &dispc_am62l_feats, },
 	{ .compatible = "ti,am65x-dss", .data = &dispc_am65x_feats, },
 	{ .compatible = "ti,j721e-dss", .data = &dispc_j721e_feats, },
 	{ }
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions
  2025-03-26 14:57 ` [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions Devarsh Thakkar
@ 2025-03-27 11:18   ` Tomi Valkeinen
  2025-04-30 15:38     ` Devarsh Thakkar
  0 siblings, 1 reply; 6+ messages in thread
From: Tomi Valkeinen @ 2025-03-27 11:18 UTC (permalink / raw)
  To: Devarsh Thakkar
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, jyri.sarha, airlied, maarten.lankhorst,
	mripard, tzimmermann, dri-devel, simona, linux-kernel, devicetree,
	robh, krzk+dt, conor+dt

Hi,

On 26/03/2025 16:57, Devarsh Thakkar wrote:
> SoCs like AM62Lx support cut-down version of K3 DSS where although same
> register space is supported as in other K3 DSS supported SoCs such as
> AM65x, AM62x, AM62Ax but some of the resources such as planes and
> corresponding register spaces are truncated.
> 
> For e.g. AM62Lx has only single VIDL pipeline supported, so corresponding
> register spaces for other video pipelines need to be skipped.
> 
> To add a generic support for future SoCs where one or more video pipelines
> can get truncated from the parent register space, move the video plane
> related information to vid_info struct which will also have a field to
> indicate hardware index of each of the available video planes, so that
> driver only maps and programs those video pipes and skips the unavailable
> ones.
> 
> While at it, also change the num_planes field in the features structure to
> num_vid so that all places in code which use vid_info structure are
> highlighted in the code.
> 
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V4:
> - Create vid_info struct only for instantiated planes
> - s/num_planes/num_vids
> - s/vid_lite/is_lite
> - Add hw_id member in vid_info struct and remove is_present
> 
> V2->V3:
> - No change (patch introduced in V3)
> 
>   drivers/gpu/drm/tidss/tidss_crtc.c  |   8 +-
>   drivers/gpu/drm/tidss/tidss_dispc.c | 135 ++++++++++++++++++++--------
>   drivers/gpu/drm/tidss/tidss_dispc.h |  11 ++-
>   drivers/gpu/drm/tidss/tidss_kms.c   |   2 +-
>   drivers/gpu/drm/tidss/tidss_plane.c |   2 +-
>   5 files changed, 114 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index 94f8e3178df5..6db100b81482 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -130,7 +130,7 @@ static void tidss_crtc_position_planes(struct tidss_device *tidss,
>   	    !to_tidss_crtc_state(cstate)->plane_pos_changed)
>   		return;
>   
> -	for (layer = 0; layer < tidss->feat->num_planes; layer++) {
> +	for (layer = 0; layer < tidss->feat->num_vids ; layer++) {
>   		struct drm_plane_state *pstate;
>   		struct drm_plane *plane;
>   		bool layer_active = false;
> @@ -271,9 +271,9 @@ static void tidss_crtc_atomic_disable(struct drm_crtc *crtc,
>   	 * another videoport, the DSS will report sync lost issues. Disable all
>   	 * the layers here as a work-around.
>   	 */
> -	for (u32 layer = 0; layer < tidss->feat->num_planes; layer++)
> -		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, layer,
> -				       false);
> +	for (u32 layer = 0; layer < tidss->feat->num_vids; layer++)
> +		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport,
> +				       tidss->feat->vid_info[layer].hw_id, false);
>   
>   	dispc_vp_disable(tidss->dispc, tcrtc->hw_videoport);
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index cacb5f3d8085..6f0255d65a06 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -103,9 +103,16 @@ const struct dispc_features dispc_k2g_feats = {
>   		},
>   	},
>   
> -	.num_planes = 1,
> -	.vid_name = { "vid1" },
> -	.vid_lite = { false },
> +	.num_vids = 1,
> +
> +	.vid_info = {
> +		{
> +			.name = "vid1",
> +			.is_lite = false,
> +			.hw_id = 0,
> +		},
> +	},
> +
>   	.vid_order = { 0 },
>   };
>   
> @@ -178,11 +185,22 @@ const struct dispc_features dispc_am65x_feats = {
>   		},
>   	},
>   
> -	.num_planes = 2,
> +	.num_vids = 2,
>   	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
> -	.vid_name = { "vid", "vidl1" },
> -	.vid_lite = { false, true, },
> -	.vid_order = { 1, 0 },
> +	.vid_info = {
> +		{
> +			.name = "vid",
> +			.is_lite = false,
> +			.hw_id = 0,
> +		},
> +		{
> +			.name = "vidl1",
> +			.is_lite = true,
> +			.hw_id = 1,
> +		},
> +	},
> +
> +	.vid_order = {1, 0},
>   };
>   
>   static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
> @@ -267,9 +285,32 @@ const struct dispc_features dispc_j721e_feats = {
>   			.gamma_type = TIDSS_GAMMA_10BIT,
>   		},
>   	},
> -	.num_planes = 4,
> -	.vid_name = { "vid1", "vidl1", "vid2", "vidl2" },
> -	.vid_lite = { 0, 1, 0, 1, },
> +
> +	.num_vids = 4,
> +
> +	.vid_info = {
> +		{
> +			.name = "vid1",
> +			.is_lite = false,
> +			.hw_id = 0,
> +		},
> +		{
> +			.name = "vidl1",
> +			.is_lite = true,
> +			.hw_id = 1,
> +		},
> +		{
> +			.name = "vid2",
> +			.is_lite = false,
> +			.hw_id = 2,
> +		},
> +		{
> +			.name = "vidl2",
> +			.is_lite = true,
> +			.hw_id = 3,
> +		},
> +	},
> +
>   	.vid_order = { 1, 3, 0, 2 },
>   };
>   
> @@ -315,11 +356,23 @@ const struct dispc_features dispc_am625_feats = {
>   		},
>   	},
>   
> -	.num_planes = 2,
> +	.num_vids = 2,
> +
>   	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
> -	.vid_name = { "vid", "vidl1" },
> -	.vid_lite = { false, true, },
> -	.vid_order = { 1, 0 },
> +	.vid_info = {
> +		{
> +			.name = "vid",
> +			.is_lite = false,
> +			.hw_id = 0,
> +		},
> +		{
> +			.name = "vidl1",
> +			.is_lite = true,
> +			.hw_id = 1,
> +		}
> +	},
> +
> +	.vid_order = {1, 0},
>   };
>   
>   const struct dispc_features dispc_am62a7_feats = {
> @@ -369,11 +422,22 @@ const struct dispc_features dispc_am62a7_feats = {
>   		},
>   	},
>   
> -	.num_planes = 2,
> -	/* note: vid is plane_id 0 and vidl1 is plane_id 1 */
> -	.vid_name = { "vid", "vidl1" },
> -	.vid_lite = { false, true, },
> -	.vid_order = { 1, 0 },
> +	.num_vids = 2,
> +
> +	.vid_info = {
> +		{
> +			.name = "vid",
> +			.is_lite = false,
> +			.hw_id = 0,
> +		},
> +		{
> +			.name = "vidl1",
> +			.is_lite = true,
> +			.hw_id = 1,
> +		}
> +	},
> +
> +	.vid_order = {1, 0},
>   };
>   
>   static const u16 *dispc_common_regmap;
> @@ -788,7 +852,8 @@ void dispc_k3_clear_irqstatus(struct dispc_device *dispc, dispc_irq_t clearmask)
>   		if (clearmask & DSS_IRQ_VP_MASK(i))
>   			dispc_k3_vp_write_irqstatus(dispc, i, clearmask);
>   	}
> -	for (i = 0; i < dispc->feat->num_planes; ++i) {
> +
> +	for (i = 0; i < dispc->feat->num_vids; ++i) {
>   		if (clearmask & DSS_IRQ_PLANE_MASK(i))
>   			dispc_k3_vid_write_irqstatus(dispc, i, clearmask);

How did you test this?

With the changes in this patch, the index (i here) can really only be 
used to reference the vid_info array. Any other use is most likely an 
error, like here.

>   	}
> @@ -809,8 +874,8 @@ dispc_irq_t dispc_k3_read_and_clear_irqstatus(struct dispc_device *dispc)
>   	for (i = 0; i < dispc->feat->num_vps; ++i)
>   		status |= dispc_k3_vp_read_irqstatus(dispc, i);
>   
> -	for (i = 0; i < dispc->feat->num_planes; ++i)
> -		status |= dispc_k3_vid_read_irqstatus(dispc, i);
> +	for (i = 0; i < dispc->feat->num_vids; ++i)
> +		status |= dispc_k3_vid_read_irqstatus(dispc, dispc->feat->vid_info[i].hw_id);

I think here and probably in almost all the cases it makes sense to use 
a helper variable "hw_id".

>   
>   	dispc_k3_clear_irqstatus(dispc, status);
>   
> @@ -825,8 +890,8 @@ static dispc_irq_t dispc_k3_read_irqenable(struct dispc_device *dispc)
>   	for (i = 0; i < dispc->feat->num_vps; ++i)
>   		enable |= dispc_k3_vp_read_irqenable(dispc, i);
>   
> -	for (i = 0; i < dispc->feat->num_planes; ++i)
> -		enable |= dispc_k3_vid_read_irqenable(dispc, i);
> +	for (i = 0; i < dispc->feat->num_vids; ++i)
> +		enable |= dispc_k3_vid_read_irqenable(dispc, dispc->feat->vid_info[i].hw_id);
>   
>   	return enable;
>   }
> @@ -849,10 +914,11 @@ static void dispc_k3_set_irqenable(struct dispc_device *dispc,
>   			main_enable |= BIT(i);		/* VP IRQ */
>   		else
>   			main_disable |= BIT(i);		/* VP IRQ */
> +
>   	}
>   
> -	for (i = 0; i < dispc->feat->num_planes; ++i) {
> -		dispc_k3_vid_set_irqenable(dispc, i, mask);
> +	for (i = 0; i < dispc->feat->num_vids; ++i) {
> +		dispc_k3_vid_set_irqenable(dispc, dispc->feat->vid_info[i].hw_id, mask);
>   		if (mask & DSS_IRQ_PLANE_MASK(i))
>   			main_enable |= BIT(i + 4);	/* VID IRQ */

And here.

>   		else
> @@ -861,7 +927,6 @@ static void dispc_k3_set_irqenable(struct dispc_device *dispc,
>   
>   	if (main_enable)
>   		dispc_write(dispc, DISPC_IRQENABLE_SET, main_enable);
> -
>   	if (main_disable)
>   		dispc_write(dispc, DISPC_IRQENABLE_CLR, main_disable);
>   
> @@ -2025,7 +2090,7 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
>   		      const struct drm_plane_state *state,
>   		      u32 hw_videoport)
>   {
> -	bool lite = dispc->feat->vid_lite[hw_plane];
> +	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
>   	u32 fourcc = state->fb->format->format;
>   	bool need_scaling = state->src_w >> 16 != state->crtc_w ||
>   		state->src_h >> 16 != state->crtc_h;
> @@ -2096,7 +2161,7 @@ void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
>   		       const struct drm_plane_state *state,
>   		       u32 hw_videoport)
>   {
> -	bool lite = dispc->feat->vid_lite[hw_plane];
> +	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
>   	u32 fourcc = state->fb->format->format;
>   	u16 cpp = state->fb->format->cpp[0];
>   	u32 fb_width = state->fb->pitches[0] / cpp;
> @@ -2210,7 +2275,7 @@ static void dispc_k2g_plane_init(struct dispc_device *dispc)
>   	/* MFLAG_START = MFLAGNORMALSTARTMODE */
>   	REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 6, 6);
>   
> -	for (hw_plane = 0; hw_plane < dispc->feat->num_planes; hw_plane++) {
> +	for (hw_plane = 0; hw_plane < dispc->feat->num_vids; hw_plane++) {
>   		u32 size = dispc_vid_get_fifo_size(dispc, hw_plane);

And here.

Maybe there's more, please check each of the cases where the index is used.

  Tomi

>   		u32 thr_low, thr_high;
>   		u32 mflag_low, mflag_high;
> @@ -2226,7 +2291,7 @@ static void dispc_k2g_plane_init(struct dispc_device *dispc)
>   
>   		dev_dbg(dispc->dev,
>   			"%s: bufsize %u, buf_threshold %u/%u, mflag threshold %u/%u preload %u\n",
> -			dispc->feat->vid_name[hw_plane],
> +			dispc->feat->vid_info[hw_plane].name,
>   			size,
>   			thr_high, thr_low,
>   			mflag_high, mflag_low,
> @@ -2265,7 +2330,7 @@ static void dispc_k3_plane_init(struct dispc_device *dispc)
>   	/* MFLAG_START = MFLAGNORMALSTARTMODE */
>   	REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 6, 6);
>   
> -	for (hw_plane = 0; hw_plane < dispc->feat->num_planes; hw_plane++) {
> +	for (hw_plane = 0; hw_plane < dispc->feat->num_vids; hw_plane++) {
>   		u32 size = dispc_vid_get_fifo_size(dispc, hw_plane);
>   		u32 thr_low, thr_high;
>   		u32 mflag_low, mflag_high;
> @@ -2281,7 +2346,7 @@ static void dispc_k3_plane_init(struct dispc_device *dispc)
>   
>   		dev_dbg(dispc->dev,
>   			"%s: bufsize %u, buf_threshold %u/%u, mflag threshold %u/%u preload %u\n",
> -			dispc->feat->vid_name[hw_plane],
> +			dispc->feat->vid_info[hw_plane].name,
>   			size,
>   			thr_high, thr_low,
>   			mflag_high, mflag_low,
> @@ -2898,8 +2963,8 @@ int dispc_init(struct tidss_device *tidss)
>   	if (r)
>   		return r;
>   
> -	for (i = 0; i < dispc->feat->num_planes; i++) {
> -		r = dispc_iomap_resource(pdev, dispc->feat->vid_name[i],
> +	for (i = 0; i < dispc->feat->num_vids; i++) {
> +		r = dispc_iomap_resource(pdev, dispc->feat->vid_info[i].name,
>   					 &dispc->base_vid[i]);
>   		if (r)
>   			return r;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 086327d51a90..72a0146e57d5 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -46,6 +46,12 @@ struct dispc_features_scaling {
>   	u32 xinc_max;
>   };
>   
> +struct dispc_vid_info {
> +	const char *name; /* Should match dt reg names */
> +	u32 hw_id;
> +	bool is_lite;
> +};
> +
>   struct dispc_errata {
>   	bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
>   };
> @@ -82,9 +88,8 @@ struct dispc_features {
>   	const char *vpclk_name[TIDSS_MAX_PORTS]; /* Should match dt clk names */
>   	const enum dispc_vp_bus_type vp_bus_type[TIDSS_MAX_PORTS];
>   	struct tidss_vp_feat vp_feat;
> -	u32 num_planes;
> -	const char *vid_name[TIDSS_MAX_PLANES]; /* Should match dt reg names */
> -	bool vid_lite[TIDSS_MAX_PLANES];
> +	u32 num_vids;
> +	struct dispc_vid_info vid_info[TIDSS_MAX_PLANES];
>   	u32 vid_order[TIDSS_MAX_PLANES];
>   };
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index f371518f8697..19432c08ec6b 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -115,7 +115,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   
>   	const struct dispc_features *feat = tidss->feat;
>   	u32 max_vps = feat->num_vps;
> -	u32 max_planes = feat->num_planes;
> +	u32 max_planes = feat->num_vids;
>   
>   	struct pipe pipes[TIDSS_MAX_PORTS];
>   	u32 num_pipes = 0;
> diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c
> index 116de124bddb..ff71370cad8b 100644
> --- a/drivers/gpu/drm/tidss/tidss_plane.c
> +++ b/drivers/gpu/drm/tidss/tidss_plane.c
> @@ -200,7 +200,7 @@ struct tidss_plane *tidss_plane_create(struct tidss_device *tidss,
>   	struct tidss_plane *tplane;
>   	enum drm_plane_type type;
>   	u32 possible_crtcs;
> -	u32 num_planes = tidss->feat->num_planes;
> +	u32 num_planes = tidss->feat->num_vids;
>   	u32 color_encodings = (BIT(DRM_COLOR_YCBCR_BT601) |
>   			       BIT(DRM_COLOR_YCBCR_BT709));
>   	u32 color_ranges = (BIT(DRM_COLOR_YCBCR_FULL_RANGE) |


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions
  2025-03-27 11:18   ` Tomi Valkeinen
@ 2025-04-30 15:38     ` Devarsh Thakkar
  0 siblings, 0 replies; 6+ messages in thread
From: Devarsh Thakkar @ 2025-04-30 15:38 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: praneeth, vigneshr, aradhya.bhatia, s-jain1, r-donadkar,
	j-choudhary, h-shenoy, jyri.sarha, airlied, maarten.lankhorst,
	mripard, tzimmermann, dri-devel, simona, linux-kernel, devicetree,
	robh, krzk+dt, conor+dt

Hi Tomi

Thanks for the review. I had missed to respond back to below comment
before sending V5, so kindly find the response for the same as below.

On 27/03/25 16:48, Tomi Valkeinen wrote:

>> *dispc, u32 hw_plane,
>>                  const struct drm_plane_state *state,
>>                  u32 hw_videoport)
>>   {
>> -    bool lite = dispc->feat->vid_lite[hw_plane];
>> +    bool lite = dispc->feat->vid_info[hw_plane].is_lite;
>>       u32 fourcc = state->fb->format->format;
>>       u16 cpp = state->fb->format->cpp[0];
>>       u32 fb_width = state->fb->pitches[0] / cpp;
>> @@ -2210,7 +2275,7 @@ static void dispc_k2g_plane_init(struct
>> dispc_device *dispc)
>>       /* MFLAG_START = MFLAGNORMALSTARTMODE */
>>       REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 6, 6);
>>   -    for (hw_plane = 0; hw_plane < dispc->feat->num_planes;
>> hw_plane++) {
>> +    for (hw_plane = 0; hw_plane < dispc->feat->num_vids; hw_plane++) {
>>           u32 size = dispc_vid_get_fifo_size(dispc, hw_plane);
> 
> And here.
> 

I don't think we need to us hw_id for dispc_vid* functions as they
directly act on VID register base which is mapped based on device-tree.
So for e.g. if an SoC does not have VID0 then it won't map that register
base at all. For e.g. AM62L does not have VID region and has only VIDL
so to access VIDL base (which is the first vid region mapped hence index
0) we only need to use hw_plane as index 0

 void __iomem *base = dispc->base_vid[hw_plane];

The hw_id is only required for dispc_k3_vid* functions which access
common register space for vid* specific registers and bits. For e.g.
AM62L does not have VID base so there is a hole there and after 0x4
offset (i.e. at index 1) VIDL starts, so in this case we need to pass
hw_id as 1 (from vid_info struct).

Regards
Devarsh

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-30 15:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-26 14:57 [PATCH v4 0/3] Add support for AM62L DSS Devarsh Thakkar
2025-03-26 14:57 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: " Devarsh Thakkar
2025-03-26 14:57 ` [PATCH v4 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions Devarsh Thakkar
2025-03-27 11:18   ` Tomi Valkeinen
2025-04-30 15:38     ` Devarsh Thakkar
2025-03-26 14:57 ` [PATCH v4 3/3] drm/tidss: Add support for AM62L display subsystem Devarsh Thakkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).