* [PATCH v3 0/2] drm/dumb-buffers: Add defaults for .dumb_map_offset and .dumb_destroy @ 2017-08-17 16:21 Noralf Trønnes 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes 2017-08-17 16:21 ` [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults Noralf Trønnes 0 siblings, 2 replies; 29+ messages in thread From: Noralf Trønnes @ 2017-08-17 16:21 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, linux, laurent.pinchart This is a follow up that lets armada also use the default .dumb_map_offset. First version: https://lists.freedesktop.org/archives/dri-devel/2017-July/148101.html Changes since version 2 ----------------------- - drm_gem_dumb_map_offset(): reject imported buffers (Daniel Vetter) - armada can now also use drm_gem_dumb_map_offset() Changes since version 1 ----------------------- - Exynos can also use drm_gem_dumb_map_offset() (Emil Velikov) - Remove drm_gem_cma_dumb_map_offset() (Philipp Zabel) Noralf. Noralf Trønnes (2): drm/gem: drm_gem_dumb_map_offset(): reject dma-buf drm/armada: Use .dumb_map_offset and .dumb_destroy defaults drivers/gpu/drm/armada/armada_drv.c | 2 -- drivers/gpu/drm/armada/armada_gem.c | 36 ------------------------------------ drivers/gpu/drm/armada/armada_gem.h | 4 ---- drivers/gpu/drm/drm_gem.c | 6 ++++++ 4 files changed, 6 insertions(+), 42 deletions(-) -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 16:21 [PATCH v3 0/2] drm/dumb-buffers: Add defaults for .dumb_map_offset and .dumb_destroy Noralf Trønnes @ 2017-08-17 16:21 ` Noralf Trønnes 2017-08-17 17:56 ` Laurent Pinchart ` (2 more replies) 2017-08-17 16:21 ` [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults Noralf Trønnes 1 sibling, 3 replies; 29+ messages in thread From: Noralf Trønnes @ 2017-08-17 16:21 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, linux, laurent.pinchart Reject mapping an imported dma-buf since is's an invalid use-case. Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/drm_gem.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad4e9cf..8da5801 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, if (!obj) return -ENOENT; + /* Don't allow imported objects to be mapped */ + if (obj->import_attach) { + ret = -EINVAL; + goto out; + } + ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes @ 2017-08-17 17:56 ` Laurent Pinchart 2017-08-17 18:30 ` Chris Wilson 2017-08-18 7:46 ` Daniel Vetter 2017-08-21 5:26 ` Thierry Reding 2 siblings, 1 reply; 29+ messages in thread From: Laurent Pinchart @ 2017-08-17 17:56 UTC (permalink / raw) To: Noralf Trønnes; +Cc: daniel.vetter, dri-devel, linux Hi Noralf, Thank you for the patch. On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > Reject mapping an imported dma-buf since is's an invalid use-case. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/drm_gem.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ad4e9cf..8da5801 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, > struct drm_device *dev, if (!obj) > return -ENOENT; > > + /* Don't allow imported objects to be mapped */ > + if (obj->import_attach) { > + ret = -EINVAL; > + goto out; > + } > + Wouldn't it be better to move this check to drm_gem_create_mmap_offset_size() to reject mapping of all imported dmabuf, not just the ones associated with dumb buffers ? > ret = drm_gem_create_mmap_offset(obj); > if (ret) > goto out; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 17:56 ` Laurent Pinchart @ 2017-08-17 18:30 ` Chris Wilson 2017-08-17 19:01 ` Laurent Pinchart 0 siblings, 1 reply; 29+ messages in thread From: Chris Wilson @ 2017-08-17 18:30 UTC (permalink / raw) To: Laurent Pinchart, Noralf Trønnes; +Cc: daniel.vetter, linux, dri-devel Quoting Laurent Pinchart (2017-08-17 18:56:09) > Hi Noralf, > > Thank you for the patch. > > On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index ad4e9cf..8da5801 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, > > struct drm_device *dev, if (!obj) > > return -ENOENT; > > > > + /* Don't allow imported objects to be mapped */ > > + if (obj->import_attach) { > > + ret = -EINVAL; > > + goto out; > > + } > > + > > Wouldn't it be better to move this check to drm_gem_create_mmap_offset_size() > to reject mapping of all imported dmabuf, not just the ones associated with > dumb buffers ? No, we use that and we don't ban mapping an imported object. -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 18:30 ` Chris Wilson @ 2017-08-17 19:01 ` Laurent Pinchart 2017-08-17 19:09 ` Chris Wilson 0 siblings, 1 reply; 29+ messages in thread From: Laurent Pinchart @ 2017-08-17 19:01 UTC (permalink / raw) To: Chris Wilson; +Cc: daniel.vetter, linux, dri-devel Hi Chris, On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > Quoting Laurent Pinchart (2017-08-17 18:56:09) > > On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Cc: Sean Paul <seanpaul@chromium.org> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > --- > > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > index ad4e9cf..8da5801 100644 > > > --- a/drivers/gpu/drm/drm_gem.c > > > +++ b/drivers/gpu/drm/drm_gem.c > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, > > > struct drm_device *dev, if (!obj) > > > > > > return -ENOENT; > > > > > > + /* Don't allow imported objects to be mapped */ > > > + if (obj->import_attach) { > > > + ret = -EINVAL; > > > + goto out; > > > + } > > > + > > > > Wouldn't it be better to move this check to > > drm_gem_create_mmap_offset_size() to reject mapping of all imported > > dmabuf, not just the ones associated with dumb buffers ? > > No, we use that and we don't ban mapping an imported object. Do you use that with driver-specific buffers only or with dumb buffers too ? -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 19:01 ` Laurent Pinchart @ 2017-08-17 19:09 ` Chris Wilson 2017-08-17 19:40 ` Laurent Pinchart 0 siblings, 1 reply; 29+ messages in thread From: Chris Wilson @ 2017-08-17 19:09 UTC (permalink / raw) To: Laurent Pinchart; +Cc: daniel.vetter, linux, dri-devel Quoting Laurent Pinchart (2017-08-17 20:01:40) > Hi Chris, > > On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > > Quoting Laurent Pinchart (2017-08-17 18:56:09) > > > On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Cc: Sean Paul <seanpaul@chromium.org> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > > --- > > > > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > index ad4e9cf..8da5801 100644 > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, > > > > struct drm_device *dev, if (!obj) > > > > > > > > return -ENOENT; > > > > > > > > + /* Don't allow imported objects to be mapped */ > > > > + if (obj->import_attach) { > > > > + ret = -EINVAL; > > > > + goto out; > > > > + } > > > > + > > > > > > Wouldn't it be better to move this check to > > > drm_gem_create_mmap_offset_size() to reject mapping of all imported > > > dmabuf, not just the ones associated with dumb buffers ? > > > > No, we use that and we don't ban mapping an imported object. > > Do you use that with driver-specific buffers only or with dumb buffers too ? For dma-buf we import? The current presumption is that the sg_table is backed by struct page. How would we know otherwise? I am actually not sure what would happen if we tried to use another mmio bar from the GPU. For dumb buffers we create, they are just ordinary buffers. -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 19:09 ` Chris Wilson @ 2017-08-17 19:40 ` Laurent Pinchart 2017-08-17 20:01 ` Chris Wilson 0 siblings, 1 reply; 29+ messages in thread From: Laurent Pinchart @ 2017-08-17 19:40 UTC (permalink / raw) To: Chris Wilson; +Cc: daniel.vetter, linux, dri-devel Hi Chris, On Thursday 17 Aug 2017 20:09:20 Chris Wilson wrote: > Quoting Laurent Pinchart (2017-08-17 20:01:40) > > On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > > > Quoting Laurent Pinchart (2017-08-17 18:56:09) > > > > > > > On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > Cc: Sean Paul <seanpaul@chromium.org> > > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > > > --- > > > > > > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > > index ad4e9cf..8da5801 100644 > > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file > > > > > *file, > > > > > struct drm_device *dev, if (!obj) > > > > > > > > > > return -ENOENT; > > > > > > > > > > + /* Don't allow imported objects to be mapped */ > > > > > + if (obj->import_attach) { > > > > > + ret = -EINVAL; > > > > > + goto out; > > > > > + } > > > > > + > > > > > > > > Wouldn't it be better to move this check to > > > > drm_gem_create_mmap_offset_size() to reject mapping of all imported > > > > dmabuf, not just the ones associated with dumb buffers ? > > > > > > No, we use that and we don't ban mapping an imported object. > > > > Do you use that with driver-specific buffers only or with dumb buffers too > > ? > > > For dma-buf we import? The current presumption is that the sg_table is > backed by struct page. How would we know otherwise? I am actually not > sure what would happen if we tried to use another mmio bar from the GPU. > > For dumb buffers we create, they are just ordinary buffers. My question was whether you need to mmap the dma-buf you import through the dumb buffers API (which this patch prevents for drivers using the GEM dumb helpers), or only through your driver-specific buffer API. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 19:40 ` Laurent Pinchart @ 2017-08-17 20:01 ` Chris Wilson 2017-08-17 22:35 ` Laurent Pinchart 0 siblings, 1 reply; 29+ messages in thread From: Chris Wilson @ 2017-08-17 20:01 UTC (permalink / raw) To: Laurent Pinchart; +Cc: daniel.vetter, linux, dri-devel Quoting Laurent Pinchart (2017-08-17 20:40:52) > Hi Chris, > > On Thursday 17 Aug 2017 20:09:20 Chris Wilson wrote: > > Quoting Laurent Pinchart (2017-08-17 20:01:40) > > > On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > > > > Quoting Laurent Pinchart (2017-08-17 18:56:09) > > > > > > > > > On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > > > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > Cc: Sean Paul <seanpaul@chromium.org> > > > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > > > > --- > > > > > > > > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > > > index ad4e9cf..8da5801 100644 > > > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file > > > > > > *file, > > > > > > struct drm_device *dev, if (!obj) > > > > > > > > > > > > return -ENOENT; > > > > > > > > > > > > + /* Don't allow imported objects to be mapped */ > > > > > > + if (obj->import_attach) { > > > > > > + ret = -EINVAL; > > > > > > + goto out; > > > > > > + } > > > > > > + > > > > > > > > > > Wouldn't it be better to move this check to > > > > > drm_gem_create_mmap_offset_size() to reject mapping of all imported > > > > > dmabuf, not just the ones associated with dumb buffers ? > > > > > > > > No, we use that and we don't ban mapping an imported object. > > > > > > Do you use that with driver-specific buffers only or with dumb buffers too > > > ? > > > > > For dma-buf we import? The current presumption is that the sg_table is > > backed by struct page. How would we know otherwise? I am actually not > > sure what would happen if we tried to use another mmio bar from the GPU. > > > > For dumb buffers we create, they are just ordinary buffers. > > My question was whether you need to mmap the dma-buf you import through the > dumb buffers API (which this patch prevents for drivers using the GEM dumb > helpers), or only through your driver-specific buffer API. i915_gem_mmap_gtt_ioctl -> drm_gem_create_mmap_offset/_size drvier->dumb_map_offset also uses the i915_gem_mmap_gtt mechanism. I was responding to the suggestion that this check could be centralised in drm_gem_create_mmap_offset_size, as that would prevent us from offering the mmap interface on imported buffers. -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 20:01 ` Chris Wilson @ 2017-08-17 22:35 ` Laurent Pinchart 2017-08-18 7:45 ` Daniel Vetter 0 siblings, 1 reply; 29+ messages in thread From: Laurent Pinchart @ 2017-08-17 22:35 UTC (permalink / raw) To: Chris Wilson; +Cc: daniel.vetter, linux, dri-devel Hi Chris, On Thursday 17 Aug 2017 21:01:54 Chris Wilson wrote: > Quoting Laurent Pinchart (2017-08-17 20:40:52) > > On Thursday 17 Aug 2017 20:09:20 Chris Wilson wrote: > >> Quoting Laurent Pinchart (2017-08-17 20:01:40) > >>> On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > >>>> Quoting Laurent Pinchart (2017-08-17 18:56:09) > >>>>> On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > >>>>>> Reject mapping an imported dma-buf since is's an invalid > >>>>>> use-case. > >>>>>> > >>>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de> > >>>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>>>> Cc: Sean Paul <seanpaul@chromium.org> > >>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>>>> --- > >>>>>> > >>>>>> drivers/gpu/drm/drm_gem.c | 6 ++++++ > >>>>>> 1 file changed, 6 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/gpu/drm/drm_gem.c > >>>>>> b/drivers/gpu/drm/drm_gem.c > >>>>>> index ad4e9cf..8da5801 100644 > >>>>>> --- a/drivers/gpu/drm/drm_gem.c > >>>>>> +++ b/drivers/gpu/drm/drm_gem.c > >>>>>> @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file > >>>>>> *file, struct drm_device *dev, > >>>>>> if (!obj) > >>>>>> return -ENOENT; > >>>>>> > >>>>>> + /* Don't allow imported objects to be mapped */ > >>>>>> + if (obj->import_attach) { > >>>>>> + ret = -EINVAL; > >>>>>> + goto out; > >>>>>> + } > >>>>>> + > >>>>> > >>>>> Wouldn't it be better to move this check to > >>>>> drm_gem_create_mmap_offset_size() to reject mapping of all > >>>>> imported dmabuf, not just the ones associated with dumb buffers ? > >>>> > >>>> No, we use that and we don't ban mapping an imported object. > >>> > >>> Do you use that with driver-specific buffers only or with dumb buffers > >>> too ? > >> > >> For dma-buf we import? The current presumption is that the sg_table is > >> backed by struct page. How would we know otherwise? I am actually not > >> sure what would happen if we tried to use another mmio bar from the GPU. > >> > >> For dumb buffers we create, they are just ordinary buffers. > > > > My question was whether you need to mmap the dma-buf you import through > > the dumb buffers API (which this patch prevents for drivers using the GEM > > dumb helpers), or only through your driver-specific buffer API. > > i915_gem_mmap_gtt_ioctl -> drm_gem_create_mmap_offset/_size > drvier->dumb_map_offset also uses the i915_gem_mmap_gtt mechanism. > > I was responding to the suggestion that this check could be centralised in > drm_gem_create_mmap_offset_size, as that would prevent us from offering > the mmap interface on imported buffers. Fair enough. I wonder, however, why mmap()ing an imported buffer through the dumb buffers API is an invalid use case, while doing the same through the driver-specific buffer API isn't. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 22:35 ` Laurent Pinchart @ 2017-08-18 7:45 ` Daniel Vetter 2017-08-18 8:01 ` Laurent Pinchart 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-18 7:45 UTC (permalink / raw) To: Laurent Pinchart; +Cc: daniel.vetter, linux, dri-devel On Fri, Aug 18, 2017 at 01:35:41AM +0300, Laurent Pinchart wrote: > Hi Chris, > > On Thursday 17 Aug 2017 21:01:54 Chris Wilson wrote: > > Quoting Laurent Pinchart (2017-08-17 20:40:52) > > > On Thursday 17 Aug 2017 20:09:20 Chris Wilson wrote: > > >> Quoting Laurent Pinchart (2017-08-17 20:01:40) > > >>> On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > > >>>> Quoting Laurent Pinchart (2017-08-17 18:56:09) > > >>>>> On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > > >>>>>> Reject mapping an imported dma-buf since is's an invalid > > >>>>>> use-case. > > >>>>>> > > >>>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de> > > >>>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >>>>>> Cc: Sean Paul <seanpaul@chromium.org> > > >>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > >>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > >>>>>> --- > > >>>>>> > > >>>>>> drivers/gpu/drm/drm_gem.c | 6 ++++++ > > >>>>>> 1 file changed, 6 insertions(+) > > >>>>>> > > >>>>>> diff --git a/drivers/gpu/drm/drm_gem.c > > >>>>>> b/drivers/gpu/drm/drm_gem.c > > >>>>>> index ad4e9cf..8da5801 100644 > > >>>>>> --- a/drivers/gpu/drm/drm_gem.c > > >>>>>> +++ b/drivers/gpu/drm/drm_gem.c > > >>>>>> @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file > > >>>>>> *file, struct drm_device *dev, > > >>>>>> if (!obj) > > >>>>>> return -ENOENT; > > >>>>>> > > >>>>>> + /* Don't allow imported objects to be mapped */ > > >>>>>> + if (obj->import_attach) { > > >>>>>> + ret = -EINVAL; > > >>>>>> + goto out; > > >>>>>> + } > > >>>>>> + > > >>>>> > > >>>>> Wouldn't it be better to move this check to > > >>>>> drm_gem_create_mmap_offset_size() to reject mapping of all > > >>>>> imported dmabuf, not just the ones associated with dumb buffers ? > > >>>> > > >>>> No, we use that and we don't ban mapping an imported object. > > >>> > > >>> Do you use that with driver-specific buffers only or with dumb buffers > > >>> too ? > > >> > > >> For dma-buf we import? The current presumption is that the sg_table is > > >> backed by struct page. How would we know otherwise? I am actually not > > >> sure what would happen if we tried to use another mmio bar from the GPU. > > >> > > >> For dumb buffers we create, they are just ordinary buffers. > > > > > > My question was whether you need to mmap the dma-buf you import through > > > the dumb buffers API (which this patch prevents for drivers using the GEM > > > dumb helpers), or only through your driver-specific buffer API. > > > > i915_gem_mmap_gtt_ioctl -> drm_gem_create_mmap_offset/_size > > drvier->dumb_map_offset also uses the i915_gem_mmap_gtt mechanism. > > > > I was responding to the suggestion that this check could be centralised in > > drm_gem_create_mmap_offset_size, as that would prevent us from offering > > the mmap interface on imported buffers. > > Fair enough. I wonder, however, why mmap()ing an imported buffer through the > dumb buffers API is an invalid use case, while doing the same through the > driver-specific buffer API isn't. Because i915 is special. Our offset-based mmap goes throug the gart, and for that all we need is to dma-map the sg table, which is the primary use-case for dma-buf. Which means we can always do that. But yeah it's special. In general dma-buf mmap on imported stuff isn't really a good idea, if you want that, use the mmap on the dma-buf itself. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 7:45 ` Daniel Vetter @ 2017-08-18 8:01 ` Laurent Pinchart 2017-08-18 8:21 ` Chris Wilson 0 siblings, 1 reply; 29+ messages in thread From: Laurent Pinchart @ 2017-08-18 8:01 UTC (permalink / raw) To: Daniel Vetter; +Cc: daniel.vetter, linux, dri-devel Hi Daniel, On Friday 18 Aug 2017 09:45:48 Daniel Vetter wrote: > On Fri, Aug 18, 2017 at 01:35:41AM +0300, Laurent Pinchart wrote: > > On Thursday 17 Aug 2017 21:01:54 Chris Wilson wrote: > >> Quoting Laurent Pinchart (2017-08-17 20:40:52) > >>> On Thursday 17 Aug 2017 20:09:20 Chris Wilson wrote: > >>>> Quoting Laurent Pinchart (2017-08-17 20:01:40) > >>>>> On Thursday 17 Aug 2017 19:30:44 Chris Wilson wrote: > >>>>>> Quoting Laurent Pinchart (2017-08-17 18:56:09) > >>>>>>> On Thursday 17 Aug 2017 18:21:30 Noralf Trønnes wrote: > >>>>>>>> Reject mapping an imported dma-buf since is's an invalid > >>>>>>>> use-case. > >>>>>>>> > >>>>>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de> > >>>>>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>>>>>> Cc: Sean Paul <seanpaul@chromium.org> > >>>>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>>>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> drivers/gpu/drm/drm_gem.c | 6 ++++++ > >>>>>>>> 1 file changed, 6 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/drivers/gpu/drm/drm_gem.c > >>>>>>>> b/drivers/gpu/drm/drm_gem.c > >>>>>>>> index ad4e9cf..8da5801 100644 > >>>>>>>> --- a/drivers/gpu/drm/drm_gem.c > >>>>>>>> +++ b/drivers/gpu/drm/drm_gem.c > >>>>>>>> @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file > >>>>>>>> *file, struct drm_device *dev, > >>>>>>>> if (!obj) > >>>>>>>> return -ENOENT; > >>>>>>>> > >>>>>>>> + /* Don't allow imported objects to be mapped */ > >>>>>>>> + if (obj->import_attach) { > >>>>>>>> + ret = -EINVAL; > >>>>>>>> + goto out; > >>>>>>>> + } > >>>>>>>> + > >>>>>>> > >>>>>>> Wouldn't it be better to move this check to > >>>>>>> drm_gem_create_mmap_offset_size() to reject mapping of all > >>>>>>> imported dmabuf, not just the ones associated with dumb buffers ? > >>>>>> > >>>>>> No, we use that and we don't ban mapping an imported object. > >>>>> > >>>>> Do you use that with driver-specific buffers only or with dumb > >>>>> buffers too ? > >>>> > >>>> For dma-buf we import? The current presumption is that the sg_table > >>>> is backed by struct page. How would we know otherwise? I am actually > >>>> not sure what would happen if we tried to use another mmio bar from > >>>> the GPU. > >>>> > >>>> For dumb buffers we create, they are just ordinary buffers. > >>> > >>> My question was whether you need to mmap the dma-buf you import > >>> through the dumb buffers API (which this patch prevents for drivers > >>> using the GEM dumb helpers), or only through your driver-specific > >>> buffer API. > >> > >> i915_gem_mmap_gtt_ioctl -> drm_gem_create_mmap_offset/_size > >> drvier->dumb_map_offset also uses the i915_gem_mmap_gtt mechanism. > >> > >> I was responding to the suggestion that this check could be centralised > >> in drm_gem_create_mmap_offset_size, as that would prevent us from > >> offering the mmap interface on imported buffers. > > > > Fair enough. I wonder, however, why mmap()ing an imported buffer through > > the dumb buffers API is an invalid use case, while doing the same through > > the driver-specific buffer API isn't. > > Because i915 is special. Our offset-based mmap goes throug the gart, and > for that all we need is to dma-map the sg table, which is the primary > use-case for dma-buf. Which means we can always do that. But yeah it's > special. I'll take this as meaning that i915 shouldn't allow this use case, but does and will need to continue doing so because we can't break userspace (it would be nice to start moving userspace away from mmap()ing imported buffers though). > In general dma-buf mmap on imported stuff isn't really a good idea, if you > want that, use the mmap on the dma-buf itself. Should we disallow this in all drivers that don't depend on it yet ? -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 8:01 ` Laurent Pinchart @ 2017-08-18 8:21 ` Chris Wilson 0 siblings, 0 replies; 29+ messages in thread From: Chris Wilson @ 2017-08-18 8:21 UTC (permalink / raw) To: Laurent Pinchart, Daniel Vetter; +Cc: daniel.vetter, linux, dri-devel Quoting Laurent Pinchart (2017-08-18 09:01:38) > Hi Daniel, > > On Friday 18 Aug 2017 09:45:48 Daniel Vetter wrote: > > On Fri, Aug 18, 2017 at 01:35:41AM +0300, Laurent Pinchart wrote: > > > Fair enough. I wonder, however, why mmap()ing an imported buffer through > > > the dumb buffers API is an invalid use case, while doing the same through > > > the driver-specific buffer API isn't. > > > > Because i915 is special. Our offset-based mmap goes throug the gart, and > > for that all we need is to dma-map the sg table, which is the primary > > use-case for dma-buf. Which means we can always do that. But yeah it's > > special. > > I'll take this as meaning that i915 shouldn't allow this use case, but does > and will need to continue doing so because we can't break userspace (it would > be nice to start moving userspace away from mmap()ing imported buffers > though). We do like pretend ingthat all GEM handles userspace has are equal. Partly this is due to the lack of side-channels in protocols like DRI2/DRI3 where all shared buffers must be fully capable or else either party may die in weird ways. Keeping everything first class allows for a simple agnostic userspace. Alas that is not the case, but the one thing we do have atm is that we can rely on using a mmap via the GTT. If we throw that out the window, we have teach all of our userspace about transfer buffers as the final fallback. (Yes, history repeats.) -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes 2017-08-17 17:56 ` Laurent Pinchart @ 2017-08-18 7:46 ` Daniel Vetter 2017-08-18 16:13 ` Noralf Trønnes 2017-08-21 5:26 ` Thierry Reding 2 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-18 7:46 UTC (permalink / raw) To: Noralf Trønnes; +Cc: daniel.vetter, linux, dri-devel, laurent.pinchart On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: > Reject mapping an imported dma-buf since is's an invalid use-case. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> I think acks from someone using mali would be good too. amdgpu already has such checks, so I think on the desktop side we're ok. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> But I think this one here definitely needs a few more acks. I could break uabi if we're unlucky, so let's not rush it. -Daniel > --- > drivers/gpu/drm/drm_gem.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ad4e9cf..8da5801 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > if (!obj) > return -ENOENT; > > + /* Don't allow imported objects to be mapped */ > + if (obj->import_attach) { > + ret = -EINVAL; > + goto out; > + } > + > ret = drm_gem_create_mmap_offset(obj); > if (ret) > goto out; > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 7:46 ` Daniel Vetter @ 2017-08-18 16:13 ` Noralf Trønnes 2017-08-18 17:41 ` Eric Anholt ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Noralf Trønnes @ 2017-08-18 16:13 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel (cc affected parties) Den 18.08.2017 09.46, skrev Daniel Vetter: > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: >> Reject mapping an imported dma-buf since is's an invalid use-case. >> >> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Sean Paul <seanpaul@chromium.org> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > I think acks from someone using mali would be good too. amdgpu already has > such checks, so I think on the desktop side we're ok. > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > But I think this one here definitely needs a few more acks. I could break > uabi if we're unlucky, so let's not rush it. Ok, I've CC'ed the affected parties to increase the odds that they look at this. These are the drivers using drm_gem_dumb_map_offset() (hopefully I got the list right): arc atmel-hlcdc cirrus exynos fsl-dcu gma500 hdlcd imx kirin mali-dp mediatek meson mxsfb pl111 rcar-du rockchip shmobile sti stm sun4i tegra tilcd vc4 zte Noralf. > -Daniel > >> --- >> drivers/gpu/drm/drm_gem.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> index ad4e9cf..8da5801 100644 >> --- a/drivers/gpu/drm/drm_gem.c >> +++ b/drivers/gpu/drm/drm_gem.c >> @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, >> if (!obj) >> return -ENOENT; >> >> + /* Don't allow imported objects to be mapped */ >> + if (obj->import_attach) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> ret = drm_gem_create_mmap_offset(obj); >> if (ret) >> goto out; >> -- >> 2.7.4 >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 16:13 ` Noralf Trønnes @ 2017-08-18 17:41 ` Eric Anholt 2017-08-18 20:17 ` Daniel Vetter 2017-08-24 10:04 ` Brian Starkey 2017-09-02 12:42 ` Noralf Trønnes 2 siblings, 1 reply; 29+ messages in thread From: Eric Anholt @ 2017-08-18 17:41 UTC (permalink / raw) To: Noralf Trønnes, Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel [-- Attachment #1.1: Type: text/plain, Size: 1287 bytes --] Noralf Trønnes <noralf@tronnes.org> writes: > (cc affected parties) > > > Den 18.08.2017 09.46, skrev Daniel Vetter: >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: >>> Reject mapping an imported dma-buf since is's an invalid use-case. >>> >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Cc: Sean Paul <seanpaul@chromium.org> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> I think acks from someone using mali would be good too. amdgpu already has >> such checks, so I think on the desktop side we're ok. >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> But I think this one here definitely needs a few more acks. I could break >> uabi if we're unlucky, so let's not rush it. > > Ok, I've CC'ed the affected parties to increase the odds that they look > at this. These are the drivers using drm_gem_dumb_map_offset() > (hopefully I got the list right): If I understand the affected path right, this would break the PL111+VC4 combination: PL111 makes (dumb) buffers for scanout, and VC4 imports them and uses them for rendering. A vc4 glReadPixels of the window system buffer would map it and fail. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 17:41 ` Eric Anholt @ 2017-08-18 20:17 ` Daniel Vetter 2017-08-18 20:34 ` Eric Anholt 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-18 20:17 UTC (permalink / raw) To: Eric Anholt Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote: > Noralf Trønnes <noralf@tronnes.org> writes: > > Den 18.08.2017 09.46, skrev Daniel Vetter: > >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: > >>> Reject mapping an imported dma-buf since is's an invalid use-case. > >>> > >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> > >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> Cc: Sean Paul <seanpaul@chromium.org> > >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >> I think acks from someone using mali would be good too. amdgpu already has > >> such checks, so I think on the desktop side we're ok. > >> > >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> > >> But I think this one here definitely needs a few more acks. I could break > >> uabi if we're unlucky, so let's not rush it. > > > > Ok, I've CC'ed the affected parties to increase the odds that they look > > at this. These are the drivers using drm_gem_dumb_map_offset() > > (hopefully I got the list right): > > If I understand the affected path right, this would break the PL111+VC4 > combination: PL111 makes (dumb) buffers for scanout, and VC4 imports > them and uses them for rendering. A vc4 glReadPixels of the window > system buffer would map it and fail. It only rejects the map call on dumb buffers, and mmap on imported dma-buf tends to not really work well, or at least break a few abstractions. Are you sure this works currently? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 20:17 ` Daniel Vetter @ 2017-08-18 20:34 ` Eric Anholt 2017-08-18 21:01 ` Daniel Vetter 0 siblings, 1 reply; 29+ messages in thread From: Eric Anholt @ 2017-08-18 20:34 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel [-- Attachment #1.1: Type: text/plain, Size: 2100 bytes --] Daniel Vetter <daniel@ffwll.ch> writes: > On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote: >> Noralf Trønnes <noralf@tronnes.org> writes: >> > Den 18.08.2017 09.46, skrev Daniel Vetter: >> >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: >> >>> Reject mapping an imported dma-buf since is's an invalid use-case. >> >>> >> >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >>> Cc: Sean Paul <seanpaul@chromium.org> >> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> >> I think acks from someone using mali would be good too. amdgpu already has >> >> such checks, so I think on the desktop side we're ok. >> >> >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> >> >> But I think this one here definitely needs a few more acks. I could break >> >> uabi if we're unlucky, so let's not rush it. >> > >> > Ok, I've CC'ed the affected parties to increase the odds that they look >> > at this. These are the drivers using drm_gem_dumb_map_offset() >> > (hopefully I got the list right): >> >> If I understand the affected path right, this would break the PL111+VC4 >> combination: PL111 makes (dumb) buffers for scanout, and VC4 imports >> them and uses them for rendering. A vc4 glReadPixels of the window >> system buffer would map it and fail. > > It only rejects the map call on dumb buffers, and mmap on imported dma-buf > tends to not really work well, or at least break a few abstractions. Are > you sure this works currently? OK, that's right -- vc4 would be doing its "native" map call (the same code), not dumb map. Furthermore, I had it backwards (I had written things both ways at different points, iirc). We have VC4 making the buffers and PL111 dma-buf importing them. I don't see X11 mapping those buffers if glamor is enabled, so this should be OK for vc4. GBM's dumb mapping looks safe to me. X11 does some dumb maps, but I don't think any of those would be on imports. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 20:34 ` Eric Anholt @ 2017-08-18 21:01 ` Daniel Vetter 2017-08-18 23:37 ` Eric Anholt 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-18 21:01 UTC (permalink / raw) To: Eric Anholt Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Fri, Aug 18, 2017 at 01:34:45PM -0700, Eric Anholt wrote: > Daniel Vetter <daniel@ffwll.ch> writes: > > > On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote: > >> Noralf Trønnes <noralf@tronnes.org> writes: > >> > Den 18.08.2017 09.46, skrev Daniel Vetter: > >> >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: > >> >>> Reject mapping an imported dma-buf since is's an invalid use-case. > >> >>> > >> >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> > >> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> >>> Cc: Sean Paul <seanpaul@chromium.org> > >> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >> >> I think acks from someone using mali would be good too. amdgpu already has > >> >> such checks, so I think on the desktop side we're ok. > >> >> > >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> >> > >> >> But I think this one here definitely needs a few more acks. I could break > >> >> uabi if we're unlucky, so let's not rush it. > >> > > >> > Ok, I've CC'ed the affected parties to increase the odds that they look > >> > at this. These are the drivers using drm_gem_dumb_map_offset() > >> > (hopefully I got the list right): > >> > >> If I understand the affected path right, this would break the PL111+VC4 > >> combination: PL111 makes (dumb) buffers for scanout, and VC4 imports > >> them and uses them for rendering. A vc4 glReadPixels of the window > >> system buffer would map it and fail. > > > > It only rejects the map call on dumb buffers, and mmap on imported dma-buf > > tends to not really work well, or at least break a few abstractions. Are > > you sure this works currently? > > OK, that's right -- vc4 would be doing its "native" map call (the same > code), not dumb map. > > Furthermore, I had it backwards (I had written things both ways at > different points, iirc). We have VC4 making the buffers and PL111 > dma-buf importing them. I don't see X11 mapping those buffers if glamor > is enabled, so this should be OK for vc4. > > GBM's dumb mapping looks safe to me. X11 does some dumb maps, but I > don't think any of those would be on imports. Yeah the idea is only to lock down the dumb mmap and make sure abi abuse (which might work on a specific combo of exporters/kms drivers) is caught for this generic interface. dumb really should only be used for unaccelarated rendering on exactly that driver. So ack from you? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 21:01 ` Daniel Vetter @ 2017-08-18 23:37 ` Eric Anholt 0 siblings, 0 replies; 29+ messages in thread From: Eric Anholt @ 2017-08-18 23:37 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel [-- Attachment #1.1: Type: text/plain, Size: 2975 bytes --] Daniel Vetter <daniel@ffwll.ch> writes: > On Fri, Aug 18, 2017 at 01:34:45PM -0700, Eric Anholt wrote: >> Daniel Vetter <daniel@ffwll.ch> writes: >> >> > On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote: >> >> Noralf Trønnes <noralf@tronnes.org> writes: >> >> > Den 18.08.2017 09.46, skrev Daniel Vetter: >> >> >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: >> >> >>> Reject mapping an imported dma-buf since is's an invalid use-case. >> >> >>> >> >> >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> >> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> >>> Cc: Sean Paul <seanpaul@chromium.org> >> >> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> >> >> I think acks from someone using mali would be good too. amdgpu already has >> >> >> such checks, so I think on the desktop side we're ok. >> >> >> >> >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> >> >> >> >> But I think this one here definitely needs a few more acks. I could break >> >> >> uabi if we're unlucky, so let's not rush it. >> >> > >> >> > Ok, I've CC'ed the affected parties to increase the odds that they look >> >> > at this. These are the drivers using drm_gem_dumb_map_offset() >> >> > (hopefully I got the list right): >> >> >> >> If I understand the affected path right, this would break the PL111+VC4 >> >> combination: PL111 makes (dumb) buffers for scanout, and VC4 imports >> >> them and uses them for rendering. A vc4 glReadPixels of the window >> >> system buffer would map it and fail. >> > >> > It only rejects the map call on dumb buffers, and mmap on imported dma-buf >> > tends to not really work well, or at least break a few abstractions. Are >> > you sure this works currently? >> >> OK, that's right -- vc4 would be doing its "native" map call (the same >> code), not dumb map. >> >> Furthermore, I had it backwards (I had written things both ways at >> different points, iirc). We have VC4 making the buffers and PL111 >> dma-buf importing them. I don't see X11 mapping those buffers if glamor >> is enabled, so this should be OK for vc4. >> >> GBM's dumb mapping looks safe to me. X11 does some dumb maps, but I >> don't think any of those would be on imports. > > Yeah the idea is only to lock down the dumb mmap and make sure abi abuse > (which might work on a specific combo of exporters/kms drivers) is caught > for this generic interface. dumb really should only be used for > unaccelarated rendering on exactly that driver. > > So ack from you? I'm hesitant, but that's mostly because I don't see the reason we're trying to lock it down, so it just looks like a chance of breakage from my perspective. However, given that some drivers are banning it already, let's make things consistent until we find we need to relax it globally. Acked-by: Eric Anholt <eric@anholt.net> [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 16:13 ` Noralf Trønnes 2017-08-18 17:41 ` Eric Anholt @ 2017-08-24 10:04 ` Brian Starkey 2017-08-25 13:34 ` Daniel Vetter 2017-09-02 12:42 ` Noralf Trønnes 2 siblings, 1 reply; 29+ messages in thread From: Brian Starkey @ 2017-08-24 10:04 UTC (permalink / raw) To: Noralf Tr??nnes Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel Hi, Thanks for the CC. On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote: >(cc affected parties) > > >Den 18.08.2017 09.46, skrev Daniel Vetter: >>On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote: >>>Reject mapping an imported dma-buf since is's an invalid use-case. >>> >>>Cc: Philipp Zabel <p.zabel@pengutronix.de> >>>Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>>Cc: Sean Paul <seanpaul@chromium.org> >>>Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>Signed-off-by: Noralf Tr??nnes <noralf@tronnes.org> >>I think acks from someone using mali would be good too. amdgpu already has >>such checks, so I think on the desktop side we're ok. >> This looks like it would break anyone running the Mali-4xx series GPUs with the Arm graphics stack (e.g. Hikey board). I don't know where that sits in terms of policy. Cheers, -Brian >>Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >>But I think this one here definitely needs a few more acks. I could break >>uabi if we're unlucky, so let's not rush it. > >Ok, I've CC'ed the affected parties to increase the odds that they look >at this. These are the drivers using drm_gem_dumb_map_offset() >(hopefully I got the list right): > >arc >atmel-hlcdc >cirrus >exynos >fsl-dcu >gma500 >hdlcd >imx >kirin >mali-dp >mediatek >meson >mxsfb >pl111 >rcar-du >rockchip >shmobile >sti >stm >sun4i >tegra >tilcd >vc4 >zte > > >Noralf. > >>-Daniel >> >>>--- >>> drivers/gpu/drm/drm_gem.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>>diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>>index ad4e9cf..8da5801 100644 >>>--- a/drivers/gpu/drm/drm_gem.c >>>+++ b/drivers/gpu/drm/drm_gem.c >>>@@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, >>> if (!obj) >>> return -ENOENT; >>>+ /* Don't allow imported objects to be mapped */ >>>+ if (obj->import_attach) { >>>+ ret = -EINVAL; >>>+ goto out; >>>+ } >>>+ >>> ret = drm_gem_create_mmap_offset(obj); >>> if (ret) >>> goto out; >>>-- >>>2.7.4 >>> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-24 10:04 ` Brian Starkey @ 2017-08-25 13:34 ` Daniel Vetter 2017-08-29 16:03 ` Brian Starkey 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-25 13:34 UTC (permalink / raw) To: Brian Starkey Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Thu, Aug 24, 2017 at 11:04:01AM +0100, Brian Starkey wrote: > Hi, > > Thanks for the CC. > > On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote: > > (cc affected parties) > > > > > > Den 18.08.2017 09.46, skrev Daniel Vetter: > > > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote: > > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Cc: Sean Paul <seanpaul@chromium.org> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Signed-off-by: Noralf Tr??nnes <noralf@tronnes.org> > > > I think acks from someone using mali would be good too. amdgpu already has > > > such checks, so I think on the desktop side we're ok. > > > > > This looks like it would break anyone running the Mali-4xx series GPUs > with the Arm graphics stack (e.g. Hikey board). > > I don't know where that sits in terms of policy. Why would this break this use-case? Is the mali blob somehow using dumb mmap for it's own buffers it shares with display? That would be rather backwards ... Either way, blob -> meh, not a concern really. -Daniel > > Cheers, > -Brian > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > But I think this one here definitely needs a few more acks. I could break > > > uabi if we're unlucky, so let's not rush it. > > > > Ok, I've CC'ed the affected parties to increase the odds that they look > > at this. These are the drivers using drm_gem_dumb_map_offset() > > (hopefully I got the list right): > > > > arc > > atmel-hlcdc > > cirrus > > exynos > > fsl-dcu > > gma500 > > hdlcd > > imx > > kirin > > mali-dp > > mediatek > > meson > > mxsfb > > pl111 > > rcar-du > > rockchip > > shmobile > > sti > > stm > > sun4i > > tegra > > tilcd > > vc4 > > zte > > > > > > Noralf. > > > > > -Daniel > > > > > > > --- > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > index ad4e9cf..8da5801 100644 > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > > > > if (!obj) > > > > return -ENOENT; > > > > + /* Don't allow imported objects to be mapped */ > > > > + if (obj->import_attach) { > > > > + ret = -EINVAL; > > > > + goto out; > > > > + } > > > > + > > > > ret = drm_gem_create_mmap_offset(obj); > > > > if (ret) > > > > goto out; > > > > -- > > > > 2.7.4 > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-25 13:34 ` Daniel Vetter @ 2017-08-29 16:03 ` Brian Starkey 2017-08-30 7:44 ` Daniel Vetter 0 siblings, 1 reply; 29+ messages in thread From: Brian Starkey @ 2017-08-29 16:03 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Fri, Aug 25, 2017 at 03:34:33PM +0200, Daniel Vetter wrote: >On Thu, Aug 24, 2017 at 11:04:01AM +0100, Brian Starkey wrote: >> Hi, >> >> Thanks for the CC. >> >> On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote: >> > (cc affected parties) >> > >> > >> > Den 18.08.2017 09.46, skrev Daniel Vetter: >> > > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote: >> > > > Reject mapping an imported dma-buf since is's an invalid use-case. >> > > > >> > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> >> > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> > > > Cc: Sean Paul <seanpaul@chromium.org> >> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> > > > Signed-off-by: Noralf Tr??nnes <noralf@tronnes.org> >> > > I think acks from someone using mali would be good too. amdgpu already has >> > > such checks, so I think on the desktop side we're ok. >> > > >> >> This looks like it would break anyone running the Mali-4xx series GPUs >> with the Arm graphics stack (e.g. Hikey board). >> >> I don't know where that sits in terms of policy. > >Why would this break this use-case? Is the mali blob somehow using dumb >mmap for it's own buffers it shares with display? That would be rather >backwards ... If it's running with a DRM display driver (which may or may not be Mali-DP, they are separate IPs), then its window system code always uses dumb mmap if it needs a pointer. That might be a native DRM buffer that the display driver allocated, or a dma-buf which it PRIME imported (which would now fail). -Brian > >Either way, blob -> meh, not a concern really. >-Daniel > >> >> Cheers, >> -Brian >> >> > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> > > >> > > But I think this one here definitely needs a few more acks. I could break >> > > uabi if we're unlucky, so let's not rush it. >> > >> > Ok, I've CC'ed the affected parties to increase the odds that they look >> > at this. These are the drivers using drm_gem_dumb_map_offset() >> > (hopefully I got the list right): >> > >> > arc >> > atmel-hlcdc >> > cirrus >> > exynos >> > fsl-dcu >> > gma500 >> > hdlcd >> > imx >> > kirin >> > mali-dp >> > mediatek >> > meson >> > mxsfb >> > pl111 >> > rcar-du >> > rockchip >> > shmobile >> > sti >> > stm >> > sun4i >> > tegra >> > tilcd >> > vc4 >> > zte >> > >> > >> > Noralf. >> > >> > > -Daniel >> > > >> > > > --- >> > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ >> > > > 1 file changed, 6 insertions(+) >> > > > >> > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> > > > index ad4e9cf..8da5801 100644 >> > > > --- a/drivers/gpu/drm/drm_gem.c >> > > > +++ b/drivers/gpu/drm/drm_gem.c >> > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, >> > > > if (!obj) >> > > > return -ENOENT; >> > > > + /* Don't allow imported objects to be mapped */ >> > > > + if (obj->import_attach) { >> > > > + ret = -EINVAL; >> > > > + goto out; >> > > > + } >> > > > + >> > > > ret = drm_gem_create_mmap_offset(obj); >> > > > if (ret) >> > > > goto out; >> > > > -- >> > > > 2.7.4 >> > > > >> > > >-- >Daniel Vetter >Software Engineer, Intel Corporation >http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-29 16:03 ` Brian Starkey @ 2017-08-30 7:44 ` Daniel Vetter 2017-08-30 8:38 ` Brian Starkey 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-30 7:44 UTC (permalink / raw) To: Brian Starkey Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Tue, Aug 29, 2017 at 05:03:19PM +0100, Brian Starkey wrote: > On Fri, Aug 25, 2017 at 03:34:33PM +0200, Daniel Vetter wrote: > > On Thu, Aug 24, 2017 at 11:04:01AM +0100, Brian Starkey wrote: > > > Hi, > > > > > > Thanks for the CC. > > > > > > On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote: > > > > (cc affected parties) > > > > > > > > > > > > Den 18.08.2017 09.46, skrev Daniel Vetter: > > > > > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote: > > > > > > Reject mapping an imported dma-buf since is's an invalid use-case. > > > > > > > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > Cc: Sean Paul <seanpaul@chromium.org> > > > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > Signed-off-by: Noralf Tr??nnes <noralf@tronnes.org> > > > > > I think acks from someone using mali would be good too. amdgpu already has > > > > > such checks, so I think on the desktop side we're ok. > > > > > > > > > > > This looks like it would break anyone running the Mali-4xx series GPUs > > > with the Arm graphics stack (e.g. Hikey board). > > > > > > I don't know where that sits in terms of policy. > > > > Why would this break this use-case? Is the mali blob somehow using dumb > > mmap for it's own buffers it shares with display? That would be rather > > backwards ... > > If it's running with a DRM display driver (which may or may not be > Mali-DP, they are separate IPs), then its window system code always > uses dumb mmap if it needs a pointer. That might be a native DRM > buffer that the display driver allocated, or a dma-buf which it PRIME > imported (which would now fail). Yeah, that's pretty much the kind of uabi abuse I want to prevent. Dumb mmap is for dumb buffers, if you have a egl platform then you need to mmap through that one anyway. If a egl stack uses dumb mmap, something went wrong somewhere. -Daniel > > -Brian > > > > > Either way, blob -> meh, not a concern really. > > -Daniel > > > > > > > > Cheers, > > > -Brian > > > > > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > > > > > But I think this one here definitely needs a few more acks. I could break > > > > > uabi if we're unlucky, so let's not rush it. > > > > > > > > Ok, I've CC'ed the affected parties to increase the odds that they look > > > > at this. These are the drivers using drm_gem_dumb_map_offset() > > > > (hopefully I got the list right): > > > > > > > > arc > > > > atmel-hlcdc > > > > cirrus > > > > exynos > > > > fsl-dcu > > > > gma500 > > > > hdlcd > > > > imx > > > > kirin > > > > mali-dp > > > > mediatek > > > > meson > > > > mxsfb > > > > pl111 > > > > rcar-du > > > > rockchip > > > > shmobile > > > > sti > > > > stm > > > > sun4i > > > > tegra > > > > tilcd > > > > vc4 > > > > zte > > > > > > > > > > > > Noralf. > > > > > > > > > -Daniel > > > > > > > > > > > --- > > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > > > index ad4e9cf..8da5801 100644 > > > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > > > > > > if (!obj) > > > > > > return -ENOENT; > > > > > > + /* Don't allow imported objects to be mapped */ > > > > > > + if (obj->import_attach) { > > > > > > + ret = -EINVAL; > > > > > > + goto out; > > > > > > + } > > > > > > + > > > > > > ret = drm_gem_create_mmap_offset(obj); > > > > > > if (ret) > > > > > > goto out; > > > > > > -- > > > > > > 2.7.4 > > > > > > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-30 7:44 ` Daniel Vetter @ 2017-08-30 8:38 ` Brian Starkey 0 siblings, 0 replies; 29+ messages in thread From: Brian Starkey @ 2017-08-30 8:38 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel On Wed, Aug 30, 2017 at 09:44:53AM +0200, Daniel Vetter wrote: >On Tue, Aug 29, 2017 at 05:03:19PM +0100, Brian Starkey wrote: >> On Fri, Aug 25, 2017 at 03:34:33PM +0200, Daniel Vetter wrote: >> > On Thu, Aug 24, 2017 at 11:04:01AM +0100, Brian Starkey wrote: >> > > Hi, >> > > >> > > Thanks for the CC. >> > > >> > > On Fri, Aug 18, 2017 at 06:13:14PM +0200, Noralf Tr??nnes wrote: >> > > > (cc affected parties) >> > > > >> > > > >> > > > Den 18.08.2017 09.46, skrev Daniel Vetter: >> > > > > On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr??nnes wrote: >> > > > > > Reject mapping an imported dma-buf since is's an invalid use-case. >> > > > > > >> > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> >> > > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> > > > > > Cc: Sean Paul <seanpaul@chromium.org> >> > > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> > > > > > Signed-off-by: Noralf Tr??nnes <noralf@tronnes.org> >> > > > > I think acks from someone using mali would be good too. amdgpu already has >> > > > > such checks, so I think on the desktop side we're ok. >> > > > > >> > > >> > > This looks like it would break anyone running the Mali-4xx series GPUs >> > > with the Arm graphics stack (e.g. Hikey board). >> > > >> > > I don't know where that sits in terms of policy. >> > >> > Why would this break this use-case? Is the mali blob somehow using dumb >> > mmap for it's own buffers it shares with display? That would be rather >> > backwards ... >> >> If it's running with a DRM display driver (which may or may not be >> Mali-DP, they are separate IPs), then its window system code always >> uses dumb mmap if it needs a pointer. That might be a native DRM >> buffer that the display driver allocated, or a dma-buf which it PRIME >> imported (which would now fail). > >Yeah, that's pretty much the kind of uabi abuse I want to prevent. Dumb >mmap is for dumb buffers, if you have a egl platform then you need to mmap >through that one anyway. If a egl stack uses dumb mmap, something went >wrong somewhere. But it exists. There's applications depending on that behaviour to continue working, so shouldn't it continue to work? I thought that was rule #1 -Brian >-Daniel > >> >> -Brian >> >> > >> > Either way, blob -> meh, not a concern really. >> > -Daniel >> > >> > > >> > > Cheers, >> > > -Brian >> > > >> > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> > > > > >> > > > > But I think this one here definitely needs a few more acks. I could break >> > > > > uabi if we're unlucky, so let's not rush it. >> > > > >> > > > Ok, I've CC'ed the affected parties to increase the odds that they look >> > > > at this. These are the drivers using drm_gem_dumb_map_offset() >> > > > (hopefully I got the list right): >> > > > >> > > > arc >> > > > atmel-hlcdc >> > > > cirrus >> > > > exynos >> > > > fsl-dcu >> > > > gma500 >> > > > hdlcd >> > > > imx >> > > > kirin >> > > > mali-dp >> > > > mediatek >> > > > meson >> > > > mxsfb >> > > > pl111 >> > > > rcar-du >> > > > rockchip >> > > > shmobile >> > > > sti >> > > > stm >> > > > sun4i >> > > > tegra >> > > > tilcd >> > > > vc4 >> > > > zte >> > > > >> > > > >> > > > Noralf. >> > > > >> > > > > -Daniel >> > > > > >> > > > > > --- >> > > > > > drivers/gpu/drm/drm_gem.c | 6 ++++++ >> > > > > > 1 file changed, 6 insertions(+) >> > > > > > >> > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> > > > > > index ad4e9cf..8da5801 100644 >> > > > > > --- a/drivers/gpu/drm/drm_gem.c >> > > > > > +++ b/drivers/gpu/drm/drm_gem.c >> > > > > > @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, >> > > > > > if (!obj) >> > > > > > return -ENOENT; >> > > > > > + /* Don't allow imported objects to be mapped */ >> > > > > > + if (obj->import_attach) { >> > > > > > + ret = -EINVAL; >> > > > > > + goto out; >> > > > > > + } >> > > > > > + >> > > > > > ret = drm_gem_create_mmap_offset(obj); >> > > > > > if (ret) >> > > > > > goto out; >> > > > > > -- >> > > > > > 2.7.4 >> > > > > > >> > > > >> > >> > -- >> > Daniel Vetter >> > Software Engineer, Intel Corporation >> > http://blog.ffwll.ch > >-- >Daniel Vetter >Software Engineer, Intel Corporation >http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-18 16:13 ` Noralf Trønnes 2017-08-18 17:41 ` Eric Anholt 2017-08-24 10:04 ` Brian Starkey @ 2017-09-02 12:42 ` Noralf Trønnes 2 siblings, 0 replies; 29+ messages in thread From: Noralf Trønnes @ 2017-09-02 12:42 UTC (permalink / raw) To: Daniel Vetter Cc: narmstrong, daniel.vetter, liviu.dudau, dri-devel, thierry.reding, laurent.pinchart, daniel.vetter, marex, boris.brezillon, abrodkin, linux, z.liuxinliang, kong.kongxinwei, tomi.valkeinen, airlied, puck.chen, jsarha, vincent.abriou, alison.wang, sw0312.kim, philippe.cornu, yannick.fertre, kyungmin.park, zourongrong, maxime.ripard, shawnguo, kraxel Den 18.08.2017 18.13, skrev Noralf Trønnes: > (cc affected parties) > > > Den 18.08.2017 09.46, skrev Daniel Vetter: >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: >>> Reject mapping an imported dma-buf since is's an invalid use-case. >>> >>> Cc: Philipp Zabel <p.zabel@pengutronix.de> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Cc: Sean Paul <seanpaul@chromium.org> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> I think acks from someone using mali would be good too. amdgpu >> already has >> such checks, so I think on the desktop side we're ok. >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> But I think this one here definitely needs a few more acks. I could >> break >> uabi if we're unlucky, so let's not rush it. > > Ok, I've CC'ed the affected parties to increase the odds that they look > at this. These are the drivers using drm_gem_dumb_map_offset() > (hopefully I got the list right): > > arc > atmel-hlcdc > cirrus > exynos > fsl-dcu > gma500 > hdlcd > imx > kirin > mali-dp > mediatek > meson > mxsfb > pl111 > rcar-du > rockchip > shmobile > sti > stm > sun4i > tegra > tilcd > vc4 > zte > Applied to drm-misc together with the armada patch. Noralf. > > Noralf. > >> -Daniel >> >>> --- >>> drivers/gpu/drm/drm_gem.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>> index ad4e9cf..8da5801 100644 >>> --- a/drivers/gpu/drm/drm_gem.c >>> +++ b/drivers/gpu/drm/drm_gem.c >>> @@ -333,6 +333,12 @@ int drm_gem_dumb_map_offset(struct drm_file >>> *file, struct drm_device *dev, >>> if (!obj) >>> return -ENOENT; >>> + /* Don't allow imported objects to be mapped */ >>> + if (obj->import_attach) { >>> + ret = -EINVAL; >>> + goto out; >>> + } >>> + >>> ret = drm_gem_create_mmap_offset(obj); >>> if (ret) >>> goto out; >>> -- >>> 2.7.4 >>> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes 2017-08-17 17:56 ` Laurent Pinchart 2017-08-18 7:46 ` Daniel Vetter @ 2017-08-21 5:26 ` Thierry Reding 2 siblings, 0 replies; 29+ messages in thread From: Thierry Reding @ 2017-08-21 5:26 UTC (permalink / raw) To: Noralf Trønnes; +Cc: daniel.vetter, linux, dri-devel, laurent.pinchart [-- Attachment #1.1: Type: text/plain, Size: 731 bytes --] On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote: > Reject mapping an imported dma-buf since is's an invalid use-case. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/drm_gem.c | 6 ++++++ > 1 file changed, 6 insertions(+) I'm not sure we've documented exactly why this is an invalid use-case. Perhaps that's something we should do along with this patch, or maybe as a follow-up. Either way, I think this is a good idea, so: Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults 2017-08-17 16:21 [PATCH v3 0/2] drm/dumb-buffers: Add defaults for .dumb_map_offset and .dumb_destroy Noralf Trønnes 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes @ 2017-08-17 16:21 ` Noralf Trønnes 2017-08-18 7:47 ` Daniel Vetter 1 sibling, 1 reply; 29+ messages in thread From: Noralf Trønnes @ 2017-08-17 16:21 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, linux, laurent.pinchart This driver can use the drm_driver.dumb_destroy and drm_driver.dumb_map_offset defaults, so no need to set them. Cc: Russell King <linux@armlinux.org.uk> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/armada/armada_drv.c | 2 -- drivers/gpu/drm/armada/armada_gem.c | 36 ------------------------------------ drivers/gpu/drm/armada/armada_gem.h | 4 ---- 3 files changed, 42 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index 0b3227c..2fbd9d3 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -70,8 +70,6 @@ static struct drm_driver armada_drm_driver = { .gem_prime_export = armada_gem_prime_export, .gem_prime_import = armada_gem_prime_import, .dumb_create = armada_gem_dumb_create, - .dumb_map_offset = armada_gem_dumb_map_offset, - .dumb_destroy = armada_gem_dumb_destroy, .gem_vm_ops = &armada_gem_vm_ops, .major = 1, .minor = 0, diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index a76ca21..7983538 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -270,42 +270,6 @@ int armada_gem_dumb_create(struct drm_file *file, struct drm_device *dev, return ret; } -int armada_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, - uint32_t handle, uint64_t *offset) -{ - struct armada_gem_object *obj; - int ret = 0; - - obj = armada_gem_object_lookup(file, handle); - if (!obj) { - DRM_ERROR("failed to lookup gem object\n"); - return -EINVAL; - } - - /* Don't allow imported objects to be mapped */ - if (obj->obj.import_attach) { - ret = -EINVAL; - goto err_unref; - } - - ret = drm_gem_create_mmap_offset(&obj->obj); - if (ret == 0) { - *offset = drm_vma_node_offset_addr(&obj->obj.vma_node); - DRM_DEBUG_DRIVER("handle %#x offset %llx\n", handle, *offset); - } - - err_unref: - drm_gem_object_unreference_unlocked(&obj->obj); - - return ret; -} - -int armada_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, - uint32_t handle) -{ - return drm_gem_handle_delete(file, handle); -} - /* Private driver gem ioctls */ int armada_gem_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file) diff --git a/drivers/gpu/drm/armada/armada_gem.h b/drivers/gpu/drm/armada/armada_gem.h index 6e524e0..1ac9079 100644 --- a/drivers/gpu/drm/armada/armada_gem.h +++ b/drivers/gpu/drm/armada/armada_gem.h @@ -35,10 +35,6 @@ struct armada_gem_object *armada_gem_alloc_private_object(struct drm_device *, size_t); int armada_gem_dumb_create(struct drm_file *, struct drm_device *, struct drm_mode_create_dumb *); -int armada_gem_dumb_map_offset(struct drm_file *, struct drm_device *, - uint32_t, uint64_t *); -int armada_gem_dumb_destroy(struct drm_file *, struct drm_device *, - uint32_t); struct dma_buf *armada_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj, int flags); struct drm_gem_object *armada_gem_prime_import(struct drm_device *, -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults 2017-08-17 16:21 ` [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults Noralf Trønnes @ 2017-08-18 7:47 ` Daniel Vetter 2017-08-30 8:25 ` Daniel Vetter 0 siblings, 1 reply; 29+ messages in thread From: Daniel Vetter @ 2017-08-18 7:47 UTC (permalink / raw) To: Noralf Trønnes; +Cc: daniel.vetter, linux, dri-devel, laurent.pinchart On Thu, Aug 17, 2017 at 06:21:31PM +0200, Noralf Trønnes wrote: > This driver can use the drm_driver.dumb_destroy and > drm_driver.dumb_map_offset defaults, so no need to set them. > > Cc: Russell King <linux@armlinux.org.uk> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Assuming patch 1 gets enough acks and lands: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/armada/armada_drv.c | 2 -- > drivers/gpu/drm/armada/armada_gem.c | 36 ------------------------------------ > drivers/gpu/drm/armada/armada_gem.h | 4 ---- > 3 files changed, 42 deletions(-) > > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c > index 0b3227c..2fbd9d3 100644 > --- a/drivers/gpu/drm/armada/armada_drv.c > +++ b/drivers/gpu/drm/armada/armada_drv.c > @@ -70,8 +70,6 @@ static struct drm_driver armada_drm_driver = { > .gem_prime_export = armada_gem_prime_export, > .gem_prime_import = armada_gem_prime_import, > .dumb_create = armada_gem_dumb_create, > - .dumb_map_offset = armada_gem_dumb_map_offset, > - .dumb_destroy = armada_gem_dumb_destroy, > .gem_vm_ops = &armada_gem_vm_ops, > .major = 1, > .minor = 0, > diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c > index a76ca21..7983538 100644 > --- a/drivers/gpu/drm/armada/armada_gem.c > +++ b/drivers/gpu/drm/armada/armada_gem.c > @@ -270,42 +270,6 @@ int armada_gem_dumb_create(struct drm_file *file, struct drm_device *dev, > return ret; > } > > -int armada_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > - uint32_t handle, uint64_t *offset) > -{ > - struct armada_gem_object *obj; > - int ret = 0; > - > - obj = armada_gem_object_lookup(file, handle); > - if (!obj) { > - DRM_ERROR("failed to lookup gem object\n"); > - return -EINVAL; > - } > - > - /* Don't allow imported objects to be mapped */ > - if (obj->obj.import_attach) { > - ret = -EINVAL; > - goto err_unref; > - } > - > - ret = drm_gem_create_mmap_offset(&obj->obj); > - if (ret == 0) { > - *offset = drm_vma_node_offset_addr(&obj->obj.vma_node); > - DRM_DEBUG_DRIVER("handle %#x offset %llx\n", handle, *offset); > - } > - > - err_unref: > - drm_gem_object_unreference_unlocked(&obj->obj); > - > - return ret; > -} > - > -int armada_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, > - uint32_t handle) > -{ > - return drm_gem_handle_delete(file, handle); > -} > - > /* Private driver gem ioctls */ > int armada_gem_create_ioctl(struct drm_device *dev, void *data, > struct drm_file *file) > diff --git a/drivers/gpu/drm/armada/armada_gem.h b/drivers/gpu/drm/armada/armada_gem.h > index 6e524e0..1ac9079 100644 > --- a/drivers/gpu/drm/armada/armada_gem.h > +++ b/drivers/gpu/drm/armada/armada_gem.h > @@ -35,10 +35,6 @@ struct armada_gem_object *armada_gem_alloc_private_object(struct drm_device *, > size_t); > int armada_gem_dumb_create(struct drm_file *, struct drm_device *, > struct drm_mode_create_dumb *); > -int armada_gem_dumb_map_offset(struct drm_file *, struct drm_device *, > - uint32_t, uint64_t *); > -int armada_gem_dumb_destroy(struct drm_file *, struct drm_device *, > - uint32_t); > struct dma_buf *armada_gem_prime_export(struct drm_device *dev, > struct drm_gem_object *obj, int flags); > struct drm_gem_object *armada_gem_prime_import(struct drm_device *, > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults 2017-08-18 7:47 ` Daniel Vetter @ 2017-08-30 8:25 ` Daniel Vetter 0 siblings, 0 replies; 29+ messages in thread From: Daniel Vetter @ 2017-08-30 8:25 UTC (permalink / raw) To: Noralf Trønnes; +Cc: daniel.vetter, linux, dri-devel, laurent.pinchart On Fri, Aug 18, 2017 at 09:47:25AM +0200, Daniel Vetter wrote: > On Thu, Aug 17, 2017 at 06:21:31PM +0200, Noralf Trønnes wrote: > > This driver can use the drm_driver.dumb_destroy and > > drm_driver.dumb_map_offset defaults, so no need to set them. > > > > Cc: Russell King <linux@armlinux.org.uk> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > Assuming patch 1 gets enough acks and lands: I think patch 1 has soaked for long enough now, feel free to push. -Daniel > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > --- > > drivers/gpu/drm/armada/armada_drv.c | 2 -- > > drivers/gpu/drm/armada/armada_gem.c | 36 ------------------------------------ > > drivers/gpu/drm/armada/armada_gem.h | 4 ---- > > 3 files changed, 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c > > index 0b3227c..2fbd9d3 100644 > > --- a/drivers/gpu/drm/armada/armada_drv.c > > +++ b/drivers/gpu/drm/armada/armada_drv.c > > @@ -70,8 +70,6 @@ static struct drm_driver armada_drm_driver = { > > .gem_prime_export = armada_gem_prime_export, > > .gem_prime_import = armada_gem_prime_import, > > .dumb_create = armada_gem_dumb_create, > > - .dumb_map_offset = armada_gem_dumb_map_offset, > > - .dumb_destroy = armada_gem_dumb_destroy, > > .gem_vm_ops = &armada_gem_vm_ops, > > .major = 1, > > .minor = 0, > > diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c > > index a76ca21..7983538 100644 > > --- a/drivers/gpu/drm/armada/armada_gem.c > > +++ b/drivers/gpu/drm/armada/armada_gem.c > > @@ -270,42 +270,6 @@ int armada_gem_dumb_create(struct drm_file *file, struct drm_device *dev, > > return ret; > > } > > > > -int armada_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > > - uint32_t handle, uint64_t *offset) > > -{ > > - struct armada_gem_object *obj; > > - int ret = 0; > > - > > - obj = armada_gem_object_lookup(file, handle); > > - if (!obj) { > > - DRM_ERROR("failed to lookup gem object\n"); > > - return -EINVAL; > > - } > > - > > - /* Don't allow imported objects to be mapped */ > > - if (obj->obj.import_attach) { > > - ret = -EINVAL; > > - goto err_unref; > > - } > > - > > - ret = drm_gem_create_mmap_offset(&obj->obj); > > - if (ret == 0) { > > - *offset = drm_vma_node_offset_addr(&obj->obj.vma_node); > > - DRM_DEBUG_DRIVER("handle %#x offset %llx\n", handle, *offset); > > - } > > - > > - err_unref: > > - drm_gem_object_unreference_unlocked(&obj->obj); > > - > > - return ret; > > -} > > - > > -int armada_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, > > - uint32_t handle) > > -{ > > - return drm_gem_handle_delete(file, handle); > > -} > > - > > /* Private driver gem ioctls */ > > int armada_gem_create_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file) > > diff --git a/drivers/gpu/drm/armada/armada_gem.h b/drivers/gpu/drm/armada/armada_gem.h > > index 6e524e0..1ac9079 100644 > > --- a/drivers/gpu/drm/armada/armada_gem.h > > +++ b/drivers/gpu/drm/armada/armada_gem.h > > @@ -35,10 +35,6 @@ struct armada_gem_object *armada_gem_alloc_private_object(struct drm_device *, > > size_t); > > int armada_gem_dumb_create(struct drm_file *, struct drm_device *, > > struct drm_mode_create_dumb *); > > -int armada_gem_dumb_map_offset(struct drm_file *, struct drm_device *, > > - uint32_t, uint64_t *); > > -int armada_gem_dumb_destroy(struct drm_file *, struct drm_device *, > > - uint32_t); > > struct dma_buf *armada_gem_prime_export(struct drm_device *dev, > > struct drm_gem_object *obj, int flags); > > struct drm_gem_object *armada_gem_prime_import(struct drm_device *, > > -- > > 2.7.4 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2017-09-02 12:43 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-17 16:21 [PATCH v3 0/2] drm/dumb-buffers: Add defaults for .dumb_map_offset and .dumb_destroy Noralf Trønnes 2017-08-17 16:21 ` [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Noralf Trønnes 2017-08-17 17:56 ` Laurent Pinchart 2017-08-17 18:30 ` Chris Wilson 2017-08-17 19:01 ` Laurent Pinchart 2017-08-17 19:09 ` Chris Wilson 2017-08-17 19:40 ` Laurent Pinchart 2017-08-17 20:01 ` Chris Wilson 2017-08-17 22:35 ` Laurent Pinchart 2017-08-18 7:45 ` Daniel Vetter 2017-08-18 8:01 ` Laurent Pinchart 2017-08-18 8:21 ` Chris Wilson 2017-08-18 7:46 ` Daniel Vetter 2017-08-18 16:13 ` Noralf Trønnes 2017-08-18 17:41 ` Eric Anholt 2017-08-18 20:17 ` Daniel Vetter 2017-08-18 20:34 ` Eric Anholt 2017-08-18 21:01 ` Daniel Vetter 2017-08-18 23:37 ` Eric Anholt 2017-08-24 10:04 ` Brian Starkey 2017-08-25 13:34 ` Daniel Vetter 2017-08-29 16:03 ` Brian Starkey 2017-08-30 7:44 ` Daniel Vetter 2017-08-30 8:38 ` Brian Starkey 2017-09-02 12:42 ` Noralf Trønnes 2017-08-21 5:26 ` Thierry Reding 2017-08-17 16:21 ` [PATCH v3 2/2] drm/armada: Use .dumb_map_offset and .dumb_destroy defaults Noralf Trønnes 2017-08-18 7:47 ` Daniel Vetter 2017-08-30 8:25 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.