[PATCH v3 0/2] Add DSS support for AM625 SoC

[PATCH v3 0/2] Add DSS support for AM625 SoC

[Aradhya Bhatia]

This patch series adds a new compatible for the DSS IP on TI's AM625
SoC. It further adds the required support for the same in the tidss
driver. The IP is a newer version of the DSS IP available on AM65X SoC,
with a major change being in the addition of another OLDI TX inside it.
With the help of 2 OLDI TXes, this new DSS IP supports OLDI displays
with a resolution of upto 2K. The OLDI support will be added
subsequently.

Changelog:

V3:
  - Changed yaml enum in alphabetical order
  - Corrected a typo

V2:
  - Removed redundant regsiter array

Aradhya Bhatia (2):
  dt-bindings: display: ti,am65x-dss: Add am625 dss compatible
  drm/tidss: Add support for AM625 DSS

 .../bindings/display/ti/ti,am65x-dss.yaml     |  4 +-
 drivers/gpu/drm/tidss/tidss_dispc.c           | 56 ++++++++++++++++++-
 drivers/gpu/drm/tidss/tidss_dispc.h           |  2 +
 drivers/gpu/drm/tidss/tidss_drv.c             |  1 +
 4 files changed, 61 insertions(+), 2 deletions(-)

-- 
2.36.1

[PATCH v3 0/2] Add DSS support for AM625 SoC

[Aradhya Bhatia]

This patch series adds a new compatible for the DSS IP on TI's AM625
SoC. It further adds the required support for the same in the tidss
driver. The IP is a newer version of the DSS IP available on AM65X SoC,
with a major change being in the addition of another OLDI TX inside it.
With the help of 2 OLDI TXes, this new DSS IP supports OLDI displays
with a resolution of upto 2K. The OLDI support will be added
subsequently.

Changelog:

V3:
  - Changed yaml enum in alphabetical order
  - Corrected a typo

V2:
  - Removed redundant regsiter array

Aradhya Bhatia (2):
  dt-bindings: display: ti,am65x-dss: Add am625 dss compatible
  drm/tidss: Add support for AM625 DSS

 .../bindings/display/ti/ti,am65x-dss.yaml     |  4 +-
 drivers/gpu/drm/tidss/tidss_dispc.c           | 56 ++++++++++++++++++-
 drivers/gpu/drm/tidss/tidss_dispc.h           |  2 +
 drivers/gpu/drm/tidss/tidss_drv.c             |  1 +
 4 files changed, 61 insertions(+), 2 deletions(-)

-- 
2.36.1

[PATCH v3 1/2] dt-bindings: display: ti, am65x-dss: Add am625 dss compatible

[Aradhya Bhatia]

Add ti,am625-dss compatible string.
The DSS IP on TI's AM625 SoC is an update from the DSS on TI's AM65X
SoC. The former has an additional OLDI TX to enable a 2K resolution on
OLDI displays or enable 2 duplicated displays with a smaller resolution.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Rahul T R <r-ravikumar@ti.com>
---
 .../devicetree/bindings/display/ti/ti,am65x-dss.yaml          | 4 +++-
 1 file changed, 3 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 5c7d2cbc4aac..6bbce921479d 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -19,7 +19,9 @@ description: |
 
 properties:
   compatible:
-    const: ti,am65x-dss
+    enum:
+      - ti,am625-dss
+      - ti,am65x-dss
 
   reg:
     description:
-- 
2.36.1

[PATCH v3 1/2] dt-bindings: display: ti,am65x-dss: Add am625 dss compatible

[Aradhya Bhatia]

Add ti,am625-dss compatible string.
The DSS IP on TI's AM625 SoC is an update from the DSS on TI's AM65X
SoC. The former has an additional OLDI TX to enable a 2K resolution on
OLDI displays or enable 2 duplicated displays with a smaller resolution.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Rahul T R <r-ravikumar@ti.com>
---
 .../devicetree/bindings/display/ti/ti,am65x-dss.yaml          | 4 +++-
 1 file changed, 3 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 5c7d2cbc4aac..6bbce921479d 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -19,7 +19,9 @@ description: |
 
 properties:
   compatible:
-    const: ti,am65x-dss
+    enum:
+      - ti,am625-dss
+      - ti,am65x-dss
 
   reg:
     description:
-- 
2.36.1

[PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Aradhya Bhatia]

Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Rahul T R <r-ravikumar@ti.com>
---
 drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
 drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index dae47853b728..f084f0688a54 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
 	.vid_order = { 1, 3, 0, 2 },
 };
 
+const struct dispc_features dispc_am625_feats = {
+	.max_pclk_khz = {
+		[DISPC_VP_DPI] = 165000,
+		[DISPC_VP_OLDI] = 165000,
+	},
+
+	.scaling = {
+		.in_width_max_5tap_rgb = 1280,
+		.in_width_max_3tap_rgb = 2560,
+		.in_width_max_5tap_yuv = 2560,
+		.in_width_max_3tap_yuv = 4096,
+		.upscale_limit = 16,
+		.downscale_limit_5tap = 4,
+		.downscale_limit_3tap = 2,
+		/*
+		 * The max supported pixel inc value is 255. The value
+		 * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
+		 * The maximum bpp of all formats supported by the HW
+		 * is 8. So the maximum supported xinc value is 32,
+		 * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
+		 */
+		.xinc_max = 32,
+	},
+
+	.subrev = DISPC_AM625,
+
+	.common = "common",
+	.common_regs = tidss_am65x_common_regs,
+
+	.num_vps = 2,
+	.vp_name = { "vp1", "vp2" },
+	.ovr_name = { "ovr1", "ovr2" },
+	.vpclk_name =  { "vp1", "vp2" },
+	.vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
+
+	.vp_feat = { .color = {
+			.has_ctm = true,
+			.gamma_size = 256,
+			.gamma_type = TIDSS_GAMMA_8BIT,
+		},
+	},
+
+	.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 },
+};
+
 static const u16 *dispc_common_regmap;
 
 struct dss_vp_data {
@@ -775,6 +824,7 @@ dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc)
 		return dispc_k2g_read_and_clear_irqstatus(dispc);
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		return dispc_k3_read_and_clear_irqstatus(dispc);
 	default:
 		WARN_ON(1);
@@ -790,6 +840,7 @@ void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask)
 		break;
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		dispc_k3_set_irqenable(dispc, mask);
 		break;
 	default:
@@ -1279,6 +1330,7 @@ void dispc_ovr_set_plane(struct dispc_device *dispc, u32 hw_plane,
 					x, y, layer);
 		break;
 	case DISPC_AM65X:
+	case DISPC_AM625:
 		dispc_am65x_ovr_set_plane(dispc, hw_plane, hw_videoport,
 					  x, y, layer);
 		break;
@@ -2202,6 +2254,7 @@ static void dispc_plane_init(struct dispc_device *dispc)
 		break;
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		dispc_k3_plane_init(dispc);
 		break;
 	default:
@@ -2307,6 +2360,7 @@ static void dispc_vp_write_gamma_table(struct dispc_device *dispc,
 		dispc_k2g_vp_write_gamma_table(dispc, hw_videoport);
 		break;
 	case DISPC_AM65X:
+	case DISPC_AM625:
 		dispc_am65x_vp_write_gamma_table(dispc, hw_videoport);
 		break;
 	case DISPC_J721E:
@@ -2580,7 +2634,7 @@ int dispc_runtime_resume(struct dispc_device *dispc)
 		REG_GET(dispc, DSS_SYSSTATUS, 2, 2),
 		REG_GET(dispc, DSS_SYSSTATUS, 3, 3));
 
-	if (dispc->feat->subrev == DISPC_AM65X)
+	if (dispc->feat->subrev == DISPC_AM65X || dispc->feat->subrev == DISPC_AM625)
 		dev_dbg(dispc->dev, "OLDI RESETDONE %d,%d,%d\n",
 			REG_GET(dispc, DSS_SYSSTATUS, 5, 5),
 			REG_GET(dispc, DSS_SYSSTATUS, 6, 6),
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index e49432f0abf5..a28005dafdc9 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -61,6 +61,7 @@ enum dispc_dss_subrevision {
 	DISPC_K2G,
 	DISPC_AM65X,
 	DISPC_J721E,
+	DISPC_AM625,
 };
 
 struct dispc_features {
@@ -88,6 +89,7 @@ struct dispc_features {
 extern const struct dispc_features dispc_k2g_feats;
 extern const struct dispc_features dispc_am65x_feats;
 extern const struct dispc_features dispc_j721e_feats;
+extern const struct dispc_features dispc_am625_feats;
 
 void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
 dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 04cfff89ee51..326059e99696 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -235,6 +235,7 @@ static const struct of_device_id tidss_of_table[] = {
 	{ .compatible = "ti,k2g-dss", .data = &dispc_k2g_feats, },
 	{ .compatible = "ti,am65x-dss", .data = &dispc_am65x_feats, },
 	{ .compatible = "ti,j721e-dss", .data = &dispc_j721e_feats, },
+	{ .compatible = "ti,am625-dss", .data = &dispc_am625_feats, },
 	{ }
 };
 
-- 
2.36.1

[PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Aradhya Bhatia]

Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Rahul T R <r-ravikumar@ti.com>
---
 drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
 drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index dae47853b728..f084f0688a54 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
 	.vid_order = { 1, 3, 0, 2 },
 };
 
+const struct dispc_features dispc_am625_feats = {
+	.max_pclk_khz = {
+		[DISPC_VP_DPI] = 165000,
+		[DISPC_VP_OLDI] = 165000,
+	},
+
+	.scaling = {
+		.in_width_max_5tap_rgb = 1280,
+		.in_width_max_3tap_rgb = 2560,
+		.in_width_max_5tap_yuv = 2560,
+		.in_width_max_3tap_yuv = 4096,
+		.upscale_limit = 16,
+		.downscale_limit_5tap = 4,
+		.downscale_limit_3tap = 2,
+		/*
+		 * The max supported pixel inc value is 255. The value
+		 * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
+		 * The maximum bpp of all formats supported by the HW
+		 * is 8. So the maximum supported xinc value is 32,
+		 * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
+		 */
+		.xinc_max = 32,
+	},
+
+	.subrev = DISPC_AM625,
+
+	.common = "common",
+	.common_regs = tidss_am65x_common_regs,
+
+	.num_vps = 2,
+	.vp_name = { "vp1", "vp2" },
+	.ovr_name = { "ovr1", "ovr2" },
+	.vpclk_name =  { "vp1", "vp2" },
+	.vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
+
+	.vp_feat = { .color = {
+			.has_ctm = true,
+			.gamma_size = 256,
+			.gamma_type = TIDSS_GAMMA_8BIT,
+		},
+	},
+
+	.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 },
+};
+
 static const u16 *dispc_common_regmap;
 
 struct dss_vp_data {
@@ -775,6 +824,7 @@ dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc)
 		return dispc_k2g_read_and_clear_irqstatus(dispc);
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		return dispc_k3_read_and_clear_irqstatus(dispc);
 	default:
 		WARN_ON(1);
@@ -790,6 +840,7 @@ void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask)
 		break;
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		dispc_k3_set_irqenable(dispc, mask);
 		break;
 	default:
@@ -1279,6 +1330,7 @@ void dispc_ovr_set_plane(struct dispc_device *dispc, u32 hw_plane,
 					x, y, layer);
 		break;
 	case DISPC_AM65X:
+	case DISPC_AM625:
 		dispc_am65x_ovr_set_plane(dispc, hw_plane, hw_videoport,
 					  x, y, layer);
 		break;
@@ -2202,6 +2254,7 @@ static void dispc_plane_init(struct dispc_device *dispc)
 		break;
 	case DISPC_AM65X:
 	case DISPC_J721E:
+	case DISPC_AM625:
 		dispc_k3_plane_init(dispc);
 		break;
 	default:
@@ -2307,6 +2360,7 @@ static void dispc_vp_write_gamma_table(struct dispc_device *dispc,
 		dispc_k2g_vp_write_gamma_table(dispc, hw_videoport);
 		break;
 	case DISPC_AM65X:
+	case DISPC_AM625:
 		dispc_am65x_vp_write_gamma_table(dispc, hw_videoport);
 		break;
 	case DISPC_J721E:
@@ -2580,7 +2634,7 @@ int dispc_runtime_resume(struct dispc_device *dispc)
 		REG_GET(dispc, DSS_SYSSTATUS, 2, 2),
 		REG_GET(dispc, DSS_SYSSTATUS, 3, 3));
 
-	if (dispc->feat->subrev == DISPC_AM65X)
+	if (dispc->feat->subrev == DISPC_AM65X || dispc->feat->subrev == DISPC_AM625)
 		dev_dbg(dispc->dev, "OLDI RESETDONE %d,%d,%d\n",
 			REG_GET(dispc, DSS_SYSSTATUS, 5, 5),
 			REG_GET(dispc, DSS_SYSSTATUS, 6, 6),
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index e49432f0abf5..a28005dafdc9 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -61,6 +61,7 @@ enum dispc_dss_subrevision {
 	DISPC_K2G,
 	DISPC_AM65X,
 	DISPC_J721E,
+	DISPC_AM625,
 };
 
 struct dispc_features {
@@ -88,6 +89,7 @@ struct dispc_features {
 extern const struct dispc_features dispc_k2g_feats;
 extern const struct dispc_features dispc_am65x_feats;
 extern const struct dispc_features dispc_j721e_feats;
+extern const struct dispc_features dispc_am625_feats;
 
 void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
 dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 04cfff89ee51..326059e99696 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -235,6 +235,7 @@ static const struct of_device_id tidss_of_table[] = {
 	{ .compatible = "ti,k2g-dss", .data = &dispc_k2g_feats, },
 	{ .compatible = "ti,am65x-dss", .data = &dispc_am65x_feats, },
 	{ .compatible = "ti,j721e-dss", .data = &dispc_j721e_feats, },
+	{ .compatible = "ti,am625-dss", .data = &dispc_am625_feats, },
 	{ }
 };
 
-- 
2.36.1

Re: [PATCH v3 1/2] dt-bindings: display: ti,am65x-dss: Add am625 dss compatible

[Krzysztof Kozlowski]

On 27/06/2022 17:11, Aradhya Bhatia wrote:
> Add ti,am625-dss compatible string.
> The DSS IP on TI's AM625 SoC is an update from the DSS on TI's AM65X
> SoC. The former has an additional OLDI TX to enable a 2K resolution on
> OLDI displays or enable 2 duplicated displays with a smaller resolution.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: display: ti, am65x-dss: Add am625 dss compatible

[Krzysztof Kozlowski]

On 27/06/2022 17:11, Aradhya Bhatia wrote:
> Add ti,am625-dss compatible string.
> The DSS IP on TI's AM625 SoC is an update from the DSS on TI's AM65X
> SoC. The former has an additional OLDI TX to enable a 2K resolution on
> OLDI displays or enable 2 duplicated displays with a smaller resolution.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Tomi Valkeinen]

On 27/06/2022 18:12, Aradhya Bhatia wrote:
> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>   3 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index dae47853b728..f084f0688a54 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>   	.vid_order = { 1, 3, 0, 2 },
>   };
>   
> +const struct dispc_features dispc_am625_feats = {
> +	.max_pclk_khz = {
> +		[DISPC_VP_DPI] = 165000,
> +		[DISPC_VP_OLDI] = 165000,
> +	},
> +
> +	.scaling = {
> +		.in_width_max_5tap_rgb = 1280,
> +		.in_width_max_3tap_rgb = 2560,
> +		.in_width_max_5tap_yuv = 2560,
> +		.in_width_max_3tap_yuv = 4096,
> +		.upscale_limit = 16,
> +		.downscale_limit_5tap = 4,
> +		.downscale_limit_3tap = 2,
> +		/*
> +		 * The max supported pixel inc value is 255. The value
> +		 * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
> +		 * The maximum bpp of all formats supported by the HW
> +		 * is 8. So the maximum supported xinc value is 32,
> +		 * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
> +		 */
> +		.xinc_max = 32,
> +	},
> +
> +	.subrev = DISPC_AM625,
> +
> +	.common = "common",
> +	.common_regs = tidss_am65x_common_regs,
> +
> +	.num_vps = 2,
> +	.vp_name = { "vp1", "vp2" },
> +	.ovr_name = { "ovr1", "ovr2" },
> +	.vpclk_name =  { "vp1", "vp2" },
> +	.vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },

This looks correct, but with the two OLDI TXes, I think there will be 
some interesting issues.

The tidss_kms.c associates a DSS VP and a DT port, but that's no longer 
true if you add the ports for both OLDI TXes, as they both use the same 
VP. I think fixing that won't affect this patch, though, and merging 
this patch will, afaik, enable similar DSS functionality as we have for 
AM65x.

So, I think these two patches could be merged, or we could wait a bit 
until the OLDI situation becomes more clear. Up to you. In any case, for 
both patches:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Tomi Valkeinen]

