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

[Paulo Zanoni]

2013/10/10 Chris Wilson <chris at chris-wilson.co.uk>:
> 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 :)

Do we have plans for a V2 based on the comments? I wanted to use the
macro on a bug fix and discovered we didn't merge this yet.

Anyway, I can write follow-up patches for my own suggestions, so if we
merge V1 it's fine for me. Reviewed-by: Paulo Zanoni
<paulo.r.zanoni at intel.com>


> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni