Re: [PATCH] Revert "media: vsp1: Add underrun debug print"

[Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>]

Quoting Tomi Valkeinen (2025-09-10 08:26:43)
> This reverts commit 1dc30075fb0fe02b74b1ea7fd1c1c734a89f1448.
> 
> There have been reports of lots of underruns happening on earlier
> generation SoCs (M3, E3) with display use cases, e.g.:
> 
> vsp1 fea28000.vsp: Underrun occurred at WPF0 (total underruns 1)
> 
> but the display still working fine, and reverting the above commit,
> which added underrun prints, makes the prints go away (obviously).
> 
> I made some tests on a remote M3, with no display connected, and I can
> confirm that there seem to be a single underrun report quite often when
> enabling a display, and an underrun flood when using interlace display
> modes.
> 
> E3 does not have interlace display support as far as I can see, so the
> interlace issue does not concern it.
> 
> Debugging display issues remotely without a display is quite
> challenging, and I did not find any issues in the code, nor could I find
> a way to stop the underruns by twiddling with the related registers.
> 
> My pure guess is that the single underruns occurring when starting the
> display hint at either a startup sequence issue, or some kind of initial
> fifo loading issue. The interlace underruns hint at a bigger
> misconfiguration, but as the display works fine, the issue might be just
> an underrun at the start of the frame and the HW quickly catching up, or
> at the end of the frame, where one block in the pipeline expects more
> data but the previous block has already stopped (so maybe a misconfig
> between using interlaced height vs progressive height?).
> 
> But at the moment I have no solution to this, and as the displays work
> fine, I think it makes sense to just revert the print.

Is there any value in instead 'ignoring' any underruns if say the frame
count is < 5 to ignore startup underruns, and keep it as an active print
if something causes underruns later once the pipeline is established?

But maybe that doesn't change much - and if there's no current perceived
issue


Anyway, I don't object to this revert. It's low impact and it's only
undoing 'your' work so no one else will complain :D

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_drm.c  |  3 ---
>  drivers/media/platform/renesas/vsp1/vsp1_drv.c  | 11 +----------
>  drivers/media/platform/renesas/vsp1/vsp1_pipe.h |  2 --
>  drivers/media/platform/renesas/vsp1/vsp1_regs.h |  2 --
>  4 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index 15d266439564..b8f211db16fc 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -721,9 +721,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>                 return 0;
>         }
>  
> -       /* Reset the underrun counter */
> -       pipe->underrun_count = 0;
> -
>         drm_pipe->width = cfg->width;
>         drm_pipe->height = cfg->height;
>         pipe->interlaced = cfg->interlaced;
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index b8d06e88c475..68e92d3c5915 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -47,8 +47,7 @@
>  
>  static irqreturn_t vsp1_irq_handler(int irq, void *data)
>  {
> -       u32 mask = VI6_WPF_IRQ_STA_DFE | VI6_WPF_IRQ_STA_FRE |
> -                  VI6_WPF_IRQ_STA_UND;
> +       u32 mask = VI6_WPF_IRQ_STA_DFE | VI6_WPF_IRQ_STA_FRE;
>         struct vsp1_device *vsp1 = data;
>         irqreturn_t ret = IRQ_NONE;
>         unsigned int i;
> @@ -63,14 +62,6 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
>                 status = vsp1_read(vsp1, VI6_WPF_IRQ_STA(i));
>                 vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status & mask);
>  
> -               if ((status & VI6_WPF_IRQ_STA_UND) && wpf->entity.pipe) {
> -                       wpf->entity.pipe->underrun_count++;
> -
> -                       dev_warn_ratelimited(vsp1->dev,
> -                               "Underrun occurred at WPF%u (total underruns %u)\n",
> -                               i, wpf->entity.pipe->underrun_count);
> -               }
> -
>                 if (status & VI6_WPF_IRQ_STA_DFE) {
>                         vsp1_pipeline_frame_end(wpf->entity.pipe);
>                         ret = IRQ_HANDLED;
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> index 7f623b8cbe5c..9cc2f1646b00 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
> @@ -137,8 +137,6 @@ struct vsp1_pipeline {
>  
>         unsigned int partitions;
>         struct vsp1_partition *part_table;
> -
> -       u32 underrun_count;
>  };
>  
>  void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> index 10cfbcd1b6e0..188d26289714 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> @@ -32,12 +32,10 @@
>  #define VI6_STATUS_SYS_ACT(n)          BIT((n) + 8)
>  
>  #define VI6_WPF_IRQ_ENB(n)             (0x0048 + (n) * 12)
> -#define VI6_WPF_IRQ_ENB_UNDE           BIT(16)
>  #define VI6_WPF_IRQ_ENB_DFEE           BIT(1)
>  #define VI6_WPF_IRQ_ENB_FREE           BIT(0)
>  
>  #define VI6_WPF_IRQ_STA(n)             (0x004c + (n) * 12)
> -#define VI6_WPF_IRQ_STA_UND            BIT(16)
>  #define VI6_WPF_IRQ_STA_DFE            BIT(1)
>  #define VI6_WPF_IRQ_STA_FRE            BIT(0)
>  
> 
> ---
> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
> change-id: 20250908-rcar-vsp-underrun-revert-f3e64612c62d
> 
> Best regards,
> -- 
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>