On Mon, 19 Jan 2015, Guennadi Liakhovetski wrote:
On Mon, 19 Jan 2015, William Towle wrote:
in the patchset Ben linked to above we think
we have the appropriate loops: a for loop for queue_buf[], and
list_for_each_safe() for anything left in priv->capture; this is
consistent with rcar_vin_fill_hw_slot() setting up queue_buf[] with
pointers unlinked from priv->capture. This in turn suggests that we
are right not to call list_del_init() in both of
rcar_vin_stop_streaming()'s loops ... as long as I've correctly
interpreted the code and everyone's feedback thus far.
I'm referring to this comment by Hans Verkuil of 14 August last year:
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.
I think, that's the right way to implement that clean up loop.
Hi,
I was describing the code in my latest patch; we start with
rcar_vin_stop_streaming() having a list_for_each_safe() loop like
that above, and leave that loop in place but add statements to it.
I add another loop to rcar_vin_stop_streaming() [which you will
have seen -in the patch Ben published in my absence over Christmas-
was particularly inelegant initially], but it can't be rewritten in
the same way. The new version is undeniably neater, though.
We believe the latest patches take Hans' comment above into
account properly, and we are looking into his latest suggestion at
the moment.
Thanks again,
Wills.
--
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
