| Message ID | 20230207124440.123910-1-tomi.valkeinen@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas: vsp1: Add underrun debug print | expand |
Hi Tomi, Thank you for the patch. On Tue, Feb 07, 2023 at 02:44:40PM +0200, Tomi Valkeinen wrote: > From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > > 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@ideasonboard.com> > --- > drivers/media/platform/renesas/vsp1/vsp1_drv.c | 8 +++++++- > drivers/media/platform/renesas/vsp1/vsp1_regs.h | 2 ++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c > index 5710152d6511..b24437b5b945 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,11 @@ 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) { > + dev_warn_ratelimited(vsp1->dev, > + "Underrun occurred at WPF%u\n", i); > + } You could drop the braces. Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > + > 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) >
On 07/02/2023 16:00, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tue, Feb 07, 2023 at 02:44:40PM +0200, Tomi Valkeinen wrote: >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> >> >> 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@ideasonboard.com> >> --- >> drivers/media/platform/renesas/vsp1/vsp1_drv.c | 8 +++++++- >> drivers/media/platform/renesas/vsp1/vsp1_regs.h | 2 ++ >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c >> index 5710152d6511..b24437b5b945 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,11 @@ 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) { >> + dev_warn_ratelimited(vsp1->dev, >> + "Underrun occurred at WPF%u\n", i); >> + } > > You could drop the braces. I could, but block is not a single line, so... Well, matter of taste =). Tomi
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c index 5710152d6511..b24437b5b945 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,11 @@ 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) { + dev_warn_ratelimited(vsp1->dev, + "Underrun occurred at WPF%u\n", 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)