Hi, Sakari, > -----Original Message----- > From: Sakari Ailus [mailto:sakari.ailus@xxxxxx] > Sent: Wednesday, February 7, 2018 11:38 PM > To: Zhi, Yong <yong.zhi@xxxxxxxxx> > Cc: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx; > tfiga@xxxxxxxxxxxx; Qiu, Tian Shu <tian.shu.qiu@xxxxxxxxx>; Zheng, Jian > Xu <jian.xu.zheng@xxxxxxxxx>; Mani, Rajmohan > <rajmohan.mani@xxxxxxxxx> > 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@xxxxxxxxx> > > Signed-off-by: Tianshu Qiu <tian.shu.qiu@xxxxxxxxx> > > --- > > 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@xxxxxx
