[PATCH] drm: rz-du: Ensure correct suspend/resume ordering with VSP

[PATCH] drm: rz-du: Ensure correct suspend/resume ordering with VSP

[Tommaso Merciai]

The VSP serves as an interface to memory and a compositor to the DU. It
therefore needs to be suspended after and resumed before the DU, to be
properly stopped and restarted in a controlled fashion driven by the DU
driver. This currently works by chance. Avoid relying on luck by
enforcing the correct suspend/resume ordering with device links.

Based on similar work done by Laurent Pinchart for R-Car DU.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c | 16 ++++++++++++++++
 drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
index bd486377f037..eb626c3cc421 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
@@ -20,6 +20,7 @@
 #include <drm/drm_vblank.h>
 
 #include <linux/bitops.h>
+#include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
@@ -293,6 +294,9 @@ static void rzg2l_du_vsp_cleanup(struct drm_device *dev, void *res)
 {
 	struct rzg2l_du_vsp *vsp = res;
 
+	if (vsp->link)
+		device_link_del(vsp->link);
+
 	put_device(vsp->vsp);
 }
 
@@ -317,6 +321,18 @@ int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node *np,
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * Enforce suspend/resume ordering between the DU (consumer) and the
+	 * VSP (supplier). The DU will be suspended before and resume after the
+	 * VSP.
+	 */
+	vsp->link = device_link_add(rcdu->dev, vsp->vsp, DL_FLAG_STATELESS);
+	if (!vsp->link) {
+		dev_err(rcdu->dev, "Failed to create device link to VSP %s\n",
+			dev_name(vsp->vsp));
+		return -EINVAL;
+	}
+
 	ret = vsp1_du_init(vsp->vsp);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
index 322eb80dcbaf..a22aaf0843ed 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
@@ -15,6 +15,7 @@
 #include <linux/scatterlist.h>
 
 struct device;
+struct device_link;
 struct drm_framebuffer;
 struct rzg2l_du_device;
 struct rzg2l_du_format_info;
@@ -29,6 +30,7 @@ struct rzg2l_du_vsp_plane {
 struct rzg2l_du_vsp {
 	unsigned int index;
 	struct device *vsp;
+	struct device_link *link;
 	struct rzg2l_du_device *dev;
 };
 
-- 
2.43.0

Re: [PATCH] drm: rz-du: Ensure correct suspend/resume ordering with VSP

[Laurent Pinchart]

On Tue, Mar 24, 2026 at 07:05:05PM +0100, Tommaso Merciai wrote:
> The VSP serves as an interface to memory and a compositor to the DU. It
> therefore needs to be suspended after and resumed before the DU, to be
> properly stopped and restarted in a controlled fashion driven by the DU
> driver. This currently works by chance. Avoid relying on luck by
> enforcing the correct suspend/resume ordering with device links.
> 
> Based on similar work done by Laurent Pinchart for R-Car DU.

If the R-Car DU patch gets merged before this one, you could mention the
commit ID here.

> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c | 16 ++++++++++++++++
>  drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> index bd486377f037..eb626c3cc421 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> @@ -20,6 +20,7 @@
>  #include <drm/drm_vblank.h>
>  
>  #include <linux/bitops.h>
> +#include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> @@ -293,6 +294,9 @@ static void rzg2l_du_vsp_cleanup(struct drm_device *dev, void *res)
>  {
>  	struct rzg2l_du_vsp *vsp = res;
>  
> +	if (vsp->link)
> +		device_link_del(vsp->link);
> +
>  	put_device(vsp->vsp);
>  }
>  
> @@ -317,6 +321,18 @@ int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node *np,
>  	if (ret < 0)
>  		return ret;
>  
> +	/*
> +	 * Enforce suspend/resume ordering between the DU (consumer) and the
> +	 * VSP (supplier). The DU will be suspended before and resume after the
> +	 * VSP.
> +	 */
> +	vsp->link = device_link_add(rcdu->dev, vsp->vsp, DL_FLAG_STATELESS);
> +	if (!vsp->link) {
> +		dev_err(rcdu->dev, "Failed to create device link to VSP %s\n",
> +			dev_name(vsp->vsp));
> +		return -EINVAL;
> +	}
> +
>  	ret = vsp1_du_init(vsp->vsp);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
> index 322eb80dcbaf..a22aaf0843ed 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h
> @@ -15,6 +15,7 @@
>  #include <linux/scatterlist.h>
>  
>  struct device;
> +struct device_link;
>  struct drm_framebuffer;
>  struct rzg2l_du_device;
>  struct rzg2l_du_format_info;
> @@ -29,6 +30,7 @@ struct rzg2l_du_vsp_plane {
>  struct rzg2l_du_vsp {
>  	unsigned int index;
>  	struct device *vsp;
> +	struct device_link *link;
>  	struct rzg2l_du_device *dev;
>  };
>  

-- 
Regards,

Laurent Pinchart

Claude review: drm: rz-du: Ensure correct suspend/resume ordering with VSP

[Claude Code Review Bot]

Overall Series Review

Subject: drm: rz-du: Ensure correct suspend/resume ordering with VSP
Author: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Patches: 2
Reviewed: 2026-03-25T06:34:36.400945

---

This is a straightforward, single-patch series that adds a `DL_FLAG_STATELESS` device link between the RZ/G2L DU (consumer) and its VSP (supplier) to enforce correct suspend/resume ordering. The approach mirrors what was already done for R-Car DU's CMM device links in `rcar_du_kms.c`. The patch is clean, small, and well-motivated.

One issue worth raising is around the cleanup ordering and error handling.

---
Generated by Claude Code Patch Reviewer

Claude review: drm: rz-du: Ensure correct suspend/resume ordering with VSP

[Claude Code Review Bot]

Patch Review

**Correctness of the device link direction:** The call `device_link_add(rcdu->dev, vsp->vsp, DL_FLAG_STATELESS)` correctly makes the DU the consumer and the VSP the supplier. With `DL_FLAG_STATELESS`, the PM core ensures the supplier (VSP) is resumed before and suspended after the consumer (DU). This matches the commit message and comment.

**Cleanup concern — resource leak on `vsp1_du_init()` failure:**
The device link is created *before* calling `vsp1_du_init()`:

```c
+	vsp->link = device_link_add(rcdu->dev, vsp->vsp, DL_FLAG_STATELESS);
+	if (!vsp->link) {
+		dev_err(rcdu->dev, "Failed to create device link to VSP %s\n",
+			dev_name(vsp->vsp));
+		return -EINVAL;
+	}
+
 	ret = vsp1_du_init(vsp->vsp);
 	if (ret < 0)
 		return ret;
```

If `vsp1_du_init()` fails, the function returns an error. The cleanup action `rzg2l_du_vsp_cleanup` (registered via `drmm_add_action_or_reset` earlier) will eventually run and correctly delete the link since the patch adds the `device_link_del(vsp->link)` call there. So this is actually fine — the `drmm` cleanup handles it.

**Minor style observation:** The R-Car DU CMM code uses a local `struct device_link *link` variable rather than storing it in a struct, because the CMM links are never explicitly deleted (they live for the lifetime of the device). Here, you store `vsp->link` and explicitly delete it in cleanup, which is a reasonable design choice given the `drmm` managed lifecycle. However, with `DL_FLAG_STATELESS` device links, the link is automatically cleaned up when either the consumer or supplier device is unregistered. Since the DU device is the consumer and owns the lifecycle, the explicit `device_link_del` in cleanup is arguably redundant — but it's also harmless and makes the intent clear. No change needed.

**Include ordering:** `#include <linux/device.h>` is added in alphabetical order — correct.

**Header forward declaration:** `struct device_link;` is added in the correct alphabetical position in the header — correct.

**Struct field placement:** `struct device_link *link` is added after `struct device *vsp` which is a logical grouping — fine.

**Overall:** The patch is correct and clean. No blocking issues. It would be worth confirming with Laurent Pinchart whether he also planned to port the R-Car DU VSP device link (not just CMM) to the rz-du driver, since the R-Car DU VSP code itself doesn't appear to use device links currently — this patch adds functionality that goes beyond what the R-Car DU VSP has.

---
Generated by Claude Code Patch Reviewer