From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
