Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Hi Ian,
Apparently, I wasn't quite right in my reply to patch 1 from this series.
It seems to be related to this one and to Hans' comment below. Taking it
into account, it seems you shouldn't remove cancelled active buffers from
the list without calling vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR)
for them. So, I think you should modify your patch 1 to either leave those
buffers on the list and report them done here, or call done for them
when removing from the list. I think I'd rather go with the latter option
to not keep failed buffers on the list and rely, that the vb2 core will
stop streaming soon. Hans, do you agree?
Thanks
Guennadi
On Mon, 4 Aug 2014, Hans Verkuil wrote:
On 07/08/2014 11:41 AM, Ian Molton wrote:
Videobuf2 complains about buffers that are still marked ACTIVE (in use by
the driver) following a call to stop_streaming().
This patch returns all active buffers to state ERROR prior to stopping.
Note: this introduces a (non fatal) race condition as the stream is not
guaranteed to be stopped at this point.
Signed-off-by: Ian Molton ian.mol...@codethink.co.uk
Signed-off-by: William Towle william.to...@codethink.co.uk
---
drivers/media/platform/soc_camera/rcar_vin.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index 7154500..06ce705 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue
*vq)
struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
struct rcar_vin_priv *priv = ici-priv;
struct list_head *buf_head, *tmp;
+ int i;
spin_lock_irq(priv-lock);
+
+ for (i = 0; i vq-num_buffers; ++i)
+ if (vq-bufs[i]-state == VB2_BUF_STATE_ACTIVE)
+ vb2_buffer_done(vq-bufs[i], VB2_BUF_STATE_ERROR);
+
list_for_each_safe(buf_head, tmp, priv-capture)
list_del_init(buf_head);
I'm assuming all buffers that are queued to the driver via buf_queue() are
linked into priv-capture. So you would typically call vb2_buffer_done
when you are walking that list:
list_for_each_safe(buf_head, tmp, priv-capture) {
// usually you go from buf_head to the real buffer struct
// containing a vb2_buffer struct
vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
list_del_init(buf_head);
}
Please use this rather than looking into internal vb2_queue datastructures.
Regards,
Hans
spin_unlock_irq(priv-lock);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Hi Ian, On Wed, 13 Aug 2014, Ian Molton wrote: Fixed kernel WARNINGs for me! \o/ Ian, perhaps it makes sense for me to take these patches into my hands? I'm planning to respin these tomorrow - is that OK? I have test hardware with two different frontends here. Any progress on this? Thanks Guennadi -Ian -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Fixed kernel WARNINGs for me! \o/ Ian, perhaps it makes sense for me to take these patches into my hands? I'm planning to respin these tomorrow - is that OK? I have test hardware with two different frontends here. -Ian -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Hello. On 08/13/2014 09:30 PM, Ian Molton wrote: Fixed kernel WARNINGs for me! \o/ Ian, perhaps it makes sense for me to take these patches into my hands? I'm planning to respin these tomorrow - is that OK? Yes. I have test hardware with two different frontends here. I'm sorry, what do you mean by frontends? -Ian WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
On 07/08/2014 01:41 PM, Ian Molton wrote: Videobuf2 complains about buffers that are still marked ACTIVE (in use by the driver) following a call to stop_streaming(). This patch returns all active buffers to state ERROR prior to stopping. Note: this introduces a (non fatal) race condition as the stream is not guaranteed to be stopped at this point. Signed-off-by: Ian Molton ian.mol...@codethink.co.uk Signed-off-by: William Towle william.to...@codethink.co.uk Fixed kernel WARNINGs for me! \o/ Ian, perhaps it makes sense for me to take these patches into my hands? WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
On 07/08/2014 11:41 AM, Ian Molton wrote:
Videobuf2 complains about buffers that are still marked ACTIVE (in use by the
driver) following a call to stop_streaming().
This patch returns all active buffers to state ERROR prior to stopping.
Note: this introduces a (non fatal) race condition as the stream is not
guaranteed to be stopped at this point.
Signed-off-by: Ian Molton ian.mol...@codethink.co.uk
Signed-off-by: William Towle william.to...@codethink.co.uk
---
drivers/media/platform/soc_camera/rcar_vin.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index 7154500..06ce705 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq)
struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
struct rcar_vin_priv *priv = ici-priv;
struct list_head *buf_head, *tmp;
+ int i;
spin_lock_irq(priv-lock);
+
+ for (i = 0; i vq-num_buffers; ++i)
+ if (vq-bufs[i]-state == VB2_BUF_STATE_ACTIVE)
+ vb2_buffer_done(vq-bufs[i], VB2_BUF_STATE_ERROR);
+
list_for_each_safe(buf_head, tmp, priv-capture)
list_del_init(buf_head);
I'm assuming all buffers that are queued to the driver via buf_queue() are
linked into priv-capture. So you would typically call vb2_buffer_done
when you are walking that list:
list_for_each_safe(buf_head, tmp, priv-capture) {
// usually you go from buf_head to the real buffer struct
// containing a vb2_buffer struct
vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
list_del_init(buf_head);
}
Please use this rather than looking into internal vb2_queue datastructures.
Regards,
Hans
spin_unlock_irq(priv-lock);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Videobuf2 complains about buffers that are still marked ACTIVE (in use by the driver) following a call to stop_streaming(). This patch returns all active buffers to state ERROR prior to stopping. Note: this introduces a (non fatal) race condition as the stream is not guaranteed to be stopped at this point. Signed-off-by: Ian Molton ian.mol...@codethink.co.uk Signed-off-by: William Towle william.to...@codethink.co.uk --- drivers/media/platform/soc_camera/rcar_vin.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index 7154500..06ce705 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq) struct soc_camera_host *ici = to_soc_camera_host(icd-parent); struct rcar_vin_priv *priv = ici-priv; struct list_head *buf_head, *tmp; + int i; spin_lock_irq(priv-lock); + + for (i = 0; i vq-num_buffers; ++i) + if (vq-bufs[i]-state == VB2_BUF_STATE_ACTIVE) + vb2_buffer_done(vq-bufs[i], VB2_BUF_STATE_ERROR); + list_for_each_safe(buf_head, tmp, priv-capture) list_del_init(buf_head); spin_unlock_irq(priv-lock); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Videobuf2 complains about buffers that are still marked ACTIVE (in use by the driver) following a call to stop_streaming(). This patch returns all active buffers to state ERROR prior to stopping. Note: this introduces a (non fatal) race condition as the stream is not guaranteed to be stopped at this point. Signed-off-by: Ian Molton ian.mol...@codethink.co.uk Signed-off-by: William Towle william.to...@codethink.co.uk --- drivers/media/platform/soc_camera/rcar_vin.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index 7154500..06ce705 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq) struct soc_camera_host *ici = to_soc_camera_host(icd-parent); struct rcar_vin_priv *priv = ici-priv; struct list_head *buf_head, *tmp; + int i; spin_lock_irq(priv-lock); + + for (i = 0; i vq-num_buffers; ++i) + if (vq-bufs[i]-state == VB2_BUF_STATE_ACTIVE) + vb2_buffer_done(vq-bufs[i], VB2_BUF_STATE_ERROR); + list_for_each_safe(buf_head, tmp, priv-capture) list_del_init(buf_head); spin_unlock_irq(priv-lock); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
