2013/7/14 Chris Wilson <chris at chris-wilson.co.uk>: > 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. While testing the Package C8+ feature I'm implementing, one of the things I tried was "for i in $(seq 20); do glxgears & done". After I ran this command, when I tried to drag the glxgears windows around I got a hard machine hang. Then I applied this patch series and now the problem is gone :) Also, "git am" complains about white space errors on patch 1 :) Thanks for the fix! Paulo > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni
