Re: [PATCH v3 12/15] media: vsp1: Fix setting control register by display list

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Oct 15, 2026 at 04:51:52PM +0900, Paul Elder wrote:
> From: Phong Hoang <phong.hoang.wz@renesas.com>
> 
> According to R-Car Hardware User's Manual, VSP HW does not
> support to download register settings for the control registers
> by display list, so set those registers by APB interface instead.
> 
> This limitation is especially enforced within the VDK.
> 
> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> index 821887815eb2e38d3a51791161de8cd3c135ef3c..4a5ad8e4903e99d5a522af2239889d7899f629cd 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> @@ -332,9 +332,8 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
>  	vsp1_wpf_write(wpf, dlb, VI6_WPF_SRCRPF, srcrpf);
>  
>  	/* Enable interrupts. */
> -	vsp1_dl_body_write(dlb, VI6_WPF_IRQ_STA(index), 0);
> -	vsp1_dl_body_write(dlb, VI6_WPF_IRQ_ENB(index),
> -			   VI6_WPF_IRQ_ENB_DFEE);
> +	vsp1_write(vsp1, VI6_WPF_IRQ_STA(index), 0);
> +	vsp1_write(vsp1, VI6_WPF_IRQ_ENB(index), VI6_WPF_IRQ_ENB_DFEE);

Unfortunately I think this will cause a problem. For the V4L2 and DRM
clients, all should be fine. The configure_stream operation is called
after starting streaming, which calls vsp1_device_get() to
runtime-resume the device. For the ISP client, however, I think this
function can be called from the .buf_queue() handler before the start
streaming operation, as vb2 enqueues buffer in the driver before calling
.start_streaming() on the queue. We would then try to write the register
while the device is suspended, which won't lead to any good outcome.

>  
>  	if (pipe->iif)
>  		return;

-- 
Regards,

Laurent Pinchart