From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: fixup i915_gem_object_get_page inline helper Date: Tue, 09 Oct 2012 21:41:19 +0100 Message-ID: <84c8a8$624sk4@orsmga001.jf.intel.com> References: <1349805542-28240-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id CCF199EC00 for ; Tue, 9 Oct 2012 13:41:34 -0700 (PDT) In-Reply-To: <1349805542-28240-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Intel Graphics Development Cc: Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On Tue, 9 Oct 2012 19:59:02 +0200, Daniel Vetter wrote: > The obj->pages to obj->pages->sgl rework introduced this helper, but > it doesn't actually work for n % SG_MAX_SINGLE_ALLOC == 0. > > This is exercised by the improved hangman tests and the gem_exec_big > test in i-g-t. > > Signed-off-by: Daniel Vetter I had to think about that harder than I should, to be sure we handled the case where the last element was a page and not a chainptr correctly. On hindsight, the code is clearly bogus for the n==SG_MAX_SINGLE_ALLOC because n is an index. My original thought was to use 'nth' instead and I believe that it would have helped prevent my confusion. Ah hindisght. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre