Re: [PATCH v2 3/3] drm/i915: Consolidate ring freespace calculations

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 18, 2014 at 08:07:22PM +0000, Dave Gordon wrote:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index ae09258..a08ae65 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -52,16 +52,27 @@ intel_ring_initialized(struct intel_engine_cs *ring)
>  
>  int __intel_ring_space(int head, int tail, int size)
>  {
> -	int space = head - (tail + I915_RING_FREE_SPACE);
> -	if (space < 0)
> +	int space = head - tail;
> +	if (space <= 0)
>  		space += size;
> -	return space;
> +	return space - I915_RING_FREE_SPACE;

Changing the free space computation doesn't seem required, but resulting
in Chris&me just discussing it on irc to convince ourselves it's not
broken accidentally now. Can you please respin you patch with this change
dropped?

In generally it's good practice to review the diff after committing a
patch and hunt for any unecessary changes. Imo even when the endresult
looks a bit ulgy (e.g. crazy ordering of static functions which requires
tons of predeclarations) it's better if the resulting diff looks cleaner.
And if things get out of hand we can always do a pure cleanup patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx