On Thu, Oct 10, 2019 at 10:02:31AM +0800, Qiang Yu wrote: > On Wed, Oct 9, 2019 at 10:57 PM Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Sep 26, 2019 at 9:12 AM Qiang Yu <yuq825@xxxxxxxxx> 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@xxxxxxxxxx> > > > > > Cc: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx> > > > > > Cc: Steven Price <steven.price@xxxxxxx> > > > > > Cc: Alyssa Rosenzweig <alyssa.rosenzweig@xxxxxxxxxxxxx> > > > > > Signed-off-by: Qiang Yu <yuq825@xxxxxxxxx> > > > > > --- > > > > > 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. Yeah I think with the new stuff we might be able to avoid the kvmalloc_array, that's not really a great idea to plug into a fastpath like execbuf. The other patches can imo still go ahead, I don't want to hold up everything :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel