Hi Tomasz,
On Friday 20 January 2012 16:53:20 Tomasz Stanislawski wrote:
On 01/20/2012 04:12 PM, Laurent Pinchart wrote:
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 Osciakpawel@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 / detachcould 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.
I haven't looked into the details, but that sounds good to me. Do we have use cases where a queue is allocated before knowing which physical device it will be used for ?