Re: [PATCH 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.

Ville Syrjälä
2014-03-13T19:00:08Z

On Fri, Mar 14, 2014 at 12:13:30AM +0530, S, Deepak wrote:
>
>
> On 3/13/2014 11:46 PM, Ville Syrjälä wrote:
> > On Thu, Mar 13, 2014 at 09:30:16PM +0530, deepak.s@linux.intel.com wrote:
> >> From: Deepak S <deepak.s@intel.com>
> >>
> >> When we use different rps events for different platform or due to wa, we
> >> mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable
> >> in dev_priv to track the enabled PM interrupts
> >>
> >> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 1 +
> >> drivers/gpu/drm/i915/i915_irq.c | 14 +++++++-------
> >> drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++-----
> >> 3 files changed, 17 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 70fbe90..d522313 100644
> > <snip>
> >> @@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev)
> >> GEN6_RP_UP_BUSY_AVG |
> >> GEN6_RP_DOWN_IDLE_AVG);
> >>
> >> + dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
> >> +
> >> /* 6: Ring frequency + overclocking (our driver does this later */
> >>
> >> gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) & 0xff00) >> 8);
> >> @@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev)
> >> dev_priv->rps.power = HIGH_POWER; /* force a reset */
> >> gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
> >>
> >> + dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
> >> gen6_enable_rps_interrupts(dev);
> >>
> >> rc6vids = 0;
> >> @@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >> dev_priv->rps.rp_up_masked = false;
> >> dev_priv->rps.rp_down_masked = false;
> >>
> >> + dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
> >> gen6_enable_rps_interrupts(dev);
> >>
> >> gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> >
> > I think we need to initialize pm_rps_events somewhere earlier since we
> > depend on it already in irq postinstall. Othwewise the patch looks
> > good.
> Adding it in functions "intel_uncore_early_sanitize" or "pm_init" as
> this gets executed before irq_install in driver_load?

intel_irq_init() might be a good choice since that's where we also
initialize the rps.work, and then it's clear it gets executed before any
other irq setup code.

--
Ville Syrjälä
Intel OTC