| Message ID | 20210519005834.8690-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 0ada1697ed4256b38225319c9896661142a3572d |
| Headers | show |
| Series | media: imx: imx7-media-csi: Fix buffer return upon stream start failure | expand |
Am Mittwoch, dem 19.05.2021 um 03:58 +0300 schrieb Laurent Pinchart: > When the stream fails to start, the first two buffers in the queue > have > been moved to the active_vb2_buf array and are returned to vb2 by > imx7_csi_dma_unsetup_vb2_buf(). The function is called with the > buffer > state set to VB2_BUF_STATE_ERROR unconditionally, which is correct > when > stopping the stream, but not when the start operation fails. In that > case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c > b/drivers/staging/media/imx/imx7-media-csi.c > index f644a640a831..da768ea21d03 100644 > --- a/drivers/staging/media/imx/imx7-media-csi.c > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -361,6 +361,7 @@ static void imx7_csi_dma_unsetup_vb2_buf(struct > imx7_csi *csi, > > vb->timestamp = ktime_get_ns(); > vb2_buffer_done(vb, return_status); > + csi->active_vb2_buf[i] = NULL; > } > } > } > @@ -386,9 +387,10 @@ static int imx7_csi_dma_setup(struct imx7_csi > *csi) > return 0; > } > > -static void imx7_csi_dma_cleanup(struct imx7_csi *csi) > +static void imx7_csi_dma_cleanup(struct imx7_csi *csi, > + enum vb2_buffer_state return_status) > { > - imx7_csi_dma_unsetup_vb2_buf(csi, VB2_BUF_STATE_ERROR); > + imx7_csi_dma_unsetup_vb2_buf(csi, return_status); > imx_media_free_dma_buf(csi->dev, &csi->underrun_buf); > } > > @@ -526,9 +528,10 @@ static int imx7_csi_init(struct imx7_csi *csi) > return 0; > } > > -static void imx7_csi_deinit(struct imx7_csi *csi) > +static void imx7_csi_deinit(struct imx7_csi *csi, > + enum vb2_buffer_state return_status) > { > - imx7_csi_dma_cleanup(csi); > + imx7_csi_dma_cleanup(csi, return_status); > imx7_csi_init_default(csi); > imx7_csi_dmareq_rff_disable(csi); > clk_disable_unprepare(csi->mclk); > @@ -691,7 +694,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev > *sd, int enable) > > ret = v4l2_subdev_call(csi->src_sd, video, s_stream, > 1); > if (ret < 0) { > - imx7_csi_deinit(csi); > + imx7_csi_deinit(csi, VB2_BUF_STATE_QUEUED); > goto out_unlock; > } > > @@ -701,7 +704,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev > *sd, int enable) > > v4l2_subdev_call(csi->src_sd, video, s_stream, 0); > > - imx7_csi_deinit(csi); > + imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR); > } > > csi->is_streaming = !!enable; This patch has not yet been accepted. Any specific reason? I need it. Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> thank you very much
Hi Martin, On Wed Jul 28, 2021 at 9:50 AM WEST, Martin Kepplinger wrote: > Am Mittwoch, dem 19.05.2021 um 03:58 +0300 schrieb Laurent Pinchart: > > When the stream fails to start, the first two buffers in the queue > > have > > been moved to the active_vb2_buf array and are returned to vb2 by > > imx7_csi_dma_unsetup_vb2_buf(). The function is called with the > > buffer > > state set to VB2_BUF_STATE_ERROR unconditionally, which is correct > > when > > stopping the stream, but not when the start operation fails. In that > > case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------ <snip> > > + imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR); > > } > > > > csi->is_streaming = !!enable; > > > This patch has not yet been accepted. Any specific reason? I need it. Good question, I gave my reviewed in May [0], maybe got lost in the merge process somewhere. Mauro? [0]: https://lore.kernel.org/linux-media/CBHA8BLTAJM1.1DIYC729ZMAYY@arch-thunder/ ------ Cheers, Rui > > Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > thank you very much
Hello, On Wed, Jul 28, 2021 at 10:19:36AM +0100, Rui Miguel Silva wrote: > On Wed Jul 28, 2021 at 9:50 AM WEST, Martin Kepplinger wrote: > > Am Mittwoch, dem 19.05.2021 um 03:58 +0300 schrieb Laurent Pinchart: > > > When the stream fails to start, the first two buffers in the queue > > > have > > > been moved to the active_vb2_buf array and are returned to vb2 by > > > imx7_csi_dma_unsetup_vb2_buf(). The function is called with the > > > buffer > > > state set to VB2_BUF_STATE_ERROR unconditionally, which is correct > > > when > > > stopping the stream, but not when the start operation fails. In that > > > case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------ > > <snip> > > > > + imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR); > > > } > > > > > > csi->is_streaming = !!enable; > > > > > > This patch has not yet been accepted. Any specific reason? I need it. > > Good question, I gave my reviewed in May [0], maybe got lost in the > merge process somewhere. Mauro? I've just sent a pull request with all the pending i.MX8 patches, including Martin's i.MX8MQ CSI-2 receiver driver. https://lore.kernel.org/linux-media/YQFY5FS8v3Y3KkEA@pendragon.ideasonboard.com/T/#u > [0]: https://lore.kernel.org/linux-media/CBHA8BLTAJM1.1DIYC729ZMAYY@arch-thunder/ > > > > > Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > > > thank you very much -- Regards, Laurent Pinchart
On Wed Jul 28, 2021 at 2:18 PM WEST, Laurent Pinchart wrote: > Hello, > > On Wed, Jul 28, 2021 at 10:19:36AM +0100, Rui Miguel Silva wrote: > > On Wed Jul 28, 2021 at 9:50 AM WEST, Martin Kepplinger wrote: > > > Am Mittwoch, dem 19.05.2021 um 03:58 +0300 schrieb Laurent Pinchart: > > > > When the stream fails to start, the first two buffers in the queue > > > > have > > > > been moved to the active_vb2_buf array and are returned to vb2 by > > > > imx7_csi_dma_unsetup_vb2_buf(). The function is called with the > > > > buffer > > > > state set to VB2_BUF_STATE_ERROR unconditionally, which is correct > > > > when > > > > stopping the stream, but not when the start operation fails. In that > > > > case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------ > > > > <snip> > > > > > > + imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR); > > > > } > > > > > > > > csi->is_streaming = !!enable; > > > > > > > > > This patch has not yet been accepted. Any specific reason? I need it. > > > > Good question, I gave my reviewed in May [0], maybe got lost in the > > merge process somewhere. Mauro? > > I've just sent a pull request with all the pending i.MX8 patches, > including Martin's i.MX8MQ CSI-2 receiver driver. Thanks Laurent. ------ Cheers, Rui >
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c index f644a640a831..da768ea21d03 100644 --- a/drivers/staging/media/imx/imx7-media-csi.c +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -361,6 +361,7 @@ static void imx7_csi_dma_unsetup_vb2_buf(struct imx7_csi *csi, vb->timestamp = ktime_get_ns(); vb2_buffer_done(vb, return_status); + csi->active_vb2_buf[i] = NULL; } } } @@ -386,9 +387,10 @@ static int imx7_csi_dma_setup(struct imx7_csi *csi) return 0; } -static void imx7_csi_dma_cleanup(struct imx7_csi *csi) +static void imx7_csi_dma_cleanup(struct imx7_csi *csi, + enum vb2_buffer_state return_status) { - imx7_csi_dma_unsetup_vb2_buf(csi, VB2_BUF_STATE_ERROR); + imx7_csi_dma_unsetup_vb2_buf(csi, return_status); imx_media_free_dma_buf(csi->dev, &csi->underrun_buf); } @@ -526,9 +528,10 @@ static int imx7_csi_init(struct imx7_csi *csi) return 0; } -static void imx7_csi_deinit(struct imx7_csi *csi) +static void imx7_csi_deinit(struct imx7_csi *csi, + enum vb2_buffer_state return_status) { - imx7_csi_dma_cleanup(csi); + imx7_csi_dma_cleanup(csi, return_status); imx7_csi_init_default(csi); imx7_csi_dmareq_rff_disable(csi); clk_disable_unprepare(csi->mclk); @@ -691,7 +694,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable) ret = v4l2_subdev_call(csi->src_sd, video, s_stream, 1); if (ret < 0) { - imx7_csi_deinit(csi); + imx7_csi_deinit(csi, VB2_BUF_STATE_QUEUED); goto out_unlock; } @@ -701,7 +704,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable) v4l2_subdev_call(csi->src_sd, video, s_stream, 0); - imx7_csi_deinit(csi); + imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR); } csi->is_streaming = !!enable;
When the stream fails to start, the first two buffers in the queue have been moved to the active_vb2_buf array and are returned to vb2 by imx7_csi_dma_unsetup_vb2_buf(). The function is called with the buffer state set to VB2_BUF_STATE_ERROR unconditionally, which is correct when stopping the stream, but not when the start operation fails. In that case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)