On Sun, 1 Jul 2012 12:48:59 +0200 Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > 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. meh, it was a cheap shot that didn't merit a response ;-) > > >> 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 > iirc in the past, disabling rc6 mean disable, and destroy the object. Now it simply means disable. If that's the case, really just a comment in the commit would have made me happy. If it's not the case, ignore. > > > >> > >> 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> Again, 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 > > > -- Ben Widawsky, Intel Open Source Technology Center