On Wed, Jun 12, 2013 at 1:05 PM, Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote: > On Wed, Jun 12, 2013 at 05:49:14PM +0100, Russell King - ARM Linux wrote: >> On Wed, Jun 12, 2013 at 09:56:22AM -0400, Rob Clark wrote: >> > On Wed, Jun 12, 2013 at 9:48 AM, Russell King - ARM Linux >> > <linux@xxxxxxxxxxxxxxxx> wrote: >> > > And having thought about this driver, DRM some more, I'm now of the >> > > opinion that DRM is not suitable for driving hardware where the GPU is >> > > an entirely separate IP block from the display side. >> > > >> > > DRM is modelled after the PC setup where your "graphics card" does >> > > everything - it has the GPU, display and connectors all integrated >> > > together. This is not the case on embedded SoCs, which can be a >> > > collection of different IPs all integrated together. >> > >> > actually it isn't even the case on desktop/laptop anymore, where you >> > can have one gpu with scanout and a second one without (or just with >> > display controller not hooked up to anything, etc, etc) >> > >> > That is the point of dmabuf and the upcoming fence/reservation stuff. >> >> Okay, but dmabuf really needs to be fixed, because as it stands this API >> is really quite broken wrt the DMA API. dma_map_sg() is (a) not supposed >> to have its return value ignored - mappings can fail, and (b) the returned >> number indicates how many entries are valid for the _mapped_ version of >> the scatterlist. >> >> Both these points are important if your DMA API implementation uses an >> IOMMU, which may coalesce the scatterlist array when creating mappings - >> much as described in Documentation/DMA-API.txt and >> Documentation/DMA-API-HOWTO.txt. >> >> That is not all DRMs fault - (a) is attributable to DRM's prime >> implementation: >> >> sgt = obj->dev->driver->gem_prime_get_sg_table(obj); >> >> if (!IS_ERR_OR_NULL(sgt)) >> dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir); >> >> and quite why it does the dma_map_sg() beneath the struct_mutex is >> beyond me - if the array of pages isn't safe without the mutex being >> held, then it isn't safe after the dma_map_sg() operation has completed >> and the mutex has been released. >> >> However, (b) is more a problem for dmabuf (which I just rather aptly >> mistyped as dmabug) because either dmabuf's .map_dma_buf method needs >> to return the value from dma_map_sg(), or it needs to stop requiring >> this of the .map_dma_buf method and have it done by the caller of >> this method so the caller can have access to that returned value. >> >> Added Sumit Semwal to this email for the dmabuf issue. >> >> Thankfully, this being correct isn't a requirement for me, but it's >> something which _should_ be fixed. (just some quick answers.. I'll read rest of thread a bit later when I have some time) > Okay, so Sumit Semwal has been a victim of TI's cuts, so don't expect > dma_buf to get sorted by the original author... Sumit is at linaro, and should still be caring for dma_buf (although w/ different email addr) > Now we come to this: > > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c: return dma_buf_export(exynos_gem_obj, &exynos_dmabuf_ops, > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c- exynos_gem_obj->base.size, flags); > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c: return dma_buf_export(obj, &omap_dmabuf_ops, obj->size, flags); > drivers/gpu/drm/drm_prime.c: return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size, > drivers/gpu/drm/drm_prime.c- 0600); > drivers/gpu/drm/i915/i915_gem_dmabuf.c: return dma_buf_export(obj, &i915_dmabuf_ops, obj->base.size, flags); the drm_prime.c version was a more recent addition of "dmabuf helpers".. not all the drivers use 'em (which seems to be a good thing) > Of the three implementations which don't use the generic version, they all > pass 'flags' to dma_buf_export. drm_prime.c doesn't, it passes a fixed > file mode. What's the correct version, or is flags | 0600 actually the > right one (as flags only contains O_CLOEXEC)? the drm_prime version is wrong.. the O_CLOEXEC flag should have been passed along from what userspace requested when exporting the dmabuf BR, -R _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
