Re: [PATCH v2 1/6] drm/gem: refine drm_gem_objects_lookup

From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Herring <robh@kernel.org>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	lima@lists.freedesktop.org,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Steven Price <steven.price@arm.com>,
	David Airlie <airlied@linux.ie>, Qiang Yu <yuq825@gmail.com>,
	Sean Paul <sean@poorly.run>,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Subject: Re: [PATCH v2 1/6] drm/gem: refine drm_gem_objects_lookup
Date: Wed, 9 Oct 2019 18:22:10 +0200	[thread overview]
Message-ID: <20191009162210.GS16989@phenom.ffwll.local> (raw)
In-Reply-To: <CAL_Jsq+dz1fU0TsiPB9qhCJpumf8Qz=ANmPyEL0SJiX+q1TXWw@mail.gmail.com>

On Wed, Oct 09, 2019 at 11:05:31AM -0500, Rob Herring wrote:
> On Wed, Oct 9, 2019 at 10:07 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Fri, Sep 27, 2019 at 09:46:11PM +0800, Qiang Yu wrote:
> > > drm_gem_objects_lookup does not use user space bo handles
> > > directly and left the user to kernel copy work to a new
> > > interface drm_gem_objects_lookup_user.
> > >
> > > This is for driver like lima which does not pass gem bo
> > > handles continously in an array in ioctl argument.
> > >
> > > v2:
> > > 1. add drm_gem_objects_lookup_user
> > > 2. remove none-zero check as all caller will check before
> > >    calling this function
> > >
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > > Cc: Steven Price <steven.price@arm.com>
> > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> > > Suggested-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_gem.c               | 57 +++++++++++++++++++------
> > >  drivers/gpu/drm/panfrost/panfrost_drv.c |  6 +--
> > >  include/drm/drm_gem.h                   |  4 +-
> > >  3 files changed, 49 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index 6854f5867d51..a5e88c3e6d25 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,26 +695,56 @@ 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)
> >
> > You can't do this change without updating all the drivers. Simpler to keep
> > this one as-is, and create a new function with an _internal suffix.
> 
> The only driver currently is panfrost and it is updated in this patch.

Oops sry missed that in the diffstat somehow.
-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