On Fri, Apr 22, 2016 at 07:22:55PM +0100, Dave Gordon wrote: > This patch simply changes the default value of "enable_guc_submission" > from 0 (never) to -1 (auto). This means that GuC submission will be > used if the platform has a GuC, the GuC supports the request submission > protocol, and any required GuC firmwware was successfully loaded. If any > of these conditions are not met, the driver will fall back to using > execlist mode. There are several shortcomings yet, in particular you've decided to add new ABI... i915_guc_wq_check_space(): Do not return ETIMEDOUT. This function's return code is ABI. Your choices are either EAGAIN if you think the hardware will catch up, or more likely EIO and reset the hardware. guc_add_workqueue_item: cannot fail It must be fixed such that it is a void function without possibility of failure. Not that you even successfully hooked up the failure paths in the current code. Same for guc_ring_doorbell. And what exactly is that atomic64_cmpxchg() serialising with? There are no other CPUs contending with the write, and neither does the GuC (and I doubt it is taking any notice of the lock cmpxchg). Using cmpxchg where a single WRITE_ONCE() of a 32bit value wins the perf prize for hotest instruction and function in the kernel. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
