[Intel-gfx] [PATCH 1/3] drm/i915: Allow i915_gem_object_get_page() on userptr as well

[Jani Nikula]

On Wed, 03 Feb 2016, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Fri, 29 Jan 2016, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> commit 033908aed5a596f6202c848c6bbc8a40fb1a8490
>> Author: Dave Gordon <david.s.gordon at intel.com>
>> Date:   Thu Dec 10 18:51:23 2015 +0000
>>
>>     drm/i915: mark GEM object pages dirty when mapped & written by the CPU
>>
>> introduced a check into i915_gem_object_get_dirty_pages() that returned
>> a NULL pointer when called with a bad object, one that was not backed by
>> shmemfs. This WARN was too strict as we can work on all struct page
>> backed objects, and resulted in a WARN + GPF for existing userspace. In
>> order to differentiate the various types of objects, add a new flags field
>> to the i915_gem_object_ops struct to describe their capabilities, with
>> the first flag being whether the object has struct pages.
>>
>> v2: Drop silly const before an integer in the structure declaration.
>>
>> Testcase: igt/gem_userptr_blits/relocations
>> Reported-and-tested-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Dave Gordon <david.s.gordon at intel.com>
>> Cc: Kristian Høgsberg Kristensen <krh at bitplanet.net>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Reviewed-by: Dave Gordon <david.s.gordon at intel.com>
>> Reviewed-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
>
> Cc: drm-intel-fixes at lists.freedesktop.org
>
> to pick for 4.5

Picked up in drm-intel-fixes.

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h         | 4 ++++
>>  drivers/gpu/drm/i915/i915_gem.c         | 3 ++-
>>  drivers/gpu/drm/i915/i915_gem_userptr.c | 3 ++-
>>  3 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 905e90f25957..a2d2f08b7515 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2000,6 +2000,9 @@ enum hdmi_force_audio {
>>  #define I915_GTT_OFFSET_NONE ((u32)-1)
>>  
>>  struct drm_i915_gem_object_ops {
>> +	unsigned int flags;
>> +#define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1
>> +
>>  	/* Interface between the GEM object and its backing storage.
>>  	 * get_pages() is called once prior to the use of the associated set
>>  	 * of pages before to binding them into the GTT, and put_pages() is
>> @@ -2015,6 +2018,7 @@ struct drm_i915_gem_object_ops {
>>  	 */
>>  	int (*get_pages)(struct drm_i915_gem_object *);
>>  	void (*put_pages)(struct drm_i915_gem_object *);
>> +
>>  	int (*dmabuf_export)(struct drm_i915_gem_object *);
>>  	void (*release)(struct drm_i915_gem_object *);
>>  };
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index a928823507c5..e9b19bca1383 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4465,6 +4465,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
>>  }
>>  
>>  static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
>> +	.flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,
>>  	.get_pages = i915_gem_object_get_pages_gtt,
>>  	.put_pages = i915_gem_object_put_pages_gtt,
>>  };
>> @@ -5309,7 +5310,7 @@ i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, int n)
>>  	struct page *page;
>>  
>>  	/* Only default objects have per-page dirty tracking */
>> -	if (WARN_ON(obj->ops != &i915_gem_object_ops))
>> +	if (WARN_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
>>  		return NULL;
>>  
>>  	page = i915_gem_object_get_page(obj, n);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
>> index 74a4d1714879..7107f2fd38f5 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
>> @@ -708,9 +708,10 @@ i915_gem_userptr_dmabuf_export(struct drm_i915_gem_object *obj)
>>  }
>>  
>>  static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
>> -	.dmabuf_export = i915_gem_userptr_dmabuf_export,
>> +	.flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,
>>  	.get_pages = i915_gem_userptr_get_pages,
>>  	.put_pages = i915_gem_userptr_put_pages,
>> +	.dmabuf_export = i915_gem_userptr_dmabuf_export,
>>  	.release = i915_gem_userptr_release,
>>  };

-- 
Jani Nikula, Intel Open Source Technology Center