All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Paulo Zanoni <przanoni@gmail.com>,
	intel-gfx@lists.freedesktop.org,
	Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 4/9] drm/i915: don't update GEN6_PMIMR when it's not needed
Date: Tue, 20 Aug 2013 16:18:37 +0200	[thread overview]
Message-ID: <20130820141837.GS776@phenom.ffwll.local> (raw)
In-Reply-To: <20130807141451.GA16248@cantiga.alporthouse.com>

On Wed, Aug 07, 2013 at 03:14:51PM +0100, Chris Wilson wrote:
> On Wed, Aug 07, 2013 at 10:34:11AM -0300, Paulo Zanoni wrote:
> > 2013/8/6 Chris Wilson <chris@chris-wilson.co.uk>:
> > > On Tue, Aug 06, 2013 at 06:57:14PM -0300, Paulo Zanoni wrote:
> > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >>
> > >> I did some brief tests and the "new_val = pmimr" condition usually
> > >> happens a few times after exiting games.
> > >>
> > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >
> > > I'm not sure of the value of this patch by itself. It did make me wonder
> > > what you were micro-optimising, and then I saw patch 5 and it made more
> > > sense.
> > 
> > Patches 4 and 5 are just micro optimizations and shouldn't be needed
> > for the PC8+, but I thought they would be useful. If you think they're
> > not worth it, we can discard them. I was trying to make the code
> > similar to the other IMR-changing functions.
> 
> Combined together, I think the micro-optimisation makes sense and would
> say it was less of a micro-optimisation than a consistent design to use
> the bookkeeping instead of touching registers. Just on its own this
> patch caused me to do a double-take and question what your motivation
> was.

I've added a small note to the commit message to (hopefully) capture this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2013-08-20 14:18 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06 21:57 [PATCH 0/9] Haswell Package C8+ Paulo Zanoni
2013-08-06 21:57 ` [PATCH 1/9] drm/i915: add the FCLK case to intel_ddi_get_cdclk_freq Paulo Zanoni
2013-08-14 18:42   ` Rodrigo Vivi
2013-08-06 21:57 ` [PATCH 2/9] drm/i915: wrap GTIMR changes Paulo Zanoni
2013-08-15  0:19   ` Rodrigo Vivi
2013-08-15 13:21     ` Paulo Zanoni
2013-08-06 21:57 ` [PATCH 3/9] drm/i915: wrap GEN6_PMIMR changes Paulo Zanoni
2013-08-15  0:22   ` Rodrigo Vivi
2013-08-15 13:23     ` Paulo Zanoni
2013-08-06 21:57 ` [PATCH 4/9] drm/i915: don't update GEN6_PMIMR when it's not needed Paulo Zanoni
2013-08-07  0:35   ` Chris Wilson
2013-08-07 13:34     ` Paulo Zanoni
2013-08-07 14:14       ` Chris Wilson
2013-08-20 14:18         ` Daniel Vetter [this message]
2013-08-15  0:28   ` Rodrigo Vivi
2013-08-06 21:57 ` [PATCH 5/9] drm/i915: add dev_priv->pm_irq_mask Paulo Zanoni
2013-08-15  0:36   ` Rodrigo Vivi
2013-08-15 13:31     ` Paulo Zanoni
2013-08-06 21:57 ` [PATCH 6/9] drm/i915: don't disable/reenable IVB error interrupts when not needed Paulo Zanoni
2013-08-15  0:41   ` Rodrigo Vivi
2013-08-20 14:21   ` Daniel Vetter
2013-08-20 14:43     ` Paulo Zanoni
2013-08-20 15:11       ` Daniel Vetter
2013-08-20 18:07         ` Paulo Zanoni
2013-08-06 21:57 ` [PATCH 7/9] drm/i915: allow package C8+ states on Haswell (disabled) Paulo Zanoni
2013-08-07  0:54   ` Chris Wilson
2013-08-07 13:38     ` Paulo Zanoni
2013-08-07 14:20       ` Chris Wilson
2013-08-07 16:02         ` Daniel Vetter
2013-08-09 20:10           ` Paulo Zanoni
2013-08-09 20:32             ` Chris Wilson
2013-08-09 21:34               ` Paulo Zanoni
2013-08-10  7:55                 ` Daniel Vetter
2013-08-10  8:04                   ` Chris Wilson
2013-08-12 22:02                     ` Paulo Zanoni
2013-08-09 20:42             ` Chris Wilson
2013-08-09 21:25               ` Paulo Zanoni
2013-08-06 21:57 ` [PATCH 8/9] drm/i915: add i915_pc8_status debugfs file Paulo Zanoni
2013-08-06 21:57 ` [PATCH 9/9] drm/i915: enable Package C8+ by default Paulo Zanoni
2013-08-06 22:31 ` [PATCH 0/9] Haswell Package C8+ Daniel Vetter
2013-08-07 13:30   ` Paulo Zanoni
2013-08-09 20:04 ` [PATCH 6.1/9] drm/i915: don't queue PM events we won't process Paulo Zanoni
2013-08-09 20:04   ` [PATCH 6.2/9] drm/i915: fix how we mask PMIMR when adding work to the queue Paulo Zanoni
2013-08-20 14:26     ` Daniel Vetter
2013-08-09 20:04   ` [PATCH 6.3/9] drm/i915: merge HSW and SNB PM irq handlers Paulo Zanoni
2013-08-14 19:21     ` Ben Widawsky
2013-08-15 14:51       ` Paulo Zanoni
2013-08-20 14:27         ` Daniel Vetter
2013-08-14 18:36   ` [PATCH 6.1/9] drm/i915: don't queue PM events we won't process Ben Widawsky
2013-08-15 14:50     ` Paulo Zanoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130820141837.GS776@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    --cc=przanoni@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.