Re: [PATCH 2/6] media: vsp1: Enable FREE interrupt

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

Hi Jacopo,

Thank you for the patch.

s/FREE/FRE/ in the subject line, as FREE stands for "FRameEnd Enable",
so the interrupt is FRE.

On Thu, Jan 23, 2025 at 06:04:03PM +0100, Jacopo Mondi wrote:
> Enable the "FrameEnd" interrupt to support the VSPX operations.
> 
> The frame completion interrupt signals that the transfer of
> the config buffer and image buffer to the ISP has completed.
> 
> Enable the interrupt source if the pipe has an IIF entity, such as
> in the VSPX case.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_drv.c | 3 ++-
>  drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index cbaad0ea0b73f90f3994bbdfb4304d2f71eabccd..5aa0751a896f8a58bd11128ccaa092c9596cdb5d 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -69,7 +69,8 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
>  				i, wpf->entity.pipe->underrun_count);
>  		}
>  
> -		if (status & VI6_WPF_IRQ_STA_DFE) {
> +		if (status & VI6_WPF_IRQ_STA_DFE ||
> +		    status & VI6_WPF_IRQ_STA_FRE) {
>  			vsp1_pipeline_frame_end(wpf->entity.pipe);
>  			ret = IRQ_HANDLED;
>  		}
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> index f176750ccd9847fdb8d51f7f51a6bd5092b70197..93a663f58a5930a3c7c40a96a30888d0b8ccb2ed 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> @@ -239,6 +239,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
>  	const struct v4l2_mbus_framefmt *source_format;
>  	const struct v4l2_mbus_framefmt *sink_format;
>  	unsigned int index = wpf->entity.index;
> +	u32 irqmask = 0;

No need to initialize this to 0.

>  	unsigned int i;
>  	u32 outfmt = 0;
>  	u32 srcrpf = 0;
> @@ -312,9 +313,9 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
>  	vsp1_wpf_write(wpf, dlb, VI6_WPF_SRCRPF, srcrpf);
>  
>  	/* Enable interrupts. */
> +	irqmask = VI6_WPF_IRQ_ENB_DFEE | (pipe->iif ? VI6_WPF_IRQ_ENB_FREE : 0);

Do we need te DFE interrupt for VSPX ? If both DFE and FRE fire,
vsp1_pipeline_frame_end() will be called twice per frame, and that
doesn't sound like what you need. If DFE is not generated, I'd rather
not enable the interrupt, to be on the safe side.

Another option, which may be better, is to use the DFE interrupt. As far
as I understand, generation of the DFE interrupt is controlled by a bit
in the display list. Is there a reason you can't use that and need FRE ?

>  	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_dl_body_write(dlb, VI6_WPF_IRQ_ENB(index), irqmask);
>  
>  	/*
>  	 * Configure writeback for display pipelines (the wpf writeback flag is
> 

-- 
Regards,

Laurent Pinchart