On 27/06/2022 18:12, Aradhya Bhatia wrote:
> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>   3 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index dae47853b728..f084f0688a54 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>   	.vid_order = { 1, 3, 0, 2 },
>   };
>   
> +const struct dispc_features dispc_am625_feats = {
> +	.max_pclk_khz = {
> +		[DISPC_VP_DPI] = 165000,
> +		[DISPC_VP_OLDI] = 165000,
> +	},
> +
> +	.scaling = {
> +		.in_width_max_5tap_rgb = 1280,
> +		.in_width_max_3tap_rgb = 2560,
> +		.in_width_max_5tap_yuv = 2560,
> +		.in_width_max_3tap_yuv = 4096,
> +		.upscale_limit = 16,
> +		.downscale_limit_5tap = 4,
> +		.downscale_limit_3tap = 2,
> +		/*
> +		 * The max supported pixel inc value is 255. The value
> +		 * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
> +		 * The maximum bpp of all formats supported by the HW
> +		 * is 8. So the maximum supported xinc value is 32,
> +		 * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
> +		 */
> +		.xinc_max = 32,
> +	},
> +
> +	.subrev = DISPC_AM625,
> +
> +	.common = "common",
> +	.common_regs = tidss_am65x_common_regs,
> +
> +	.num_vps = 2,
> +	.vp_name = { "vp1", "vp2" },
> +	.ovr_name = { "ovr1", "ovr2" },
> +	.vpclk_name =  { "vp1", "vp2" },
> +	.vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },

This looks correct, but with the two OLDI TXes, I think there will be 
some interesting issues.

The tidss_kms.c associates a DSS VP and a DT port, but that's no longer 
true if you add the ports for both OLDI TXes, as they both use the same 
VP. I think fixing that won't affect this patch, though, and merging 
this patch will, afaik, enable similar DSS functionality as we have for 
AM65x.

So, I think these two patches could be merged, or we could wait a bit 
until the OLDI situation becomes more clear. Up to you. In any case, for 
both patches:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Aradhya Bhatia]

Hi Tomi,

On 28-Jul-22 17:34, Tomi Valkeinen wrote:
> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>> ---
>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>> index dae47853b728..f084f0688a54 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>       .vid_order = { 1, 3, 0, 2 },
>>   };
>> +const struct dispc_features dispc_am625_feats = {
>> +    .max_pclk_khz = {
>> +        [DISPC_VP_DPI] = 165000,
>> +        [DISPC_VP_OLDI] = 165000,
>> +    },
>> +
>> +    .scaling = {
>> +        .in_width_max_5tap_rgb = 1280,
>> +        .in_width_max_3tap_rgb = 2560,
>> +        .in_width_max_5tap_yuv = 2560,
>> +        .in_width_max_3tap_yuv = 4096,
>> +        .upscale_limit = 16,
>> +        .downscale_limit_5tap = 4,
>> +        .downscale_limit_3tap = 2,
>> +        /*
>> +         * The max supported pixel inc value is 255. The value
>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>> +         * The maximum bpp of all formats supported by the HW
>> +         * is 8. So the maximum supported xinc value is 32,
>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>> +         */
>> +        .xinc_max = 32,
>> +    },
>> +
>> +    .subrev = DISPC_AM625,
>> +
>> +    .common = "common",
>> +    .common_regs = tidss_am65x_common_regs,
>> +
>> +    .num_vps = 2,
>> +    .vp_name = { "vp1", "vp2" },
>> +    .ovr_name = { "ovr1", "ovr2" },
>> +    .vpclk_name =  { "vp1", "vp2" },
>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
> 
> This looks correct, but with the two OLDI TXes, I think there will be 
> some interesting issues.
> 
> The tidss_kms.c associates a DSS VP and a DT port, but that's no longer 
> true if you add the ports for both OLDI TXes, as they both use the same 
> VP. I think fixing that won't affect this patch, though, and merging 
> this patch will, afaik, enable similar DSS functionality as we have for 
> AM65x.
> 
> So, I think these two patches could be merged, or we could wait a bit 
> until the OLDI situation becomes more clear. Up to you. In any case, for 
> both patches:
> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\

Thank you for the review!

This patch set is required for the dss DT patches to be upstreamed for
the AM625-SK, so I would like them to get merged.

Since these were posted in the previous merge window, I will re-send 
with your tag.

Regards
Aradhya

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Tomi Valkeinen]

On 09/08/2022 09:08, Aradhya Bhatia wrote:
> Hi Tomi,
> 
> On 28-Jul-22 17:34, Tomi Valkeinen wrote:
>> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>>
>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>>> ---
>>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 ++++++++++++++++++++++++++++-
>>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>> index dae47853b728..f084f0688a54 100644
>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>>       .vid_order = { 1, 3, 0, 2 },
>>>   };
>>> +const struct dispc_features dispc_am625_feats = {
>>> +    .max_pclk_khz = {
>>> +        [DISPC_VP_DPI] = 165000,
>>> +        [DISPC_VP_OLDI] = 165000,
>>> +    },
>>> +
>>> +    .scaling = {
>>> +        .in_width_max_5tap_rgb = 1280,
>>> +        .in_width_max_3tap_rgb = 2560,
>>> +        .in_width_max_5tap_yuv = 2560,
>>> +        .in_width_max_3tap_yuv = 4096,
>>> +        .upscale_limit = 16,
>>> +        .downscale_limit_5tap = 4,
>>> +        .downscale_limit_3tap = 2,
>>> +        /*
>>> +         * The max supported pixel inc value is 255. The value
>>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>>> +         * The maximum bpp of all formats supported by the HW
>>> +         * is 8. So the maximum supported xinc value is 32,
>>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>>> +         */
>>> +        .xinc_max = 32,
>>> +    },
>>> +
>>> +    .subrev = DISPC_AM625,
>>> +
>>> +    .common = "common",
>>> +    .common_regs = tidss_am65x_common_regs,
>>> +
>>> +    .num_vps = 2,
>>> +    .vp_name = { "vp1", "vp2" },
>>> +    .ovr_name = { "ovr1", "ovr2" },
>>> +    .vpclk_name =  { "vp1", "vp2" },
>>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
>>
>> This looks correct, but with the two OLDI TXes, I think there will be 
>> some interesting issues.
>>
>> The tidss_kms.c associates a DSS VP and a DT port, but that's no 
>> longer true if you add the ports for both OLDI TXes, as they both use 
>> the same VP. I think fixing that won't affect this patch, though, and 
>> merging this patch will, afaik, enable similar DSS functionality as we 
>> have for AM65x.
>>
>> So, I think these two patches could be merged, or we could wait a bit 
>> until the OLDI situation becomes more clear. Up to you. In any case, 
>> for both patches:
>>
>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\
> 
> Thank you for the review!
> 
> This patch set is required for the dss DT patches to be upstreamed for
> the AM625-SK, so I would like them to get merged.
> 
> Since these were posted in the previous merge window, I will re-send 
> with your tag.

I'd like to understand better the dual OLDI TX case before merging any 
AM625 dss changes.

At the moment you have only one port in the DT for the OLDI TX for 
AM625, right? I don't see how that is supposed to work as there are two 
OLDI outputs. And if we do add a new port, it perhaps makes sense to 
have two OLDI TX ports as ports 0 and 1, and the DPI as port 2, which is 
then different from AM65x.

  Tomi

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Aradhya Bhatia]

Hi Tomi,

On 09-Aug-22 12:01, Tomi Valkeinen wrote:
> On 09/08/2022 09:08, Aradhya Bhatia wrote:
>> Hi Tomi,
>>
>> On 28-Jul-22 17:34, Tomi Valkeinen wrote:
>>> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>>>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>>>
>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>>>> ---
>>>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 
>>>> ++++++++++++++++++++++++++++-
>>>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> index dae47853b728..f084f0688a54 100644
>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>>>       .vid_order = { 1, 3, 0, 2 },
>>>>   };
>>>> +const struct dispc_features dispc_am625_feats = {
>>>> +    .max_pclk_khz = {
>>>> +        [DISPC_VP_DPI] = 165000,
>>>> +        [DISPC_VP_OLDI] = 165000,
>>>> +    },
>>>> +
>>>> +    .scaling = {
>>>> +        .in_width_max_5tap_rgb = 1280,
>>>> +        .in_width_max_3tap_rgb = 2560,
>>>> +        .in_width_max_5tap_yuv = 2560,
>>>> +        .in_width_max_3tap_yuv = 4096,
>>>> +        .upscale_limit = 16,
>>>> +        .downscale_limit_5tap = 4,
>>>> +        .downscale_limit_3tap = 2,
>>>> +        /*
>>>> +         * The max supported pixel inc value is 255. The value
>>>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>>>> +         * The maximum bpp of all formats supported by the HW
>>>> +         * is 8. So the maximum supported xinc value is 32,
>>>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>>>> +         */
>>>> +        .xinc_max = 32,
>>>> +    },
>>>> +
>>>> +    .subrev = DISPC_AM625,
>>>> +
>>>> +    .common = "common",
>>>> +    .common_regs = tidss_am65x_common_regs,
>>>> +
>>>> +    .num_vps = 2,
>>>> +    .vp_name = { "vp1", "vp2" },
>>>> +    .ovr_name = { "ovr1", "ovr2" },
>>>> +    .vpclk_name =  { "vp1", "vp2" },
>>>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
>>>
>>> This looks correct, but with the two OLDI TXes, I think there will be 
>>> some interesting issues.
>>>
>>> The tidss_kms.c associates a DSS VP and a DT port, but that's no 
>>> longer true if you add the ports for both OLDI TXes, as they both use 
>>> the same VP. I think fixing that won't affect this patch, though, and 
>>> merging this patch will, afaik, enable similar DSS functionality as 
>>> we have for AM65x.
>>>
>>> So, I think these two patches could be merged, or we could wait a bit 
>>> until the OLDI situation becomes more clear. Up to you. In any case, 
>>> for both patches:
>>>
>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\
>>
>> Thank you for the review!
>>
>> This patch set is required for the dss DT patches to be upstreamed for
>> the AM625-SK, so I would like them to get merged.
>>
>> Since these were posted in the previous merge window, I will re-send 
>> with your tag.
> 
> I'd like to understand better the dual OLDI TX case before merging any 
> AM625 dss changes.
> 
> At the moment you have only one port in the DT for the OLDI TX for 
> AM625, right? I don't see how that is supposed to work as there are two 
> OLDI outputs. 
The OLDI node doesn't have node of its own at all. Its the dss port that
gets directly connected to the panel ports.

> And if we do add a new port, it perhaps makes sense to 
> have two OLDI TX ports as ports 0 and 1, and the DPI as port 2, which is 
> then different from AM65x.
The DSS still has a single (DPI) VP for the OLDI outputs. Both the OLDI 
TXes receive the same input from the DSS VP.

Wouldn't having them modeled as videp ports 0 and 1 would mean that the
DSS is capable of driving 2 different OLDI displays? (which is not the
case here).

Regards
Aradhya

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Aradhya Bhatia]

Hi Tomi,

On 09-Aug-22 12:01, Tomi Valkeinen wrote:
> On 09/08/2022 09:08, Aradhya Bhatia wrote:
>> Hi Tomi,
>>
>> On 28-Jul-22 17:34, Tomi Valkeinen wrote:
>>> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>>>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>>>
>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>>>> ---
>>>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 
>>>> ++++++++++++++++++++++++++++-
>>>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> index dae47853b728..f084f0688a54 100644
>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>>>       .vid_order = { 1, 3, 0, 2 },
>>>>   };
>>>> +const struct dispc_features dispc_am625_feats = {
>>>> +    .max_pclk_khz = {
>>>> +        [DISPC_VP_DPI] = 165000,
>>>> +        [DISPC_VP_OLDI] = 165000,
>>>> +    },
>>>> +
>>>> +    .scaling = {
>>>> +        .in_width_max_5tap_rgb = 1280,
>>>> +        .in_width_max_3tap_rgb = 2560,
>>>> +        .in_width_max_5tap_yuv = 2560,
>>>> +        .in_width_max_3tap_yuv = 4096,
>>>> +        .upscale_limit = 16,
>>>> +        .downscale_limit_5tap = 4,
>>>> +        .downscale_limit_3tap = 2,
>>>> +        /*
>>>> +         * The max supported pixel inc value is 255. The value
>>>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>>>> +         * The maximum bpp of all formats supported by the HW
>>>> +         * is 8. So the maximum supported xinc value is 32,
>>>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>>>> +         */
>>>> +        .xinc_max = 32,
>>>> +    },
>>>> +
>>>> +    .subrev = DISPC_AM625,
>>>> +
>>>> +    .common = "common",
>>>> +    .common_regs = tidss_am65x_common_regs,
>>>> +
>>>> +    .num_vps = 2,
>>>> +    .vp_name = { "vp1", "vp2" },
>>>> +    .ovr_name = { "ovr1", "ovr2" },
>>>> +    .vpclk_name =  { "vp1", "vp2" },
>>>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
>>>
>>> This looks correct, but with the two OLDI TXes, I think there will be 
>>> some interesting issues.
>>>
>>> The tidss_kms.c associates a DSS VP and a DT port, but that's no 
>>> longer true if you add the ports for both OLDI TXes, as they both use 
>>> the same VP. I think fixing that won't affect this patch, though, and 
>>> merging this patch will, afaik, enable similar DSS functionality as 
>>> we have for AM65x.
>>>
>>> So, I think these two patches could be merged, or we could wait a bit 
>>> until the OLDI situation becomes more clear. Up to you. In any case, 
>>> for both patches:
>>>
>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\
>>
>> Thank you for the review!
>>
>> This patch set is required for the dss DT patches to be upstreamed for
>> the AM625-SK, so I would like them to get merged.
>>
>> Since these were posted in the previous merge window, I will re-send 
>> with your tag.
> 
> I'd like to understand better the dual OLDI TX case before merging any 
> AM625 dss changes.
> 
> At the moment you have only one port in the DT for the OLDI TX for 
> AM625, right? I don't see how that is supposed to work as there are two 
> OLDI outputs. 
The OLDI node doesn't have node of its own at all. Its the dss port that
gets directly connected to the panel ports.

> And if we do add a new port, it perhaps makes sense to 
> have two OLDI TX ports as ports 0 and 1, and the DPI as port 2, which is 
> then different from AM65x.
The DSS still has a single (DPI) VP for the OLDI outputs. Both the OLDI 
TXes receive the same input from the DSS VP.

Wouldn't having them modeled as videp ports 0 and 1 would mean that the
DSS is capable of driving 2 different OLDI displays? (which is not the
case here).

Regards
Aradhya

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Tomi Valkeinen]

