Hi Marek, On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote: > On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote: > > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote: > > > >> 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 ? > > > > > > I don't think so. In case of S5P drivers, vb2_queue_init is called > > > while opening /dev/videoX. > > > > > > BTW. This struct device may help vb2 to produce logs with more > > > descriptive client annotation. > > > > > > What happens if such a device is NULL. It would happen for vmalloc > > > allocator used by VIVI? > > > > Good question. Should dma-buf accept NULL devices ? Or should vivi pass > > its V4L2 device to vb2 ? > > I assume you suggested using struct video_device->dev entry in such case. > It will not work. DMA-mapping API requires some parameters to be set for > the client device, like for example dma mask. struct video_device contains > only an artificial struct device entry, which has no relation to any > physical device and cannot be used for calling DMA-mapping functions. > > Performing dma_map_* operations with such artificial struct device doesn't > make any sense. It also slows down things significantly due to cache > flushing (forced by dma-mapping) which should be avoided if the buffer is > accessed only with CPU (like it is done by vb2-vmalloc style drivers). I agree that mapping the buffer to the physical device doesn't make any sense, as there's simple no physical device to map the buffer to. In that case we could simply skip the dma_map/dma_unmap calls. Note, however, that dma-buf v1 explicitly does not support CPU access by the importer. > IMHO this case perfectly shows the design mistake that have been made. The > current version simply tries to do too much. > > Each client of dma_buf should 'map' the provided sgtable/scatterlist on its > own. Only the client device driver has all knowledge to make a proper > 'mapping'. Real physical devices usually will use dma_map_sg() for such > operation, while some virtual ones will only create a kernel mapping for > the provided scatterlist (like vivi with vmalloc memory module). I tend to agree with that. Depending on the importer device, drivers could then map/unmap the buffer around each DMA access, or keep a mapping and sync the buffer. What about splitting the map_dma_buf operation into an operation that backs the buffer with pages and returns an sg_list, and an operation that performs DMA synchronization with the exporter ? unmap_dma_buf would similarly be split in two operations. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
