On Tue, 28 Jan 2014, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > Anything more than just one bool parameter is just a pain to read, > symbolic constants are much better. > > Split out from Chris' vma-binding rework patch. > > v2: Undo the behaviour change in object_pin that Chris spotted. > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> [snip] > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 032def901f98..9399a6fa4c2f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -544,19 +544,23 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, > struct drm_i915_gem_object *obj = vma->obj; > struct drm_i915_gem_exec_object2 *entry = vma->exec_entry; > bool has_fenced_gpu_access = INTEL_INFO(ring->dev)->gen < 4; > - bool need_fence, need_mappable; > - u32 flags = (entry->flags & EXEC_OBJECT_NEEDS_GTT) && > - !vma->obj->has_global_gtt_mapping ? GLOBAL_BIND : 0; > + bool need_fence; > + unsigned flags; > int ret; > > + flags = 0; > + > need_fence = > has_fenced_gpu_access && > entry->flags & EXEC_OBJECT_NEEDS_FENCE && > obj->tiling_mode != I915_TILING_NONE; > - need_mappable = need_fence || need_reloc_mappable(vma); > + if (need_fence || need_reloc_mappable(vma)) > + flags |= PIN_MAPPABLE; > + > + if (entry->flags & EXEC_OBJECT_NEEDS_GTT) > + flags |= PIN_GLOBAL; It is not obvious to me that this together with the PIN_GLOBAL handling in i915_gem_object_pin() do not introduce a functional change. (Stress on obvious to _me_; it may be obvious to you.) I would have thought it better to first change the two bool parameters to two flags, and then add the new flag in a separate patch to not confuse poor reviewers like myself. [snip] > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index d897a19f887f..a0793c929b95 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -531,9 +531,11 @@ init_pipe_control(struct intel_ring_buffer *ring) > goto err; > } > > - i915_gem_object_set_cache_level(ring->scratch.obj, I915_CACHE_LLC); > + ret = i915_gem_object_set_cache_level(ring->scratch.obj, I915_CACHE_LLC); > + if (ret) > + goto err_unref; This should be split out from the patch just like patch 4/9. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
