From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 0/9] Haswell Package C8+
Date: Tue, 6 Aug 2013 18:57:10 -0300 [thread overview]
Message-ID: <1375826239-3060-1-git-send-email-przanoni@gmail.com> (raw)
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Hi
Here's my next attempt at the Haswell PC8+ feature.
What's new?
1. I hope I implemented the feedback from Chris. Many function renames, a few
style changes. Chris' biggest concern was about the interrupts. So in this
series you'll see patches 2-6 moving some interrupt code around, so all
changes to each IMR register should go through a single function. With this
change we can easily detect when someone wants to change IMR but interrupts
are disabled. As he suggested, we also don't enable the interrupts in that
case.
2. Another difference is that now we also disable PC8 when there's GPU work to
do. The lack of interrupts made things like glxgears work at 2 FPS. I talked
to Ben about this and what I understood is that the lack of interrupts is
not really a problem, it just makes things slower. But still I decided to
write the code to disable PC8 when we have GPU work to do, so background
apps can run faster than 2 FPS.
3. Another difference is that now we enable PC8 on a delayed work function that
only gets called if we stay with 0 refcount for at least 5 seconds. So when
someone runs "xrandr" we won't enable/disable PC8 dozens of times. Also, on
"xset dpms force off" we'll only get to PC8 if the desktop environment
decides to not do rendering every second. Not getting to PC8? Fix your
desktop environment!
4. Due to 3 and the fact that Daniel didn't seem to like my "do DP AUX and
GMBUS without interrupts" approach, the previous patches that implemented
this just got dropped: we now have the delayed work thing which should
replace them.
5. I also added code to wake up from PC8 when doing suspend, we need this.
Despite this, the other thing to discuss is about the size of patch 7. I can
certainly try to split it, but I really think that if all you want is to take a
brief look at the code just and drop some bikesheds, then having a big patch
that implements everything in one piece seems better. I can split this later if
needed. I'm also open to ideas on how to really split this patch. Also notice
that even if the patch is big, it doesn't remove a single line of the current
code. And it adds PC8 disabled by default, so if git bisect points to that patch
the surface to look will be small.
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.
Even if we decide to delay patch 7, we could try to merge patches 1-6 if they
look acceptable. I'm also not really sure if we want the last patch yet, but
it's there just in case.
Also, I couldn't do i-g-t regression testing since the i-g-t test suite is quite
unreliable on Haswell right now.
Thanks,
Paulo
Paulo Zanoni (9):
drm/i915: add the FCLK case to intel_ddi_get_cdclk_freq
drm/i915: wrap GTIMR changes
drm/i915: wrap GEN6_PMIMR changes
drm/i915: don't update GEN6_PMIMR when it's not needed
drm/i915: add dev_priv->pm_irq_mask
drm/i915: don't disable/reenable IVB error interrupts when not needed
drm/i915: allow package C8+ states on Haswell (disabled)
drm/i915: add i915_pc8_status debugfs file
drm/i915: enable Package C8+ by default
drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++
drivers/gpu/drm/i915/i915_dma.c | 10 ++
drivers/gpu/drm/i915/i915_drv.c | 11 ++
drivers/gpu/drm/i915/i915_drv.h | 73 ++++++++++++
drivers/gpu/drm/i915/i915_irq.c | 202 +++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_ddi.c | 9 +-
drivers/gpu/drm/i915/intel_display.c | 164 ++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_dp.c | 3 +
drivers/gpu/drm/i915/intel_drv.h | 14 +++
drivers/gpu/drm/i915/intel_i2c.c | 2 +
drivers/gpu/drm/i915/intel_pm.c | 13 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++---
12 files changed, 518 insertions(+), 40 deletions(-)
--
1.8.1.2
next reply other threads:[~2013-08-06 21:57 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-06 21:57 Paulo Zanoni [this message]
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
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=1375826239-3060-1-git-send-email-przanoni@gmail.com \
--to=przanoni@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=paulo.r.zanoni@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).