On 12/06/2012 10:36 PM, Inki Dae wrote: > > Hi, > > CCing media guys. > > I agree with you but we should consider one issue released to v4l2. > > As you may know, V4L2-based driver uses vb2 as buffer manager and the > vb2 includes dmabuf feature>(import and export) And v4l2 uses streaming > concept>(qbuf and dqbuf) > With dmabuf and iommu, generally qbuf imports a fd into its own buffer > and maps it with its own iommu table calling dma_buf_map_attachment(). > And dqbuf calls dma_buf_unmap_attachment() to unmap that buffer from its > own iommu table. > But now vb2's unmap_dma_buf callback is nothing to do. I think that the > reason is the below issue, > > If qbuf maps buffer with iomm table and dqbuf unmaps it from iommu table > then it has performance deterioration because qbuf and dqbuf are called > repeatedly. > And this means map/unmap are repeated also. So I think media guys moved > dma_unmap_sg call from its own unmap_dma_buf callback to detach callback > instead. > For this, you can refer to vb2_dc_dmabuf_ops_unmap and > vb2_dc_dmabuf_ops_detach function. > > So I added the below patch to avoid that performance deterioration and > am testing it now.(this patch is derived from videobuf2-dma-contig.c) > http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git;a=commit;h=576b1c3de8b90cf1570b8418b60afd1edaae4e30 > > Thus, I'm not sure that your common set could cover all the cases > including other frameworks. Please give me any opinions.
It seems like this adjustment would make perfect sense to add to the helper layer I suggested. E.g., instead of having an exynos_attach structure that caches the sgt, there'd be a struct drm_gem_prime_attach that would do the same thing, and save the sgt it gets from driver->gem_prime_get_sg. Then it would benefit nouveau and radeon, too. Alternatively, patch #4 could be dropped and Exynos can continue to reimplement all of this core functionality, since the helpers are optional, but I don't see anything about this change that should make it Exynos-specific, unless I'm missing something. -- Aaron
