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@xxxxxxxxxxxxxx>
> >>>>>> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >>>>>> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> >>>>>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >>>>>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> >>>>>> ---
> >>>>>>
> >>>>>> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
