Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

[Tomasz Stanislawski <t.stanislaws@samsung.com>]

Hi Laurent,

On 01/20/2012 04:12 PM, Laurent Pinchart wrote:
> Hi Tomasz,
>
> On Friday 20 January 2012 11:58:39 Tomasz Stanislawski wrote:
>> On 01/20/2012 11:41 AM, Sumit Semwal wrote:
>>> On 20 January 2012 00:37, Pawel Osciak<pawel@osciak.com>   wrote:
>>>> Hi Sumit,
>>>> Thank you for your work. Please find my comments below.
>>>
>>> Hi Pawel,
>>>
<snip>
>>>>>    struct vb2_mem_ops {
>>>>>
>>>>>          void            *(*alloc)(void *alloc_ctx, unsigned long size);
>>>>>
>>>>> @@ -65,6 +82,16 @@ struct vb2_mem_ops {
>>>>>
>>>>>                                          unsigned long size, int write);
>>>>>
>>>>>          void            (*put_userptr)(void *buf_priv);
>>>>>
>>>>> +       /* Comment from Rob Clark: XXX: I think the attach / detach
>>>>> could be handled +        * in the vb2 core, and vb2_mem_ops really
>>>>> just need to get/put the +        * sglist (and make sure that the
>>>>> sglist fits it's needs..) +        */
>>>>
>>>> I *strongly* agree with Rob here. Could you explain the reason behind
>>>> not doing this?
>>>> Allocator should ideally not have to be aware of attaching/detaching,
>>>> this is not specific to an allocator.
>>>
>>> Ok, I thought we'll start with this version first, and then refine.
>>> But you guys are right.
>>
>> I think that it is not possible to move attach/detach to vb2-core. The
>> problem is that dma_buf_attach needs 'struct device' argument. This
>> pointer is not available in vb2-core. This pointer is delivered by
>> device's driver in "void *alloc_context".
>>
>> Moving information about device would introduce new problems like:
>> - breaking layering in vb2
>> - some allocators like vb2-vmalloc do not posses any device related
>> attributes
>
> What about passing the device to vb2-core then ?
>

IMO, One way to do this is adding field 'struct device *dev' to struct 
vb2_queue. This field should be filled by a driver prior to calling 
vb2_queue_init.

Regards,
Tomasz Stanislawski