Skip to contentSkip to conversation

drm/bridge: cdns-dsi: Replace deprecated UNIVERSAL_DEV_PM_OPS()

PATCH Superseded Vitor Soares 3 participants 7 messages May 12, 2025 – Jan 12

Subsystems (4)
Stable branch DRM drivers DRM misc DRM drivers for bridge chips

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Vitor Soares

author
#0 lore raw

PATCH v3 1/1 drm/bridge: cdns-dsi: Replace deprecated UNIVERSAL_DEV_PM_OPS()

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

---
v2 -> v3
- Fix warning: 'cdns_dsi_suspend' defined but not used [-Wunused-function]
- Fix warning: 'cdns_dsi_resume' defined but not used [-Wunused-function]

v1 -> v2
- Rely only on SET_RUNTIME_PM_OPS() for the PM.

 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +6 -5
index b022dd6e6b6e..6429d541889c 100644
@@ -1258,7 +1258,7 @@ static const struct mipi_dsi_host_ops cdns_dsi_ops = {
 	.transfer = cdns_dsi_transfer,
 };
 
-static int __maybe_unused cdns_dsi_resume(struct device *dev)
+static int cdns_dsi_resume(struct device *dev)
 {
 	struct cdns_dsi *dsi = dev_get_drvdata(dev);
 

@@ -1269,7 +1269,7 @@ static int __maybe_unused cdns_dsi_resume(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused cdns_dsi_suspend(struct device *dev)
+static int cdns_dsi_suspend(struct device *dev)
 {
 	struct cdns_dsi *dsi = dev_get_drvdata(dev);
 

@@ -1279,8 +1279,9 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev)
 	return 0;
 }
 
-static UNIVERSAL_DEV_PM_OPS(cdns_dsi_pm_ops, cdns_dsi_suspend, cdns_dsi_resume,
-			    NULL);
+static const struct dev_pm_ops cdns_dsi_pm_ops = {
+	RUNTIME_PM_OPS(cdns_dsi_suspend, cdns_dsi_resume, NULL)
+};
 
 static int cdns_dsi_drm_probe(struct platform_device *pdev)
 {

@@ -1427,7 +1428,7 @@ static struct platform_driver cdns_dsi_platform_driver = {
 	.driver = {
 		.name   = "cdns-dsi",
 		.of_match_table = cdns_dsi_of_match,
-		.pm = &cdns_dsi_pm_ops,
+		.pm = pm_ptr(&cdns_dsi_pm_ops),
 	},
 };
 module_platform_driver(cdns_dsi_platform_driver);
Signature · 1 line
2.34.1
Reviewed-by on v3 1/1
#1 lore raw
Quoted 78 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

---
v2 -> v3
- Fix warning: 'cdns_dsi_suspend' defined but not used [-Wunused-function]
- Fix warning: 'cdns_dsi_resume' defined but not used [-Wunused-function]

v1 -> v2
- Rely only on SET_RUNTIME_PM_OPS() for the PM.

  drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 11 ++++++-----
  1 file changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +5 -4
index b022dd6e6b6e..6429d541889c 100644
@@ -1258,7 +1258,7 @@ static const struct mipi_dsi_host_ops cdns_dsi_ops = {
  	.transfer = cdns_dsi_transfer,
  };
 
-static int __maybe_unused cdns_dsi_resume(struct device *dev)
+static int cdns_dsi_resume(struct device *dev)
  {
  	struct cdns_dsi *dsi = dev_get_drvdata(dev);
 

@@ -1269,7 +1269,7 @@ static int __maybe_unused cdns_dsi_resume(struct device *dev)
  	return 0;
  }
 
-static int __maybe_unused cdns_dsi_suspend(struct device *dev)
+static int cdns_dsi_suspend(struct device *dev)
  {
  	struct cdns_dsi *dsi = dev_get_drvdata(dev);
 

@@ -1279,8 +1279,9 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev)
  	return 0;
  }
 
-static UNIVERSAL_DEV_PM_OPS(cdns_dsi_pm_ops, cdns_dsi_suspend, cdns_dsi_resume,
-			    NULL);
+static const struct dev_pm_ops cdns_dsi_pm_ops = {
+	RUNTIME_PM_OPS(cdns_dsi_suspend, cdns_dsi_resume, NULL)
+};
 
  static int cdns_dsi_drm_probe(struct platform_device *pdev)
  {

drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c static struct platform_driver cdns_dsi_platform_driver = { · line 1434

Quoted 8 lines
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +1 -1
@@ -1427,7 +1428,7 @@ static struct platform_driver cdns_dsi_platform_driver = {
  	.driver = {
  		.name   = "cdns-dsi",
  		.of_match_table = cdns_dsi_of_match,
-		.pm = &cdns_dsi_pm_ops,
+		.pm = pm_ptr(&cdns_dsi_pm_ops),
  	},
  };
  module_platform_driver(cdns_dsi_platform_driver);
Reviewed-by Tomi Valkeinen <[email protected]>

Tomi

#2 lore raw

Hello,

Quoted 25 lines · Tomi Valkeinen
Quoted 24 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368
Quoted 8 lines
Quoted 8 lines

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

ping on this, Tomi, maybe you can pick this one or is there any concern ?

Francesco

#3 lore raw

Hi,

Quoted 33 lines · Francesco Dolcini

Hello,

Quoted 30 lines · Tomi Valkeinen
Quoted 29 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

ping on this, Tomi, maybe you can pick this one or is there any concern ?

No concern from my side, but I'm not a bridge or cdns-dsi maintainer, so I don't pick any of these to drm-misc by default.

Aaand now as I wrote that, I realized I just some time ago pushed the cdns-dsi series ("drm/bridge: cdns-dsi: Make it work a bit better") to drm-misc without bridge maintainer's ack... So that didn't go according to the rules, sorry.

Tomi

#4 lore raw
Quoted 36 lines · Tomi Valkeinen
Quoted 35 lines · Francesco Dolcini
Quoted 33 lines · Tomi Valkeinen
Quoted 32 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>
Quoted 8 lines
Quoted 2 lines

ping on this, Tomi, maybe you can pick this one or is there any concern ?

No concern from my side, but I'm not a bridge or cdns-dsi maintainer, so I don't pick any of these to drm-misc by default.

Aaand now as I wrote that, I realized I just some time ago pushed the cdns-dsi series ("drm/bridge: cdns-dsi: Make it work a bit better") to

Yep, that's the reason I assumed you could also pick this one. Any DRM bridge maintainer can help here?

Thanks,
Francesco

#5 lore raw

Hello all,

Quoted 40 lines · Francesco Dolcini
Quoted 39 lines · Tomi Valkeinen
Quoted 37 lines · Francesco Dolcini
Quoted 25 lines · Tomi Valkeinen
Quoted 24 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368
Quoted 8 lines
Quoted 8 lines

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

ping on this, Tomi, maybe you can pick this one or is there any concern ?

Quoted 8 lines
Quoted 5 lines

No concern from my side, but I'm not a bridge or cdns-dsi maintainer, so I don't pick any of these to drm-misc by default.

Aaand now as I wrote that, I realized I just some time ago pushed the cdns-dsi series ("drm/bridge: cdns-dsi: Make it work a bit better") to

Yep, that's the reason I assumed you could also pick this one. Any DRM bridge maintainer can help here?

Another ping here ... I am not sure on how to move this forward.

Andrzej? Neil? Robert?

Considering that this is a bug fix, the patch is reviewed since long and it was sent 4 months ago would be fine that you merge it Tomi?

Francesco

#6 lore raw
In reply to Vitor Soares

Hello Maarten, Maxime, Thomas, Simona, David,

Quoted 24 lines · Vitor Soares

From: Vitor Soares <[email protected]>

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks for both runtime PM and system sleep. This causes the DSI clocks to be disabled twice: once during runtime suspend and again during system suspend, resulting in a WARN message from the clock framework when attempting to disable already-disabled clocks.

[   84.384540] clk:231:5 already disabled
[   84.388314] WARNING: CPU: 2 PID: 531 at /drivers/clk/clk.c:1181 clk_core_disable+0xa4/0xac
...
[   84.579183] Call trace:
[   84.581624]  clk_core_disable+0xa4/0xac
[   84.585457]  clk_disable+0x30/0x4c
[   84.588857]  cdns_dsi_suspend+0x20/0x58 [cdns_dsi]
[   84.593651]  pm_generic_suspend+0x2c/0x44
[   84.597661]  ti_sci_pd_suspend+0xbc/0x15c
[   84.601670]  dpm_run_callback+0x8c/0x14c
[   84.605588]  __device_suspend+0x1a0/0x56c
[   84.609594]  dpm_suspend+0x17c/0x21c
[   84.613165]  dpm_suspend_start+0xa0/0xa8
[   84.617083]  suspend_devices_and_enter+0x12c/0x634
[   84.621872]  pm_suspend+0x1fc/0x368
Quoted 8 lines

To address this issue, replace UNIVERSAL_DEV_PM_OPS() with SET_RUNTIME_PM_OPS(), enabling suspend/resume handling through the _enable()/_disable() hooks managed by the DRM framework for both runtime and system-wide PM.

Cc <[email protected]> # 6.1.x
Fixes e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by Vitor Soares <[email protected]>

I am a little bit stuck on what is the best way to have this patch moving forward.

Who can help on this?
Can any of you guide me a little bit?

Francesco