[Lima] [PATCH 1/6] drm/gem: refine drm_gem_objects_lookup

[Qiang Yu]

On Wed, Oct 9, 2019 at 10:57 PM Daniel Vetter <daniel at ffwll.ch> wrote:
>
> On Fri, Sep 27, 2019 at 08:09:52AM +0800, Qiang Yu wrote:
> > On Thu, Sep 26, 2019 at 11:01 PM Rob Herring <robh at kernel.org> wrote:
> > >
> > > On Thu, Sep 26, 2019 at 9:12 AM Qiang Yu <yuq825 at gmail.com> wrote:
> > > >
> > > > Do not use user space bo handles directly and left the user
> > > > to kernel copy work to drivers calling this function.
> > > >
> > > > This is for driver like lima which does not pass gem bo
> > > > handles continously in an array in ioctl argument.
> > > >
> > > > Cc: Rob Herring <robh at kernel.org>
> > > > Cc: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> > > > Cc: Steven Price <steven.price at arm.com>
> > > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
> > > > Signed-off-by: Qiang Yu <yuq825 at gmail.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_gem.c               | 28 +++++++------------------
> > > >  drivers/gpu/drm/panfrost/panfrost_drv.c | 23 +++++++++++++++++---
> > >
> > > Please keep the current variant. While only panfrost is converted ATM,
> > > vc4 and v3d will use this too.
> > >
> > > >  include/drm/drm_gem.h                   |  2 +-
> > > >  3 files changed, 29 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > > index 6854f5867d51..9f73e5f3b53f 100644
> > > > --- a/drivers/gpu/drm/drm_gem.c
> > > > +++ b/drivers/gpu/drm/drm_gem.c
> > > > @@ -679,11 +679,11 @@ static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
> > > >  /**
> > > >   * drm_gem_objects_lookup - look up GEM objects from an array of handles
> > > >   * @filp: DRM file private date
> > > > - * @bo_handles: user pointer to array of userspace handle
> > > > + * @bo_handles: array of GEM object handles
> > > >   * @count: size of handle array
> > > >   * @objs_out: returned pointer to array of drm_gem_object pointers
> > > >   *
> > > > - * Takes an array of userspace handles and returns a newly allocated array of
> > > > + * Takes an array of GEM object handles and returns a newly allocated array of
> > > >   * GEM objects.
> > > >   *
> > > >   * For a single handle lookup, use drm_gem_object_lookup().
> > > > @@ -695,11 +695,10 @@ static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
> > > >   * failure. 0 is returned on success.
> > > >   *
> > > >   */
> > > > -int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles,
> > > > +int drm_gem_objects_lookup(struct drm_file *filp, u32 *bo_handles,
> > > >                            int count, struct drm_gem_object ***objs_out)
> > >
> > > Either split into drm_gem_objects_lookup() and a
> > > drm_gem_objects_lookup_user() with the latter calling the former or
> > > just make the helper take both a user and u32* ptr with the
> > > expectation that only one of them is non-NULL.
> > >
> > OK, I prefer the first way, will send v2 for it.
>
> Note that hopefully soon (Christian König is working on it) we'll have
> ww_mutex locking helpers, which will introduce a drm_gem_operation_ctx.
> Once we have that I think we can unify a lot of these helpers (Gerd
> Hoffmann has looked into it) all while making them more flexible (i.e. not
> only works with arrays). So might be worth to coordinate a bit here, and
> maybe hold off on just this part for lima for a bit.

I don't know the context of these works, so I think I'd better wait some time
for the new interface and send the rest of lima patches as v4.

Thanks,
Qiang

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch