diff mbox

[01/13] drm/i915: Refactor shmem pread setup

Message ID 1391032514-19136-2-git-send-email-bradley.d.volkin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

bradley.d.volkin@intel.com Jan. 29, 2014, 9:55 p.m. UTC 
From: Brad Volkin <bradley.d.volkin@intel.com>

The command parser is going to need the same synchronization and
setup logic, so factor it out for reuse.

Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 14 deletions(-)

Comments

Daniel Vetter Jan. 30, 2014, 8:36 a.m. UTC | #1 
On Wed, Jan 29, 2014 at 01:55:02PM -0800, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
> 
> The command parser is going to need the same synchronization and
> setup logic, so factor it out for reuse.
> 
> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  3 +++
>  drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++++++++++++------------
>  2 files changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3673ba1..bfb30df 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2045,6 +2045,9 @@ void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
>  void i915_gem_release_mmap(struct drm_i915_gem_object *obj);
>  void i915_gem_lastclose(struct drm_device *dev);
>  
> +int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
> +				    int *needs_clflush);
> +
>  int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
>  static inline struct page *i915_gem_object_get_page(struct drm_i915_gem_object *obj, int n)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 39770f7..fdc1f40 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -332,6 +332,39 @@ __copy_from_user_swizzled(char *gpu_vaddr, int gpu_offset,
>  	return 0;
>  }
>  
> +/*
> + * Pins the specified object's pages and synchronizes the object with
> + * GPU accesses. Sets needs_clflush to non-zero if the caller should
> + * flush the object from the CPU cache.
> + */
> +int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
> +				    int *needs_clflush)
> +{
> +	int ret;

I think for safety reasons (userspace can blow up the kernel otherwise
with the execbuf cmd parser) we need to replicate the "is this really
shmem backed" check from i915_gem_pread_ioctl to this place here.
-Daniel

> +
> +	*needs_clflush = 0;
> +
> +	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
> +		/* If we're not in the cpu read domain, set ourself into the gtt
> +		 * read domain and manually flush cachelines (if required). This
> +		 * optimizes for the case when the gpu will dirty the data
> +		 * anyway again before the next pread happens. */
> +		*needs_clflush = !cpu_cache_is_coherent(obj->base.dev,
> +							obj->cache_level);
> +		ret = i915_gem_object_wait_rendering(obj, true);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = i915_gem_object_get_pages(obj);
> +	if (ret)
> +		return ret;
> +
> +	i915_gem_object_pin_pages(obj);
> +
> +	return ret;
> +}
> +
>  /* Per-page copy function for the shmem pread fastpath.
>   * Flushes invalid cachelines before reading the target if
>   * needs_clflush is set. */
> @@ -429,23 +462,10 @@ i915_gem_shmem_pread(struct drm_device *dev,
>  
>  	obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj);
>  
> -	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
> -		/* If we're not in the cpu read domain, set ourself into the gtt
> -		 * read domain and manually flush cachelines (if required). This
> -		 * optimizes for the case when the gpu will dirty the data
> -		 * anyway again before the next pread happens. */
> -		needs_clflush = !cpu_cache_is_coherent(dev, obj->cache_level);
> -		ret = i915_gem_object_wait_rendering(obj, true);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	ret = i915_gem_object_get_pages(obj);
> +	ret = i915_gem_obj_prepare_shmem_read(obj, &needs_clflush);
>  	if (ret)
>  		return ret;
>  
> -	i915_gem_object_pin_pages(obj);
> -
>  	offset = args->offset;
>  
>  	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,
> -- 
> 1.8.5.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3673ba1..bfb30df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2045,6 +2045,9 @@  void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
 void i915_gem_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_lastclose(struct drm_device *dev);
 
+int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
+				    int *needs_clflush);
+
 int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
 static inline struct page *i915_gem_object_get_page(struct drm_i915_gem_object *obj, int n)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 39770f7..fdc1f40 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -332,6 +332,39 @@  __copy_from_user_swizzled(char *gpu_vaddr, int gpu_offset,
 	return 0;
 }
 
+/*
+ * Pins the specified object's pages and synchronizes the object with
+ * GPU accesses. Sets needs_clflush to non-zero if the caller should
+ * flush the object from the CPU cache.
+ */
+int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
+				    int *needs_clflush)
+{
+	int ret;
+
+	*needs_clflush = 0;
+
+	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
+		/* If we're not in the cpu read domain, set ourself into the gtt
+		 * read domain and manually flush cachelines (if required). This
+		 * optimizes for the case when the gpu will dirty the data
+		 * anyway again before the next pread happens. */
+		*needs_clflush = !cpu_cache_is_coherent(obj->base.dev,
+							obj->cache_level);
+		ret = i915_gem_object_wait_rendering(obj, true);
+		if (ret)
+			return ret;
+	}
+
+	ret = i915_gem_object_get_pages(obj);
+	if (ret)
+		return ret;
+
+	i915_gem_object_pin_pages(obj);
+
+	return ret;
+}
+
 /* Per-page copy function for the shmem pread fastpath.
  * Flushes invalid cachelines before reading the target if
  * needs_clflush is set. */
@@ -429,23 +462,10 @@  i915_gem_shmem_pread(struct drm_device *dev,
 
 	obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj);
 
-	if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) {
-		/* If we're not in the cpu read domain, set ourself into the gtt
-		 * read domain and manually flush cachelines (if required). This
-		 * optimizes for the case when the gpu will dirty the data
-		 * anyway again before the next pread happens. */
-		needs_clflush = !cpu_cache_is_coherent(dev, obj->cache_level);
-		ret = i915_gem_object_wait_rendering(obj, true);
-		if (ret)
-			return ret;
-	}
-
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_obj_prepare_shmem_read(obj, &needs_clflush);
 	if (ret)
 		return ret;
 
-	i915_gem_object_pin_pages(obj);
-
 	offset = args->offset;
 
 	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,