[v3,05/14] media: platform: pxa_camera: convert to vb2

Message ID 87zioht3zi.fsf@belgarion.home (mailing list archive)
State Superseded, archived
Delegated to: Hans Verkuil
Headers

Commit Message

Robert Jarzmik Aug. 13, 2016, 9:25 a.m. UTC
  Hi Hans,

Robert Jarzmik <robert.jarzmik@free.fr> writes:
> Convert pxa_camera from videobuf to videobuf2.
...zip...

> +static int pxac_vb2_queue_setup(struct vb2_queue *vq,
> +				unsigned int *nbufs,
> +				unsigned int *num_planes, unsigned int sizes[],
> +				void *alloc_ctxs[])

There is an API change here that happened since I wrote this code, ie. void
*alloc_ctxs became struct device *alloc_devs.

I made the incremental patch in [1] accrodingly to prepare the v4 iteration, but
it triggers new errors in v4l2-compliance -s :
Streaming ioctls:
	test read/write: OK (Not Supported)
		fail: v4l2-test-buffers.cpp(293): !(g_flags() & V4L2_BUF_FLAG_DONE)
		fail: v4l2-test-buffers.cpp(703): buf.check(q, last_seq)
		fail: v4l2-test-buffers.cpp(976): captureBufs(node, q, m2m_q, frame_count, false)
	test MMAP: FAIL
		fail: v4l2-test-buffers.cpp(1075): can_stream && ret != EINVAL
	test USERPTR: FAIL
	test DMABUF: Cannot test, specify --expbuf-device
Total: 45, Succeeded: 43, Failed: 2, Warnings: 6

I'm a bit puzzled how this change brought this in, so in case you've already
encountered this, it could save me investigating more. If nothing obvious
appears to you, I'll dig in.

Cheers.
  

Comments

Hans Verkuil Aug. 13, 2016, 9:29 a.m. UTC | #1
On 08/13/2016 11:25 AM, Robert Jarzmik wrote:
> Hi Hans,
> 
> Robert Jarzmik <robert.jarzmik@free.fr> writes:
>> Convert pxa_camera from videobuf to videobuf2.
> ...zip...
> 
>> +static int pxac_vb2_queue_setup(struct vb2_queue *vq,
>> +				unsigned int *nbufs,
>> +				unsigned int *num_planes, unsigned int sizes[],
>> +				void *alloc_ctxs[])
> 
> There is an API change here that happened since I wrote this code, ie. void
> *alloc_ctxs became struct device *alloc_devs.
> 
> I made the incremental patch in [1] accrodingly to prepare the v4 iteration, but
> it triggers new errors in v4l2-compliance -s :
> Streaming ioctls:
> 	test read/write: OK (Not Supported)
> 		fail: v4l2-test-buffers.cpp(293): !(g_flags() & V4L2_BUF_FLAG_DONE)
> 		fail: v4l2-test-buffers.cpp(703): buf.check(q, last_seq)
> 		fail: v4l2-test-buffers.cpp(976): captureBufs(node, q, m2m_q, frame_count, false)
> 	test MMAP: FAIL
> 		fail: v4l2-test-buffers.cpp(1075): can_stream && ret != EINVAL
> 	test USERPTR: FAIL
> 	test DMABUF: Cannot test, specify --expbuf-device
> Total: 45, Succeeded: 43, Failed: 2, Warnings: 6
> 
> I'm a bit puzzled how this change brought this in, so in case you've already
> encountered this, it could save me investigating more. If nothing obvious
> appears to you, I'll dig in.

Make sure you have the latest v4l2-compliance code as well. A fix went into vb2
that corrected a bug relating to the V4L2_BUF_FLAG_DONE, but that required a fix for
v4l2-compliance as well. I'd say that's what you are seeing here.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Robert Jarzmik Aug. 13, 2016, 11:15 a.m. UTC | #2
Hans Verkuil <hverkuil@xs4all.nl> writes:

> On 08/13/2016 11:25 AM, Robert Jarzmik wrote:
>> Hi Hans,
>> 
>> Robert Jarzmik <robert.jarzmik@free.fr> writes:
>>> Convert pxa_camera from videobuf to videobuf2.
>> ...zip...
>> 
>>> +static int pxac_vb2_queue_setup(struct vb2_queue *vq,
>>> +				unsigned int *nbufs,
>>> +				unsigned int *num_planes, unsigned int sizes[],
>>> +				void *alloc_ctxs[])
>> 
>> There is an API change here that happened since I wrote this code, ie. void
>> *alloc_ctxs became struct device *alloc_devs.
>> 
>> I made the incremental patch in [1] accrodingly to prepare the v4 iteration, but
>> it triggers new errors in v4l2-compliance -s :
>> Streaming ioctls:
>> 	test read/write: OK (Not Supported)
>> 		fail: v4l2-test-buffers.cpp(293): !(g_flags() & V4L2_BUF_FLAG_DONE)
>> 		fail: v4l2-test-buffers.cpp(703): buf.check(q, last_seq)
>> 		fail: v4l2-test-buffers.cpp(976): captureBufs(node, q, m2m_q, frame_count, false)
>> 	test MMAP: FAIL
>> 		fail: v4l2-test-buffers.cpp(1075): can_stream && ret != EINVAL
>> 	test USERPTR: FAIL
>> 	test DMABUF: Cannot test, specify --expbuf-device
>> Total: 45, Succeeded: 43, Failed: 2, Warnings: 6
>> 
>> I'm a bit puzzled how this change brought this in, so in case you've already
>> encountered this, it could save me investigating more. If nothing obvious
>> appears to you, I'll dig in.
>
> Make sure you have the latest v4l2-compliance code as well. A fix went into vb2
> that corrected a bug relating to the V4L2_BUF_FLAG_DONE, but that required a fix for
> v4l2-compliance as well. I'd say that's what you are seeing here.

Indeed, this is fixed in v4l-utils somewhere between v4l-utils-1.8.1 and master,
and I don't get any error anymore.

Thanks for the info, I've updated accordingly my work tree :
       https://github.com/rjarzmik/linux.git work/v4l2

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index a161b64d420d..41359a183e83 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -269,8 +269,6 @@  struct pxa_camera_dev {
 	struct tasklet_struct	task_eof;
 
 	u32			save_cicr[5];
-
-	void			*alloc_ctx;
 };
 
 struct pxa_cam {
@@ -1043,7 +1041,7 @@  static int pxac_vb2_init(struct vb2_buffer *vb)
 static int pxac_vb2_queue_setup(struct vb2_queue *vq,
 				unsigned int *nbufs,
 				unsigned int *num_planes, unsigned int sizes[],
-				void *alloc_ctxs[])
+				struct device *alloc_devs[])
 {
 	struct pxa_camera_dev *pcdev = vb2_get_drv_priv(vq);
 	int size = pcdev->current_pix.sizeimage;
@@ -1069,7 +1067,6 @@  static int pxac_vb2_queue_setup(struct vb2_queue *vq,
 		return -EINVAL;
 	}
 
-	alloc_ctxs[0] = pcdev->alloc_ctx;
 	if (!*nbufs)
 		*nbufs = 1;
 
@@ -1125,6 +1122,7 @@  static int pxa_camera_init_videobuf2(struct pxa_camera_dev *pcdev)
 	vq->drv_priv = pcdev;
 	vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	vq->buf_struct_size = sizeof(struct pxa_buffer);
+	vq->dev = pcdev->v4l2_dev.dev;
 
 	vq->ops = &pxac_vb2_ops;
 	vq->mem_ops = &vb2_dma_sg_memops;
@@ -1918,10 +1916,6 @@  static int pxa_camera_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	pcdev->alloc_ctx = vb2_dma_sg_init_ctx(&pdev->dev);
-	if (IS_ERR(pcdev->alloc_ctx))
-		return PTR_ERR(pcdev->alloc_ctx);
-
 	pcdev->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pcdev->clk))
 		return PTR_ERR(pcdev->clk);
@@ -2091,9 +2085,8 @@  static int pxa_camera_remove(struct platform_device *pdev)
 	dma_release_channel(pcdev->dma_chans[0]);
 	dma_release_channel(pcdev->dma_chans[1]);
 	dma_release_channel(pcdev->dma_chans[2]);
-	vb2_dma_sg_cleanup_ctx(pcdev->alloc_ctx);
-	v4l2_clk_unregister(pcdev->mclk_clk);
 
+	v4l2_clk_unregister(pcdev->mclk_clk);
 	v4l2_device_unregister(&pcdev->v4l2_dev);
 
 	dev_info(&pdev->dev, "PXA Camera driver unloaded\n");