| Message ID | 20190326170218.13255-1-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Update size upon return from GEM_CREATE | expand |
On Tue, Mar 26, 2019 at 06:02:18PM +0100, Michał Winiarski wrote: >Since GEM_CREATE is trying to outsmart the user by rounding up unaligned >objects, we used to update the size returned to userspace. >This update seems to have been lost throughout the history. > >v2: Use round_up(), reorder locals (Chris) > >References: ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)") >Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> >Cc: Chris Wilson <chris@chris-wilson.co.uk> >Cc: Janusz Krzysztofik <janusz.krzysztofik@intel.com> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >--- > drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >index f6cdd5fb9deb..e506e43cfade 100644 >--- a/drivers/gpu/drm/i915/i915_gem.c >+++ b/drivers/gpu/drm/i915/i915_gem.c >@@ -622,14 +622,15 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, > static int > i915_gem_create(struct drm_file *file, > struct drm_i915_private *dev_priv, >- u64 size, >+ u64 *size_p, > u32 *handle_p) > { > struct drm_i915_gem_object *obj; >- int ret; > u32 handle; >+ u64 size; >+ int ret; > >- size = roundup(size, PAGE_SIZE); >+ size = round_up(*size_p, PAGE_SIZE); > if (size == 0) > return -EINVAL; > Does it make more sense to check the size prior to doing a roundup? >@@ -645,6 +646,7 @@ i915_gem_create(struct drm_file *file, > return ret; > > *handle_p = handle; >+ *size_p = obj->base.size; > return 0; > } > >@@ -657,7 +659,7 @@ i915_gem_dumb_create(struct drm_file *file, > args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > args->size = args->pitch * args->height; > return i915_gem_create(file, to_i915(dev), >- args->size, &args->handle); >+ &args->size, &args->handle); > } > > static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj) >@@ -682,7 +684,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, > i915_gem_flush_free_objects(dev_priv); > > return i915_gem_create(file, dev_priv, >- args->size, &args->handle); >+ &args->size, &args->handle); > } > > static inline enum fb_op_origin >-- >2.20.1 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Vanshidhar Konda (2019-03-26 17:29:07) > On Tue, Mar 26, 2019 at 06:02:18PM +0100, Michał Winiarski wrote: > >Since GEM_CREATE is trying to outsmart the user by rounding up unaligned > >objects, we used to update the size returned to userspace. > >This update seems to have been lost throughout the history. > > > >v2: Use round_up(), reorder locals (Chris) > > > >References: ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)") > >Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > >Cc: Chris Wilson <chris@chris-wilson.co.uk> > >Cc: Janusz Krzysztofik <janusz.krzysztofik@intel.com> > >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > >--- > > drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >index f6cdd5fb9deb..e506e43cfade 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -622,14 +622,15 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, > > static int > > i915_gem_create(struct drm_file *file, > > struct drm_i915_private *dev_priv, > >- u64 size, > >+ u64 *size_p, > > u32 *handle_p) > > { > > struct drm_i915_gem_object *obj; > >- int ret; > > u32 handle; > >+ u64 size; > >+ int ret; > > > >- size = roundup(size, PAGE_SIZE); > >+ size = round_up(*size_p, PAGE_SIZE); > > if (size == 0) > > return -EINVAL; > > > > Does it make more sense to check the size prior to doing a roundup? The only failure conditions known at this point are size==0 and size > -PAGE_SIZE which is more concisely handled as a single test afterwards. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f6cdd5fb9deb..e506e43cfade 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -622,14 +622,15 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, static int i915_gem_create(struct drm_file *file, struct drm_i915_private *dev_priv, - u64 size, + u64 *size_p, u32 *handle_p) { struct drm_i915_gem_object *obj; - int ret; u32 handle; + u64 size; + int ret; - size = roundup(size, PAGE_SIZE); + size = round_up(*size_p, PAGE_SIZE); if (size == 0) return -EINVAL; @@ -645,6 +646,7 @@ i915_gem_create(struct drm_file *file, return ret; *handle_p = handle; + *size_p = obj->base.size; return 0; } @@ -657,7 +659,7 @@ i915_gem_dumb_create(struct drm_file *file, args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); args->size = args->pitch * args->height; return i915_gem_create(file, to_i915(dev), - args->size, &args->handle); + &args->size, &args->handle); } static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj) @@ -682,7 +684,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, i915_gem_flush_free_objects(dev_priv); return i915_gem_create(file, dev_priv, - args->size, &args->handle); + &args->size, &args->handle); } static inline enum fb_op_origin
Since GEM_CREATE is trying to outsmart the user by rounding up unaligned objects, we used to update the size returned to userspace. This update seems to have been lost throughout the history. v2: Use round_up(), reorder locals (Chris) References: ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)") Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Janusz Krzysztofik <janusz.krzysztofik@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)