From: Matthew Auld Date: Fri, 6 Oct 2017 23:18:17 +0100 Subject: drm/i915: push set_pages down to the callers Git-commit: b91b09eea7a15ab417aa9ed6502b3be12f5283f8 Patch-mainline: v4.15-rc1 References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166 Each backend is now responsible for calling __i915_gem_object_set_pages upon successfully gathering its backing storage. This eliminates the inconsistency between the async and sync paths, which stands out even more when we start throwing around an sg_mask in a later patch. Suggested-by: Chris Wilson Signed-off-by: Matthew Auld Cc: Joonas Lahtinen Cc: Chris Wilson Reviewed-by: Joonas Lahtinen Reviewed-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20171006145041.21673-6-matthew.auld@intel.com Signed-off-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20171006221833.32439-5-chris@chris-wilson.co.uk Acked-by: Petr Tesarik --- drivers/gpu/drm/i915/i915_gem.c | 45 ++++++++++++----------- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 15 +++++-- drivers/gpu/drm/i915/i915_gem_internal.c | 15 ++++--- drivers/gpu/drm/i915/i915_gem_object.h | 2 - drivers/gpu/drm/i915/i915_gem_stolen.c | 16 +++++--- drivers/gpu/drm/i915/i915_gem_userptr.c | 12 +++--- drivers/gpu/drm/i915/selftests/huge_gem_object.c | 14 ++++--- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 12 +++--- 8 files changed, 77 insertions(+), 54 deletions(-) --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -162,8 +162,7 @@ i915_gem_get_aperture_ioctl(struct drm_d return 0; } -static struct sg_table * -i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) +static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) { struct address_space *mapping = obj->base.filp->f_mapping; drm_dma_handle_t *phys; @@ -171,9 +170,10 @@ i915_gem_object_get_pages_phys(struct dr struct scatterlist *sg; char *vaddr; int i; + int err; if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj))) - return ERR_PTR(-EINVAL); + return -EINVAL; /* Always aligning to the object size, allows a single allocation * to handle all possible callers, and given typical object sizes, @@ -183,7 +183,7 @@ i915_gem_object_get_pages_phys(struct dr roundup_pow_of_two(obj->base.size), roundup_pow_of_two(obj->base.size)); if (!phys) - return ERR_PTR(-ENOMEM); + return -ENOMEM; vaddr = phys->vaddr; for (i = 0; i < obj->base.size / PAGE_SIZE; i++) { @@ -192,7 +192,7 @@ i915_gem_object_get_pages_phys(struct dr page = shmem_read_mapping_page(mapping, i); if (IS_ERR(page)) { - st = ERR_CAST(page); + err = PTR_ERR(page); goto err_phys; } @@ -209,13 +209,13 @@ i915_gem_object_get_pages_phys(struct dr st = kmalloc(sizeof(*st), GFP_KERNEL); if (!st) { - st = ERR_PTR(-ENOMEM); + err = -ENOMEM; goto err_phys; } if (sg_alloc_table(st, 1, GFP_KERNEL)) { kfree(st); - st = ERR_PTR(-ENOMEM); + err = -ENOMEM; goto err_phys; } @@ -227,11 +227,15 @@ i915_gem_object_get_pages_phys(struct dr sg_dma_len(sg) = obj->base.size; obj->phys_handle = phys; - return st; + + __i915_gem_object_set_pages(obj, st); + + return 0; err_phys: drm_pci_free(obj->base.dev, phys); - return st; + + return err; } static void __start_cpu_write(struct drm_i915_gem_object *obj) @@ -2294,8 +2298,7 @@ static bool i915_sg_trim(struct sg_table return true; } -static struct sg_table * -i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) +static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) { struct drm_i915_private *dev_priv = to_i915(obj->base.dev); const unsigned long page_count = obj->base.size / PAGE_SIZE; @@ -2319,12 +2322,12 @@ i915_gem_object_get_pages_gtt(struct drm st = kmalloc(sizeof(*st), GFP_KERNEL); if (st == NULL) - return ERR_PTR(-ENOMEM); + return -ENOMEM; rebuild_st: if (sg_alloc_table(st, page_count, GFP_KERNEL)) { kfree(st); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } /* Get the list of pages out of our struct file. They'll be pinned @@ -2432,7 +2435,9 @@ rebuild_st: if (i915_gem_object_needs_bit17_swizzle(obj)) i915_gem_object_do_bit_17_swizzle(obj, st); - return st; + __i915_gem_object_set_pages(obj, st); + + return 0; err_sg: sg_mark_end(sg); @@ -2453,7 +2458,7 @@ err_pages: if (ret == -ENOSPC) ret = -ENOMEM; - return ERR_PTR(ret); + return ret; } void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, @@ -2476,19 +2481,17 @@ void __i915_gem_object_set_pages(struct static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj) { - struct sg_table *pages; + int err; if (unlikely(obj->mm.madv != I915_MADV_WILLNEED)) { DRM_DEBUG("Attempting to obtain a purgeable object\n"); return -EFAULT; } - pages = obj->ops->get_pages(obj); - if (unlikely(IS_ERR(pages))) - return PTR_ERR(pages); + err = obj->ops->get_pages(obj); + GEM_BUG_ON(!err && IS_ERR_OR_NULL(obj->mm.pages)); - __i915_gem_object_set_pages(obj, pages); - return 0; + return err; } /* Ensure that the associated pages are gathered from the backing storage --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -256,11 +256,18 @@ struct dma_buf *i915_gem_prime_export(st return drm_gem_dmabuf_export(dev, &exp_info); } -static struct sg_table * -i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) +static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) { - return dma_buf_map_attachment(obj->base.import_attach, - DMA_BIDIRECTIONAL); + struct sg_table *pages; + + pages = dma_buf_map_attachment(obj->base.import_attach, + DMA_BIDIRECTIONAL); + if (IS_ERR(pages)) + return PTR_ERR(pages); + + __i915_gem_object_set_pages(obj, pages); + + return 0; } static void i915_gem_object_put_pages_dmabuf(struct drm_i915_gem_object *obj, --- a/drivers/gpu/drm/i915/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/i915_gem_internal.c @@ -44,8 +44,7 @@ static void internal_free_pages(struct s kfree(st); } -static struct sg_table * -i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) +static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) { struct drm_i915_private *i915 = to_i915(obj->base.dev); struct sg_table *st; @@ -78,12 +77,12 @@ i915_gem_object_get_pages_internal(struc create_st: st = kmalloc(sizeof(*st), GFP_KERNEL); if (!st) - return ERR_PTR(-ENOMEM); + return -ENOMEM; npages = obj->base.size / PAGE_SIZE; if (sg_alloc_table(st, npages, GFP_KERNEL)) { kfree(st); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } sg = st->sgl; @@ -132,13 +131,17 @@ create_st: * object are only valid whilst active and pinned. */ obj->mm.madv = I915_MADV_DONTNEED; - return st; + + __i915_gem_object_set_pages(obj, st); + + return 0; err: sg_set_page(sg, NULL, 0, 0); sg_mark_end(sg); internal_free_pages(st); - return ERR_PTR(-ENOMEM); + + return -ENOMEM; } static void i915_gem_object_put_pages_internal(struct drm_i915_gem_object *obj, --- a/drivers/gpu/drm/i915/i915_gem_object.h +++ b/drivers/gpu/drm/i915/i915_gem_object.h @@ -69,7 +69,7 @@ struct drm_i915_gem_object_ops { * being released or under memory pressure (where we attempt to * reap pages for the shrinker). */ - struct sg_table *(*get_pages)(struct drm_i915_gem_object *); + int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *, struct sg_table *); int (*pwrite)(struct drm_i915_gem_object *, --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -539,12 +539,18 @@ i915_pages_create_for_stolen(struct drm_ return st; } -static struct sg_table * -i915_gem_object_get_pages_stolen(struct drm_i915_gem_object *obj) +static int i915_gem_object_get_pages_stolen(struct drm_i915_gem_object *obj) { - return i915_pages_create_for_stolen(obj->base.dev, - obj->stolen->start, - obj->stolen->size); + struct sg_table *pages = + i915_pages_create_for_stolen(obj->base.dev, + obj->stolen->start, + obj->stolen->size); + if (IS_ERR(pages)) + return PTR_ERR(pages); + + __i915_gem_object_set_pages(obj, pages); + + return 0; } static void i915_gem_object_put_pages_stolen(struct drm_i915_gem_object *obj, --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -434,6 +434,8 @@ alloc_table: return ERR_PTR(ret); } + __i915_gem_object_set_pages(obj, st); + return st; } @@ -521,7 +523,6 @@ __i915_gem_userptr_get_pages_worker(stru pages = __i915_gem_userptr_alloc_pages(obj, pvec, npages); if (!IS_ERR(pages)) { - __i915_gem_object_set_pages(obj, pages); pinned = 0; pages = NULL; } @@ -582,8 +583,7 @@ __i915_gem_userptr_get_pages_schedule(st return ERR_PTR(-EAGAIN); } -static struct sg_table * -i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) +static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) { const int num_pages = obj->base.size >> PAGE_SHIFT; struct mm_struct *mm = obj->userptr.mm->mm; @@ -612,9 +612,9 @@ i915_gem_userptr_get_pages(struct drm_i9 if (obj->userptr.work) { /* active flag should still be held for the pending work */ if (IS_ERR(obj->userptr.work)) - return ERR_CAST(obj->userptr.work); + return PTR_ERR(obj->userptr.work); else - return ERR_PTR(-EAGAIN); + return -EAGAIN; } pvec = NULL; @@ -650,7 +650,7 @@ i915_gem_userptr_get_pages(struct drm_i9 release_pages(pvec, pinned); kvfree(pvec); - return pages; + return PTR_ERR_OR_ZERO(pages); } static void --- a/drivers/gpu/drm/i915/selftests/huge_gem_object.c +++ b/drivers/gpu/drm/i915/selftests/huge_gem_object.c @@ -37,8 +37,7 @@ static void huge_free_pages(struct drm_i kfree(pages); } -static struct sg_table * -huge_get_pages(struct drm_i915_gem_object *obj) +static int huge_get_pages(struct drm_i915_gem_object *obj) { #define GFP (GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY) const unsigned long nreal = obj->scratch / PAGE_SIZE; @@ -49,11 +48,11 @@ huge_get_pages(struct drm_i915_gem_objec pages = kmalloc(sizeof(*pages), GFP); if (!pages) - return ERR_PTR(-ENOMEM); + return -ENOMEM; if (sg_alloc_table(pages, npages, GFP)) { kfree(pages); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } sg = pages->sgl; @@ -81,11 +80,14 @@ huge_get_pages(struct drm_i915_gem_objec if (i915_gem_gtt_prepare_pages(obj, pages)) goto err; - return pages; + __i915_gem_object_set_pages(obj, pages); + + return 0; err: huge_free_pages(obj, pages); - return ERR_PTR(-ENOMEM); + + return -ENOMEM; #undef GFP } --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -39,8 +39,7 @@ static void fake_free_pages(struct drm_i kfree(pages); } -static struct sg_table * -fake_get_pages(struct drm_i915_gem_object *obj) +static int fake_get_pages(struct drm_i915_gem_object *obj) { #define GFP (GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY) #define PFN_BIAS 0x1000 @@ -50,12 +49,12 @@ fake_get_pages(struct drm_i915_gem_objec pages = kmalloc(sizeof(*pages), GFP); if (!pages) - return ERR_PTR(-ENOMEM); + return -ENOMEM; rem = round_up(obj->base.size, BIT(31)) >> 31; if (sg_alloc_table(pages, rem, GFP)) { kfree(pages); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } rem = obj->base.size; @@ -72,7 +71,10 @@ fake_get_pages(struct drm_i915_gem_objec GEM_BUG_ON(rem); obj->mm.madv = I915_MADV_DONTNEED; - return pages; + + __i915_gem_object_set_pages(obj, pages); + + return 0; #undef GFP }