[Intel-gfx] [PATCH] drm/i915: fix up ilk rc6 disabling confusion

[Daniel Vetter]

On Sun, Jul 1, 2012 at 5:04 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Fri, 29 Jun 2012 23:32:16 +0200
> Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
>> While creating the new enable/disable_gt_powersave functions in
>>
>> commit 8090c6b9daa04dda649ac0a2209601042abfb0a4
>> Author: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Date:   Sun Jun 24 16:42:32 2012 +0200
>>
>>     drm/i915: wrap up gt powersave enabling functions
>>
>> I've botched up the handling of ironlake_disable_rc6. Fix this up by
>> calling it at the right place. Note though that ironlake_disable_rc6
>> does a bit more than just disabling rc6 - it also tears down all the
>> allocated context objects.
>
> I can only hope that one day you will appreciate the API I set up in
> the context code to handle this.

Ffs I'd like to merge this for the renderctx, my gripes have only been
with shoehorning the powerctx stuff into your ctx code. That one
simply doesn't make sense to me.

>> Hence we need to move intel_teardown_rc6 out and directly call it from
>> intel_modeset_cleanup.
>
> I don't mind that you've done this, however could you explain/modify the
> comment when we want to disable without destroying the backing context
> object?

Hm, I don't see the comment you're talking about ... Neither
ironlake_disable_rc6 nor ironlake_teardown_rc6 nor any of the callers
have one. Or am I totally missing your point?
-Daniel

>
>>
>> Also properly mark ironlake_enable_rc6 as static and kill the un-used
>> declaration in i915_drv.h.
>>
>> Cc: Ben Widawsky <ben at bwidawsk.net>
>> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h      |    1 -
>>  drivers/gpu/drm/i915/intel_display.c |    2 ++
>>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>>  drivers/gpu/drm/i915/intel_pm.c      |   13 +++++++------
>>  4 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a0c15ab..6c077e9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1510,7 +1510,6 @@ extern bool intel_fbc_enabled(struct drm_device *dev);
>>  extern void intel_disable_fbc(struct drm_device *dev);
>>  extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
>>  extern void ironlake_init_pch_refclk(struct drm_device *dev);
>> -extern void ironlake_enable_rc6(struct drm_device *dev);
>>  extern void gen6_set_rps(struct drm_device *dev, u8 val);
>>  extern void intel_detect_pch(struct drm_device *dev);
>>  extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3fbc802..717b553 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7270,6 +7270,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
>>
>>       intel_disable_gt_powersave(dev);
>>
>> +     ironlake_teardown_rc6(dev);
>> +
>>       if (IS_VALLEYVIEW(dev))
>>               vlv_init_dpio(dev);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index cc1573b..db226c5 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -493,6 +493,7 @@ extern void intel_gpu_ips_teardown(void);
>>
>>  extern void intel_enable_gt_powersave(struct drm_device *dev);
>>  extern void intel_disable_gt_powersave(struct drm_device *dev);
>> +extern void ironlake_teardown_rc6(struct drm_device *dev);
>>
>>  extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
>>  extern void intel_ddi_mode_set(struct drm_encoder *encoder,
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 99bc1f3..ec25eca 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2547,7 +2547,7 @@ static void gen6_update_ring_freq(struct drm_device *dev)
>>       }
>>  }
>>
>> -static void ironlake_teardown_rc6(struct drm_device *dev)
>> +void ironlake_teardown_rc6(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> @@ -2564,7 +2564,7 @@ static void ironlake_teardown_rc6(struct drm_device *dev)
>>       }
>>  }
>>
>> -void ironlake_disable_rc6(struct drm_device *dev)
>> +static void ironlake_disable_rc6(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> @@ -2580,8 +2580,6 @@ void ironlake_disable_rc6(struct drm_device *dev)
>>               I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT);
>>               POSTING_READ(RSTDBYCTL);
>>       }
>> -
>> -     ironlake_teardown_rc6(dev);
>>  }
>>
>>  static int ironlake_setup_rc6(struct drm_device *dev)
>> @@ -2603,7 +2601,7 @@ static int ironlake_setup_rc6(struct drm_device *dev)
>>       return 0;
>>  }
>>
>> -void ironlake_enable_rc6(struct drm_device *dev)
>> +static void ironlake_enable_rc6(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
>> @@ -3225,8 +3223,11 @@ static void intel_init_emon(struct drm_device *dev)
>>
>>  void intel_disable_gt_powersave(struct drm_device *dev)
>>  {
>> -     if (IS_IRONLAKE_M(dev))
>> +     if (IS_IRONLAKE_M(dev)) {
>>               ironlake_disable_drps(dev);
>> +             ironlake_disable_rc6(dev);
>> +     }
>> +
>>       if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
>>               gen6_disable_rps(dev);
>>  }
>
>
> --
> Ben Widawsky, Intel Open Source Technology Center



-- 
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch