Re: [PATCH 11/12] drm/i915: Clarify the semantics of tiling-changed by renaming it

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 17, 2012 at 03:31:34PM +0100, Chris Wilson wrote:
> Rename obj->tiling_changed to obj->fence_change so that it is clear that
> it flags when the parameters for an active fence (including the
> no-fence) register are changed.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I've picked up all the patches for -next up to this one. Thanks a lot for
cleaning out the cruft here. For this patch I've got a bit confused about
fence_changed. Chris suggested fence_dirty on irc and I like it.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    7 ++++++-
>  drivers/gpu/drm/i915/i915_gem.c        |    8 ++++----
>  drivers/gpu/drm/i915/i915_gem_tiling.c |    5 ++++-
>  3 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c32ada..3d7ad9b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -869,7 +869,12 @@ struct drm_i915_gem_object {
>  	 * Current tiling mode for the object.
>  	 */
>  	unsigned int tiling_mode:2;
> -	unsigned int tiling_changed:1;
> +	/**
> +	 * Whether the tiling parameters for the currently associated fence
> +	 * register have changed. Note that for the purposes of tracking
> +	 * tiling changes we also treat the unfenced register as a "fence".
> +	 */
> +	unsigned int fence_changed:1;
>  
>  	/** How many users have pinned this object in GTT space. The following
>  	 * users can each hold at most one reference: pwrite/pread, pin_ioctl
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7bc4a40..5edab3f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -66,7 +66,7 @@ static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
>  	/* As we do not have an associated fence register, we will force
>  	 * a tiling change if we ever need to acquire one.
>  	 */
> -	obj->tiling_changed = false;
> +	obj->fence_changed = false;
>  	obj->fence_reg = I915_FENCE_REG_NONE;
>  }
>  
> @@ -2456,7 +2456,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
>  	/* Have we updated the tiling parameters upon the object and so
>  	 * will need to serialise the write to the associated fence register?
>  	 */
> -	if (obj->tiling_changed) {
> +	if (obj->fence_changed) {
>  		ret = i915_gem_object_flush_fence(obj);
>  		if (ret)
>  			return ret;
> @@ -2465,7 +2465,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
>  	/* Just update our place in the LRU if our fence is getting reused. */
>  	if (obj->fence_reg != I915_FENCE_REG_NONE) {
>  		reg = &dev_priv->fence_regs[obj->fence_reg];
> -		if (!obj->tiling_changed) {
> +		if (!obj->fence_changed) {
>  			list_move_tail(&reg->lru_list,
>  				       &dev_priv->mm.fence_list);
>  			return 0;
> @@ -2488,7 +2488,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
>  		return 0;
>  
>  	i915_gem_object_update_fence(obj, reg, enable);
> -	obj->tiling_changed = false;
> +	obj->fence_changed = false;
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 1a93066..e51d892 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -374,7 +374,10 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
>  		}
>  
>  		if (ret == 0) {
> -			obj->tiling_changed = true;
> +			obj->fence_changed =
> +				obj->fenced_gpu_access ||
> +				obj->fence_reg != I915_FENCE_REG_NONE;
> +
>  			obj->tiling_mode = args->tiling_mode;
>  			obj->stride = args->stride;
>  		}
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48