Re: [PATCH 08/15] drm/i915: Move execlists defines from .c to .h

[Chris Wilson <chris@chris-wilson.co.uk>]

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@intel.com>
> >>
> >> 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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx