Re: [PATCH 08/43] drm/i915: drop register special-casing in forcewake

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

[-- Attachment #1: Type: text/plain, Size: 2149 bytes --]

On Thu, 15 Dec 2011 11:21:27 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Wed, Dec 14, 2011 at 16:05, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Wed, 14 Dec 2011 13:57:05 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >> We currently have 3 register for which we must not grab forcewake for:
> >> FORCEWAKE, FROCEWAKE_MT and ECOBUS.
> >> - FORCEWAKE is excluded in the NEEDS_FORCE_WAKE macro and accessed
> >>   with _NOTRACE.
> >> - FORCEWAKE_MT is just accessed with _NOTRACE.
> >> - ECOBUS is only excluded in the macro.
> >>
> >> In fear of an ever-growing list of special cases and to cut down the
> >> confusion, just access all of them with the _NOTRACE variants.
> >
> > Instead you build in future confusion by making us guess wtf is this using
> > *_NOTRACE. The NOTRACE macro needs a bit of explanation as it now is
> > more than simply skipping the tracepoints, and why certain registers
> > must be accessed through the macro. Also add that warning to the
> > register define.
> 
> When I last checked _NOTRACE was only used to avoid the forcewake
> dance, hence why I didn't add any comment. Would renaming it to
> _NO_FORCEWAKE make you happy, too?

Yeah, the macro did get abused past its original intentions and I'm just
catching up with my complaint. I'd suggest __I915_READ.

> Otherwise I think I'll call it _RAW
> and smash a bunch of comments all over the place, but imo that's
> overkill (and especially in such architectural corner-cases comments
> tend to get stale fast or at least not really reflect reality fully
> correctly).

Right. So the best place for the warning would be next to the register
define that it needs to avoid the forcewake dance, and a mention in the
forcewake dance that some registers are special. Despite the seemingly
futile nature, keeping the relevant information near the code is even
more important when it changes frequently. Knowing precisely which
assumptions and knowledge that was used when the code is written helps
when we need to adapt to new architectures and looking for
oversights/bugs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx