From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH 3/8] drm/i915: allow package C8+ states on
 Haswell (disabled)
Date: Wed, 31 Jul 2013 16:01:27 +0100
Message-ID: <20130731150127.GC3637@cantiga.alporthouse.com>
References: <1375130907-4054-1-git-send-email-przanoni@gmail.com>
	<1375130907-4054-4-git-send-email-przanoni@gmail.com>
	<20130729214205.GA17854@cantiga.alporthouse.com>
	<CA+gsUGTecz4MTg4ubOCYvx6wK0Kfdr9Gbbw6xUgmxa=F-R5Tug@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from fireflyinternet.com (s16502780.onlinehome-server.info
	[87.106.93.118])
	by gabe.freedesktop.org (Postfix) with ESMTP id CEB1BE6417
	for <intel-gfx@lists.freedesktop.org>;
	Wed, 31 Jul 2013 08:01:38 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <CA+gsUGTecz4MTg4ubOCYvx6wK0Kfdr9Gbbw6xUgmxa=F-R5Tug@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
List-Id: intel-gfx@lists.freedesktop.org

On Wed, Jul 31, 2013 at 11:24:22AM -0300, Paulo Zanoni wrote:
> 2013/7/29 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Mon, Jul 29, 2013 at 05:48:22PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@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.

> > 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