<div class="gmail_quote">2012/12/8 Aaron Plattner <<a href="mailto:aplattner@nvidia.com" target="_blank">aplattner@nvidia.com</a>> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On 12/06/2012 10:36 PM, Inki Dae wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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) <a href="http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git;a=commit;h=576b1c3de8b90cf1570b8418b60afd1edaae4e30" target="_blank">http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git;a=commit;h=576b1c3de8b90cf1570b8418b60afd1edaae4e30</a> Thus, I'm not sure that your common set could cover all the cases including other frameworks. Please give me any opinions. </blockquote> </div></div> 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. </blockquote><div> If this change is applied to common helper and also that could be accepted by other maintainers then I think it's better to use this common helper instead of specific one. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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,</blockquote> <div> I agree with you. I also think this change isn't specific to Exynos. But you need to check if this is a reasonable change for other drivers also. Thanks, Inki Dae </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> unless I'm missing something. </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -- Aaron<div class="HOEnZb"><div class="h5"> _______________________________________________ dri-devel mailing list <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a> </div></div></blockquote></div>