Enable pread/pwrite for prime objects

Enable pread/pwrite for prime objects

[Chris Wilson]

One of the intentions for the pread/pwrite overhaul was to enable access
to !shmemfs tests. Let's finish that task.
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Extract checking for backing struct pages to a helper

[Chris Wilson]

Currently to see if an object is backed by struct pages (as opposed to
being a simple pointer to stolen memory, for example) we do a manual
check on the obj->ops->flags. This is quite shouty and before adding
more checks in future, we should make it a bit calmer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h | 6 ++++++
 drivers/gpu/drm/i915/i915_gem.c | 4 ++--
 2 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 25ebe4660764..48928227bdcc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2266,6 +2266,12 @@ struct drm_i915_gem_object {
 };
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
 
+static inline bool
+i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
+{
+	return obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE;
+}
+
 /*
  * Optimised SGL iterator for GEM objects
  */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 21d0dea57312..7f6879cc82fb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -508,7 +508,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
 
 	*needs_clflush = 0;
 
-	if (WARN_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
+	if (WARN_ON(!i915_gem_object_has_struct_page(obj)))
 		return -EINVAL;
 
 	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
@@ -5528,7 +5528,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->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
+	if (WARN_ON(!i915_gem_object_has_struct_page(obj)))
 		return NULL;
 
 	page = i915_gem_object_get_page(obj, n);
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Extract checking for backing struct pages to a helper

[Tvrtko Ursulin]

On 20/06/16 10:31, Chris Wilson wrote:
> Currently to see if an object is backed by struct pages (as opposed to
> being a simple pointer to stolen memory, for example) we do a manual
> check on the obj->ops->flags. This is quite shouty and before adding
> more checks in future, we should make it a bit calmer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 6 ++++++
>   drivers/gpu/drm/i915/i915_gem.c | 4 ++--
>   2 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 25ebe4660764..48928227bdcc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2266,6 +2266,12 @@ struct drm_i915_gem_object {
>   };
>   #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
>
> +static inline bool
> +i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
> +{
> +	return obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE;
> +}
> +
>   /*
>    * Optimised SGL iterator for GEM objects
>    */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 21d0dea57312..7f6879cc82fb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -508,7 +508,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
>
>   	*needs_clflush = 0;
>
> -	if (WARN_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
> +	if (WARN_ON(!i915_gem_object_has_struct_page(obj)))
>   		return -EINVAL;
>
>   	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
> @@ -5528,7 +5528,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->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
> +	if (WARN_ON(!i915_gem_object_has_struct_page(obj)))
>   		return NULL;
>
>   	page = i915_gem_object_get_page(obj, n);
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Chris Wilson]

The idea behind relaxing the restriction for pread/pwrite was to handle
!obj->base.flip, i.e. non-shmemfs backed objects, which only requires
that the object provide struct pages.

v2: Remove excess (). Note enough editing after copy'n'paste.
v3: Use new i915_gem_object_has_struct_page()

Testcase: igt/prime_vgem/read
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7f6879cc82fb..604989b81131 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
 	int needs_clflush = 0;
 	struct sg_page_iter sg_iter;
 
-	if (!obj->base.filp)
+	if (!i915_gem_object_has_struct_page(obj))
 		return -ENODEV;
 
 	user_data = u64_to_user_ptr(args->data_ptr);
@@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	 * pread/pwrite currently are reading and writing from the CPU
 	 * perspective, requiring manual detiling by the client.
 	 */
-	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
+	if (!i915_gem_object_has_struct_page(obj) ||
+	    cpu_write_needs_clflush(obj)) {
 		ret = i915_gem_gtt_pwrite_fast(dev_priv, obj, args, file);
 		/* Note that the gtt paths might fail with non-page-backed user
 		 * pointers (e.g. gtt mappings when moving data between
@@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	if (ret == -EFAULT) {
 		if (obj->phys_handle)
 			ret = i915_gem_phys_pwrite(obj, args, file);
-		else if (obj->base.filp)
+		else if (i915_gem_object_has_struct_page(obj))
 			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
 		else
 			ret = -ENODEV;
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Tvrtko Ursulin]

On 20/06/16 10:31, Chris Wilson wrote:
> The idea behind relaxing the restriction for pread/pwrite was to handle
> !obj->base.flip, i.e. non-shmemfs backed objects, which only requires
> that the object provide struct pages.
>
> v2: Remove excess (). Note enough editing after copy'n'paste.
> v3: Use new i915_gem_object_has_struct_page()
>
> Testcase: igt/prime_vgem/read
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7f6879cc82fb..604989b81131 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>   	int needs_clflush = 0;
>   	struct sg_page_iter sg_iter;
>
> -	if (!obj->base.filp)
> +	if (!i915_gem_object_has_struct_page(obj))
>   		return -ENODEV;
>
>   	user_data = u64_to_user_ptr(args->data_ptr);
> @@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	 * pread/pwrite currently are reading and writing from the CPU
>   	 * perspective, requiring manual detiling by the client.
>   	 */
> -	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
> +	if (!i915_gem_object_has_struct_page(obj) ||
> +	    cpu_write_needs_clflush(obj)) {
>   		ret = i915_gem_gtt_pwrite_fast(dev_priv, obj, args, file);
>   		/* Note that the gtt paths might fail with non-page-backed user
>   		 * pointers (e.g. gtt mappings when moving data between
> @@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	if (ret == -EFAULT) {
>   		if (obj->phys_handle)
>   			ret = i915_gem_phys_pwrite(obj, args, file);
> -		else if (obj->base.filp)
> +		else if (i915_gem_object_has_struct_page(obj))
>   			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
>   		else
>   			ret = -ENODEV;
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx