[Intel-gfx] [PATCH 1/6] drm/i915: Colocate all GT access routines in the same file

[Chris Wilson]

On Sun, Jul 14, 2013 at 12:42:49PM -0700, Ben Widawsky wrote:
> On Fri, Jul 12, 2013 at 06:08:22PM +0100, Chris Wilson wrote:
> > Currently, the register access code is split between i915_drv.c and
> > intel_pm.c. It only bares a superficial resemblance to the reset of the
> > powermanagement code, so move it all into its own file. This is to ease
> > further patches to enforce serialised register access.
> > 
> > v2: Scan for random abuse of I915_WRITE_NOTRACE
> > v3: Take the opportunity to rename the GT functions as uncore. Uncore is
> > the term used by the hardware design (and bspec) for all functions
> > outside of the GPU (and CPU) cores in what is also known as the System
> > Agent.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> git am complains about a missing newline at EOF, but I guess Daniel will
> fix it on merge.
> 
> Just bikesheds:
> As before, intel_uncore_(clear|check)_errors seems silly to me. 

I still don't understand what you mean. They are just the code that
existed before, so what is silly in moving them since they depend upon
bypassing the common i915_write/read code?

> Also if
> you extracted those as a separate patch to the gt funcs, you could have
> had just a simple file move + rename. And as you made me realize, I'm
> not horribly thrilled with the name uncore, I liked gt. For me, the
> distinction between _pm, and _uncore isn't really large enough, ie. many
> things in _pm are really uncore also (anything touching the punit/rps,
> etc). Also, since gt_funcs never really expanded, maybe just call it
> forcewake_funcs and be done with that (since uncore forcewake sounds
> weird to me).

Looks like you made the distinction pretty clear in that paragraph
between gt and pm. The name change is mostly a whim because we no longer
refer to this as being the GT.
 
> Final bikeshed, I would like to see the reset code in a separate file as
> well (included in this could be any GEM functions we have for reset
> only, and display as well).

No. Look at the reset code, it is far too incestrous with the gt/uncore
mechanics to be anywhere else. And it didn't seem right to separate it
up.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre