From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 7/9] drm/i915: allow package C8+ states on Haswell (disabled) Date: Wed, 7 Aug 2013 18:02:43 +0200 Message-ID: <20130807160243.GX22035@phenom.ffwll.local> References: <1375826239-3060-1-git-send-email-przanoni@gmail.com> <1375826239-3060-8-git-send-email-przanoni@gmail.com> <20130807005449.GB10740@cantiga.alporthouse.com> <20130807142009.GB16248@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f46.google.com (mail-ee0-f46.google.com [74.125.83.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 43E03E60D3 for ; Wed, 7 Aug 2013 09:02:37 -0700 (PDT) Received: by mail-ee0-f46.google.com with SMTP id c13so941029eek.19 for ; Wed, 07 Aug 2013 09:02:36 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130807142009.GB16248@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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: Chris Wilson , Paulo Zanoni , intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Wed, Aug 07, 2013 at 03:20:09PM +0100, Chris Wilson wrote: > On Wed, Aug 07, 2013 at 10:38:19AM -0300, Paulo Zanoni wrote: > > 2013/8/6 Chris Wilson : > > > On Tue, Aug 06, 2013 at 06:57:17PM -0300, Paulo Zanoni wrote: > > >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > >> index 2ef4335..7f6ec9e 100644 > > >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > >> @@ -1590,6 +1590,8 @@ void intel_ring_advance(struct intel_ring_buffer *ring) > > >> { > > >> struct drm_i915_private *dev_priv = ring->dev->dev_private; > > >> > > >> + hsw_package_c8_gpu_busy(dev_priv); > > >> + > > > > > > Ahem, what is this doing here? If only we had something that was the > > > opposite of intel_mark_idle... At this point, I am going to get some > > > sleep... > > > > You're suggesting me to use intel_mark_busy? I have to admit I saw it, > > but I noticed intel_mark_busy is called after intel_ring_advance, and > > I was trying to follow what Ben suggested me. If you're suggesting > > this, I guess it's ok, so I will test this possibility. > > intel_mark_busy() is where we note the transition in userspace activity, > it should be where we put all the little hooks and pm tweaks required. > If you however need the pc8 disable earlier than the ring register write, > we should do it in intel_ring_begin(). Ultimately, though we should be > able to squash it into a single operation such that we never submit a > command before intel_mark_busy(). Hmm, easier in fact just to move > intel_mark_busy() to intel_ring_begin() and I think everybody is happy. We have oddball places where we add stuff to the ring but not a request, so we'd miss the eventual mark_idle. But since this is only to make sure we have working interrupts the current place of mark_busy should be good enough. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch