From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 19/23] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled
Date: Fri, 28 Feb 2014 09:15:56 -0800 [thread overview]
Message-ID: <20140228091556.0fe2324d@jbarnes-desktop> (raw)
In-Reply-To: <1393540010-1582-20-git-send-email-przanoni@gmail.com>
On Thu, 27 Feb 2014 19:26:46 -0300
Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> When other platforms add runtime PM support they will also need to
> disable interrupts, so move the variable to the runtime PM struct.
>
> v2: - Rebase.
> v3: - Rebase.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 10 +++----
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 58 ++++++++++++++++++------------------
> drivers/gpu/drm/i915/intel_display.c | 4 +--
> drivers/gpu/drm/i915/intel_drv.h | 4 +--
> drivers/gpu/drm/i915/intel_pm.c | 3 +-
> 7 files changed, 42 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 93d6065..213b093 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1999,7 +1999,7 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
> mutex_lock(&dev_priv->pc8.lock);
> seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy));
> seq_printf(m, "IRQs disabled: %s\n",
> - yesno(dev_priv->pc8.irqs_disabled));
> + yesno(dev_priv->pm.irqs_disabled));
> mutex_unlock(&dev_priv->pc8.lock);
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 16e344e..74fba1c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1350,8 +1350,12 @@ struct ilk_wm_values {
> * For more, read "Display Sequences for Package C8" on our documentation.
> */
> struct i915_package_c8 {
> - bool irqs_disabled;
> struct mutex lock;
> +};
> +
> +struct i915_runtime_pm {
> + bool suspended;
> + bool irqs_disabled;
>
> struct {
> uint32_t deimr;
> @@ -1362,10 +1366,6 @@ struct i915_package_c8 {
> } regsave;
> };
>
> -struct i915_runtime_pm {
> - bool suspended;
> -};
> -
> enum intel_pipe_crc_source {
> INTEL_PIPE_CRC_SOURCE_NONE,
> INTEL_PIPE_CRC_SOURCE_PLANE1,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6978e69..998dabe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1017,7 +1017,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> unsigned long timeout_expire;
> int ret;
>
> - WARN(dev_priv->pc8.irqs_disabled, "IRQs disabled\n");
> + WARN(dev_priv->pm.irqs_disabled, "IRQs disabled\n");
>
> if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f68aee3..14b26f2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -86,9 +86,9 @@ ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
> {
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (dev_priv->pc8.irqs_disabled) {
> + if (dev_priv->pm.irqs_disabled) {
> WARN(1, "IRQs disabled\n");
> - dev_priv->pc8.regsave.deimr &= ~mask;
> + dev_priv->pm.regsave.deimr &= ~mask;
> return;
> }
>
> @@ -104,9 +104,9 @@ ironlake_disable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
> {
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (dev_priv->pc8.irqs_disabled) {
> + if (dev_priv->pm.irqs_disabled) {
> WARN(1, "IRQs disabled\n");
> - dev_priv->pc8.regsave.deimr |= mask;
> + dev_priv->pm.regsave.deimr |= mask;
> return;
> }
>
> @@ -129,10 +129,10 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
> {
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (dev_priv->pc8.irqs_disabled) {
> + if (dev_priv->pm.irqs_disabled) {
> WARN(1, "IRQs disabled\n");
> - dev_priv->pc8.regsave.gtimr &= ~interrupt_mask;
> - dev_priv->pc8.regsave.gtimr |= (~enabled_irq_mask &
> + dev_priv->pm.regsave.gtimr &= ~interrupt_mask;
> + dev_priv->pm.regsave.gtimr |= (~enabled_irq_mask &
> interrupt_mask);
> return;
> }
> @@ -167,10 +167,10 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
>
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (dev_priv->pc8.irqs_disabled) {
> + if (dev_priv->pm.irqs_disabled) {
> WARN(1, "IRQs disabled\n");
> - dev_priv->pc8.regsave.gen6_pmimr &= ~interrupt_mask;
> - dev_priv->pc8.regsave.gen6_pmimr |= (~enabled_irq_mask &
> + dev_priv->pm.regsave.gen6_pmimr &= ~interrupt_mask;
> + dev_priv->pm.regsave.gen6_pmimr |= (~enabled_irq_mask &
> interrupt_mask);
> return;
> }
> @@ -313,11 +313,11 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (dev_priv->pc8.irqs_disabled &&
> + if (dev_priv->pm.irqs_disabled &&
> (interrupt_mask & SDE_HOTPLUG_MASK_CPT)) {
> WARN(1, "IRQs disabled\n");
> - dev_priv->pc8.regsave.sdeimr &= ~interrupt_mask;
> - dev_priv->pc8.regsave.sdeimr |= (~enabled_irq_mask &
> + dev_priv->pm.regsave.sdeimr &= ~interrupt_mask;
> + dev_priv->pm.regsave.sdeimr |= (~enabled_irq_mask &
> interrupt_mask);
> return;
> }
> @@ -4016,32 +4016,32 @@ void intel_hpd_init(struct drm_device *dev)
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> -/* Disable interrupts so we can allow Package C8+. */
> -void hsw_pc8_disable_interrupts(struct drm_device *dev)
> +/* Disable interrupts so we can allow runtime PM. */
> +void hsw_runtime_pm_disable_interrupts(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long irqflags;
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>
> - dev_priv->pc8.regsave.deimr = I915_READ(DEIMR);
> - dev_priv->pc8.regsave.sdeimr = I915_READ(SDEIMR);
> - dev_priv->pc8.regsave.gtimr = I915_READ(GTIMR);
> - dev_priv->pc8.regsave.gtier = I915_READ(GTIER);
> - dev_priv->pc8.regsave.gen6_pmimr = I915_READ(GEN6_PMIMR);
> + dev_priv->pm.regsave.deimr = I915_READ(DEIMR);
> + dev_priv->pm.regsave.sdeimr = I915_READ(SDEIMR);
> + dev_priv->pm.regsave.gtimr = I915_READ(GTIMR);
> + dev_priv->pm.regsave.gtier = I915_READ(GTIER);
> + dev_priv->pm.regsave.gen6_pmimr = I915_READ(GEN6_PMIMR);
>
> ironlake_disable_display_irq(dev_priv, 0xffffffff);
> ibx_disable_display_interrupt(dev_priv, 0xffffffff);
> ilk_disable_gt_irq(dev_priv, 0xffffffff);
> snb_disable_pm_irq(dev_priv, 0xffffffff);
>
> - dev_priv->pc8.irqs_disabled = true;
> + dev_priv->pm.irqs_disabled = true;
>
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> -/* Restore interrupts so we can recover from Package C8+. */
> -void hsw_pc8_restore_interrupts(struct drm_device *dev)
> +/* Restore interrupts so we can recover from runtime PM. */
> +void hsw_runtime_pm_restore_interrupts(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long irqflags;
> @@ -4061,13 +4061,13 @@ void hsw_pc8_restore_interrupts(struct drm_device *dev)
> val = I915_READ(GEN6_PMIMR);
> WARN(val != 0xffffffff, "GEN6_PMIMR is 0x%08x\n", val);
>
> - dev_priv->pc8.irqs_disabled = false;
> + dev_priv->pm.irqs_disabled = false;
>
> - ironlake_enable_display_irq(dev_priv, ~dev_priv->pc8.regsave.deimr);
> - ibx_enable_display_interrupt(dev_priv, ~dev_priv->pc8.regsave.sdeimr);
> - ilk_enable_gt_irq(dev_priv, ~dev_priv->pc8.regsave.gtimr);
> - snb_enable_pm_irq(dev_priv, ~dev_priv->pc8.regsave.gen6_pmimr);
> - I915_WRITE(GTIER, dev_priv->pc8.regsave.gtier);
> + ironlake_enable_display_irq(dev_priv, ~dev_priv->pm.regsave.deimr);
> + ibx_enable_display_interrupt(dev_priv, ~dev_priv->pm.regsave.sdeimr);
> + ilk_enable_gt_irq(dev_priv, ~dev_priv->pm.regsave.gtimr);
> + snb_enable_pm_irq(dev_priv, ~dev_priv->pm.regsave.gen6_pmimr);
> + I915_WRITE(GTIER, dev_priv->pm.regsave.gtier);
>
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 271dd66..420fad5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6667,7 +6667,7 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
> }
>
> lpt_disable_clkout_dp(dev);
> - hsw_pc8_disable_interrupts(dev);
> + hsw_runtime_pm_disable_interrupts(dev);
> hsw_disable_lcpll(dev_priv, true, true);
> }
>
> @@ -6681,7 +6681,7 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
> DRM_DEBUG_KMS("Disabling package C8+\n");
>
> hsw_restore_lcpll(dev_priv);
> - hsw_pc8_restore_interrupts(dev);
> + hsw_runtime_pm_restore_interrupts(dev);
> lpt_init_pch_refclk(dev);
>
> if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2c02d68..d0434e8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -616,8 +616,8 @@ void ilk_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void ilk_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void snb_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void snb_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> -void hsw_pc8_disable_interrupts(struct drm_device *dev);
> -void hsw_pc8_restore_interrupts(struct drm_device *dev);
> +void hsw_runtime_pm_disable_interrupts(struct drm_device *dev);
> +void hsw_runtime_pm_restore_interrupts(struct drm_device *dev);
>
>
> /* intel_crt.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c85507a..5ff4b59 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5770,7 +5770,8 @@ void intel_pm_setup(struct drm_device *dev)
> mutex_init(&dev_priv->rps.hw_lock);
>
> mutex_init(&dev_priv->pc8.lock);
> - dev_priv->pc8.irqs_disabled = false;
> INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
> intel_gen6_powersave_work);
> +
> + dev_priv->pm.irqs_disabled = false;
> }
I wonder if we should eventually not bother with saving/restoring the
interrupt state and do a full re-init like we'll have to in the display
power well case. But that's a separate issue really...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
next prev parent reply other threads:[~2014-02-28 17:15 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-27 22:26 [PATCH 00/23] Merge PC8 with runtime PM, v2 Paulo Zanoni
2014-02-27 22:26 ` [PATCH 01/23] drm/i915: Accurately track when we mark the hardware as idle/busy Paulo Zanoni
2014-02-27 22:26 ` [PATCH 02/23] drm/i915: put runtime PM only at the end of intel_mark_idle Paulo Zanoni
2014-02-27 22:26 ` [PATCH 03/23] drm/i915: put runtime PM only when we actually release force_wake Paulo Zanoni
2014-02-27 22:26 ` [PATCH 04/23] drm/i915: kill dev_priv->pc8.gpu_idle Paulo Zanoni
2014-02-28 15:23 ` Imre Deak
2014-02-27 22:26 ` [PATCH 05/23] drm/i915: rename modeset_update_power_wells Paulo Zanoni
2014-02-27 22:26 ` [PATCH 06/23] drm/i915: get/put runtime PM without holding rps.hw_lock Paulo Zanoni
2014-02-27 22:26 ` [PATCH 07/23] drm/i915: add forcewake functions that don't touch runtime PM Paulo Zanoni
2014-02-28 8:44 ` Chris Wilson
2014-02-28 19:38 ` Paulo Zanoni
2014-02-28 19:46 ` Chris Wilson
2014-03-05 16:56 ` Daniel Vetter
2014-02-27 22:26 ` [PATCH 08/23] drm/i915: extract __hsw_do_{en, dis}able_package_c8 Paulo Zanoni
2014-02-27 22:26 ` [PATCH 09/23] drm/i915: make PC8 be part of runtime PM suspend/resume Paulo Zanoni
2014-02-28 9:42 ` Chris Wilson
2014-03-06 22:48 ` Daniel Vetter
2014-02-27 22:26 ` [PATCH 10/23] drm/i915: get/put runtime PM when we get/put a power domain Paulo Zanoni
2014-02-28 15:45 ` Imre Deak
2014-02-28 19:54 ` Paulo Zanoni
2014-02-27 22:26 ` [PATCH 11/23] drm/i915: remove dev_priv->pc8.requirements_met Paulo Zanoni
2014-02-27 22:26 ` [PATCH 12/23] drm/i915: get runtime PM references when the GPU is idle/busy Paulo Zanoni
2014-02-28 17:08 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 13/23] drm/i915: kill pc8.disable_count Paulo Zanoni
2014-02-28 17:10 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 14/23] drm/i915: remove an indirection level on PC8 functions Paulo Zanoni
2014-02-28 17:11 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 15/23] drm/i915: don't get/put PC8 reference on freeze/thaw Paulo Zanoni
2014-02-28 17:11 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 16/23] drm/i915: get/put runtime PM references for GMBUS and DP AUX Paulo Zanoni
2014-02-28 17:13 ` Jesse Barnes
2014-02-28 17:38 ` Imre Deak
2014-02-28 17:55 ` Jesse Barnes
2014-02-28 18:20 ` Imre Deak
2014-02-28 19:07 ` Paulo Zanoni
2014-03-05 17:04 ` Daniel Vetter
2014-02-27 22:26 ` [PATCH 17/23] drm/i915: don't get/put PC8 when getting/putting power wells Paulo Zanoni
2014-02-28 17:13 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 18/23] drm/i915: remove dev_priv->pc8.enabled Paulo Zanoni
2014-02-28 17:14 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 19/23] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled Paulo Zanoni
2014-02-28 17:15 ` Jesse Barnes [this message]
2014-02-28 18:17 ` Paulo Zanoni
2014-02-27 22:26 ` [PATCH 20/23] drm/i915: kill struct i915_package_c8 Paulo Zanoni
2014-02-28 17:16 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 21/23] drm/i915: rename __hsw_do_{en, dis}able_pc8 Paulo Zanoni
2014-02-28 17:17 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 22/23] drm/i915: update the PC8 and runtime PM documentation Paulo Zanoni
2014-02-28 17:19 ` Jesse Barnes
2014-02-27 22:26 ` [PATCH 23/23] drm/i915: init pm.suspended earlier Paulo Zanoni
2014-02-28 17:20 ` Jesse Barnes
2014-03-05 17:09 ` [PATCH 00/23] Merge PC8 with runtime PM, v2 Daniel Vetter
2014-03-06 22:23 ` Paulo Zanoni
2014-03-06 22:45 ` Daniel Vetter
2014-03-07 8:51 ` Daniel Vetter
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=20140228091556.0fe2324d@jbarnes-desktop \
--to=jbarnes@virtuousgeek.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox