On 17/06/15 08:59, Chris Wilson wrote: > On Wed, Jun 17, 2015 at 08:31:59AM +0100, Dave Gordon wrote: >> On 16/06/15 10:37, Chris Wilson wrote: >>> On Mon, Jun 15, 2015 at 07:36:26PM +0100, Dave Gordon wrote: >>>> From: "Michael H. Nguyen" <michael.h.nguyen@xxxxxxxxx> >>>> >>>> Move defines from intel_lrc.c to i915_reg.h so they are accessible >>>> to the GuC submission code; and expose a previously static function >>>> in the execlist code which will also be required for GuC submission. >>> >>> What would have been better would have to been to split the lrc code >>> from the execlists code so that the sharing is more obvious and the >>> overloading separate from the common code. >>> -Chris >> >> What would have been better is not to have put these fairly generic >> details about the hardware into a C file in the first place. And not to >> have split execlist and ringbuffer modes into two entirely different >> paths. And various other historical decisions. But we can only fix the >> code as it stands, not as it ought to have been. >> >> Anyway, this is just a bulk cut-n-paste, so I'm not inclined to do any >> restructuring on it during this process. But someone working on >> execlists could certainly tidy it up later, perhaps as part of a general >> drive towards deduplicating the code paths and partitioning (context vs >> ringbuffer vs engine) functionality in a more coherent way. > > More to the point, you are increasing the technical debt of the code > rather than reducing it. Code will just become less and less > maintainable. > -Chris OK, I have abolished the bulk cut'n'paste :) Turns out we really only needed a bit of it, and then I spotted a way to reuse some of the "execlists" code (which is really LRC code) instead of having a GuC version of same (which is what needed some of these #defines). So in the end, this patch is replaced by simply renaming-and-exposing the /two/ functions in intel_lrc.c that we actually need for GuC submission. Better still, one of them may go away entirely once we eat some of Chris' low-hanging fruit :) .Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx