Hi Tomi,
Thank you for the patch.
On Tue, Feb 14, 2023 at 06:42:23PM +0200, Tomi Valkeinen wrote:
> Print underrun interrupts with ratelimited print.
>
> Note that we don't enable the underrun interrupt. If we have underruns,
> we don't want to get flooded with interrupts about them. It's enough to
> see that an underrun happened at the end of a frame.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> ---
>
> Changes in v3:
> - Reset underrun counter when enabling VSP
>
> I have to say I'm not familiar enough with the VSP driver to say if
> these are the correct places where to reset the counters.
It's fine. We could factor it out to a clear function, but it's not
worth it if there's nothing else to factor out. It could be done later.
> There's also a
> possibility of a race, but my assumption is that we cannot get underrun
> interrupts for the WPF we are currently enabling.
It should be fine.
> Also, I realized the underrun counter could be moved to struct
> vsp1_rwpf, but as that's used also for RPF, I didn't do that change.
Another option would be to store it in the pipeline structure, as a
pipeline has one and only one WPF. What do you think ?
> drivers/media/platform/renesas/vsp1/vsp1.h | 2 ++
> drivers/media/platform/renesas/vsp1/vsp1_drm.c | 3 +++
> drivers/media/platform/renesas/vsp1/vsp1_drv.c | 11 ++++++++++-
> drivers/media/platform/renesas/vsp1/vsp1_regs.h | 2 ++
> drivers/media/platform/renesas/vsp1/vsp1_video.c | 3 +++
> 5 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
> index 2f6f0c6ae555..9eb023f4fee8 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1.h
> @@ -107,6 +107,8 @@ struct vsp1_device {
> struct media_entity_operations media_ops;
>
> struct vsp1_drm *drm;
> +
> + u32 underrun_count[VSP1_MAX_WPF];
> };
>
> int vsp1_device_get(struct vsp1_device *vsp1);
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index c6f25200982c..e3b4e993787c 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -710,6 +710,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> return 0;
> }
>
> + /* Reset the underrun counter */
> + vsp1->underrun_count[pipe->output->entity.index] = 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 5710152d6511..f9be0fda1659 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -45,7 +45,8 @@
>
> static irqreturn_t vsp1_irq_handler(int irq, void *data)
> {
> - u32 mask = VI6_WPF_IRQ_STA_DFE | VI6_WPF_IRQ_STA_FRE;
> + u32 mask = VI6_WPF_IRQ_STA_DFE | VI6_WPF_IRQ_STA_FRE |
> + VI6_WPF_IRQ_STA_UND;
> struct vsp1_device *vsp1 = data;
> irqreturn_t ret = IRQ_NONE;
> unsigned int i;
> @@ -60,6 +61,14 @@ 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) {
> + vsp1->underrun_count[i]++;
> +
> + dev_warn_ratelimited(vsp1->dev,
> + "Underrun occurred at WPF%u (total underruns %u)\n",
> + i, vsp1->underrun_count[i]);
> + }
> +
> 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_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> index d94343ae57a1..7eca82e0ba7e 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> @@ -32,10 +32,12 @@
> #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)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 544012fd1fe9..6eca2b9c8dee 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -1062,6 +1062,9 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
> if (ret < 0)
> goto err_stop;
>
> + /* Reset the underrun counter */
> + video->vsp1->underrun_count[pipe->output->entity.index] = 0;
> +
> /* Start the queue. */
> ret = vb2_streamon(&video->queue, type);
> if (ret < 0)
--
Regards,
Laurent Pinchart
