[Intel-gfx] [PATCH 2/2] drm/i915: Do not enable package C8 on unsupported hardware

[Chris Wilson]

On Thu, Oct 10, 2013 at 05:17:31PM -0300, Paulo Zanoni wrote:
> 2013/10/10 Chris Wilson <chris at chris-wilson.co.uk>:
> > If the hardware does not support package C8, then do not even schedule
> > work to enable it. Thereby we can eliminate a bunch of dangerous work.
> 
> As I already explained, this should not be a problem since non-Haswell
> platforms don't have a way to make the refcount become zero (unless we
> have a bug). I also asked people's opinions about this specific
> decision in one of my cover letters, but no one said anything:
> http://lists.freedesktop.org/archives/intel-gfx/2013-August/031440.html.
> 
> Quoting the email: "Another thing worth mentioning is that all this
> code doesn't have IS_HASWELL checks, and on non-Haswell platforms the
> refcount will never reach 0, so we won't ever try to enable PC8. I'm
> not sure if that's what we want, so please comment on that.".
> 
> That said, I'm not against your changes.

If they don't actually fix anything, they are low priority as they only
remove a mutex lock at most 10Hz. Maybe a comment would be good to remind
the next person that nothing gets enabled except on hsw.

> > +#define HAS_PC8(dev)           (IS_HASWELL(dev)) /* XXX HSW:ULX */
> 
> What exactly do you mean with this comment? Did you actually mean
> "IS_ULT()"? Even though only ULT has PC8-10 residencies, non-ULT seems
> to work fine with this code, so I thought it wouldn't be a problem.

It means I didn't actually check the valid restrictions :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre