On Wed, Aug 14, 2013 at 10:18:55AM +0200, Daniel Vetter wrote:
> On Tue, Aug 13, 2013 at 06:09:07PM -0700, Ben Widawsky wrote:
> > Cleanup the map and fenceable setting during bind to make more sense,
> > and not check i915_is_ggtt() 2 unnecessary times
> >
> > v2: Move the bools into the if block (Chris) - There are ways to tidy
> > this function (fence calculations for instance) even further, but they
> > are quite invasive, so I am punting on those unless specifically asked.
> >
> > v3: Add newline between variable declaration and logic (Chris)
> >
> > Recommended-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 19 +++++++++----------
> > 1 file changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 4a58ead..01cc016 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3106,7 +3106,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
> > struct drm_device *dev = obj->base.dev;
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > u32 size, fence_size, fence_alignment, unfenced_alignment;
> > - bool mappable, fenceable;
> > size_t gtt_max =
> > map_and_fenceable ? dev_priv->gtt.mappable_end : vm->total;
> > struct i915_vma *vma;
> > @@ -3191,18 +3190,18 @@ search_free:
> > list_move_tail(&obj->global_list, &dev_priv->mm.bound_list);
> > list_add_tail(&vma->mm_list, &vm->inactive_list);
> >
> > - fenceable =
> > - i915_is_ggtt(vm) &&
> > - i915_gem_obj_ggtt_size(obj) == fence_size &&
> > - (i915_gem_obj_ggtt_offset(obj) & (fence_alignment - 1)) == 0;
> > + if (i915_is_ggtt(vm)) {
> > + bool mappable, fenceable;
> >
> > - mappable =
> > - i915_is_ggtt(vm) &&
> > - vma->node.start + obj->base.size <= dev_priv->gtt.mappable_end;
> > + fenceable =
> > + i915_gem_obj_ggtt_size(obj) == fence_size &&
> > + (i915_gem_obj_ggtt_offset(obj) & (fence_alignment - 1)) == 0;
>
> Why not go the full mounty and also use vma->node here? Would also make
> checkpatch a bit more happy. I'll do a follow-up commit.
> -Daniel
>
You've already done it, so it's moot. The idea I had was to use "ggtt"
as much as possible when it can only every be ggtt. I think this would
be helpful for both clarity, and debug.
The extra indirection would be immeasurable.
Again, you've already changed it, so meh.
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
