On Fri, Nov 29, 2013 at 11:05:49AM -0200, Rodrigo Vivi wrote: > I think this one and last one could be only one patch, but anyway:
Yeah, agreed that the split doesn't make too much sense. What I'd really like to see in these commits (and also all the following ones that sprinkle runtime pm get/put calls all over the place) is a reference to the relevant subtest. And with that a natural way to split stuff would be so that each patch adds the get/put calls for a given feature and hence testcase. I don't care too much about the split since it's all disabled by default, so as-is is imo ok. But the tests should be mentioned. That applies in general btw: If there's a specific igt testcase for a feature or bugfix, the kernel commit message should mention the testname. If people want, we could use a standardized patter like IGT-Testcase: igt/pm_pc8/gem_mmap_check or something like that. Cheers, Daniel > > Reviewed-by Rodrigo Vivi <rodrigo.v...@gmail.com> > > On Wed, Nov 27, 2013 at 06:21:54PM -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zan...@intel.com> > > > > These are needed when we cat the debugfs and sysfs files. > > > > V2: - Rebase > > V3: - Rebase > > V4: - Rebase > > > > Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 45 > > ++++++++++++++++++++++++++++++++++--- > > drivers/gpu/drm/i915/i915_sysfs.c | 14 ++++++++++-- > > drivers/gpu/drm/i915/intel_dp.c | 11 +++++++-- > > drivers/gpu/drm/i915/intel_panel.c | 3 +++ > > drivers/gpu/drm/i915/intel_uncore.c | 4 ++++ > > 5 files changed, 70 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index 13accf7..6badc15 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -564,10 +564,12 @@ static int i915_gem_seqno_info(struct seq_file *m, > > void *data) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > for_each_ring(ring, dev_priv, i) > > i915_ring_seqno_info(m, ring); > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -585,6 +587,7 @@ static int i915_interrupt_info(struct seq_file *m, void > > *data) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > if (INTEL_INFO(dev)->gen >= 8) { > > int i; > > @@ -711,6 +714,7 @@ static int i915_interrupt_info(struct seq_file *m, void > > *data) > > } > > i915_ring_seqno_info(m, ring); > > } > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -904,9 +908,11 @@ static int i915_rstdby_delays(struct seq_file *m, void > > *unused) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > crstanddelay = I915_READ16(CRSTANDVID); > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > seq_printf(m, "w/ctx: %d, w/o ctx: %d\n", (crstanddelay >> 8) & 0x3f, > > (crstanddelay & 0x3f)); > > @@ -919,7 +925,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void > > *unused) > > struct drm_info_node *node = (struct drm_info_node *) m->private; > > struct drm_device *dev = node->minor->dev; > > drm_i915_private_t *dev_priv = dev->dev_private; > > - int ret; > > + int ret = 0; > > + > > + intel_runtime_pm_get(dev_priv); > > > > flush_delayed_work(&dev_priv->rps.delayed_resume_work); > > > > @@ -945,7 +953,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void > > *unused) > > /* RPSTAT1 is in the GT power well */ > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > - return ret; > > + goto out; > > > > gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL); > > > > @@ -1033,7 +1041,9 @@ static int i915_cur_delayinfo(struct seq_file *m, > > void *unused) > > seq_puts(m, "no P-state info available\n"); > > } > > > > - return 0; > > +out: > > + intel_runtime_pm_put(dev_priv); > > + return ret; > > } > > > > static int i915_delayfreq_table(struct seq_file *m, void *unused) > > @@ -1047,6 +1057,7 @@ static int i915_delayfreq_table(struct seq_file *m, > > void *unused) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > for (i = 0; i < 16; i++) { > > delayfreq = I915_READ(PXVFREQ_BASE + i * 4); > > @@ -1054,6 +1065,8 @@ static int i915_delayfreq_table(struct seq_file *m, > > void *unused) > > (delayfreq & PXVFREQ_PX_MASK) >> PXVFREQ_PX_SHIFT); > > } > > > > + intel_runtime_pm_put(dev_priv); > > + > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -1075,12 +1088,14 @@ static int i915_inttoext_table(struct seq_file *m, > > void *unused) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > for (i = 1; i <= 32; i++) { > > inttoext = I915_READ(INTTOEXT_BASE_ILK + i * 4); > > seq_printf(m, "INTTOEXT%02d: 0x%08x\n", i, inttoext); > > } > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -1098,11 +1113,13 @@ static int ironlake_drpc_info(struct seq_file *m) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > rgvmodectl = I915_READ(MEMMODECTL); > > rstdbyctl = I915_READ(RSTDBYCTL); > > crstandvid = I915_READ16(CRSTANDVID); > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > seq_printf(m, "HD boost: %s\n", (rgvmodectl & MEMMODE_BOOST_EN) ? > > @@ -1166,6 +1183,7 @@ static int gen6_drpc_info(struct seq_file *m) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > spin_lock_irq(&dev_priv->uncore.lock); > > forcewake_count = dev_priv->uncore.forcewake_count; > > @@ -1191,6 +1209,8 @@ static int gen6_drpc_info(struct seq_file *m) > > sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids); > > mutex_unlock(&dev_priv->rps.hw_lock); > > > > + intel_runtime_pm_put(dev_priv); > > + > > seq_printf(m, "Video Turbo Mode: %s\n", > > yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO)); > > seq_printf(m, "HW control enabled: %s\n", > > @@ -1405,6 +1425,7 @@ static int i915_ring_freq_table(struct seq_file *m, > > void *unused) > > ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > seq_puts(m, "GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring > > freq (MHz)\n"); > > > > @@ -1421,6 +1442,7 @@ static int i915_ring_freq_table(struct seq_file *m, > > void *unused) > > ((ia_freq >> 8) & 0xff) * 100); > > } > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev_priv->rps.hw_lock); > > > > return 0; > > @@ -1436,8 +1458,10 @@ static int i915_gfxec(struct seq_file *m, void > > *unused) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > seq_printf(m, "GFXEC: %ld\n", (unsigned long)I915_READ(0x112f4)); > > + intel_runtime_pm_put(dev_priv); > > > > mutex_unlock(&dev->struct_mutex); > > > > @@ -1617,6 +1641,7 @@ static int i915_swizzle_info(struct seq_file *m, void > > *data) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > seq_printf(m, "bit6 swizzle for X-tiling = %s\n", > > swizzle_string(dev_priv->mm.bit_6_swizzle_x)); > > @@ -1648,6 +1673,7 @@ static int i915_swizzle_info(struct seq_file *m, void > > *data) > > seq_printf(m, "DISP_ARB_CTL = 0x%08x\n", > > I915_READ(DISP_ARB_CTL)); > > } > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -1708,16 +1734,19 @@ static int i915_ppgtt_info(struct seq_file *m, void > > *data) > > { > > struct drm_info_node *node = (struct drm_info_node *) m->private; > > struct drm_device *dev = node->minor->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > > int ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > if (INTEL_INFO(dev)->gen >= 8) > > gen8_ppgtt_info(m, dev); > > else if (INTEL_INFO(dev)->gen >= 6) > > gen6_ppgtt_info(m, dev); > > > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > return 0; > > @@ -1791,6 +1820,8 @@ static int i915_edp_psr_status(struct seq_file *m, > > void *data) > > u32 psrperf = 0; > > bool enabled = false; > > > > + intel_runtime_pm_get(dev_priv); > > + > > seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); > > seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); > > > > @@ -1803,6 +1834,7 @@ static int i915_edp_psr_status(struct seq_file *m, > > void *data) > > EDP_PSR_PERF_CNT_MASK; > > seq_printf(m, "Performance_Counter: %u\n", psrperf); > > > > + intel_runtime_pm_put(dev_priv); > > return 0; > > } > > > > @@ -3016,8 +3048,11 @@ i915_cache_sharing_get(void *data, u64 *val) > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > > > snpcr = I915_READ(GEN6_MBCUNIT_SNPCR); > > + > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev_priv->dev->struct_mutex); > > > > *val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT; > > @@ -3038,6 +3073,7 @@ i915_cache_sharing_set(void *data, u64 val) > > if (val > 3) > > return -EINVAL; > > > > + intel_runtime_pm_get(dev_priv); > > DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val); > > > > /* Update the cache sharing policy here as well */ > > @@ -3046,6 +3082,7 @@ i915_cache_sharing_set(void *data, u64 val) > > snpcr |= (val << GEN6_MBC_SNPCR_SHIFT); > > I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr); > > > > + intel_runtime_pm_put(dev_priv); > > return 0; > > } > > > > @@ -3061,6 +3098,7 @@ static int i915_forcewake_open(struct inode *inode, > > struct file *file) > > if (INTEL_INFO(dev)->gen < 6) > > return 0; > > > > + intel_runtime_pm_get(dev_priv); > > gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL); > > > > return 0; > > @@ -3075,6 +3113,7 @@ static int i915_forcewake_release(struct inode > > *inode, struct file *file) > > return 0; > > > > gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); > > + intel_runtime_pm_put(dev_priv); > > > > return 0; > > } > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c > > b/drivers/gpu/drm/i915/i915_sysfs.c > > index 05d8b16..33bcae3 100644 > > --- a/drivers/gpu/drm/i915/i915_sysfs.c > > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > > @@ -40,10 +40,13 @@ static u32 calc_residency(struct drm_device *dev, const > > u32 reg) > > struct drm_i915_private *dev_priv = dev->dev_private; > > u64 raw_time; /* 32b value may overflow during fixed point math */ > > u64 units = 128ULL, div = 100000ULL, bias = 100ULL; > > + u32 ret; > > > > if (!intel_enable_rc6(dev)) > > return 0; > > > > + intel_runtime_pm_get(dev_priv); > > + > > /* On VLV, residency time is in CZ units rather than 1.28us */ > > if (IS_VALLEYVIEW(dev)) { > > u32 clkctl2; > > @@ -52,7 +55,8 @@ static u32 calc_residency(struct drm_device *dev, const > > u32 reg) > > CLK_CTL2_CZCOUNT_30NS_SHIFT; > > if (!clkctl2) { > > WARN(!clkctl2, "bogus CZ count value"); > > - return 0; > > + ret = 0; > > + goto out; > > } > > units = DIV_ROUND_UP_ULL(30ULL * bias, (u64)clkctl2); > > if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) > > @@ -62,7 +66,11 @@ static u32 calc_residency(struct drm_device *dev, const > > u32 reg) > > } > > > > raw_time = I915_READ(reg) * units; > > - return DIV_ROUND_UP_ULL(raw_time, div); > > + ret = DIV_ROUND_UP_ULL(raw_time, div); > > + > > +out: > > + intel_runtime_pm_put(dev_priv); > > + return ret; > > } > > > > static ssize_t > > @@ -448,7 +456,9 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, > > struct device_attribute *attr > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > return ret; > > + intel_runtime_pm_get(dev_priv); > > rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev->struct_mutex); > > > > if (attr == &dev_attr_gt_RP0_freq_mhz) { > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 1e372d5..28fc070 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3082,9 +3082,12 @@ intel_dp_detect(struct drm_connector *connector, > > bool force) > > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > struct intel_encoder *intel_encoder = &intel_dig_port->base; > > struct drm_device *dev = connector->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > enum drm_connector_status status; > > struct edid *edid = NULL; > > > > + intel_runtime_pm_get(dev_priv); > > + > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > connector->base.id, drm_get_connector_name(connector)); > > > > @@ -3096,7 +3099,7 @@ intel_dp_detect(struct drm_connector *connector, bool > > force) > > status = g4x_dp_detect(intel_dp); > > > > if (status != connector_status_connected) > > - return status; > > + goto out; > > > > intel_dp_probe_oui(intel_dp); > > > > @@ -3112,7 +3115,11 @@ intel_dp_detect(struct drm_connector *connector, > > bool force) > > > > if (intel_encoder->type != INTEL_OUTPUT_EDP) > > intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > - return connector_status_connected; > > + status = connector_status_connected; > > + > > +out: > > + intel_runtime_pm_put(dev_priv); > > + return status; > > } > > > > static int intel_dp_get_modes(struct drm_connector *connector) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index e480cf4..4b7925b 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -845,11 +845,14 @@ static int > > intel_backlight_device_get_brightness(struct backlight_device *bd) > > { > > struct intel_connector *connector = bl_get_data(bd); > > struct drm_device *dev = connector->base.dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > int ret; > > > > + intel_runtime_pm_get(dev_priv); > > mutex_lock(&dev->mode_config.mutex); > > ret = intel_panel_get_backlight(connector); > > mutex_unlock(&dev->mode_config.mutex); > > + intel_runtime_pm_put(dev_priv); > > > > return ret; > > } > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > > b/drivers/gpu/drm/i915/intel_uncore.c > > index 00e5ced..2d745a6 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -366,6 +366,8 @@ void gen6_gt_force_wake_get(struct drm_i915_private > > *dev_priv, int fw_engine) > > if (!dev_priv->uncore.funcs.force_wake_get) > > return; > > > > + intel_runtime_pm_get(dev_priv); > > + > > /* Redirect to VLV specific routine */ > > if (IS_VALLEYVIEW(dev_priv->dev)) > > return vlv_force_wake_get(dev_priv, fw_engine); > > @@ -399,6 +401,8 @@ void gen6_gt_force_wake_put(struct drm_i915_private > > *dev_priv, int fw_engine) > > 1); > > } > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > + > > + intel_runtime_pm_put(dev_priv); > > } > > > > /* We give fast paths for the really cool registers */ > > -- > > 1.8.3.1 > > > > _______________________________________________ > > 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx