Re: [PATCH] drm/i915: Remove redundant get_pages call

[Dave Gordon <david.s.gordon@intel.com>]

On 28/10/15 12:08, ankitprasad.r.sharma@intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>
> A call to i915_gem_obj_ggtt_pin is being made after this, which again
> calls the get_pages function. Hence removing the redundant call to
> get_pages.
>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 792d0b9..30237e2 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -649,11 +649,6 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
>   	if (!obj)
>   		return NULL;
>
> -	if (i915_gem_object_get_pages(obj)) {
> -		drm_gem_object_unreference(&obj->base);
> -		return NULL;
> -	}
> -
>   	if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
>   			PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) {
>   		drm_gem_object_unreference(&obj->base);

I suppose it is technically redundant, but it's actually quite difficult 
to verify that the call to i915_gem_obj_ggtt_pin() *will* actually take 
the path that, *six levels deeper*, includes the call to 
i915_gem_object_get_pages().

Is there any advantage to calling i915_gem_object_get_pages() later (or 
later)? Does it improve/worsen the chances of hitting a failure path? 
Handling an error from get_pages here is simple, whereas it looks like 
backing out of a failure in the middle of (six levels of) ggtt_pin might 
not be?

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