[PATCH v3 0/3] Add Display support for AM62P SoC

[PATCH v3 0/3] Add Display support for AM62P SoC

[Swamil Jain]

Display Controller Overview:
TI's AM62P[1] SoC has two instances of TI's Display Subsystem (DSS).
Each instance contains two video ports. Combined, both instances support
up to three independent video streams: OLDI, DPI, and DSI.

This series:
  1. Updates bindings (PATCH 1/3)
    - Adds "ti,am62p-dss" compatible string
    - Modifies power-domain requirements
  2. Updates driver (PATCH 2/3 and 3/3)
    - Adds power management for attached PM domains
    - Enables AM62P DSS support by adding compatible to the driver

Note:
  - Device-tree changes will be submitted after this series is merged.  
  - The device-tree patches are available here[2]

[1]: https://www.ti.com/product/AM62P
[2]: https://github.com/swamiljain/linux-next/tree/AM62P_J722S_DSS_v1
---
Changelog:
v2->v3:
  - PATCH 1/3 - Add a broader range for top-level constraints in the 
                bindings to resolve dt_biniding_check conflicts
  - PATCH 2/3 - Remove and modify some comments
              - Use IS_ERR_OR_NULL() instead od checking for both NULL
                and IS_ERR()
              - Changes in error handling paths
  - PATCH 3/3 - Pick R-by tag

Link to v2:
https://lore.kernel.org/all/20251125165942.2586341-1-s-jain1@ti.com/

v1->v2:
  - PATCH 1/3: - Remove unnecessary example
               - Use "am62p-dss" compatible check for multiple
                 power-domains
  - PATCH 2/3:   Add Signed-off-by tag

Link to v1:
https://lore.kernel.org/all/20251114064336.3683731-1-s-jain1@ti.com/
---
Devarsh Thakkar (1):
  drm/tidss: Power up attached PM domains on probe

Swamil Jain (2):
  dt-bindings: display: ti,am65x-dss: Add am62p dss compatible
  drm: tidss: tidss_drv: Add support for AM62P display subsystem

 .../bindings/display/ti/ti,am65x-dss.yaml     | 33 +++++++-
 drivers/gpu/drm/tidss/tidss_drv.c             | 84 ++++++++++++++++++-
 drivers/gpu/drm/tidss/tidss_drv.h             |  4 +
 3 files changed, 116 insertions(+), 5 deletions(-)

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

[Swamil Jain]

TI's AM62P SoC contains two instances of the TI Keystone Display
SubSystem (DSS), each with two video ports and two video planes. These
instances support up to three independent video streams through OLDI,
DPI, and DSI interfaces.

DSS0 (first instance) supports:
 - Two OLDI transmitters on video port 1, configurable in dual-link or
   single-link mode.
 - DPI output on video port 2.

DSS1 (second instance) supports:
 - One OLDI transmitter on video port 1 (single-link mode only).
 - DSI controller output on video port 2.

The two OLDI transmitters can be configured in clone mode to drive a
pair of identical OLDI single-link displays. DPI outputs from
DSS0 VP2, DSS1 VP1, and DSS1 VP2 are multiplexed, allowing only one
DPI output at a time.

Add the compatible string "ti,am62p-dss" and update related
description accordingly.

AM62P has different power domains for DSS and OLDI compared to other
Keystone SoCs. Therefore, add 'minItems' and set to 1 and 'maxItems'
field in the power-domains property to 3 for the "ti,am62p-dss"
compatible entry to reflect this hardware difference.

Signed-off-by: Swamil Jain <s-jain1@ti.com>
---
 .../bindings/display/ti/ti,am65x-dss.yaml     | 33 ++++++++++++++++++-
 1 file changed, 32 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 38fcee91211e..e74e710934fc 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -24,6 +24,19 @@ description: |
   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.
+  The AM62P has two instances of TI Keystone Display SubSystem, each with two
+  video ports and two video planes. These instances can support up to 3
+  independent video streams through OLDI, DPI, and DSI interfaces.
+  DSS0 (first instance) supports:
+    - Two OLDI TXes on video port 1, configurable in dual-link or
+      single link clone mode
+    - DPI output on video port 2
+  DSS1 (second instance) supports:
+    - One OLDI TX on video port 1 (single-link mode only)
+    - DSI controller output on video port 2
+  The two OLDI TXes can be configured in clone mode to drive a pair of
+  identical OLDI single-link displays. DPI outputs from DSS0 VP2, DSS1 VP1,
+  and DSS1 VP2 are muxed, allowing only one DPI output at a time.
 
 properties:
   compatible:
@@ -31,6 +44,7 @@ properties:
       - ti,am625-dss
       - ti,am62a7-dss
       - ti,am62l-dss
+      - ti,am62p-dss
       - ti,am65x-dss
 
   reg:
@@ -81,7 +95,8 @@ properties:
     maxItems: 1
 
   power-domains:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
     description: phandle to the associated power domain
 
   dma-coherent: true
@@ -196,6 +211,22 @@ allOf:
               properties:
                 endpoint@1: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,am62p-dss
+    then:
+      properties:
+        power-domains:
+          minItems: 1
+          maxItems: 3
+    else:
+      properties:
+        power-domains:
+          minItems: 1
+          maxItems: 1
+
 required:
   - compatible
   - reg

[PATCH v3 2/3] drm/tidss: Power up attached PM domains on probe

[Swamil Jain]

From: Devarsh Thakkar <devarsht@ti.com>

Some SoC's such as AM62P have dedicated power domains
for OLDI which need to be powered on separately along
with display controller.

So during driver probe, power up all attached PM domains
enumerated in devicetree node for DSS.

This also prepares base to add display support for AM62P.

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
[j-choudhary@ti.com: fix PM call sequence causing kernel crash in OLDI]
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Swamil Jain <s-jain1@ti.com>
---
 drivers/gpu/drm/tidss/tidss_drv.c | 83 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/tidss/tidss_drv.h |  4 ++
 2 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 1c8cc18bc53c..33a10fba4325 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -8,6 +8,7 @@
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
 #include <linux/aperture.h>
 
 #include <drm/clients/drm_client_setup.h>
@@ -107,6 +108,68 @@ static const struct drm_driver tidss_driver = {
 	.minor			= 0,
 };
 
+static int tidss_detach_pm_domains(struct tidss_device *tidss)
+{
+	int i;
+
+	if (tidss->num_domains <= 1)
+		return 0;
+
+	for (i = 0; i < tidss->num_domains; i++) {
+		if (!IS_ERR_OR_NULL(tidss->pd_link[i]))
+			device_link_del(tidss->pd_link[i]);
+		if (!IS_ERR_OR_NULL(tidss->pd_dev[i]))
+			dev_pm_domain_detach(tidss->pd_dev[i], true);
+		tidss->pd_dev[i] = NULL;
+		tidss->pd_link[i] = NULL;
+	}
+
+	return 0;
+}
+
+static int tidss_attach_pm_domains(struct tidss_device *tidss)
+{
+	struct device *dev = tidss->dev;
+	int i;
+	int ret;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *np = pdev->dev.of_node;
+
+	tidss->num_domains = of_count_phandle_with_args(np, "power-domains",
+							"#power-domain-cells");
+
+	tidss->pd_dev = devm_kmalloc_array(dev, tidss->num_domains,
+					   sizeof(*tidss->pd_dev), GFP_KERNEL);
+	if (!tidss->pd_dev)
+		return -ENOMEM;
+
+	tidss->pd_link = devm_kmalloc_array(dev, tidss->num_domains,
+					    sizeof(*tidss->pd_link), GFP_KERNEL);
+	if (!tidss->pd_link)
+		return -ENOMEM;
+
+	for (i = 0; i < tidss->num_domains; i++) {
+		tidss->pd_dev[i] = dev_pm_domain_attach_by_id(dev, i);
+		if (IS_ERR(tidss->pd_dev[i])) {
+			ret = PTR_ERR(tidss->pd_dev[i]);
+			goto fail;
+		}
+
+		tidss->pd_link[i] = device_link_add(dev, tidss->pd_dev[i],
+						    DL_FLAG_STATELESS |
+						    DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
+		if (!tidss->pd_link[i]) {
+			ret = -EINVAL;
+			goto fail;
+		}
+	}
+
+	return 0;
+fail:
+	tidss_detach_pm_domains(tidss);
+	return ret;
+}
+
 static int tidss_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -129,15 +192,21 @@ static int tidss_probe(struct platform_device *pdev)
 
 	spin_lock_init(&tidss->irq_lock);
 
+	ret = tidss_attach_pm_domains(tidss);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to attach power domains\n");
+
 	ret = dispc_init(tidss);
 	if (ret) {
-		dev_err(dev, "failed to initialize dispc: %d\n", ret);
-		return ret;
+		dev_err_probe(dev, ret, "failed to initialize dispc\n");
+		goto err_detach_pm_domains;
 	}
 
 	ret = tidss_oldi_init(tidss);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to init OLDI\n");
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to init OLDI\n");
+		goto err_oldi_deinit;
+	}
 
 	pm_runtime_enable(dev);
 
@@ -203,8 +272,12 @@ static int tidss_probe(struct platform_device *pdev)
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_disable(dev);
 
+err_oldi_deinit:
 	tidss_oldi_deinit(tidss);
 
+err_detach_pm_domains:
+	tidss_detach_pm_domains(tidss);
+
 	return ret;
 }
 
@@ -232,6 +305,8 @@ static void tidss_remove(struct platform_device *pdev)
 	/* devm allocated dispc goes away with the dev so mark it NULL */
 	dispc_remove(tidss);
 
+	tidss_detach_pm_domains(tidss);
+
 	dev_dbg(dev, "%s done\n", __func__);
 }
 
diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
index e1c1f41d8b4b..6625b989b815 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.h
+++ b/drivers/gpu/drm/tidss/tidss_drv.h
@@ -41,6 +41,10 @@ struct tidss_device {
 	/* protects the irq masks field and irqenable/irqstatus registers */
 	spinlock_t irq_lock;
 	dispc_irq_t irq_mask;	/* enabled irqs */
+
+	int num_domains; /* Count of PM domains to be handled */
+	struct device **pd_dev;
+	struct device_link **pd_link;
 };
 
 #define to_tidss(__dev) container_of(__dev, struct tidss_device, ddev)

[PATCH v3 3/3] drm: tidss: tidss_drv: Add support for AM62P display subsystem

[Swamil Jain]

The DSS controller on TI's AM62P SoC features two instances of the TI
DSS. Each DSS instance supports two video ports, similar to the DSS
controller found on the TI AM62X SoC. This allows three independent
video streams to be supported: OLDI, DPI, and DSI.

Since the DSS instances on AM62P are architecturally similar to those
on the AM62X DSS controller, the existing dispc_am625_feats
configuration can be reused for the AM62P DSS support.

This patch adds the necessary device tree compatibility entry for
"ti,am62p-dss" in the tidss driver, pointing to dispc_am625_feats,
thereby enabling DSS support on AM62P devices.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Swamil Jain <s-jain1@ti.com>
---
 drivers/gpu/drm/tidss/tidss_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 33a10fba4325..0c6087adee2c 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -320,6 +320,7 @@ static const struct of_device_id tidss_of_table[] = {
 	{ .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,am62p-dss", .data = &dispc_am625_feats, },
 	{ .compatible = "ti,am65x-dss", .data = &dispc_am65x_feats, },
 	{ .compatible = "ti,j721e-dss", .data = &dispc_j721e_feats, },
 	{ }

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

[Krzysztof Kozlowski]

On Wed, Jan 07, 2026 at 11:15:23PM +0530, Swamil Jain wrote:
> TI's AM62P SoC contains two instances of the TI Keystone Display
> SubSystem (DSS), each with two video ports and two video planes. These
> instances support up to three independent video streams through OLDI,
> DPI, and DSI interfaces.
> 
> DSS0 (first instance) supports:
>  - Two OLDI transmitters on video port 1, configurable in dual-link or
>    single-link mode.
>  - DPI output on video port 2.
> 
> DSS1 (second instance) supports:
>  - One OLDI transmitter on video port 1 (single-link mode only).
>  - DSI controller output on video port 2.
> 
> The two OLDI transmitters can be configured in clone mode to drive a
> pair of identical OLDI single-link displays. DPI outputs from
> DSS0 VP2, DSS1 VP1, and DSS1 VP2 are multiplexed, allowing only one
> DPI output at a time.
> 
> Add the compatible string "ti,am62p-dss" and update related
> description accordingly.
> 
> AM62P has different power domains for DSS and OLDI compared to other
> Keystone SoCs. Therefore, add 'minItems' and set to 1 and 'maxItems'
> field in the power-domains property to 3 for the "ti,am62p-dss"
> compatible entry to reflect this hardware difference.

Last sentence is redundant. You are again explain repeating the diff
which is pointless, but did not explain WHY you think 2 power domains is
correct.

> 
> Signed-off-by: Swamil Jain <s-jain1@ti.com>
> ---
>  .../bindings/display/ti/ti,am65x-dss.yaml     | 33 ++++++++++++++++++-
>  1 file changed, 32 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 38fcee91211e..e74e710934fc 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> @@ -24,6 +24,19 @@ description: |
>    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.
> +  The AM62P has two instances of TI Keystone Display SubSystem, each with two
> +  video ports and two video planes. These instances can support up to 3
> +  independent video streams through OLDI, DPI, and DSI interfaces.
> +  DSS0 (first instance) supports:
> +    - Two OLDI TXes on video port 1, configurable in dual-link or
> +      single link clone mode
> +    - DPI output on video port 2
> +  DSS1 (second instance) supports:
> +    - One OLDI TX on video port 1 (single-link mode only)
> +    - DSI controller output on video port 2
> +  The two OLDI TXes can be configured in clone mode to drive a pair of
> +  identical OLDI single-link displays. DPI outputs from DSS0 VP2, DSS1 VP1,
> +  and DSS1 VP2 are muxed, allowing only one DPI output at a time.
>  
>  properties:
>    compatible:
> @@ -31,6 +44,7 @@ properties:
>        - ti,am625-dss
>        - ti,am62a7-dss
>        - ti,am62l-dss
> +      - ti,am62p-dss
>        - ti,am65x-dss
>  
>    reg:
> @@ -81,7 +95,8 @@ properties:
>      maxItems: 1
>  
>    power-domains:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3
>      description: phandle to the associated power domain
>  
>    dma-coherent: true
> @@ -196,6 +211,22 @@ allOf:
>                properties:
>                  endpoint@1: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,am62p-dss
> +    then:
> +      properties:
> +        power-domains:
> +          minItems: 1
> +          maxItems: 3

This is still not constrained enough. You need to define the items
instead. I still do not understand why number of power domains is
flexible.

> +    else:
> +      properties:
> +        power-domains:
> +          minItems: 1

You can drop this one.

> +          maxItems: 1
> +
>  required:
>    - compatible
>    - reg

Re: [PATCH v3 2/3] drm/tidss: Power up attached PM domains on probe

[Tomi Valkeinen]

Hi,

On 07/01/2026 19:45, Swamil Jain wrote:
> From: Devarsh Thakkar <devarsht@ti.com>
> 
> Some SoC's such as AM62P have dedicated power domains
> for OLDI which need to be powered on separately along
> with display controller.
> 
> So during driver probe, power up all attached PM domains
> enumerated in devicetree node for DSS.
> 
> This also prepares base to add display support for AM62P.
> 
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> [j-choudhary@ti.com: fix PM call sequence causing kernel crash in OLDI]
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Swamil Jain <s-jain1@ti.com>
> ---
>  drivers/gpu/drm/tidss/tidss_drv.c | 83 +++++++++++++++++++++++++++++--
>  drivers/gpu/drm/tidss/tidss_drv.h |  4 ++
>  2 files changed, 83 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 1c8cc18bc53c..33a10fba4325 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -8,6 +8,7 @@
>  #include <linux/of.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pm_domain.h>
>  #include <linux/aperture.h>
>  
>  #include <drm/clients/drm_client_setup.h>
> @@ -107,6 +108,68 @@ static const struct drm_driver tidss_driver = {
>  	.minor			= 0,
>  };
>  
> +static int tidss_detach_pm_domains(struct tidss_device *tidss)
> +{
> +	int i;
> +
> +	if (tidss->num_domains <= 1)
> +		return 0;
> +
> +	for (i = 0; i < tidss->num_domains; i++) {
> +		if (!IS_ERR_OR_NULL(tidss->pd_link[i]))
> +			device_link_del(tidss->pd_link[i]);
> +		if (!IS_ERR_OR_NULL(tidss->pd_dev[i]))
> +			dev_pm_domain_detach(tidss->pd_dev[i], true);
> +		tidss->pd_dev[i] = NULL;
> +		tidss->pd_link[i] = NULL;
> +	}
> +
> +	return 0;
> +}

This should be void, especially as you don't even check the return value
below.

> +
> +static int tidss_attach_pm_domains(struct tidss_device *tidss)
> +{
> +	struct device *dev = tidss->dev;
> +	int i;
> +	int ret;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct device_node *np = pdev->dev.of_node;

You should try to order these lines from longest to the shortest (not
possible for every line).

> +
> +	tidss->num_domains = of_count_phandle_with_args(np, "power-domains",
> +							"#power-domain-cells");
> +
> +	tidss->pd_dev = devm_kmalloc_array(dev, tidss->num_domains,
> +					   sizeof(*tidss->pd_dev), GFP_KERNEL);
> +	if (!tidss->pd_dev)
> +		return -ENOMEM;
> +
> +	tidss->pd_link = devm_kmalloc_array(dev, tidss->num_domains,
> +					    sizeof(*tidss->pd_link), GFP_KERNEL);
> +	if (!tidss->pd_link)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < tidss->num_domains; i++) {
> +		tidss->pd_dev[i] = dev_pm_domain_attach_by_id(dev, i);
> +		if (IS_ERR(tidss->pd_dev[i])) {
> +			ret = PTR_ERR(tidss->pd_dev[i]);
> +			goto fail;
> +		}
> +
> +		tidss->pd_link[i] = device_link_add(dev, tidss->pd_dev[i],
> +						    DL_FLAG_STATELESS |
> +						    DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
> +		if (!tidss->pd_link[i]) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +	}
> +
> +	return 0;
> +fail:
> +	tidss_detach_pm_domains(tidss);
> +	return ret;
> +}
> +
>  static int tidss_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -129,15 +192,21 @@ static int tidss_probe(struct platform_device *pdev)
>  
>  	spin_lock_init(&tidss->irq_lock);
>  
> +	ret = tidss_attach_pm_domains(tidss);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to attach power domains\n");
> +
>  	ret = dispc_init(tidss);
>  	if (ret) {
> -		dev_err(dev, "failed to initialize dispc: %d\n", ret);
> -		return ret;
> +		dev_err_probe(dev, ret, "failed to initialize dispc\n");
> +		goto err_detach_pm_domains;
>  	}
>  
>  	ret = tidss_oldi_init(tidss);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to init OLDI\n");
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to init OLDI\n");
> +		goto err_oldi_deinit;

This doesn't look right.

 Tomi


> +	}
>  
>  	pm_runtime_enable(dev);
>  
> @@ -203,8 +272,12 @@ static int tidss_probe(struct platform_device *pdev)
>  	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_disable(dev);
>  
> +err_oldi_deinit:
>  	tidss_oldi_deinit(tidss);
>  
> +err_detach_pm_domains:
> +	tidss_detach_pm_domains(tidss);
> +
>  	return ret;
>  }
>  
> @@ -232,6 +305,8 @@ static void tidss_remove(struct platform_device *pdev)
>  	/* devm allocated dispc goes away with the dev so mark it NULL */
>  	dispc_remove(tidss);
>  
> +	tidss_detach_pm_domains(tidss);
> +
>  	dev_dbg(dev, "%s done\n", __func__);
>  }
>  
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
> index e1c1f41d8b4b..6625b989b815 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -41,6 +41,10 @@ struct tidss_device {
>  	/* protects the irq masks field and irqenable/irqstatus registers */
>  	spinlock_t irq_lock;
>  	dispc_irq_t irq_mask;	/* enabled irqs */
> +
> +	int num_domains; /* Count of PM domains to be handled */
> +	struct device **pd_dev;
> +	struct device_link **pd_link;
>  };
>  
>  #define to_tidss(__dev) container_of(__dev, struct tidss_device, ddev)

