[0/4] media: add missing wait_prepare/finish ops

Without these ops the v4l2-compliance blocking wait test will fail.
These ops are required to ensure that when VIDIOC_DQBUF has to
wait for buffers to arrive, the queue lock is correctly released
and retaken. Otherwise the wait for a buffer would block all other
queue ioctls.

I would appreciate it if these patches can be tested.

I also updated v4l2-compliance so that it executes the blocking wait
test even if the '-s' option was not specified. This should improve
coverage since not everyone uses that option.

After this series is (hopefully) applied I plan to make changes to
avoid the need to set these two ops, unless you need a custom
implementation (omap3isp).

Regards,

	Hans

Hans Verkuil (4):
  media: atomisp: add missing wait_prepare/finish ops
  media: omap3isp: add missing wait_prepare/finish ops
  media: pisp_be: add missing wait_prepare/finish ops
  media: venus: add missing wait_prepare/finish ops

 drivers/media/platform/qcom/venus/vdec.c       |  2 ++
 drivers/media/platform/qcom/venus/venc.c       |  2 ++
 .../platform/raspberrypi/pisp_be/pisp_be.c     |  2 ++
 drivers/media/platform/ti/omap3isp/ispvideo.c  | 18 ++++++++++++++++++
 .../staging/media/atomisp/pci/atomisp_fops.c   |  2 ++
 5 files changed, 26 insertions(+)

Hi Hans

On Mon, Sep 02, 2024 at 10:31:23AM GMT, Hans Verkuil wrote:
> Without these ops the v4l2-compliance blocking wait test will fail.
> These ops are required to ensure that when VIDIOC_DQBUF has to
> wait for buffers to arrive, the queue lock is correctly released
> and retaken. Otherwise the wait for a buffer would block all other
> queue ioctls.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Cc: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I can confirm it fixes a previously failing test

was:

                fail: v4l2-test-buffers.cpp(3050): !thread_streamoff.done
                fail: v4l2-test-buffers.cpp(3078): testBlockingDQBuf(node, q)
	test blocking wait: FAIL

now:
	test blocking wait: OK

Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  drivers/media/platform/raspberrypi/pisp_be/pisp_be.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> index 65ff2382cffe..7ce3be626c4a 100644
> --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
> @@ -964,6 +964,8 @@ static const struct vb2_ops pispbe_node_queue_ops = {
>  	.buf_queue = pispbe_node_buffer_queue,
>  	.start_streaming = pispbe_node_start_streaming,
>  	.stop_streaming = pispbe_node_stop_streaming,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
>  };
>
>  static const struct v4l2_file_operations pispbe_fops = {
> --
> 2.43.0
>
>

Hi,

On 9/3/24 9:59 AM, Hans Verkuil wrote:
> Hi Hans,
> 
> On 02/09/2024 12:39, Hans de Goede wrote:
>> Hi Hans,
>>
>> On 9/2/24 10:31 AM, Hans Verkuil wrote:
>>> Without these ops the v4l2-compliance blocking wait test will fail.
>>> These ops are required to ensure that when VIDIOC_DQBUF has to
>>> wait for buffers to arrive, the queue lock is correctly released
>>> and retaken. Otherwise the wait for a buffer would block all other
>>> queue ioctls.
>>>
>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>
>> Thank you for this patch.
>>
>> I have merged this in my media-atomisp branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp
>>
>> I'll run some tests when I'm back home tonight (with access to
>> atomisp2 hw) before included this in my upcoming atomisp pull-request
>> for 6.12.
> 
> Would you be able to post a PR today? We want to switch over to our new gitlab
> tree (https://gitlab.freedesktop.org/linux-media/media-staging), but we'd like
> to get all PRs for 6.12 merged first.

Yes I already reviewed + merged all the pending atomisp stuff yesterday.

I just need to merge your "media: staging: atomisp: set lock before
calling vb2_queue_init()" and run a quick test and then I'll send out
the PR today.

Regards,
 
Hans