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. 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... 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); 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)? _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
