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

Rodrigo Vivi
2013-11-29T13:34:20Z

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

(resent with correct smtp)

On Fri, Nov 29, 2013 at 10:35 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> On Wed, Nov 27, 2013 at 05:59:22PM -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" subtest from the "pm_pc8" test from
>> intel-gpu-tools.
>>
>> v2: - Rebase (i915_disable_power_well).
>> v3: - More reabase.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 432ec13..1cb02b1 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5677,6 +5677,8 @@ static void hsw_set_power_well(struct drm_device *dev,
>> unsigned long irqflags;
>> 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;
>> @@ -5736,17 +5738,26 @@ static void hsw_set_power_well(struct drm_device *dev,
>> static void __intel_power_well_get(struct drm_device *dev,
>> struct i915_power_well *power_well)
>> {
>> - if (!power_well->count++ && power_well->set)
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + if (!power_well->count++ && power_well->set) {
>> + hsw_disable_package_c8(dev_priv);
>> power_well->set(dev, power_well, 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 && power_well->set && i915_disable_power_well)
>> + if (!--power_well->count && power_well->set &&
>> + i915_disable_power_well) {
>> power_well->set(dev, power_well, false);
>> + hsw_enable_package_c8(dev_priv);
>> + }
>> }
>>
>> void intel_display_power_get(struct drm_device *dev,
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br