[Intel-gfx] [PATCH] drm/i915: VGA also requires the power well

[Paulo Zanoni]

2013/6/6 Ville Syrjälä <ville.syrjala at linux.intel.com>:
> On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> So add a power domain and check for it before we try to read
>> VGA_CONTROL.
>>
>> This fixes unclaimed register messages that happen on suspend/resume.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h      | 1 +
>>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>>  drivers/gpu/drm/i915/intel_pm.c      | 1 +
>>  3 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 46b1f70..d51ce13 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -89,6 +89,7 @@ enum port {
>>  #define port_name(p) ((p) + 'A')
>>
>>  enum intel_display_power_domain {
>> +     POWER_DOMAIN_VGA,
>>       POWER_DOMAIN_PIPE_A,
>>       POWER_DOMAIN_PIPE_B,
>>       POWER_DOMAIN_PIPE_C,
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4c8fcec..3719d99 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device *dev)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       u32 vga_reg = i915_vgacntrl_reg(dev);
>>
>> +     if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA))
>> +             return;
>> +
>
> So it looks like you're essentially making intel_redisable_vga() a nop
> for HSW.

It's not a nop for HSW, it's only a nop if the power well is disabled,
which means VGA is disabled, so it's a nop if VGA is disabled. But if
you look at the current function it's also a nop if VGA is disabled.
So we're keeping the same behavior, but checking the power well before
checking vga_reg.

VGA mode requires the power well to be enabled, we can be sure that if
the power well is disabled, then VGA is disabled, so you don't need to
do the check for VGA_DISP_DISABLE.


> Shouldn't we instead enable the power well during resume?

So far we don't need it. I hope it stays this way.


>
>>       if (I915_READ(vga_reg) != VGA_DISP_DISABLE) {
>>               DRM_DEBUG_KMS("Something enabled VGA plane, disabling it\n");
>>               i915_disable_vga(dev);
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 50fe3d7..47ef4a6 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5000,6 +5000,7 @@ bool intel_display_power_enabled(struct drm_device *dev,
>>       case POWER_DOMAIN_PIPE_A:
>>       case POWER_DOMAIN_TRANSCODER_EDP:
>>               return true;
>> +     case POWER_DOMAIN_VGA:
>>       case POWER_DOMAIN_PIPE_B:
>>       case POWER_DOMAIN_PIPE_C:
>>       case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC



-- 
Paulo Zanoni