Re: [PATCH v2 5/15] drm/i915: take init power domain for sysfs/debugfs entries where needed

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 09, 2014 at 05:21:47PM +0300, Imre Deak wrote:
> On Wed, 2014-04-09 at 16:15 +0200, Daniel Vetter wrote:
> > On Tue, Apr 08, 2014 at 10:34:41PM +0300, Imre Deak wrote:
> > > v2:
> > > - make it actually compile, I managed to send the wrong version as v1
> > >   somehow
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > 
> > Usually if we just want the device to be out of D3 we just use
> > intel_runtime_pm_get/put. On a quick look it seems like that's enough for
> > these places here, too?
> 
> For drpc yes, but for the others we need the DPIO power wells up too. I
> got for all VLV case the power well ref for simplicity, I can change it
> to an RPM ref for drpc, if that's ok.

The dpio_info file would need DPIO wells up. However I think we should
just kill that file. It just dumps a small subset of the registers w/o
decoding anything, and we can get more extensive dumps w/ igt, so I
don't see the point of this file.

The error handler should already be able to deal with the wells being
down no?

The gt_cutr_freq file just talks to the punit so no need for any power
wells either AFAICS.

> 
> --Imre
> 
> > -Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++++++++++--
> > >  drivers/gpu/drm/i915/i915_sysfs.c   |  4 ++++
> > >  2 files changed, 20 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 02f1b39..ac91a44 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -1239,9 +1239,13 @@ static int vlv_drpc_info(struct seq_file *m)
> > >  	u32 rpmodectl1, rcctl1;
> > >  	unsigned fw_rendercount = 0, fw_mediacount = 0;
> > >  
> > > +	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
> > >  	rcctl1 = I915_READ(GEN6_RC_CONTROL);
> > >  
> > > +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	seq_printf(m, "Video Turbo Mode: %s\n",
> > >  		   yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
> > >  	seq_printf(m, "Turbo enabled: %s\n",
> > > @@ -1916,9 +1920,11 @@ static int i915_dpio_info(struct seq_file *m, void *data)
> > >  		return 0;
> > >  	}
> > >  
> > > +	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	ret = mutex_lock_interruptible(&dev_priv->dpio_lock);
> > >  	if (ret)
> > > -		return ret;
> > > +		goto out;
> > >  
> > >  	seq_printf(m, "DPIO_CTL: 0x%08x\n", I915_READ(DPIO_CTL));
> > >  
> > > @@ -1946,8 +1952,10 @@ static int i915_dpio_info(struct seq_file *m, void *data)
> > >  		   vlv_dpio_read(dev_priv, PIPE_A, VLV_CMN_DW0));
> > >  
> > >  	mutex_unlock(&dev_priv->dpio_lock);
> > > +out:
> > > +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> > >  
> > > -	return 0;
> > > +	return ret;
> > >  }
> > >  
> > >  static int i915_llc(struct seq_file *m, void *data)
> > > @@ -3304,9 +3312,15 @@ static int
> > >  i915_wedged_set(void *data, u64 val)
> > >  {
> > >  	struct drm_device *dev = data;
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +
> > > +	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > >  
> > >  	i915_handle_error(dev, val,
> > >  			  "Manually setting wedged to %llu", val);
> > > +
> > > +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > > index 9c57029..552c4ed 100644
> > > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > > @@ -263,6 +263,8 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > >  
> > >  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > >  
> > > +	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	mutex_lock(&dev_priv->rps.hw_lock);
> > >  	if (IS_VALLEYVIEW(dev_priv->dev)) {
> > >  		u32 freq;
> > > @@ -273,6 +275,8 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > >  	}
> > >  	mutex_unlock(&dev_priv->rps.hw_lock);
> > >  
> > > +	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> > > +
> > >  	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> > >  }
> > >  
> > > -- 
> > > 1.8.3.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC