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

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

Hi Sumit and Pawel,
Please find comments below.
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,
>
> Thank you for finding time for this review, and your comments :) - my
> comments inline.
> [Also, as an aside, Tomasz has also been working on the vb2 adaptation
> to dma-buf, and his patches should be more comprehensive, in that he
> is also planning to include 'vb2 as exporter' of dma-buf. He might
> take and improve on this RFC, so it might be worthwhile to wait for
> it?]
>>

<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

Best regards,
Tomasz Stanislawski

>
>> --
>> Best regards,
>> Pawel Osciak
>