diff mbox series

drm/i915: Update size upon return from GEM_CREATE

Message ID 20190326163345.5606-1-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Update size upon return from GEM_CREATE | expand

Commit Message

Michał Winiarski March 26, 2019, 4:33 p.m. UTC
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.

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>
---
 drivers/gpu/drm/i915/i915_gem.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Chris Wilson March 26, 2019, 4:44 p.m. UTC | #1
Quoting Michał Winiarski (2019-03-26 16:33:45)
> 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.
> 
> 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>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f6cdd5fb9deb..77b82cb81a6c 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;
> +       u64 size;
>         u32 handle;
>  
> -       size = roundup(size, PAGE_SIZE);
> +       size = roundup(*size_p, PAGE_SIZE);

You could have snuck in a round_up() and reordered the locals.

>         if (size == 0)
>                 return -EINVAL;
>  
> @@ -645,6 +646,8 @@ i915_gem_create(struct drm_file *file,
>                 return ret;
>  
>         *handle_p = handle;
> +       *size_p = obj->base.size;

No need for whitespace between the out params and return. Without the
break, it reinforces that they are all part of the return imo.

Sneak in a round_up, and have
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f6cdd5fb9deb..77b82cb81a6c 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;
+	u64 size;
 	u32 handle;
 
-	size = roundup(size, PAGE_SIZE);
+	size = roundup(*size_p, PAGE_SIZE);
 	if (size == 0)
 		return -EINVAL;
 
@@ -645,6 +646,8 @@  i915_gem_create(struct drm_file *file,
 		return ret;
 
 	*handle_p = handle;
+	*size_p = obj->base.size;
+
 	return 0;
 }
 
@@ -657,7 +660,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 +685,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