On Tue,  4 Sep 2012 21:02:55 +0100
Chris Wilson <ch...@chris-wilson.co.uk> wrote:

> By using the recently introduced pinning of pages, we can safely drop
> the mutex in the knowledge that the pages are not going to disappear
> beneath us, and so we can simplify the code for iterating over the pages.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index aa088ef..8a4eac0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -690,7 +690,7 @@ shmem_pwrite_fast(struct page *page, int 
> shmem_page_offset, int page_length,
>                                      page_length);
>       kunmap_atomic(vaddr);
>  
> -     return ret;
> +     return ret ? -EFAULT : 0;
>  }
>  
>  /* Only difference to the fast-path function is that this can handle bit17
> @@ -724,7 +724,7 @@ shmem_pwrite_slow(struct page *page, int 
> shmem_page_offset, int page_length,
>                                            page_do_bit17_swizzling);
>       kunmap(page);
>  
> -     return ret;
> +     return ret ? -EFAULT : 0;
>  }
>  
>  static int
> @@ -733,7 +733,6 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>                     struct drm_i915_gem_pwrite *args,
>                     struct drm_file *file)
>  {
> -     struct address_space *mapping = 
> obj->base.filp->f_path.dentry->d_inode->i_mapping;
>       ssize_t remain;
>       loff_t offset;
>       char __user *user_data;
> @@ -742,7 +741,6 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>       int hit_slowpath = 0;
>       int needs_clflush_after = 0;
>       int needs_clflush_before = 0;
> -     int release_page;
>  
>       user_data = (char __user *) (uintptr_t) args->data_ptr;
>       remain = args->size;
> @@ -768,6 +766,12 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>           && obj->cache_level == I915_CACHE_NONE)
>               needs_clflush_before = 1;
>  
> +     ret = i915_gem_object_get_pages(obj);
> +     if (ret)
> +             return ret;
> +
> +     i915_gem_object_pin_pages(obj);
> +
>       offset = args->offset;
>       obj->dirty = 1;
>  
> @@ -793,18 +797,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>                       ((shmem_page_offset | page_length)
>                               & (boot_cpu_data.x86_clflush_size - 1));
>  
> -             if (obj->pages) {
> -                     page = obj->pages[offset >> PAGE_SHIFT];
> -                     release_page = 0;
> -             } else {
> -                     page = shmem_read_mapping_page(mapping, offset >> 
> PAGE_SHIFT);
> -                     if (IS_ERR(page)) {
> -                             ret = PTR_ERR(page);
> -                             goto out;
> -                     }
> -                     release_page = 1;
> -             }
> -
> +             page = obj->pages[offset >> PAGE_SHIFT];
>               page_do_bit17_swizzling = obj_do_bit17_swizzling &&
>                       (page_to_phys(page) & (1 << 17)) != 0;
>  
> @@ -816,26 +809,20 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
>                       goto next_page;
>  
>               hit_slowpath = 1;
> -             page_cache_get(page);
>               mutex_unlock(&dev->struct_mutex);
> -
>               ret = shmem_pwrite_slow(page, shmem_page_offset, page_length,
>                                       user_data, page_do_bit17_swizzling,
>                                       partial_cacheline_write,
>                                       needs_clflush_after);
>  
>               mutex_lock(&dev->struct_mutex);
> -             page_cache_release(page);
> +
>  next_page:
>               set_page_dirty(page);
>               mark_page_accessed(page);
> -             if (release_page)
> -                     page_cache_release(page);
>  
> -             if (ret) {
> -                     ret = -EFAULT;
> +             if (ret)
>                       goto out;
> -             }
>  
>               remain -= page_length;
>               user_data += page_length;
> @@ -843,6 +830,8 @@ next_page:
>       }
>  
>  out:
> +     i915_gem_object_unpin_pages(obj);
> +
>       if (hit_slowpath) {
>               /* Fixup: Kill any reinstated backing storage pages */
>               if (obj->madv == __I915_MADV_PURGED)

I'll leave the pread/pwrite reviewing to Daniel...

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to