Re: [PATCH v3 2/3] drm/tidss: Power up attached PM domains on probe

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Wed Jan 7, 2026 at 6:45 PM CET, Swamil Jain wrote:
> From: Devarsh Thakkar <devarsht@ti.com>
>
> Some SoC's such as AM62P have dedicated power domains
> for OLDI which need to be powered on separately along
> with display controller.
>
> So during driver probe, power up all attached PM domains
> enumerated in devicetree node for DSS.
>
> This also prepares base to add display support for AM62P.
>
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> [j-choudhary@ti.com: fix PM call sequence causing kernel crash in OLDI]
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Swamil Jain <s-jain1@ti.com>

Tested-by: Michael Walle <mwalle@kernel.org> # on am67a

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

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

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]

Hi,

On Thu Jan 8, 2026 at 9:51 AM CET, Krzysztof Kozlowski wrote:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: ti,am62p-dss
>> +    then:
>> +      properties:
>> +        power-domains:
>> +          minItems: 1
>> +          maxItems: 3
>
> This is still not constrained enough. You need to define the items
> instead. I still do not understand why number of power domains is
> flexible.

So looking at the downstream devicetree, there is one power domain
for each OLDI and for the DSS itself. Thus, in the am62p case, there
are two DSS as described above, so DSS0 has a power domain for dss0
and two power domains for the OLDI transmitters. The same for dss1
but with just one OLDI transmitter.

So I don't know why there is minItems: 1 because it's either 2 or 3.

What about the following:

..
  - if:
      properties:
        compatible:
          contains:
            const: ti,am62p-dss
    then:
      properties:
        power-domains:
          minItems: 2
          items:
            - description: DSS controller
            - description: OLDI0 transmitter
            - description: OLDI1 transmitter
    else:
      properties:
        power-domains:
          maxItems: 1

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

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

[Swamil Jain]

Hi Krzysztof,

On 1/8/26 14:21, Krzysztof Kozlowski wrote:
> On Wed, Jan 07, 2026 at 11:15:23PM +0530, Swamil Jain wrote:
>> TI's AM62P SoC contains two instances of the TI Keystone Display
>> SubSystem (DSS), each with two video ports and two video planes. These
>> instances support up to three independent video streams through OLDI,
>> DPI, and DSI interfaces.
>>
>> DSS0 (first instance) supports:
>>   - Two OLDI transmitters on video port 1, configurable in dual-link or
>>     single-link mode.
>>   - DPI output on video port 2.
>>
>> DSS1 (second instance) supports:
>>   - One OLDI transmitter on video port 1 (single-link mode only).
>>   - DSI controller output on video port 2.
>>
>> The two OLDI transmitters can be configured in clone mode to drive a
>> pair of identical OLDI single-link displays. DPI outputs from
>> DSS0 VP2, DSS1 VP1, and DSS1 VP2 are multiplexed, allowing only one
>> DPI output at a time.
>>
>> Add the compatible string "ti,am62p-dss" and update related
>> description accordingly.
>>
>> AM62P has different power domains for DSS and OLDI compared to other
>> Keystone SoCs. Therefore, add 'minItems' and set to 1 and 'maxItems'
>> field in the power-domains property to 3 for the "ti,am62p-dss"
>> compatible entry to reflect this hardware difference.
> 
> Last sentence is redundant. You are again explain repeating the diff
> which is pointless, but did not explain WHY you think 2 power domains is
> correct.
> 

Will explain properly in the commit message why 2 power domains are 
correct in v4.

>>
>> Signed-off-by: Swamil Jain <s-jain1@ti.com>
>> ---
>>   .../bindings/display/ti/ti,am65x-dss.yaml     | 33 ++++++++++++++++++-
>>   1 file changed, 32 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 38fcee91211e..e74e710934fc 100644
>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> @@ -24,6 +24,19 @@ description: |
>>     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.
>> +  The AM62P has two instances of TI Keystone Display SubSystem, each with two
>> +  video ports and two video planes. These instances can support up to 3
>> +  independent video streams through OLDI, DPI, and DSI interfaces.
>> +  DSS0 (first instance) supports:
>> +    - Two OLDI TXes on video port 1, configurable in dual-link or
>> +      single link clone mode
>> +    - DPI output on video port 2
>> +  DSS1 (second instance) supports:
>> +    - One OLDI TX on video port 1 (single-link mode only)
>> +    - DSI controller output on video port 2
>> +  The two OLDI TXes can be configured in clone mode to drive a pair of
>> +  identical OLDI single-link displays. DPI outputs from DSS0 VP2, DSS1 VP1,
>> +  and DSS1 VP2 are muxed, allowing only one DPI output at a time.
>>   
>>   properties:
>>     compatible:
>> @@ -31,6 +44,7 @@ properties:
>>         - ti,am625-dss
>>         - ti,am62a7-dss
>>         - ti,am62l-dss
>> +      - ti,am62p-dss
>>         - ti,am65x-dss
>>   
>>     reg:
>> @@ -81,7 +95,8 @@ properties:
>>       maxItems: 1
>>   
>>     power-domains:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
>>       description: phandle to the associated power domain
>>   
>>     dma-coherent: true
>> @@ -196,6 +211,22 @@ allOf:
>>                 properties:
>>                   endpoint@1: false
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: ti,am62p-dss
>> +    then:
>> +      properties:
>> +        power-domains:
>> +          minItems: 1
>> +          maxItems: 3
> 
> This is still not constrained enough. You need to define the items
> instead. I still do not understand why number of power domains is
> flexible.

Planning to add:
```
power-domains:
   minItems: 1
   description:
     phandle to the associated power domain(s).
   items:
     - description: DSS controller power domain
     - description: OLDI0 power domain
     - description: OLDI1 power domain
```

There can be up to 3 power-domains in a DSS instance on AM62P SoC.
Please check the Technical Reference Manual for AM62P SoC[0].
On page 542 it is mentioned LPSC_main_dss0 has a partial dependence
on LPSC_main_oldi0 and LPSC_main_oldi1, and, similarly for
LPSC_main_dss1 there is a partial dependence on LPSC_main_oldi1.
This mean if you are only enabling DSS0 Video port 1 for HDMI output
only you need not mention other power-domains and similarly for DSS1
if you need OLDI1 output you need to use DSS1 and OLDI1 power-domains.
So, we can use up to 3 power-domains depending on the use-case.

[0]: https://www.ti.com/lit/ug/spruj83c/spruj83c.pdf
> 
>> +    else:
>> +      properties:
>> +        power-domains:
>> +          minItems: 1
> 
> You can drop this one.
> 

Yeah, sure will drop this in v4.

>> +          maxItems: 1
>> +
>>   required:
>>     - compatible
>>     - reg

Regards,
Swamil.

Re: [PATCH v3 2/3] drm/tidss: Power up attached PM domains on probe

[Swamil Jain]

Hi Tomi,

On 1/9/26 19:48, Tomi Valkeinen wrote:
> Hi,
> 
> On 07/01/2026 19:45, Swamil Jain wrote:
>> From: Devarsh Thakkar <devarsht@ti.com>
>>
>> Some SoC's such as AM62P have dedicated power domains
>> for OLDI which need to be powered on separately along
>> with display controller.
>>
>> So during driver probe, power up all attached PM domains
>> enumerated in devicetree node for DSS.
>>
>> This also prepares base to add display support for AM62P.
>>
>> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
>> [j-choudhary@ti.com: fix PM call sequence causing kernel crash in OLDI]
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> Signed-off-by: Swamil Jain <s-jain1@ti.com>
>> ---
>>   drivers/gpu/drm/tidss/tidss_drv.c | 83 +++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/tidss/tidss_drv.h |  4 ++
>>   2 files changed, 83 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
>> index 1c8cc18bc53c..33a10fba4325 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.c
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/of.h>
>>   #include <linux/module.h>
>>   #include <linux/pm_runtime.h>
>> +#include <linux/pm_domain.h>
>>   #include <linux/aperture.h>
>>   
>>   #include <drm/clients/drm_client_setup.h>
>> @@ -107,6 +108,68 @@ static const struct drm_driver tidss_driver = {
>>   	.minor			= 0,
>>   };
>>   
>> +static int tidss_detach_pm_domains(struct tidss_device *tidss)
>> +{
>> +	int i;
>> +
>> +	if (tidss->num_domains <= 1)
>> +		return 0;
>> +
>> +	for (i = 0; i < tidss->num_domains; i++) {
>> +		if (!IS_ERR_OR_NULL(tidss->pd_link[i]))
>> +			device_link_del(tidss->pd_link[i]);
>> +		if (!IS_ERR_OR_NULL(tidss->pd_dev[i]))
>> +			dev_pm_domain_detach(tidss->pd_dev[i], true);
>> +		tidss->pd_dev[i] = NULL;
>> +		tidss->pd_link[i] = NULL;
>> +	}
>> +
>> +	return 0;
>> +}
> 
> This should be void, especially as you don't even check the return value
> below.

Sure, will make it void in v4.

> 
>> +
>> +static int tidss_attach_pm_domains(struct tidss_device *tidss)
>> +{
>> +	struct device *dev = tidss->dev;
>> +	int i;
>> +	int ret;
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct device_node *np = pdev->dev.of_node;
> 
> You should try to order these lines from longest to the shortest (not
> possible for every line).

Acked. But, here we are using pdev, so, can't re-order here.

> 
>> +
>> +	tidss->num_domains = of_count_phandle_with_args(np, "power-domains",
>> +							"#power-domain-cells");
>> +
>> +	tidss->pd_dev = devm_kmalloc_array(dev, tidss->num_domains,
>> +					   sizeof(*tidss->pd_dev), GFP_KERNEL);
>> +	if (!tidss->pd_dev)
>> +		return -ENOMEM;
>> +
>> +	tidss->pd_link = devm_kmalloc_array(dev, tidss->num_domains,
>> +					    sizeof(*tidss->pd_link), GFP_KERNEL);
>> +	if (!tidss->pd_link)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < tidss->num_domains; i++) {
>> +		tidss->pd_dev[i] = dev_pm_domain_attach_by_id(dev, i);
>> +		if (IS_ERR(tidss->pd_dev[i])) {
>> +			ret = PTR_ERR(tidss->pd_dev[i]);
>> +			goto fail;
>> +		}
>> +
>> +		tidss->pd_link[i] = device_link_add(dev, tidss->pd_dev[i],
>> +						    DL_FLAG_STATELESS |
>> +						    DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
>> +		if (!tidss->pd_link[i]) {
>> +			ret = -EINVAL;
>> +			goto fail;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +fail:
>> +	tidss_detach_pm_domains(tidss);
>> +	return ret;
>> +}
>> +
>>   static int tidss_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>> @@ -129,15 +192,21 @@ static int tidss_probe(struct platform_device *pdev)
>>   
>>   	spin_lock_init(&tidss->irq_lock);
>>   
>> +	ret = tidss_attach_pm_domains(tidss);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "failed to attach power domains\n");
>> +
>>   	ret = dispc_init(tidss);
>>   	if (ret) {
>> -		dev_err(dev, "failed to initialize dispc: %d\n", ret);
>> -		return ret;
>> +		dev_err_probe(dev, ret, "failed to initialize dispc\n");
>> +		goto err_detach_pm_domains;
>>   	}
>>   
>>   	ret = tidss_oldi_init(tidss);
>> -	if (ret)
>> -		return dev_err_probe(dev, ret, "failed to init OLDI\n");
>> +	if (ret) {
>> +		dev_err_probe(dev, ret, "failed to init OLDI\n");
>> +		goto err_oldi_deinit;
> 
> This doesn't look right.
> 

Will use err_detach_pm_domains in v4 and will take oldi_deinit() issue 
separately as tidss_oldi_init() is not doing own cleanup if it fails.

Regards,
Swamil.

>   Tomi
> 
> 
>> +	}
>>   
>>   	pm_runtime_enable(dev);
>>   
>> @@ -203,8 +272,12 @@ static int tidss_probe(struct platform_device *pdev)
>>   	pm_runtime_dont_use_autosuspend(dev);
>>   	pm_runtime_disable(dev);
>>   
>> +err_oldi_deinit:
>>   	tidss_oldi_deinit(tidss);
>>   
>> +err_detach_pm_domains:
>> +	tidss_detach_pm_domains(tidss);
>> +
>>   	return ret;
>>   }
>>   
>> @@ -232,6 +305,8 @@ static void tidss_remove(struct platform_device *pdev)
>>   	/* devm allocated dispc goes away with the dev so mark it NULL */
>>   	dispc_remove(tidss);
>>   
>> +	tidss_detach_pm_domains(tidss);
>> +
>>   	dev_dbg(dev, "%s done\n", __func__);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
>> index e1c1f41d8b4b..6625b989b815 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.h
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
>> @@ -41,6 +41,10 @@ struct tidss_device {
>>   	/* protects the irq masks field and irqenable/irqstatus registers */
>>   	spinlock_t irq_lock;
>>   	dispc_irq_t irq_mask;	/* enabled irqs */
>> +
>> +	int num_domains; /* Count of PM domains to be handled */
>> +	struct device **pd_dev;
>> +	struct device_link **pd_link;
>>   };
>>   
>>   #define to_tidss(__dev) container_of(__dev, struct tidss_device, ddev)
> 

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

[Swamil Jain]

Hi Michael,

On 1/14/26 16:11, Michael Walle wrote:
> Hi,
> 
> On Thu Jan 8, 2026 at 9:51 AM CET, Krzysztof Kozlowski wrote:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: ti,am62p-dss
>>> +    then:
>>> +      properties:
>>> +        power-domains:
>>> +          minItems: 1
>>> +          maxItems: 3
>>
>> This is still not constrained enough. You need to define the items
>> instead. I still do not understand why number of power domains is
>> flexible.
> 
> So looking at the downstream devicetree, there is one power domain
> for each OLDI and for the DSS itself. Thus, in the am62p case, there
> are two DSS as described above, so DSS0 has a power domain for dss0
> and two power domains for the OLDI transmitters. The same for dss1
> but with just one OLDI transmitter.
> 
> So I don't know why there is minItems: 1 because it's either 2 or 3.

One can use DSS power-domain only, if they don't want to use OLDI0 or OLDI1.

> 
> What about the following:
> 
> ..
>    - if:
>        properties:
>          compatible:
>            contains:
>              const: ti,am62p-dss
>      then:
>        properties:
>          power-domains:
>            minItems: 2
>            items:
>              - description: DSS controller
>              - description: OLDI0 transmitter
>              - description: OLDI1 transmitter
>      else:
>        properties:
>          power-domains:
>            maxItems: 1
> 
> -michael

Using the following:
..
   power-domains:
     minItems: 1
     description:
       phandle to the associated power domain(s).
     items:
       - description: DSS controller power domain
       - description: OLDI0 power domain
       - description: OLDI1 power domain

And using constraints for "ti,am62p-dss" and other compatibles 
separately. Please check v4[1].

[1]: https://lore.kernel.org/all/20260116095406.2544565-2-s-jain1@ti.com/

Regards,
Swamil.

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

[Krzysztof Kozlowski]

On 15/01/2026 17:54, Swamil Jain wrote:
> Hi Krzysztof,
> 
> On 1/8/26 14:21, Krzysztof Kozlowski wrote:
>> On Wed, Jan 07, 2026 at 11:15:23PM +0530, Swamil Jain wrote:
>>> TI's AM62P SoC contains two instances of the TI Keystone Display
>>> SubSystem (DSS), each with two video ports and two video planes. These
>>> instances support up to three independent video streams through OLDI,
>>> DPI, and DSI interfaces.
>>>
>>> DSS0 (first instance) supports:
>>>   - Two OLDI transmitters on video port 1, configurable in dual-link or
>>>     single-link mode.
>>>   - DPI output on video port 2.
>>>
>>> DSS1 (second instance) supports:
>>>   - One OLDI transmitter on video port 1 (single-link mode only).
>>>   - DSI controller output on video port 2.
>>>
>>> The two OLDI transmitters can be configured in clone mode to drive a
>>> pair of identical OLDI single-link displays. DPI outputs from
>>> DSS0 VP2, DSS1 VP1, and DSS1 VP2 are multiplexed, allowing only one
>>> DPI output at a time.
>>>
>>> Add the compatible string "ti,am62p-dss" and update related
>>> description accordingly.
>>>
>>> AM62P has different power domains for DSS and OLDI compared to other
>>> Keystone SoCs. Therefore, add 'minItems' and set to 1 and 'maxItems'
>>> field in the power-domains property to 3 for the "ti,am62p-dss"
>>> compatible entry to reflect this hardware difference.
>>
>> Last sentence is redundant. You are again explain repeating the diff
>> which is pointless, but did not explain WHY you think 2 power domains is
>> correct.
>>
> 
> Will explain properly in the commit message why 2 power domains are 
> correct in v4.
> 
>>>
>>> Signed-off-by: Swamil Jain <s-jain1@ti.com>
>>> ---
>>>   .../bindings/display/ti/ti,am65x-dss.yaml     | 33 ++++++++++++++++++-
>>>   1 file changed, 32 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 38fcee91211e..e74e710934fc 100644
>>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>>> @@ -24,6 +24,19 @@ description: |
>>>     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.
>>> +  The AM62P has two instances of TI Keystone Display SubSystem, each with two
>>> +  video ports and two video planes. These instances can support up to 3
>>> +  independent video streams through OLDI, DPI, and DSI interfaces.
>>> +  DSS0 (first instance) supports:
>>> +    - Two OLDI TXes on video port 1, configurable in dual-link or
>>> +      single link clone mode
>>> +    - DPI output on video port 2
>>> +  DSS1 (second instance) supports:
>>> +    - One OLDI TX on video port 1 (single-link mode only)
>>> +    - DSI controller output on video port 2
>>> +  The two OLDI TXes can be configured in clone mode to drive a pair of
>>> +  identical OLDI single-link displays. DPI outputs from DSS0 VP2, DSS1 VP1,
>>> +  and DSS1 VP2 are muxed, allowing only one DPI output at a time.
>>>   
>>>   properties:
>>>     compatible:
>>> @@ -31,6 +44,7 @@ properties:
>>>         - ti,am625-dss
>>>         - ti,am62a7-dss
>>>         - ti,am62l-dss
>>> +      - ti,am62p-dss
>>>         - ti,am65x-dss
>>>   
>>>     reg:
>>> @@ -81,7 +95,8 @@ properties:
>>>       maxItems: 1
>>>   
>>>     power-domains:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 3
>>>       description: phandle to the associated power domain
>>>   
>>>     dma-coherent: true
>>> @@ -196,6 +211,22 @@ allOf:
>>>                 properties:
>>>                   endpoint@1: false
>>>   
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: ti,am62p-dss
>>> +    then:
>>> +      properties:
>>> +        power-domains:
>>> +          minItems: 1
>>> +          maxItems: 3
>>
>> This is still not constrained enough. You need to define the items
>> instead. I still do not understand why number of power domains is
>> flexible.
> 
> Planning to add:
> ```
> power-domains:
>    minItems: 1
>    description:
>      phandle to the associated power domain(s).
>    items:
>      - description: DSS controller power domain
>      - description: OLDI0 power domain
>      - description: OLDI1 power domain
> ```
> 
> There can be up to 3 power-domains in a DSS instance on AM62P SoC.
> Please check the Technical Reference Manual for AM62P SoC[0].
> On page 542 it is mentioned LPSC_main_dss0 has a partial dependence
> on LPSC_main_oldi0 and LPSC_main_oldi1, and, similarly for
> LPSC_main_dss1 there is a partial dependence on LPSC_main_oldi1.
> This mean if you are only enabling DSS0 Video port 1 for HDMI output
> only you need not mention other power-domains and similarly for DSS1
> if you need OLDI1 output you need to use DSS1 and OLDI1 power-domains.
> So, we can use up to 3 power-domains depending on the use-case.

Still wrong. The block is still part of three power domains. They are
always there even if other connectors are disconnected.

And if some other than HDMI things are disconnected, then the outputs
remain off thus driver will turn off also the power domains.

Best regards,
Krzysztof