From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> Hi Some time ago I sent the first versions of patches to allow PC8+ and received some feedback from Daniel. Based on his feedback and subsequent conversations, we agreed that the right way to go would be to modify our "irq_uninstall" function so it would allow uninstalling "everything but PCH HPD and master". Then when we allow PC8+ we'd call the modified irq_uninstall, and then when we disallow PC8+ we'call irq_postinstall. So I started studying our interrupt code and noticed that if we wanted to allow irq_uninstall/irq_postinstall multiple times on our code we'd have to change a lot of stuff. So I wrote that code and also fixed other issues I also identified. Some of these patches were already sent to the mailing list (and even merged), but the part that allows us to call irq_uninstall+irq_postinstall multiple times was not sent to the list yet. In the end I felt unhappy with the resulting code: it looked ugly and still didn't look bullet-proof (disable_irq is not cool). So I had a new idea: instead of completely disabling the interrupts I decided to only disable the IMR registers. We already touch the IMR registers everywhere on our code and avoid the save/restore procedures on the IRQ handlers, so it should be safe to deal with them. I added a small save/restore procedure for the IMR registers and also added some WARNs that will tell us in case anybody tries to touch those registers while PC8 is allowed. Even if we're still using a small register save/restore file, I think this solution is much cleaner than the original patch I proposed, so maybe Daniel thinks it's acceptable. A small notice about the regfile: if you take a close look you'll see that we also save/restore GTIER. This is because the GTIER register is reset when we enter PC8. It should be safe to save/restore GTIER since it's only written at our preinstall/postinstall/uninstall functions. Patch 3 is the big patch which you should take a look. I couldn't find a nice way to break it into more patches without writing patches that would just add static functions that no one calls. It's probably easier to review this patch as a whole than to review each piece of the puzzle in separate. But I'm open to suggestions here. The other patches are very small and self-explanatory. Notice that I won't just discard all the IRQ cleanups I already wrote, I will still send them. I will just skip the ugly controversial patches that were only useful for the PC8 work. Thanks, Paulo Paulo Zanoni (8): drm/i915: add the FCLK case to intel_ddi_get_cdclk_freq drm/i915: don't disable/reenable IVB error interrupts when not needed drm/i915: allow package C8+ states on Haswell (disabled) drm/i915: avoid waking up from PC8 on DP AUX operations drm/i915: avoid waking up from PC8 on GMBUS operations drm/i915: silence message about the DDI buffers drm/i915: add i915_pc8_status debugfs file drm/i915: allow Package C8+ by default drivers/gpu/drm/i915/i915_debugfs.c | 21 +++++++ drivers/gpu/drm/i915/i915_dma.c | 4 ++ drivers/gpu/drm/i915/i915_drv.c | 4 ++ drivers/gpu/drm/i915/i915_drv.h | 23 +++++++ drivers/gpu/drm/i915/i915_irq.c | 73 ++++++++++++++++++++++- drivers/gpu/drm/i915/intel_ddi.c | 13 ++-- drivers/gpu/drm/i915/intel_display.c | 112 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_dp.c | 25 +++++++- drivers/gpu/drm/i915/intel_drv.h | 6 ++ drivers/gpu/drm/i915/intel_i2c.c | 73 ++++++++++++++++------- drivers/gpu/drm/i915/intel_pm.c | 15 +++++ 11 files changed, 336 insertions(+), 33 deletions(-) -- 1.8.1.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