On 09/08/2022 12:21, Aradhya Bhatia wrote:
> Hi Tomi,
> 
> On 09-Aug-22 12:01, Tomi Valkeinen wrote:
>> On 09/08/2022 09:08, Aradhya Bhatia wrote:
>>> Hi Tomi,
>>>
>>> On 28-Jul-22 17:34, Tomi Valkeinen wrote:
>>>> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>>>>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>>>>
>>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>>>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>>>>> ---
>>>>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 
>>>>> ++++++++++++++++++++++++++++-
>>>>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>>>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>>>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> index dae47853b728..f084f0688a54 100644
>>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>>>>       .vid_order = { 1, 3, 0, 2 },
>>>>>   };
>>>>> +const struct dispc_features dispc_am625_feats = {
>>>>> +    .max_pclk_khz = {
>>>>> +        [DISPC_VP_DPI] = 165000,
>>>>> +        [DISPC_VP_OLDI] = 165000,
>>>>> +    },
>>>>> +
>>>>> +    .scaling = {
>>>>> +        .in_width_max_5tap_rgb = 1280,
>>>>> +        .in_width_max_3tap_rgb = 2560,
>>>>> +        .in_width_max_5tap_yuv = 2560,
>>>>> +        .in_width_max_3tap_yuv = 4096,
>>>>> +        .upscale_limit = 16,
>>>>> +        .downscale_limit_5tap = 4,
>>>>> +        .downscale_limit_3tap = 2,
>>>>> +        /*
>>>>> +         * The max supported pixel inc value is 255. The value
>>>>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>>>>> +         * The maximum bpp of all formats supported by the HW
>>>>> +         * is 8. So the maximum supported xinc value is 32,
>>>>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>>>>> +         */
>>>>> +        .xinc_max = 32,
>>>>> +    },
>>>>> +
>>>>> +    .subrev = DISPC_AM625,
>>>>> +
>>>>> +    .common = "common",
>>>>> +    .common_regs = tidss_am65x_common_regs,
>>>>> +
>>>>> +    .num_vps = 2,
>>>>> +    .vp_name = { "vp1", "vp2" },
>>>>> +    .ovr_name = { "ovr1", "ovr2" },
>>>>> +    .vpclk_name =  { "vp1", "vp2" },
>>>>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
>>>>
>>>> This looks correct, but with the two OLDI TXes, I think there will 
>>>> be some interesting issues.
>>>>
>>>> The tidss_kms.c associates a DSS VP and a DT port, but that's no 
>>>> longer true if you add the ports for both OLDI TXes, as they both 
>>>> use the same VP. I think fixing that won't affect this patch, 
>>>> though, and merging this patch will, afaik, enable similar DSS 
>>>> functionality as we have for AM65x.
>>>>
>>>> So, I think these two patches could be merged, or we could wait a 
>>>> bit until the OLDI situation becomes more clear. Up to you. In any 
>>>> case, for both patches:
>>>>
>>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\
>>>
>>> Thank you for the review!
>>>
>>> This patch set is required for the dss DT patches to be upstreamed for
>>> the AM625-SK, so I would like them to get merged.
>>>
>>> Since these were posted in the previous merge window, I will re-send 
>>> with your tag.
>>
>> I'd like to understand better the dual OLDI TX case before merging any 
>> AM625 dss changes.
>>
>> At the moment you have only one port in the DT for the OLDI TX for 
>> AM625, right? I don't see how that is supposed to work as there are 
>> two OLDI outputs. 
> The OLDI node doesn't have node of its own at all. Its the dss port that
> gets directly connected to the panel ports.
> 
>> And if we do add a new port, it perhaps makes sense to have two OLDI 
>> TX ports as ports 0 and 1, and the DPI as port 2, which is then 
>> different from AM65x.
> The DSS still has a single (DPI) VP for the OLDI outputs. Both the OLDI 
> TXes receive the same input from the DSS VP.

Yes, but don't mix the DSS VP and the DT port. They are not the same thing.

> Wouldn't having them modeled as videp ports 0 and 1 would mean that the
> DSS is capable of driving 2 different OLDI displays? (which is not the
> case here).

If you use the OLDI cloning, the AM625 is driving two OLDI displays, no? 
In theory the panels could be of different model, as long as they both 
support the same video mode, and they could be managed by different 
drivers. This requires two ports so that you can connect the panels in 
the DT.

But let's continue this discussion in the "[PATCH 4/8] drm/tidss: Add 
support for Dual Link LVDS Bus Format" thread, no need to discuss the 
same things in two threads =).

  Tomi

Re: [PATCH v3 2/2] drm/tidss: Add support for AM625 DSS

[Tomi Valkeinen]

