Re: [PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Tomasz Figa]

On Thu, Apr 26, 2018 at 1:39 AM Yong Zhi  wrote:

> From: Bingbu Cao 

> Interrupt behavior shows that some time the frame end and frame start
> of next frame is unstable and can range from several to hundreds
> of micro-sec. In the case of ~10us, isr may not clear next sof interrupt
> status in single handling, which prevents new interrupts from coming.

> Fix this by handling all pending IRQs before exiting isr, so
> any abnormal behavior results from very short interrupt status
> changes is protected.

> Signed-off-by: Andy Yeh 
> Signed-off-by: Bingbu Cao 
> Signed-off-by: Yong Zhi 
> ---
>   drivers/media/pci/intel/ipu3/ipu3-cio2.c | 32
++--
>   1 file changed, 22 insertions(+), 10 deletions(-)

In case you're waiting for me with this one:

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz

Re: [PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Sakari Ailus]

On Mon, Apr 30, 2018 at 09:30:40AM -0500, Yong Zhi wrote:
> From: Bingbu Cao 
> 
> Interrupt behavior shows that some time the frame end and frame start
> of next frame is unstable and can range from several to hundreds of micro-sec.
> In the case of ~10us, isr may not clear next sof interrupt status in
> single handling, which prevents new interrupts from coming.
> 
> Fix this by handling all pending IRQs before exiting isr, so any abnormal
> behavior results from very short interrupt status changes is protected.
> 
> Signed-off-by: Bingbu Cao 
> Signed-off-by: Andy Yeh 
> Signed-off-by: Yong Zhi 
> ---
> Hi, Sakari,
> 
> Re-send with correct signed-off-by order.

Thanks, Yong. I've replaced the patch in my ipu3 branch, with the commit
message wrapped --- after adding four spaces to the left margin it exceeded
80 characters.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Yong Zhi]

From: Bingbu Cao 

Interrupt behavior shows that some time the frame end and frame start
of next frame is unstable and can range from several to hundreds of micro-sec.
In the case of ~10us, isr may not clear next sof interrupt status in
single handling, which prevents new interrupts from coming.

Fix this by handling all pending IRQs before exiting isr, so any abnormal
behavior results from very short interrupt status changes is protected.

Signed-off-by: Bingbu Cao 
Signed-off-by: Andy Yeh 
Signed-off-by: Yong Zhi 
---
Hi, Sakari,

Re-send with correct signed-off-by order.
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 7d768ec0f824..29027159eced 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -640,18 +640,10 @@ static const char *const cio2_port_errs[] = {
"PKT2LONG",
 };
 
-static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+static void cio2_irq_handle_once(struct cio2_device *cio2, u32 int_status)
 {
-   struct cio2_device *cio2 = cio2_ptr;
void __iomem *const base = cio2->base;
struct device *dev = >pci_dev->dev;
-   u32 int_status, int_clear;
-
-   int_status = readl(base + CIO2_REG_INT_STS);
-   int_clear = int_status;
-
-   if (!int_status)
-   return IRQ_NONE;
 
if (int_status & CIO2_INT_IOOE) {
/*
@@ -770,9 +762,29 @@ static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
int_status &= ~(CIO2_INT_IOIE | CIO2_INT_IOIRQ);
}
 
-   writel(int_clear, base + CIO2_REG_INT_STS);
if (int_status)
dev_warn(dev, "unknown interrupt 0x%x on INT\n", int_status);
+}
+
+static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+{
+   struct cio2_device *cio2 = cio2_ptr;
+   void __iomem *const base = cio2->base;
+   struct device *dev = >pci_dev->dev;
+   u32 int_status;
+
+   int_status = readl(base + CIO2_REG_INT_STS);
+   dev_dbg(dev, "isr enter - interrupt status 0x%x\n", int_status);
+   if (!int_status)
+   return IRQ_NONE;
+
+   do {
+   writel(int_status, base + CIO2_REG_INT_STS);
+   cio2_irq_handle_once(cio2, int_status);
+   int_status = readl(base + CIO2_REG_INT_STS);
+   if (int_status)
+   dev_dbg(dev, "pending status 0x%x\n", int_status);
+   } while (int_status);
 
return IRQ_HANDLED;
 }
-- 
2.7.4

[PATCH] media: intel-ipu3: cio2: Handle IRQs until INT_STS is cleared

[Yong Zhi]

From: Bingbu Cao 

Interrupt behavior shows that some time the frame end and frame start
of next frame is unstable and can range from several to hundreds
of micro-sec. In the case of ~10us, isr may not clear next sof interrupt
status in single handling, which prevents new interrupts from coming.

Fix this by handling all pending IRQs before exiting isr, so
any abnormal behavior results from very short interrupt status
changes is protected.

Signed-off-by: Andy Yeh 
Signed-off-by: Bingbu Cao 
Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 7d768ec..2902715 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -640,18 +640,10 @@ static const char *const cio2_port_errs[] = {
"PKT2LONG",
 };
 
-static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+static void cio2_irq_handle_once(struct cio2_device *cio2, u32 int_status)
 {
-   struct cio2_device *cio2 = cio2_ptr;
void __iomem *const base = cio2->base;
struct device *dev = >pci_dev->dev;
-   u32 int_status, int_clear;
-
-   int_status = readl(base + CIO2_REG_INT_STS);
-   int_clear = int_status;
-
-   if (!int_status)
-   return IRQ_NONE;
 
if (int_status & CIO2_INT_IOOE) {
/*
@@ -770,9 +762,29 @@ static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
int_status &= ~(CIO2_INT_IOIE | CIO2_INT_IOIRQ);
}
 
-   writel(int_clear, base + CIO2_REG_INT_STS);
if (int_status)
dev_warn(dev, "unknown interrupt 0x%x on INT\n", int_status);
+}
+
+static irqreturn_t cio2_irq(int irq, void *cio2_ptr)
+{
+   struct cio2_device *cio2 = cio2_ptr;
+   void __iomem *const base = cio2->base;
+   struct device *dev = >pci_dev->dev;
+   u32 int_status;
+
+   int_status = readl(base + CIO2_REG_INT_STS);
+   dev_dbg(dev, "isr enter - interrupt status 0x%x\n", int_status);
+   if (!int_status)
+   return IRQ_NONE;
+
+   do {
+   writel(int_status, base + CIO2_REG_INT_STS);
+   cio2_irq_handle_once(cio2, int_status);
+   int_status = readl(base + CIO2_REG_INT_STS);
+   if (int_status)
+   dev_dbg(dev, "pending status 0x%x\n", int_status);
+   } while (int_status);
 
return IRQ_HANDLED;
 }
-- 
2.7.4