Re: [PATCH 55/59] drm/i915: Remove fallback poll for ring buffer space

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 31, 2015 at 07:13:40PM +0100, Tomas Elf wrote:
> On 19/03/2015 12:31, John.C.Harrison@Intel.com wrote:
> >From: John Harrison <John.C.Harrison@Intel.com>
> >@@ -652,8 +653,11 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
> >  			break;
> >  	}
> >
> >-	if (&request->list == &ring->request_list)
> >+	/* It should always be possible to find a suitable request! */
> >+	if (&request->list == &ring->request_list) {
> >+		WARN_ON(true);
> 
> I agree with Thomas Daniel's earlier review comment that WARN_ON(true) is
> not very helpful. You could do something like:
> 	
> 	WARN_ON(ret = <expression>);
> 	if (ret)
> 		return -ENOSPC;
> 
> That way you don't have the redundancy of doing
> 
> 	WARN_ON(expression);
> 	if (expression)
> 		return -ENOSPC;
> 
> 
> and you don't have to do if (WARN_ON(expression)) if you don't like it.	

if (WARN_ON(expression)) is prefectly acceptable style in i915 (there's
lots more of these) and what I'd go with here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx