On Wed, Jun 05, 2013 at 11:52:59AM +0900, 김승우 wrote:
On 2013년 06월 04일 21:55, Daniel Vetter wrote:
On Tue, Jun 04, 2013 at 07:42:22PM +0900, 김승우 wrote:
On 2013년 06월 01일 00:29, Daniel Vetter wrote:
On Fri, May 31, 2013 at 07:22:24PM +0900, 김승우 wrote:
Hello Daniel,
Thanks for your comment.
On 2013년 05월 31일 18:14, Daniel Vetter wrote:
On Fri, May 31, 2013 at 10:54 AM, Seung-Woo Kim sw0312.kim@samsung.com wrote: > importer private data in dma-buf attachment can be used by importer to > reimport same dma-buf. > > Seung-Woo Kim (2): > dma-buf: add importer private data to attachment > drm/prime: find gem object from the reimported dma-buf
Self-import should already work (at least with the latest refcount fixes merged). At least the tests to check both re-import on the same drm fd and on a different all work as expected now.
Currently, prime works well for all case including self-importing, importing, and reimporting as you describe. Just, importing dma-buf from other driver twice with different drm_fd, each import create its own gem object even two import is done for same buffer because prime_priv is in struct drm_file. This means mapping to the device is done also twice. IMHO, these duplicated creations and maps are not necessary if drm can find previous import in different prime_priv.
Well, that's imo a bug with the other driver. If it doesn't export something really simple (e.g. contiguous memory which doesn't require any mmio resources at all) it should have a cache of exported dma_buf fds so that it hands out the same dma_buf every time.
Hm, all existing dma-buf exporter including i915 driver implements its map_dma_buf callback as allocating scatter-gather table with pages in its buffer and calling dma_map_sg() with the sgt. With different drm_fds, importing one dma-buf *twice*, then importer calls dma_buf_attach() and dma_buf_map_attachment() twice at least in drm importer because re-importing case can only checked with prime_priv in drm_file as I described.
Well, but thanks to all the self-import and re-import checks, it's _impossible_ to import the same dma_buf twice without noticing (presuming both importer and exporter are drm devices).
No, it is possible. Prime function, drm_gem_prime_fd_to_handle(), checks re-import with following code.
ret = drm_prime_lookup_buf_handle(&file_priv->prime, dma_buf, handle);
Unfortunately, file_priv is allocated per each open of drm node so this code can only find re-import within same drm open context.
And driver specific import functions, like drm_gem_prime_import(), only check self-import like following code.
if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) { obj = dma_buf->priv; if (obj->dev == dev) { /* ... */ } }
This means some application like following can make re-import to different gem objects.
int drm_fd1, drm_fd2, ret; int dma_buf_fd; struct drm_prime_handle prime1, prime2;
drm_fd1 = open(DRM_NODE, O_RDWR, 0); drm_fd2 = open(DRM_NODE, O_RDWR, 0);
/* get some dma-buf_fd from other dma-buf exporter */ prime1.fd = dma_buf_fd; prime2.fd = dma_buf_fd;
ret = ioctl(drm_fd1, DRM_IOCTL_PRIME_FD_TO_HANDLE, &prime1); ret = ioctl(drm_fd2, DRM_IOCTL_PRIME_FD_TO_HANDLE, &prime2);
This will import same dma-buf twice as different GEM object because above checking codes can not check already imported gem object from the dma-buf.
Oh right, now I understand. Somehow I've always thought we already take care of this case, since I remember discussing it.
To fix that we need a device-global import cache similar to how we already have one for each file_priv. I think we can reuse the same locking and refcounting scheme, but I haven't checked. The commit messages of the past few prime changes have fairly good explanations of the tricky stuff going on there.
Sorry for being dense for so long, I should have checked my idea of what the drm prime code does with reality sooner ;-)
Cheers, Daniel