[Intel-gfx] [PATCH 3/8] drm/i915: allow package C8+ states on Haswell (disabled)

[Paulo Zanoni]

2013/7/31 Chris Wilson <chris at chris-wilson.co.uk>:
> On Wed, Jul 31, 2013 at 11:24:22AM -0300, Paulo Zanoni wrote:
>> 2013/7/29 Chris Wilson <chris at chris-wilson.co.uk>:
>> > On Mon, Jul 29, 2013 at 05:48:22PM -0300, Paulo Zanoni wrote:
>> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> >>
>> >> This patch allows PC8+ states on Haswell. These states can only be
>> >> reached when all the display outputs are disabled, and they allow some
>> >> more power savings.
>> >>
>> >> The fact that the graphics device is allowing PC8+ doesn't mean that
>> >> the machine will actually enter PC8+: all the other devices also need
>> >> to allow PC8+.
>> >>
>> >> For now this option is disabled by default. You need i915.allow_pc8=1
>> >> if you want it.
>> >
>> > Still dislike the names. hsw_pc8 is good, so use it consistently.
>>
>> Do you mean i915.allow_hsw_pc8? Or i915.enable_pc8? Or
>> i915.enable_hsw_pc8? (You suggested to change from "allow" to
>> "enable").
>
> i915.enable_pc8 to be consistent with
> i915.enable_psr
> i915.enable_fbc
> i915.enable_rc6
> i915.enable_rps
>
>> > Just call forbid_refcnt, forbid_count (though I'm still liking
>> > wake_count). And replace allowing with display_power_well_active,
>> > verbosity is good here.
>>
>> You mean replace dev_priv->pc8.allowing with
>> dev->priv->pc8.display_power_well_active? That's not good, because
>> when you have eDP-only the display power well is disabled but you
>> can't allow PC8, and then you have more than one output the power well
>> is enabled but you can't allow PC8.
>
> That is not what your code says.

The code says "power well disabled and all outputs disabled". But I
agree that just "allowing" is a bad name, I'll try to think on a good
name.

>
>> > s/i915_allow_pc8/i915_enable_pc8/ for
>> > consistency.
>>
>> I use the word "allow" because even if we allow PC8 it doesn't mean it
>> will actually be enabled, other drivers also need to allow it. But, of
>> course, I could change this anyway.
>
> Right. But as far as we are concerned, and more importantly our
> bookkeeping, we enable it. Whether it is enabled is up to the hardware.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni