[Intel-gfx] [PATCH] drm/i915: Remove redundant get_pages call

[Daniel Vetter]

On Tue, Dec 01, 2015 at 06:11:16PM +0000, Dave Gordon wrote:
> On 28/10/15 12:08, ankitprasad.r.sharma at intel.com wrote:
> >From: Ankitprasad Sharma <ankitprasad.r.sharma at 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 at 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?

pin into ggtt pretty much means that the backing storage must be there.
Yes we totally suck at documenting GEM, but then people also get dragged
away from upstream again before there's time to write docs or clean up
some of the incidental confusion that's sprinkled on top of the
fundamental design ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch