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 <[email protected]> > > Signed-off-by: Tomi Valkeinen <[email protected]> > --- > 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 <[email protected]> >
