[PATCH 0/4] PC8 fixes

[PATCH 0/4] PC8 fixes

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

Patch 1 was sent by Chris a few weeks ago, and patch 2 is my bikeshed to his
original patch. They shouldn't fix any particular bug, and patch 2 will WARN in
case we hit the bug that should not happen.

Patches 3 and 4 fix problems that I discovered while working on the runtime D3.
Since these bugs affect PC8 even without D3, I think they should be merged
before D3. I already have subtests for these problems on the pc8 test, so we
should be fine.

Thanks,
Paulo

Chris Wilson (1):
  drm/i915: Do not enable package C8 on unsupported hardware

Paulo Zanoni (3):
  drm/i915: WARN if !HAS_PC8 when enabling/disabling PC8
  drm/i915: get a PC8 reference when enabling the power well
  drm/i915: get/put PC8 when we get/put a CRTC

 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 48 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_pm.c      | 14 +++++++++--
 3 files changed, 57 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] drm/i915: Do not enable package C8 on unsupported hardware

[Paulo Zanoni]

From: Chris Wilson <chris@chris-wilson.co.uk>

If the hardware does not support package C8, then do not even schedule
work to enable it. Thereby we can eliminate a bunch of dangerous work.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2c1921d..a28a676 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1790,6 +1790,7 @@ struct drm_i915_file_private {
 #define HAS_POWER_WELL(dev)	(IS_HASWELL(dev))
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
 #define HAS_PSR(dev)		(IS_HASWELL(dev))
+#define HAS_PC8(dev)		(IS_HASWELL(dev)) /* XXX HSW:ULX */
 
 #define INTEL_PCH_DEVICE_ID_MASK		0xff00
 #define INTEL_PCH_IBX_DEVICE_ID_TYPE		0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f0b42bc..14327ff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6473,6 +6473,9 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
+	if (!HAS_PC8(dev_priv->dev))
+		return;
+
 	mutex_lock(&dev_priv->pc8.lock);
 	__hsw_enable_package_c8(dev_priv);
 	mutex_unlock(&dev_priv->pc8.lock);
@@ -6480,6 +6483,9 @@ void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 
 void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 {
+	if (!HAS_PC8(dev_priv->dev))
+		return;
+
 	mutex_lock(&dev_priv->pc8.lock);
 	__hsw_disable_package_c8(dev_priv);
 	mutex_unlock(&dev_priv->pc8.lock);
@@ -6517,6 +6523,9 @@ static void hsw_update_package_c8(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	bool allow;
 
+	if (!HAS_PC8(dev_priv->dev))
+		return;
+
 	if (!i915_enable_pc8)
 		return;
 
@@ -6540,6 +6549,9 @@ done:
 
 static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
 {
+	if (!HAS_PC8(dev_priv->dev))
+		return;
+
 	if (!dev_priv->pc8.gpu_idle) {
 		dev_priv->pc8.gpu_idle = true;
 		hsw_enable_package_c8(dev_priv);
@@ -6548,6 +6560,9 @@ static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
 
 static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
 {
+	if (!HAS_PC8(dev_priv->dev))
+		return;
+
 	if (dev_priv->pc8.gpu_idle) {
 		dev_priv->pc8.gpu_idle = false;
 		hsw_disable_package_c8(dev_priv);
-- 
1.8.3.1

[PATCH 2/4] drm/i915: WARN if !HAS_PC8 when enabling/disabling PC8

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We already have some checks and shouldn't be reaching these places on
!HAS_PC8 platforms, but add a WARN,  just in case.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 14327ff..c226f4d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6402,6 +6402,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
 
+	WARN_ON(!HAS_PC8(dev));
+
 	if (dev_priv->pc8.enabled)
 		return;
 
@@ -6447,6 +6449,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	if (dev_priv->pc8.disable_count != 1)
 		return;
 
+	WARN_ON(!HAS_PC8(dev));
+
 	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
 	if (!dev_priv->pc8.enabled)
 		return;
-- 
1.8.3.1

[PATCH 3/4] drm/i915: get a PC8 reference when enabling the power well

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

In the current code, at haswell_modeset_global_resources, first we
decide if we want to enable/disable the power well, then we decide if
we want to enable/disable PC8. On the case where we're enabling PC8
this works fine, but on the case where we disable PC8 due to a non-eDP
monitor being enabled, we first enable the power well and then disable
PC8. Although wrong, this doesn't seem to be causing any problems now,
and we don't even see anything in dmesg. But the patches for runtime
D3 turn this problem into a real bug, so we need to fix it.

This fixes the "modeset-non-lpsp" test from both "pc8" and
"runtime_pm" tests from intel-gpu-tools.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a0c907f..5b50083 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5547,6 +5547,8 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
 	bool is_enabled, enable_requested;
 	uint32_t tmp;
 
+	WARN_ON(dev_priv->pc8.enabled);
+
 	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
 	is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED;
 	enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST;
@@ -5591,16 +5593,24 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
 static void __intel_power_well_get(struct drm_device *dev,
 				   struct i915_power_well *power_well)
 {
-	if (!power_well->count++)
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!power_well->count++) {
+		hsw_disable_package_c8(dev_priv);
 		__intel_set_power_well(dev, true);
+	}
 }
 
 static void __intel_power_well_put(struct drm_device *dev,
 				   struct i915_power_well *power_well)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	WARN_ON(!power_well->count);
-	if (!--power_well->count)
+	if (!--power_well->count) {
 		__intel_set_power_well(dev, false);
+		hsw_enable_package_c8(dev_priv);
+	}
 }
 
 void intel_display_power_get(struct drm_device *dev,
-- 
1.8.3.1

Re: [PATCH 3/4] drm/i915: get a PC8 reference when enabling the power well

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 2523 bytes --]

On Wed, 2013-10-30 at 19:40 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> In the current code, at haswell_modeset_global_resources, first we
> decide if we want to enable/disable the power well, then we decide if
> we want to enable/disable PC8. On the case where we're enabling PC8
> this works fine, but on the case where we disable PC8 due to a non-eDP
> monitor being enabled, we first enable the power well and then disable
> PC8. Although wrong, this doesn't seem to be causing any problems now,
> and we don't even see anything in dmesg. But the patches for runtime
> D3 turn this problem into a real bug, so we need to fix it.
> 
> This fixes the "modeset-non-lpsp" test from both "pc8" and
> "runtime_pm" tests from intel-gpu-tools.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a0c907f..5b50083 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5547,6 +5547,8 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>  	bool is_enabled, enable_requested;
>  	uint32_t tmp;
>  
> +	WARN_ON(dev_priv->pc8.enabled);
> +
>  	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
>  	is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED;
>  	enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST;
> @@ -5591,16 +5593,24 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>  static void __intel_power_well_get(struct drm_device *dev,
>  				   struct i915_power_well *power_well)
>  {
> -	if (!power_well->count++)
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (!power_well->count++) {
> +		hsw_disable_package_c8(dev_priv);
>  		__intel_set_power_well(dev, true);
> +	}
>  }
>  
>  static void __intel_power_well_put(struct drm_device *dev,
>  				   struct i915_power_well *power_well)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
>  	WARN_ON(!power_well->count);
> -	if (!--power_well->count)
> +	if (!--power_well->count) {
>  		__intel_set_power_well(dev, false);
> +		hsw_enable_package_c8(dev_priv);
> +	}
>  }

It would be better to add these __intel_set_power_well(), which has all
the HSW specific bits already.

--Imre

>  
>  void intel_display_power_get(struct drm_device *dev,


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: get a PC8 reference when enabling the power well

[Paulo Zanoni]

2013/10/31 Imre Deak <imre.deak@intel.com>:
> On Wed, 2013-10-30 at 19:40 -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> In the current code, at haswell_modeset_global_resources, first we
>> decide if we want to enable/disable the power well, then we decide if
>> we want to enable/disable PC8. On the case where we're enabling PC8
>> this works fine, but on the case where we disable PC8 due to a non-eDP
>> monitor being enabled, we first enable the power well and then disable
>> PC8. Although wrong, this doesn't seem to be causing any problems now,
>> and we don't even see anything in dmesg. But the patches for runtime
>> D3 turn this problem into a real bug, so we need to fix it.
>>
>> This fixes the "modeset-non-lpsp" test from both "pc8" and
>> "runtime_pm" tests from intel-gpu-tools.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index a0c907f..5b50083 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5547,6 +5547,8 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>>       bool is_enabled, enable_requested;
>>       uint32_t tmp;
>>
>> +     WARN_ON(dev_priv->pc8.enabled);
>> +
>>       tmp = I915_READ(HSW_PWR_WELL_DRIVER);
>>       is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED;
>>       enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST;
>> @@ -5591,16 +5593,24 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable)
>>  static void __intel_power_well_get(struct drm_device *dev,
>>                                  struct i915_power_well *power_well)
>>  {
>> -     if (!power_well->count++)
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +     if (!power_well->count++) {
>> +             hsw_disable_package_c8(dev_priv);
>>               __intel_set_power_well(dev, true);
>> +     }
>>  }
>>
>>  static void __intel_power_well_put(struct drm_device *dev,
>>                                  struct i915_power_well *power_well)
>>  {
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>>       WARN_ON(!power_well->count);
>> -     if (!--power_well->count)
>> +     if (!--power_well->count) {
>>               __intel_set_power_well(dev, false);
>> +             hsw_enable_package_c8(dev_priv);
>> +     }
>>  }
>
> It would be better to add these __intel_set_power_well(), which has all
> the HSW specific bits already.

The problem is that __intel_set_power_well() is also called by
intel_power_domains_resume(), which is just used to set the power well
on the state we expect it to already be (without changing the
refcounts). So in the current code I can safely assume that the get
and put calls are balanced, while if I move the code to
__intel_set_power_well() I will lose this guarantee and have to work
around it.

Besides, it should be safe to call these functions on non-HSW
platforms since they have a check. And in the future, when I merge PC8
and D3 as a single feature, this will be a call to D3 get, which will
be needed on all platforms anyway.

>
> --Imre
>
>>
>>  void intel_display_power_get(struct drm_device *dev,
>



-- 
Paulo Zanoni

[PATCH 4/4] drm/i915: get/put PC8 when we get/put a CRTC

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Currently, PC8 is enabled at modeset_global_resources, which is called
after intel_modeset_update_state. Due to this, there's a small race
condition on the case where we start enabling PC8, then do a modeset
while PC8 is still being enabled. The racing condition triggers a WARN
because intel_modeset_update_state will mark the CRTC as enabled, then
the thread that's still enabling PC8 might look at the data structure
and think that PC8 is being enabled while a pipe is enabled. Despite
the WARN, this is not really a bug since we'll wait for the
PC8-enabling thread to finish when we call modeset_global_resources.

So this patch makes sure we get/put PC8 before we update
drm_crtc->enabled, because this will grab the PC8 lock, which will
wait for the PC8-enable task to finish.

The side-effect benefit of this patch is that we have a nice place to
track enabled/disabled CRTCs, so we may want to move some code from
modeset_global_resources to intel_crtc_set_state in the future.

The problem fixed by this patch can be reproduced by the
modeset-lpsp-stress-no-wait subtest from the pc8 test of
intel-gpu-tools.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c226f4d..e841cd7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6409,6 +6409,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 
 	DRM_DEBUG_KMS("Enabling package C8+\n");
 
+	mutex_lock(&dev_priv->pc8.lock);
 	dev_priv->pc8.enabled = true;
 
 	if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
@@ -6420,6 +6421,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 	lpt_disable_clkout_dp(dev);
 	hsw_pc8_disable_interrupts(dev);
 	hsw_disable_lcpll(dev_priv, true, true);
+	mutex_unlock(&dev_priv->pc8.lock);
 }
 
 static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
@@ -8880,6 +8882,24 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
 	return false;
 }
 
+/* Sets crtc->base.enabled and gets/puts whatever resources are needed by the
+ * CRTC. */
+static void intel_crtc_set_state(struct intel_crtc *crtc, bool enabled)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (enabled == crtc->base.enabled)
+		return;
+
+	if (enabled)
+		hsw_disable_package_c8(dev_priv);
+	else
+		hsw_enable_package_c8(dev_priv);
+
+	crtc->base.enabled = enabled;
+}
+
 static void
 intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 {
@@ -8903,7 +8923,8 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 	/* Update computed state. */
 	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
 			    base.head) {
-		intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base);
+		intel_crtc_set_state(intel_crtc,
+				     intel_crtc_in_use(&intel_crtc->base));
 	}
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -10695,7 +10716,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		}
 
 		WARN_ON(crtc->active);
-		crtc->base.enabled = false;
+		intel_crtc_set_state(crtc, false);
 	}
 
 	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
@@ -10722,7 +10743,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 			      crtc->base.enabled ? "enabled" : "disabled",
 			      crtc->active ? "enabled" : "disabled");
 
-		crtc->base.enabled = crtc->active;
+		intel_crtc_set_state(crtc, crtc->active);
 
 		/* Because we only establish the connector -> encoder ->
 		 * crtc links if something is active, this means the
@@ -10819,7 +10840,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		crtc->active = dev_priv->display.get_pipe_config(crtc,
 								 &crtc->config);
 
-		crtc->base.enabled = crtc->active;
+		intel_crtc_set_state(crtc, crtc->active);
 		crtc->primary_enabled = crtc->active;
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
-- 
1.8.3.1

Re: [PATCH 4/4] drm/i915: get/put PC8 when we get/put a CRTC

[Paulo Zanoni]

2013/10/30 Paulo Zanoni <przanoni@gmail.com>:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Currently, PC8 is enabled at modeset_global_resources, which is called
> after intel_modeset_update_state. Due to this, there's a small race
> condition on the case where we start enabling PC8, then do a modeset
> while PC8 is still being enabled. The racing condition triggers a WARN
> because intel_modeset_update_state will mark the CRTC as enabled, then
> the thread that's still enabling PC8 might look at the data structure
> and think that PC8 is being enabled while a pipe is enabled. Despite
> the WARN, this is not really a bug since we'll wait for the
> PC8-enabling thread to finish when we call modeset_global_resources.
>
> So this patch makes sure we get/put PC8 before we update
> drm_crtc->enabled, because this will grab the PC8 lock, which will
> wait for the PC8-enable task to finish.
>
> The side-effect benefit of this patch is that we have a nice place to
> track enabled/disabled CRTCs, so we may want to move some code from
> modeset_global_resources to intel_crtc_set_state in the future.
>
> The problem fixed by this patch can be reproduced by the
> modeset-lpsp-stress-no-wait subtest from the pc8 test of
> intel-gpu-tools.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

This specific patch introduces a locking problem. Please ignore it for
now. The previous 3 patches are fine.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c226f4d..e841cd7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6409,6 +6409,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>
>         DRM_DEBUG_KMS("Enabling package C8+\n");
>
> +       mutex_lock(&dev_priv->pc8.lock);
>         dev_priv->pc8.enabled = true;
>
>         if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> @@ -6420,6 +6421,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>         lpt_disable_clkout_dp(dev);
>         hsw_pc8_disable_interrupts(dev);
>         hsw_disable_lcpll(dev_priv, true, true);
> +       mutex_unlock(&dev_priv->pc8.lock);
>  }
>
>  static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
> @@ -8880,6 +8882,24 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
>         return false;
>  }
>
> +/* Sets crtc->base.enabled and gets/puts whatever resources are needed by the
> + * CRTC. */
> +static void intel_crtc_set_state(struct intel_crtc *crtc, bool enabled)
> +{
> +       struct drm_device *dev = crtc->base.dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +       if (enabled == crtc->base.enabled)
> +               return;
> +
> +       if (enabled)
> +               hsw_disable_package_c8(dev_priv);
> +       else
> +               hsw_enable_package_c8(dev_priv);
> +
> +       crtc->base.enabled = enabled;
> +}
> +
>  static void
>  intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
>  {
> @@ -8903,7 +8923,8 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
>         /* Update computed state. */
>         list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
>                             base.head) {
> -               intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base);
> +               intel_crtc_set_state(intel_crtc,
> +                                    intel_crtc_in_use(&intel_crtc->base));
>         }
>
>         list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> @@ -10695,7 +10716,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>                 }
>
>                 WARN_ON(crtc->active);
> -               crtc->base.enabled = false;
> +               intel_crtc_set_state(crtc, false);
>         }
>
>         if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
> @@ -10722,7 +10743,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>                               crtc->base.enabled ? "enabled" : "disabled",
>                               crtc->active ? "enabled" : "disabled");
>
> -               crtc->base.enabled = crtc->active;
> +               intel_crtc_set_state(crtc, crtc->active);
>
>                 /* Because we only establish the connector -> encoder ->
>                  * crtc links if something is active, this means the
> @@ -10819,7 +10840,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>                 crtc->active = dev_priv->display.get_pipe_config(crtc,
>                                                                  &crtc->config);
>
> -               crtc->base.enabled = crtc->active;
> +               intel_crtc_set_state(crtc, crtc->active);
>                 crtc->primary_enabled = crtc->active;
>
>                 DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> --
> 1.8.3.1
>



-- 
Paulo Zanoni