On Mon, 22 Apr 2013 14:39:17 +0300 Ville Syrjälä <[email protected]> wrote:
> On Fri, Apr 19, 2013 at 10:22:45AM -0700, Jesse Barnes wrote: > > On VLV, the Punit doesn't automatically drop the GPU to it's minimum > > voltage level when entering RC6, so we arm a timer to do it for us from > > the RPS interrupt handler. It'll generally only fire when we go idle > > (or if for some reason there's a long delay between RPS interrupts), but > > won't be re-armed again until the next RPS event, so shouldn't affect > > power consumption after we go idle and it triggers. > > > > Signed-off-by: Jesse Barnes <[email protected]> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > > drivers/gpu/drm/i915/i915_irq.c | 11 +++++++++++ > > drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++++++++++++++ > > 3 files changed, 47 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 2557fc7..2900dd4 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -660,6 +660,7 @@ struct i915_suspend_saved_registers { > > > > struct intel_gen6_power_mgmt { > > struct work_struct work; > > + struct work_struct vlv_work; > > u32 pm_iir; > > /* lock - irqsave spinlock that protectects the work_struct and > > * pm_iir. */ > > @@ -670,6 +671,7 @@ struct intel_gen6_power_mgmt { > > u8 cur_delay; > > u8 min_delay; > > u8 max_delay; > > + u8 rpe_delay; > > u8 hw_max; > > > > struct delayed_work delayed_resume_work; > > @@ -945,6 +947,8 @@ typedef struct drm_i915_private { > > } hpd_stats[HPD_NUM_PINS]; > > struct timer_list hotplug_reenable_timer; > > > > + struct timer_list vlv_rps_timer; > > + > > int num_pch_pll; > > int num_plane; > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 932e7f8..99b2e1c 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -488,6 +488,17 @@ static void gen6_pm_rps_work(struct work_struct *work) > > gen6_set_rps(dev_priv->dev, new_delay); > > } > > > > + if (IS_VALLEYVIEW(dev_priv->dev)) { > > + /* > > + * On VLV, when we enter RC6 we may not be at the minimum > > + * voltage level, so arm a timer to check. It should only > > + * fire when there's activity or once after we've entered > > + * RC6, and then won't be re-armed until the next RPS interrupt. > > + */ > > + mod_timer(&dev_priv->vlv_rps_timer, jiffies + > > + msecs_to_jiffies(100)); > > + } > > + > > mutex_unlock(&dev_priv->rps.hw_lock); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 2557926..c1311c9 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2545,6 +2545,9 @@ static void gen6_disable_rps(struct drm_device *dev) > > spin_unlock_irq(&dev_priv->rps.lock); > > > > I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); > > + > > + if (IS_VALLEYVIEW(dev)) > > + del_timer_sync(&dev_priv->vlv_rps_timer); > > } > > > > int intel_enable_rc6(const struct drm_device *dev) > > @@ -2822,6 +2825,30 @@ int valleyview_rps_min_freq(struct drm_i915_private > > *dev_priv) > > return val & 0xff; > > } > > > > +static void vlv_rps_timer_work(struct work_struct *work) > > +{ > > + drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, > > + rps.vlv_work); > > + > > + /* > > + * Timer fired, we must be idle. Drop to min voltage state. > > + * Note: we use RPe here since it should match the > > + * Vmin we were shooting for. That should give us better > > + * perf when we come back out of RC6 than if we used the > > + * min freq available. > > + */ > > + mutex_lock(&dev_priv->rps.hw_lock); > > + valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay); > > + mutex_unlock(&dev_priv->rps.hw_lock); > > +} > > + > > +static void valleyview_rps_timer_func(unsigned long data) > > +{ > > + struct drm_i915_private *dev_priv = (struct drm_i915_private *)data; > > + > > + queue_work(dev_priv->wq, &dev_priv->rps.vlv_work); > > +} > > This looks somewhat like open coded delayed work to me. Would the > standard delayed work be OK for this, or was there some specific reason > you did it this way? No particular reason other than not thinking about delayed work when I wrote it. :) I can fix it up; I guess it'll save us having the rps_timer_func. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
