| Message ID | 1518043670-4602-1-git-send-email-yong.zhi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Yong, On Wed, Feb 07, 2018 at 02:47:50PM -0800, Yong Zhi wrote: > This is to avoid pending interrupts to be handled during > stream off, in which case, the ready buffer will be removed > from buffer list, thus not all buffers can be returned to VB2 > as expected. Disable CIO2 irq at cio2_hw_exit() so no new > interrupts are generated. > > Signed-off-by: Yong Zhi <yong.zhi@intel.com> > Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com> > --- > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index 725973f..8d75146 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -518,6 +518,8 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q) > unsigned int i, maxloops = 1000; > > /* Disable CSI receiver and MIPI backend devices */ > + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_MASK); > + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_ENABLE); > writel(0, q->csi_rx_base + CIO2_REG_CSIRX_ENABLE); > writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_ENABLE); > > @@ -1027,6 +1029,7 @@ static void cio2_vb2_stop_streaming(struct vb2_queue *vq) > "failed to stop sensor streaming\n"); > > cio2_hw_exit(cio2, q); > + synchronize_irq(cio2->pci_dev->irq); Shouldn't this be put in cio2_hw_exit(), which is called from multiple locations? Presumably the same issue exists there, too. > cio2_vb2_return_all_buffers(q, VB2_BUF_STATE_ERROR); > media_pipeline_stop(&q->vdev.entity); > pm_runtime_put(&cio2->pci_dev->dev);
Hi, Sakari, > -----Original Message----- > From: Sakari Ailus [mailto:sakari.ailus@iki.fi] > Sent: Wednesday, February 7, 2018 11:38 PM > To: Zhi, Yong <yong.zhi@intel.com> > Cc: linux-media@vger.kernel.org; sakari.ailus@linux.intel.com; > tfiga@chromium.org; Qiu, Tian Shu <tian.shu.qiu@intel.com>; Zheng, Jian > Xu <jian.xu.zheng@intel.com>; Mani, Rajmohan > <rajmohan.mani@intel.com> > Subject: Re: [PATCH] media: intel-ipu3: cio2: Synchronize irqs at > stop_streaming > > Hi Yong, > > On Wed, Feb 07, 2018 at 02:47:50PM -0800, Yong Zhi wrote: > > This is to avoid pending interrupts to be handled during stream off, > > in which case, the ready buffer will be removed from buffer list, thus > > not all buffers can be returned to VB2 as expected. Disable CIO2 irq > > at cio2_hw_exit() so no new interrupts are generated. > > > > Signed-off-by: Yong Zhi <yong.zhi@intel.com> > > Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com> > > --- > > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > index 725973f..8d75146 100644 > > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > > @@ -518,6 +518,8 @@ static void cio2_hw_exit(struct cio2_device *cio2, > struct cio2_queue *q) > > unsigned int i, maxloops = 1000; > > > > /* Disable CSI receiver and MIPI backend devices */ > > + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_MASK); > > + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_ENABLE); > > writel(0, q->csi_rx_base + CIO2_REG_CSIRX_ENABLE); > > writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_ENABLE); > > > > @@ -1027,6 +1029,7 @@ static void cio2_vb2_stop_streaming(struct > vb2_queue *vq) > > "failed to stop sensor streaming\n"); > > > > cio2_hw_exit(cio2, q); > > + synchronize_irq(cio2->pci_dev->irq); > > Shouldn't this be put in cio2_hw_exit(), which is called from multiple > locations? Presumably the same issue exists there, too. > Thanks for catching this, cio2_hw_exit() is used at two other places, and only one of them is subject to racing, so I will add synchronize_irq there in next update if it's OK. > > cio2_vb2_return_all_buffers(q, VB2_BUF_STATE_ERROR); > > media_pipeline_stop(&q->vdev.entity); > > pm_runtime_put(&cio2->pci_dev->dev); > > -- > Regards, > > Sakari Ailus > e-mail: sakari.ailus@iki.fi
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index 725973f..8d75146 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -518,6 +518,8 @@ static void cio2_hw_exit(struct cio2_device *cio2, struct cio2_queue *q) unsigned int i, maxloops = 1000; /* Disable CSI receiver and MIPI backend devices */ + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_MASK); + writel(0, q->csi_rx_base + CIO2_REG_IRQCTRL_ENABLE); writel(0, q->csi_rx_base + CIO2_REG_CSIRX_ENABLE); writel(0, q->csi_rx_base + CIO2_REG_MIPIBE_ENABLE); @@ -1027,6 +1029,7 @@ static void cio2_vb2_stop_streaming(struct vb2_queue *vq) "failed to stop sensor streaming\n"); cio2_hw_exit(cio2, q); + synchronize_irq(cio2->pci_dev->irq); cio2_vb2_return_all_buffers(q, VB2_BUF_STATE_ERROR); media_pipeline_stop(&q->vdev.entity); pm_runtime_put(&cio2->pci_dev->dev);