Re: [PATCH] drm/i915: Defer adding preallocated stolen objects to the VM list

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 24 Sep 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> When preallocating a stolen object during early initialisation, we may
> be running before we have setup the the global GTT VM state, in
> particular before we have initialised the range manager and associated
> lists. As this is the case, we defer binding the stolen object until we
> call i915_gem_setup_global_gtt(). Not only should we defer the binding,
> but we should also defer the VM list manipulation.
>
> Fixes regression from commit a2cad9dff4dd44d0244b966d980de9d602d87593
> Author: Michał Winiarski <michal.winiarski@intel.com>
> Date:   Wed Sep 16 11:49:00 2015 +0200
>
>     drm/i915/gtt: Do not initialize drm_mm twice.
>
> Whilst I am here remove the duplicate work leaving dangling pointers
> from the error path...
>
> Reported-by: Jesse Barnes <jesse@virtuousgeek.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92099
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jesse@virtuousgeek.org>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

  CC [M]  drivers/gpu/drm/i915/i915_gem_stolen.o
drivers/gpu/drm/i915/i915_gem_gtt.c: In function ‘i915_gem_setup_global_gtt’:
drivers/gpu/drm/i915/i915_gem_gtt.c:2659:33: error: ‘ggtt’ undeclared (first use in this function)
   list_add_tail(&vma->mm_list, &ggtt->inactive_list);
                                 ^
Jani.

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 16 ++++++----------
>  2 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 01f3521e77d3..9cc68624410e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2533,7 +2533,6 @@ static int ggtt_bind_vma(struct i915_vma *vma,
>  		 * the bound flag ourselves.
>  		 */
>  		vma->bound |= GLOBAL_BIND;
> -
>  	}
>  
>  	if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) {
> @@ -2657,6 +2656,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>  			return ret;
>  		}
>  		vma->bound |= GLOBAL_BIND;
> +		list_add_tail(&vma->mm_list, &ggtt->inactive_list);
>  	}
>  
>  	/* Clear any non-preallocated blocks */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 55df6ce34751..15207796e1b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -584,7 +584,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  	vma = i915_gem_obj_lookup_or_create_vma(obj, ggtt);
>  	if (IS_ERR(vma)) {
>  		ret = PTR_ERR(vma);
> -		goto err_out;
> +		goto err;
>  	}
>  
>  	/* To simplify the initialisation sequence between KMS and GTT,
> @@ -598,23 +598,19 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  		ret = drm_mm_reserve_node(&ggtt->mm, &vma->node);
>  		if (ret) {
>  			DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
> -			goto err_vma;
> +			goto err;
>  		}
> -	}
>  
> -	vma->bound |= GLOBAL_BIND;
> +		vma->bound |= GLOBAL_BIND;
> +		list_add_tail(&vma->mm_list, &ggtt->inactive_list);
> +	}
>  
>  	list_add_tail(&obj->global_list, &dev_priv->mm.bound_list);
> -	list_add_tail(&vma->mm_list, &ggtt->inactive_list);
>  	i915_gem_object_pin_pages(obj);
>  
>  	return obj;
>  
> -err_vma:
> -	i915_gem_vma_destroy(vma);
> -err_out:
> -	i915_gem_stolen_remove_node(dev_priv, stolen);
> -	kfree(stolen);
> +err:
>  	drm_gem_object_unreference(&obj->base);
>  	return NULL;
>  }
> -- 
> 2.5.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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