Re: [PATCH] V4L: soc-camera: Add support for custom host mmap

[Sergio Aguirre <saaguirre@ti.com>]

Hi Guennadi,

On 03/08/2011 01:17 AM, Guennadi Liakhovetski wrote:
> Hi Sergio
>
> On Mon, 7 Mar 2011, Sergio Aguirre wrote:
>
>> This helps redirect mmap calls to custom memory managers which
>> already have preallocated space to use by the device.
>>
>> Otherwise, device might not support the allocation attempted
>> generically by videobuf.
>>
>> Signed-off-by: Sergio Aguirre<saaguirre@ti.com>
>> ---
>>   drivers/media/video/soc_camera.c |    7 ++++++-
>>   include/media/soc_camera.h       |    2 ++
>>   2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
>> index 59dc71d..d361ba0 100644
>> --- a/drivers/media/video/soc_camera.c
>> +++ b/drivers/media/video/soc_camera.c
>> @@ -512,6 +512,7 @@ static ssize_t soc_camera_read(struct file *file, char __user *buf,
>>   static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma)
>>   {
>>   	struct soc_camera_device *icd = file->private_data;
>> +	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
>
> This doesn't seem to be needed

It's needed to call the custom mmaper.

ici->ops->mmap

Otherwise, how can I access the soc camera host ops?

>
>>   	int err;
>>
>>   	dev_dbg(&icd->dev, "mmap called, vma=0x%08lx\n", (unsigned long)vma);
>> @@ -519,7 +520,11 @@ static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma)
>>   	if (icd->streamer != file)
>>   		return -EBUSY;
>>
>> -	err = videobuf_mmap_mapper(&icd->vb_vidq, vma);
>> +	/* Check for an interface custom mmaper */
>
> mmapper - double 'p'

Oops. Will fix.

>
>> +	if (ici->ops->mmap)
>> +		err = ici->ops->mmap(&icd->vb_vidq, icd, vma);
>> +	else
>> +		err = videobuf_mmap_mapper(&icd->vb_vidq, vma);
>
> You're patching an old version of soc-camera. Please use a current one
> with support for videobuf2. Further, wouldn't it be possible for you to
> just replace the videobuf mmap_mapper() (videobuf2 q->mem_ops->mmap())
> method? I am not sure how possible this is, maybe one of videobuf2 experts
> could help us? BTW, you really should be using the videobuf2 API.

I'm basing this patches on mainline, commit:

commit 214d93b02c4fe93638ad268613c9702a81ed9192
Merge: ad4a4a8 077f8ec
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon Mar 7 13:15:02 2011 -0800

     Merge branch 'omap-fixes-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6

And i don't see videobuf2 there.

Should I rebase my patches on another tree?

Regards,
Sergio


>
>>
>>   	dev_dbg(&icd->dev, "vma start=0x%08lx, size=%ld, ret=%d\n",
>>   		(unsigned long)vma->vm_start,
>> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
>> index de81370..11350c2 100644
>> --- a/include/media/soc_camera.h
>> +++ b/include/media/soc_camera.h
>> @@ -87,6 +87,8 @@ struct soc_camera_host_ops {
>>   	int (*set_ctrl)(struct soc_camera_device *, struct v4l2_control *);
>>   	int (*get_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
>>   	int (*set_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
>> +	int (*mmap)(struct videobuf_queue *, struct soc_camera_device *,
>> +		     struct vm_area_struct *);
>>   	unsigned int (*poll)(struct file *, poll_table *);
>>   	const struct v4l2_queryctrl *controls;
>>   	int num_controls;
>> --
>> 1.7.1
>>
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/