On 09/08/2022 12:21, Aradhya Bhatia wrote:
> Hi Tomi,
> 
> On 09-Aug-22 12:01, Tomi Valkeinen wrote:
>> On 09/08/2022 09:08, Aradhya Bhatia wrote:
>>> Hi Tomi,
>>>
>>> On 28-Jul-22 17:34, Tomi Valkeinen wrote:
>>>> On 27/06/2022 18:12, Aradhya Bhatia wrote:
>>>>> Add support for the DSS IP on TI's new AM625 SoC in the tidss driver.
>>>>>
>>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>>>>> Reviewed-by: Rahul T R <r-ravikumar@ti.com>
>>>>> ---
>>>>>   drivers/gpu/drm/tidss/tidss_dispc.c | 56 
>>>>> ++++++++++++++++++++++++++++-
>>>>>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>>>>>   drivers/gpu/drm/tidss/tidss_drv.c   |  1 +
>>>>>   3 files changed, 58 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
>>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> index dae47853b728..f084f0688a54 100644
>>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>>>>> @@ -272,6 +272,55 @@ const struct dispc_features dispc_j721e_feats = {
>>>>>       .vid_order = { 1, 3, 0, 2 },
>>>>>   };
>>>>> +const struct dispc_features dispc_am625_feats = {
>>>>> +    .max_pclk_khz = {
>>>>> +        [DISPC_VP_DPI] = 165000,
>>>>> +        [DISPC_VP_OLDI] = 165000,
>>>>> +    },
>>>>> +
>>>>> +    .scaling = {
>>>>> +        .in_width_max_5tap_rgb = 1280,
>>>>> +        .in_width_max_3tap_rgb = 2560,
>>>>> +        .in_width_max_5tap_yuv = 2560,
>>>>> +        .in_width_max_3tap_yuv = 4096,
>>>>> +        .upscale_limit = 16,
>>>>> +        .downscale_limit_5tap = 4,
>>>>> +        .downscale_limit_3tap = 2,
>>>>> +        /*
>>>>> +         * The max supported pixel inc value is 255. The value
>>>>> +         * of pixel inc is calculated like this: 1+(xinc-1)*bpp.
>>>>> +         * The maximum bpp of all formats supported by the HW
>>>>> +         * is 8. So the maximum supported xinc value is 32,
>>>>> +         * because 1+(32-1)*8 < 255 < 1+(33-1)*4.
>>>>> +         */
>>>>> +        .xinc_max = 32,
>>>>> +    },
>>>>> +
>>>>> +    .subrev = DISPC_AM625,
>>>>> +
>>>>> +    .common = "common",
>>>>> +    .common_regs = tidss_am65x_common_regs,
>>>>> +
>>>>> +    .num_vps = 2,
>>>>> +    .vp_name = { "vp1", "vp2" },
>>>>> +    .ovr_name = { "ovr1", "ovr2" },
>>>>> +    .vpclk_name =  { "vp1", "vp2" },
>>>>> +    .vp_bus_type = { DISPC_VP_OLDI, DISPC_VP_DPI },
>>>>
>>>> This looks correct, but with the two OLDI TXes, I think there will 
>>>> be some interesting issues.
>>>>
>>>> The tidss_kms.c associates a DSS VP and a DT port, but that's no 
>>>> longer true if you add the ports for both OLDI TXes, as they both 
>>>> use the same VP. I think fixing that won't affect this patch, 
>>>> though, and merging this patch will, afaik, enable similar DSS 
>>>> functionality as we have for AM65x.
>>>>
>>>> So, I think these two patches could be merged, or we could wait a 
>>>> bit until the OLDI situation becomes more clear. Up to you. In any 
>>>> case, for both patches:
>>>>
>>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\
>>>
>>> Thank you for the review!
>>>
>>> This patch set is required for the dss DT patches to be upstreamed for
>>> the AM625-SK, so I would like them to get merged.
>>>
>>> Since these were posted in the previous merge window, I will re-send 
>>> with your tag.
>>
>> I'd like to understand better the dual OLDI TX case before merging any 
>> AM625 dss changes.
>>
>> At the moment you have only one port in the DT for the OLDI TX for 
>> AM625, right? I don't see how that is supposed to work as there are 
>> two OLDI outputs. 
> The OLDI node doesn't have node of its own at all. Its the dss port that
> gets directly connected to the panel ports.
> 
>> And if we do add a new port, it perhaps makes sense to have two OLDI 
>> TX ports as ports 0 and 1, and the DPI as port 2, which is then 
>> different from AM65x.
> The DSS still has a single (DPI) VP for the OLDI outputs. Both the OLDI 
> TXes receive the same input from the DSS VP.

Yes, but don't mix the DSS VP and the DT port. They are not the same thing.

> Wouldn't having them modeled as videp ports 0 and 1 would mean that the
> DSS is capable of driving 2 different OLDI displays? (which is not the
> case here).

If you use the OLDI cloning, the AM625 is driving two OLDI displays, no? 
In theory the panels could be of different model, as long as they both 
support the same video mode, and they could be managed by different 
drivers. This requires two ports so that you can connect the panels in 
the DT.

But let's continue this discussion in the "[PATCH 4/8] drm/tidss: Add 
support for Dual Link LVDS Bus Format" thread, no need to discuss the 
same things in two threads =).

  Tomi