Re: [PATCH 40/59] drm/vram-helper: Drop drm_gem_prime_export/import

Daniel Vetter
2019-06-18T07:59:46Z

On Tue, Jun 18, 2019 at 6:49 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Hi,
>
> > > While most callbacks are pretty straight forward (just hook the same
> > > callbacks into the drm_gem_object_funcs. struct) the mmap bits are a
> > > bit more obscure.
> > >
> > > First, there seem to be two ways to mmap a gem buffer:
> > >
> > > (1) drm_driver->fops->mmap, and
> > > (2) drm_driver->gem_prime_mmap.
> > >
> > > drm_gem_object_funcs has just a single vm_ops ...
> > >
> > > Also it is not obvious how one would convert something which basically
> > > calls ttm_bo_mmap() in drm_driver->fops->mmap to the new interface.
> >
> > Yeah the mmap side is still a mess, but my series here was getting a bit
> > too long already. There's a bunch of problems here:
> >
> > drm_driver->gem_prime_mmap could be nuked and instead we use
> > drm_gem_prime_mmap everywhere. Especially the various versions in helpers
> > really don't add much.
>
> Well, everything using ttm has the problem that we have another
> duplication here: both gem and ttm have a vma_node ...
>
> So (for example) drm_gem_vram_driver_gem_prime_mmap() is a thin wrapper
> which does (a) copy vm_node.start from ttm to gem vma_node and (b) calls
> drm_gem_prime_mmap().

Hm ... maybe we should ditch the ttm vma offset stuff and fold that
over entirely to the gem way of doing things. The only thing you're
going to loose is the ->verify_access callback, which again is just to
get back to gem I think. You would need a slightly differrent vm_ops
structure though, since the ttm vm ops expect a ttm_buffer_object,
whereas gem gives you a drm_gem_buffer_object. So either need to
overwrite all those, or maybe it's inded time to just make ttm_bo a
subclass of gem_bo.

> > Second one is drm_driver->fops->mmap. That one we need to keep, but this
> > isn't mmap on a buffer, but mmap on the entire drm_device. The one which
> > should be replaced by drm_gem_object_funcs.vm_ops is
> > drm_driver->gem_vm_ops.
>
> Hmm, seems ttm hasn't something I can hook into drm_driver->gem_vm_ops ...

ttm_bo_vm_ops seems to be the thing you want.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch