[Intel-gfx] [PATCH 1/2] drm/i915: Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset

[Chris Wilson]

On Wed, Sep 30, 2015 at 04:45:18PM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 30/09/15 15:36, Michel Thierry wrote:
> >diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >index 67ef118..6ca39c1 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >@@ -589,11 +589,20 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> >  	if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
> >  		flags |= PIN_GLOBAL;
> >
> >+	/* Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset,
> >+	 * limit address to the first 4GBs for unflagged objects.
> >+	 */
> >+	flags |= PIN_ZONE_4G;
> >+	if (entry->flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)
> >+		flags &= ~PIN_ZONE_4G;
> 
> I spotted this patch purely accidentally since it was probably the
> reason pad_to_size IGT started failing in the Android tree - given
> how there is mention of changing the allocation order. Anyway beside
> the point..
> 
> Point is when I spotted it by accident, I also spotted this unusual
> handling of flags - set then conditionally clear. Why not
> conditionally set for one fewer line of code?

Because for the next few years entry->flags & 48B will be in the
minority. The other reason is that is emphasizes that we limit
everything by default and only allow the special objects to use highmem.

The idiom of:
  x = default/expected;
  if (unlikely)
  	x = something else;
is fairly commonplace in the kernel, at least before gcc had likely().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre