Re: [PATCH 16/59] drm/i915: Add flag to i915_add_request() to skip the cache flush

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 31, 2015 at 05:32:07PM +0100, Tomas Elf wrote:
> On 19/03/2015 12:30, John.C.Harrison@Intel.com wrote:
> >@@ -2361,12 +2362,14 @@ void __i915_add_request(struct intel_engine_cs *ring,
> >  	 * is that the flush _must_ happen before the next request, no matter
> >  	 * what.
> >  	 */
> >-	if (i915.enable_execlists)
> >-		ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
> >-	else
> >-		ret = intel_ring_flush_all_caches(ring);
> >-	/* Not allowed to fail! */
> >-	WARN_ON(ret);
> >+	if (flush_caches) {
> >+		if (i915.enable_execlists)
> >+			ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
> >+		else
> >+			ret = intel_ring_flush_all_caches(ring);
> >+		/* Not allowed to fail! */
> >+		WARN_ON(ret);
> 
> There has been a discussion about whether it's ok to use
> WARN_ON(<variable/constant>) since it doesn't add any useful information
> about the actual failure case to the kernel log. By that logic you should
> add WARN_ON to each individual call to _flush_all_caches above but I would
> be inclined to say that that would be slightly redundant. It could be argued
> that if you get a WARN stack_dump in the kernel log at this point you pretty
> much know from where it came.
> 
> Although, if you want to be able to rely entirely on the kernel log and
> don't want to read the source code to figure out what function call failed
> then you would have to either add WARN_ONs to each individual function call
> or replace WARN_ON(ret) with something like WARN(ret, "flush_all_caches
> returned %d", ret) or something.
> 
> Any other opinion on how we should do this from anyone? What pattern should
> we be following here?

WARN_ON dumps the full backtrace plus current EIP/RIP including basic
decoding using debug information (if compiled in). Maybe you don't have
that enabled in Android builds? Or is there additional information you
want to dump?
-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