Re: [PATCH 22/40] drm/i915: Syntatic sugar for using intel_runtime_pm

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 19/09/2018 20:55, Chris Wilson wrote:
> Frequently, we use intel_runtime_pm_get/_put around a small block.
> Formalise that usage by providing a macro to define such a block with an
> automatic closure to scope the intel_runtime_pm wakeref to that block,
> i.e. macro abuse smelling of python.

Interesting and tempting but I cannot shake the feeling it is maybe 
going a bit too far. We could add with_lock, with_mutex, 
with_runtime_pm_and_lock and where does it end? :) Don't know.. ping 
some more people on this one I guess.

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c           | 163 ++++++++----------
>   drivers/gpu/drm/i915/i915_gem.c               |  10 +-
>   drivers/gpu/drm/i915/i915_gem_gtt.c           |  23 ++-
>   drivers/gpu/drm/i915/i915_gem_shrinker.c      |  44 ++---
>   drivers/gpu/drm/i915/i915_pmu.c               |   7 +-
>   drivers/gpu/drm/i915/i915_sysfs.c             |   7 +-
>   drivers/gpu/drm/i915/intel_drv.h              |   8 +
>   drivers/gpu/drm/i915/intel_guc_log.c          |  26 ++-
>   drivers/gpu/drm/i915/intel_huc.c              |   7 +-
>   drivers/gpu/drm/i915/intel_panel.c            |  18 +-
>   drivers/gpu/drm/i915/intel_uncore.c           |  30 ++--
>   drivers/gpu/drm/i915/selftests/i915_gem.c     |  34 ++--
>   .../gpu/drm/i915/selftests/i915_gem_context.c |  12 +-
>   .../gpu/drm/i915/selftests/i915_gem_object.c  |  11 +-
>   .../drm/i915/selftests/intel_workarounds.c    |  12 +-
>   15 files changed, 193 insertions(+), 219 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index dbfe4e456d97..98fa216d19bb 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -952,9 +952,9 @@ static int i915_gpu_info_open(struct inode *inode, struct file *file)
>   	struct i915_gpu_state *gpu;
>   	intel_wakeref_t wakeref;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	gpu = i915_capture_gpu_state(i915);
> -	intel_runtime_pm_put(i915, wakeref);
> +	gpu = NULL;
> +	with_intel_runtime_pm(i915, wakeref)
> +		gpu = i915_capture_gpu_state(i915);
>   	if (!gpu)
>   		return -ENOMEM;
>   
> @@ -1015,9 +1015,8 @@ i915_next_seqno_set(void *data, u64 val)
>   	if (ret)
>   		return ret;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	ret = i915_gem_set_global_seqno(dev, val);
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		ret = i915_gem_set_global_seqno(dev, val);
>   
>   	mutex_unlock(&dev->struct_mutex);
>   
> @@ -1305,17 +1304,15 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
>   		return 0;
>   	}
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		for_each_engine(engine, dev_priv, id) {
> +			acthd[id] = intel_engine_get_active_head(engine);
> +			seqno[id] = intel_engine_get_seqno(engine);
> +		}
>   
> -	for_each_engine(engine, dev_priv, id) {
> -		acthd[id] = intel_engine_get_active_head(engine);
> -		seqno[id] = intel_engine_get_seqno(engine);
> +		intel_engine_get_instdone(dev_priv->engine[RCS], &instdone);
>   	}
>   
> -	intel_engine_get_instdone(dev_priv->engine[RCS], &instdone);
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
> -
>   	if (timer_pending(&dev_priv->gpu_error.hangcheck_work.timer))
>   		seq_printf(m, "Hangcheck active, timer fires in %dms\n",
>   			   jiffies_to_msecs(dev_priv->gpu_error.hangcheck_work.timer.expires -
> @@ -1591,18 +1588,16 @@ static int i915_drpc_info(struct seq_file *m, void *unused)
>   {
>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>   	intel_wakeref_t wakeref;
> -	int err;
> -
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		err = vlv_drpc_info(m);
> -	else if (INTEL_GEN(dev_priv) >= 6)
> -		err = gen6_drpc_info(m);
> -	else
> -		err = ironlake_drpc_info(m);
> +	int err = -ENODEV;
>   
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +			err = vlv_drpc_info(m);
> +		else if (INTEL_GEN(dev_priv) >= 6)
> +			err = gen6_drpc_info(m);
> +		else
> +			err = ironlake_drpc_info(m);
> +	}
>   
>   	return err;
>   }
> @@ -2167,9 +2162,8 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
>   	p = drm_seq_file_printer(m);
>   	intel_uc_fw_dump(&dev_priv->huc.fw, &p);
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
>   
>   	return 0;
>   }
> @@ -2179,7 +2173,6 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>   	intel_wakeref_t wakeref;
>   	struct drm_printer p;
> -	u32 tmp, i;
>   
>   	if (!HAS_GUC(dev_priv))
>   		return -ENODEV;
> @@ -2187,22 +2180,23 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
>   	p = drm_seq_file_printer(m);
>   	intel_uc_fw_dump(&dev_priv->guc.fw, &p);
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	tmp = I915_READ(GUC_STATUS);
> -
> -	seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
> -	seq_printf(m, "\tBootrom status = 0x%x\n",
> -		(tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
> -	seq_printf(m, "\tuKernel status = 0x%x\n",
> -		(tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
> -	seq_printf(m, "\tMIA Core status = 0x%x\n",
> -		(tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
> -	seq_puts(m, "\nScratch registers:\n");
> -	for (i = 0; i < 16; i++)
> -		seq_printf(m, "\t%2d: \t0x%x\n", i, I915_READ(SOFT_SCRATCH(i)));
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		u32 tmp = I915_READ(GUC_STATUS);
> +		u32 i;
> +
> +		seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
> +		seq_printf(m, "\tBootrom status = 0x%x\n",
> +			   (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
> +		seq_printf(m, "\tuKernel status = 0x%x\n",
> +			   (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
> +		seq_printf(m, "\tMIA Core status = 0x%x\n",
> +			   (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
> +		seq_puts(m, "\nScratch registers:\n");
> +		for (i = 0; i < 16; i++) {
> +			seq_printf(m, "\t%2d: \t0x%x\n",
> +				   i, I915_READ(SOFT_SCRATCH(i)));
> +		}
> +	}
>   
>   	return 0;
>   }
> @@ -2675,19 +2669,14 @@ static int i915_energy_uJ(struct seq_file *m, void *data)
>   	if (INTEL_GEN(dev_priv) < 6)
>   		return -ENODEV;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) {
> -		intel_runtime_pm_put(dev_priv, wakeref);
> +	if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
>   		return -ENODEV;
> -	}
>   
>   	units = (power & 0x1f00) >> 8;
> -	power = I915_READ(MCH_SECP_NRG_STTS);
> -	power = (1000000 * power) >> units; /* convert to uJ */
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		power = I915_READ(MCH_SECP_NRG_STTS);
>   
> +	power = (1000000 * power) >> units; /* convert to uJ */
>   	seq_printf(m, "%llu", power);
>   
>   	return 0;
> @@ -3268,22 +3257,20 @@ static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
>   	struct seq_file *m = file->private_data;
>   	struct drm_i915_private *dev_priv = m->private;
>   	intel_wakeref_t wakeref;
> -	int ret;
>   	bool enable;
> +	int ret;
>   
>   	ret = kstrtobool_from_user(ubuf, len, &enable);
>   	if (ret < 0)
>   		return ret;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	if (!dev_priv->ipc_enabled && enable)
> -		DRM_INFO("Enabling IPC: WM will be proper only after next commit\n");
> -	dev_priv->wm.distrust_bios_wm = true;
> -	dev_priv->ipc_enabled = enable;
> -	intel_enable_ipc(dev_priv);
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		if (!dev_priv->ipc_enabled && enable)
> +			DRM_INFO("Enabling IPC: WM will be proper only after next commit\n");
> +		dev_priv->wm.distrust_bios_wm = true;
> +		dev_priv->ipc_enabled = enable;
> +		intel_enable_ipc(dev_priv);
> +	}
>   
>   	return len;
>   }
> @@ -4127,16 +4114,13 @@ i915_cache_sharing_get(void *data, u64 *val)
>   {
>   	struct drm_i915_private *dev_priv = data;
>   	intel_wakeref_t wakeref;
> -	u32 snpcr;
> +	u32 snpcr = 0;
>   
>   	if (!(IS_GEN6(dev_priv) || IS_GEN7(dev_priv)))
>   		return -ENODEV;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
>   
>   	*val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
>   
> @@ -4148,7 +4132,6 @@ i915_cache_sharing_set(void *data, u64 val)
>   {
>   	struct drm_i915_private *dev_priv = data;
>   	intel_wakeref_t wakeref;
> -	u32 snpcr;
>   
>   	if (!(IS_GEN6(dev_priv) || IS_GEN7(dev_priv)))
>   		return -ENODEV;
> @@ -4156,16 +4139,17 @@ i915_cache_sharing_set(void *data, u64 val)
>   	if (val > 3)
>   		return -EINVAL;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
>   	DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		u32 snpcr;
> +
> +		/* Update the cache sharing policy here as well */
> +		snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> +		snpcr &= ~GEN6_MBC_SNPCR_MASK;
> +		snpcr |= val << GEN6_MBC_SNPCR_SHIFT;
> +		I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
> +	}
>   
> -	/* Update the cache sharing policy here as well */
> -	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> -	snpcr &= ~GEN6_MBC_SNPCR_MASK;
> -	snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
> -	I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
> -
> -	intel_runtime_pm_put(dev_priv, wakeref);
>   	return 0;
>   }
>   
> @@ -4402,20 +4386,17 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>   	sseu.max_eus_per_subslice =
>   		INTEL_INFO(dev_priv)->sseu.max_eus_per_subslice;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -
> -	if (IS_CHERRYVIEW(dev_priv)) {
> -		cherryview_sseu_device_status(dev_priv, &sseu);
> -	} else if (IS_BROADWELL(dev_priv)) {
> -		broadwell_sseu_device_status(dev_priv, &sseu);
> -	} else if (IS_GEN9(dev_priv)) {
> -		gen9_sseu_device_status(dev_priv, &sseu);
> -	} else if (INTEL_GEN(dev_priv) >= 10) {
> -		gen10_sseu_device_status(dev_priv, &sseu);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		if (IS_CHERRYVIEW(dev_priv))
> +			cherryview_sseu_device_status(dev_priv, &sseu);
> +		else if (IS_BROADWELL(dev_priv))
> +			broadwell_sseu_device_status(dev_priv, &sseu);
> +		else if (IS_GEN9(dev_priv))
> +			gen9_sseu_device_status(dev_priv, &sseu);
> +		else if (INTEL_GEN(dev_priv) >= 10)
> +			gen10_sseu_device_status(dev_priv, &sseu);
>   	}
>   
> -	intel_runtime_pm_put(dev_priv, wakeref);
> -
>   	i915_print_sseu_info(m, false, &sseu);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b6da68846164..233a65487c3f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -814,13 +814,13 @@ void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
>   
>   	i915_gem_chipset_flush(dev_priv);
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	spin_lock_irq(&dev_priv->uncore.lock);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		spin_lock_irq(&dev_priv->uncore.lock);
>   
> -	POSTING_READ_FW(RING_HEAD(RENDER_RING_BASE));
> +		POSTING_READ_FW(RING_HEAD(RENDER_RING_BASE));
>   
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +		spin_unlock_irq(&dev_priv->uncore.lock);
> +	}
>   }
>   
>   static void
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 04710d0ecf8c..ef38d09b6ce0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2720,9 +2720,8 @@ static int ggtt_bind_vma(struct i915_vma *vma,
>   	if (i915_gem_object_is_readonly(obj))
>   		pte_flags |= PTE_READ_ONLY;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref)
> +		vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
>   
>   	vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
>   
> @@ -2741,9 +2740,8 @@ static void ggtt_unbind_vma(struct i915_vma *vma)
>   	struct drm_i915_private *i915 = vma->vm->i915;
>   	intel_wakeref_t wakeref;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	vma->vm->clear_range(vma->vm, vma->node.start, vma->size);
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref)
> +		vma->vm->clear_range(vma->vm, vma->node.start, vma->size);
>   }
>   
>   static int aliasing_gtt_bind_vma(struct i915_vma *vma,
> @@ -2777,9 +2775,10 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
>   	if (flags & I915_VMA_GLOBAL_BIND) {
>   		intel_wakeref_t wakeref;
>   
> -		wakeref = intel_runtime_pm_get(i915);
> -		vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
> -		intel_runtime_pm_put(i915, wakeref);
> +		with_intel_runtime_pm(i915, wakeref) {
> +			vma->vm->insert_entries(vma->vm, vma,
> +						cache_level, pte_flags);
> +		}
>   	}
>   
>   	return 0;
> @@ -2790,11 +2789,11 @@ static void aliasing_gtt_unbind_vma(struct i915_vma *vma)
>   	struct drm_i915_private *i915 = vma->vm->i915;
>   
>   	if (vma->flags & I915_VMA_GLOBAL_BIND) {
> +		struct i915_address_space *vm = vma->vm;
>   		intel_wakeref_t wakeref;
>   
> -		wakeref = intel_runtime_pm_get(i915);
> -		vma->vm->clear_range(vma->vm, vma->node.start, vma->size);
> -		intel_runtime_pm_put(i915, wakeref);
> +		with_intel_runtime_pm(i915, wakeref)
> +			vm->clear_range(vm, vma->node.start, vma->size);
>   	}
>   
>   	if (vma->flags & I915_VMA_LOCAL_BIND) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index f25e4c7c71b1..fe198824dba1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -299,14 +299,14 @@ i915_gem_shrink(struct drm_i915_private *i915,
>   unsigned long i915_gem_shrink_all(struct drm_i915_private *i915)
>   {
>   	intel_wakeref_t wakeref;
> -	unsigned long freed;
> +	unsigned long freed = 0;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	freed = i915_gem_shrink(i915, -1UL, NULL,
> -				I915_SHRINK_BOUND |
> -				I915_SHRINK_UNBOUND |
> -				I915_SHRINK_ACTIVE);
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref) {
> +		freed = i915_gem_shrink(i915, -1UL, NULL,
> +					I915_SHRINK_BOUND |
> +					I915_SHRINK_UNBOUND |
> +					I915_SHRINK_ACTIVE);
> +	}
>   
>   	return freed;
>   }
> @@ -379,14 +379,14 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
>   	if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) {
>   		intel_wakeref_t wakeref;
>   
> -		wakeref = intel_runtime_pm_get(i915);
> -		freed += i915_gem_shrink(i915,
> -					 sc->nr_to_scan - sc->nr_scanned,
> -					 &sc->nr_scanned,
> -					 I915_SHRINK_ACTIVE |
> -					 I915_SHRINK_BOUND |
> -					 I915_SHRINK_UNBOUND);
> -		intel_runtime_pm_put(i915, wakeref);
> +		with_intel_runtime_pm(i915, wakeref) {
> +			freed += i915_gem_shrink(i915,
> +						 sc->nr_to_scan - sc->nr_scanned,
> +						 &sc->nr_scanned,
> +						 I915_SHRINK_ACTIVE |
> +						 I915_SHRINK_BOUND |
> +						 I915_SHRINK_UNBOUND);
> +		}
>   	}
>   
>   	shrinker_unlock(i915, unlock);
> @@ -483,13 +483,13 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
>   	if (ret)
>   		goto out;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	freed_pages += i915_gem_shrink(i915, -1UL, NULL,
> -				       I915_SHRINK_BOUND |
> -				       I915_SHRINK_UNBOUND |
> -				       I915_SHRINK_ACTIVE |
> -				       I915_SHRINK_VMAPS);
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref) {
> +		freed_pages += i915_gem_shrink(i915, -1UL, NULL,
> +					       I915_SHRINK_BOUND |
> +					       I915_SHRINK_UNBOUND |
> +					       I915_SHRINK_ACTIVE |
> +					       I915_SHRINK_VMAPS);
> +	}
>   
>   	/* We also want to clear any cached iomaps as they wrap vmap */
>   	list_for_each_entry_safe(vma, next,
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 3d43fc9dd25d..b1cb2d3cae16 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -230,14 +230,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
>   
>   		val = dev_priv->gt_pm.rps.cur_freq;
>   		if (dev_priv->gt.awake) {
> -			intel_wakeref_t wakeref =
> -				intel_runtime_pm_get_if_in_use(dev_priv);
> +			intel_wakeref_t wakeref;
>   
> -			if (wakeref) {
> +			with_intel_runtime_pm_if_in_use(dev_priv, wakeref)
>   				val = intel_get_cagf(dev_priv,
>   						     I915_READ_NOTRACE(GEN6_RPSTAT1));
> -				intel_runtime_pm_put(dev_priv, wakeref);
> -			}
>   		}
>   
>   		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT],
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index a959aee208de..e126196413a4 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -43,11 +43,10 @@ static u32 calc_residency(struct drm_i915_private *dev_priv,
>   			  i915_reg_t reg)
>   {
>   	intel_wakeref_t wakeref;
> -	u64 res;
> +	u64 res = 0;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	res = intel_rc6_residency_us(dev_priv, reg);
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		res = intel_rc6_residency_us(dev_priv, reg);
>   
>   	return DIV_ROUND_CLOSEST_ULL(res, 1000);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5d88b63a3d6b..fd85a937b237 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2062,6 +2062,14 @@ intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915);
>   intel_wakeref_t intel_runtime_pm_get_if_in_use(struct drm_i915_private *i915);
>   intel_wakeref_t intel_runtime_pm_get_noresume(struct drm_i915_private *i915);
>   
> +#define with_intel_runtime_pm(i915, wf) \
> +	for (wf = intel_runtime_pm_get(i915); wf; \
> +	     intel_runtime_pm_put(i915, wf), wf = 0)
> +
> +#define with_intel_runtime_pm_if_in_use(i915, wf) \
> +	for (wf = intel_runtime_pm_get_if_in_use(i915); wf; \
> +	     intel_runtime_pm_put(i915, wf), wf = 0)
> +
>   void intel_runtime_pm_put_unchecked(struct drm_i915_private *i915);
>   #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>   void intel_runtime_pm_put(struct drm_i915_private *i915, intel_wakeref_t wref);
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 20c0b36d748e..b53582c0c6c1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -444,9 +444,8 @@ static void guc_log_capture_logs(struct intel_guc_log *log)
>   	 * Generally device is expected to be active only at this
>   	 * time, so get/put should be really quick.
>   	 */
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	guc_action_flush_log_complete(guc);
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		guc_action_flush_log_complete(guc);
>   }
>   
>   int intel_guc_log_create(struct intel_guc_log *log)
> @@ -507,7 +506,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
>   	struct intel_guc *guc = log_to_guc(log);
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	intel_wakeref_t wakeref;
> -	int ret;
> +	int ret = 0;
>   
>   	BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN != 0);
>   	GEM_BUG_ON(!log->vma);
> @@ -521,16 +520,14 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
>   
>   	mutex_lock(&dev_priv->drm.struct_mutex);
>   
> -	if (log->level == level) {
> -		ret = 0;
> +	if (log->level == level)
>   		goto out_unlock;
> -	}
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	ret = guc_action_control_log(guc, GUC_LOG_LEVEL_IS_VERBOSE(level),
> -				     GUC_LOG_LEVEL_IS_ENABLED(level),
> -				     GUC_LOG_LEVEL_TO_VERBOSITY(level));
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		ret = guc_action_control_log(guc,
> +					     GUC_LOG_LEVEL_IS_VERBOSE(level),
> +					     GUC_LOG_LEVEL_IS_ENABLED(level),
> +					     GUC_LOG_LEVEL_TO_VERBOSITY(level));
>   	if (ret) {
>   		DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret);
>   		goto out_unlock;
> @@ -611,9 +608,8 @@ void intel_guc_log_relay_flush(struct intel_guc_log *log)
>   	 */
>   	flush_work(&log->relay.flush_work);
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -	guc_action_flush_log(guc);
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref)
> +		guc_action_flush_log(guc);
>   
>   	/* GuC would have updated log buffer by now, so capture it */
>   	guc_log_capture_logs(log);
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index a61c944c3036..fb69cf195253 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -115,14 +115,13 @@ int intel_huc_check_status(struct intel_huc *huc)
>   {
>   	struct drm_i915_private *dev_priv = huc_to_i915(huc);
>   	intel_wakeref_t wakeref;
> -	u32 status;
> +	u32 status = 0;
>   
>   	if (!HAS_HUC(dev_priv))
>   		return -ENODEV;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref)
> +		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
>   
>   	return status;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e57c42323b17..d36fccaccefa 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1204,17 +1204,19 @@ static int intel_backlight_device_get_brightness(struct backlight_device *bd)
>   	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = to_i915(dev);
>   	intel_wakeref_t wakeref;
> -	u32 hw_level;
> -	int ret;
> +	int ret = 0;
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		u32 hw_level;
>   
> -	hw_level = intel_panel_get_backlight(connector);
> -	ret = scale_hw_to_user(connector, hw_level, bd->props.max_brightness);
> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>   
> -	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +		hw_level = intel_panel_get_backlight(connector);
> +		ret = scale_hw_to_user(connector,
> +				       hw_level, bd->props.max_brightness);
> +
> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +	}
>   
>   	return ret;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 5b842318edbc..e611dfb2543e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1696,21 +1696,21 @@ int i915_reg_read_ioctl(struct drm_device *dev,
>   
>   	flags = reg->offset & (entry->size - 1);
>   
> -	wakeref = intel_runtime_pm_get(dev_priv);
> -	if (entry->size == 8 && flags == I915_REG_READ_8B_WA)
> -		reg->val = I915_READ64_2x32(entry->offset_ldw,
> -					    entry->offset_udw);
> -	else if (entry->size == 8 && flags == 0)
> -		reg->val = I915_READ64(entry->offset_ldw);
> -	else if (entry->size == 4 && flags == 0)
> -		reg->val = I915_READ(entry->offset_ldw);
> -	else if (entry->size == 2 && flags == 0)
> -		reg->val = I915_READ16(entry->offset_ldw);
> -	else if (entry->size == 1 && flags == 0)
> -		reg->val = I915_READ8(entry->offset_ldw);
> -	else
> -		ret = -EINVAL;
> -	intel_runtime_pm_put(dev_priv, wakeref);
> +	with_intel_runtime_pm(dev_priv, wakeref) {
> +		if (entry->size == 8 && flags == I915_REG_READ_8B_WA)
> +			reg->val = I915_READ64_2x32(entry->offset_ldw,
> +						    entry->offset_udw);
> +		else if (entry->size == 8 && flags == 0)
> +			reg->val = I915_READ64(entry->offset_ldw);
> +		else if (entry->size == 4 && flags == 0)
> +			reg->val = I915_READ(entry->offset_ldw);
> +		else if (entry->size == 2 && flags == 0)
> +			reg->val = I915_READ16(entry->offset_ldw);
> +		else if (entry->size == 1 && flags == 0)
> +			reg->val = I915_READ8(entry->offset_ldw);
> +		else
> +			ret = -EINVAL;
> +	}
>   
>   	return ret;
>   }
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index 896f0b37b34c..8db2720b8028 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -98,26 +98,22 @@ static void pm_suspend(struct drm_i915_private *i915)
>   {
>   	intel_wakeref_t wakeref;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -
> -	i915_gem_suspend_gtt_mappings(i915);
> -	i915_gem_suspend_late(i915);
> -
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref) {
> +		i915_gem_suspend_gtt_mappings(i915);
> +		i915_gem_suspend_late(i915);
> +	}
>   }
>   
>   static void pm_hibernate(struct drm_i915_private *i915)
>   {
>   	intel_wakeref_t wakeref;
>   
> -	wakeref = intel_runtime_pm_get(i915);
> -
> -	i915_gem_suspend_gtt_mappings(i915);
> -
> -	i915_gem_freeze(i915);
> -	i915_gem_freeze_late(i915);
> +	with_intel_runtime_pm(i915, wakeref) {
> +		i915_gem_suspend_gtt_mappings(i915);
>   
> -	intel_runtime_pm_put(i915, wakeref);
> +		i915_gem_freeze(i915);
> +		i915_gem_freeze_late(i915);
> +	}
>   }
>   
>   static void pm_resume(struct drm_i915_private *i915)
> @@ -128,13 +124,11 @@ static void pm_resume(struct drm_i915_private *i915)
>   	 * Both suspend and hibernate follow the same wakeup path and assume
>   	 * that runtime-pm just works.
>   	 */
> -	wakeref = intel_runtime_pm_get(i915);
> -
> -	intel_engines_sanitize(i915);
> -	i915_gem_sanitize(i915);
> -	i915_gem_resume(i915);
> -
> -	intel_runtime_pm_put(i915, wakeref);
> +	with_intel_runtime_pm(i915, wakeref) {
> +		intel_engines_sanitize(i915);
> +		i915_gem_sanitize(i915);
> +		i915_gem_resume(i915);
> +	}
>   }
>   
>   static int igt_gem_suspend(void *arg)
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index e5db71e3991d..c7c891a287f5 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -580,9 +580,9 @@ static int igt_ctx_exec(void *arg)
>   				}
>   			}
>   
> -			wakeref = intel_runtime_pm_get(i915);
> -			err = gpu_fill(obj, ctx, engine, dw);
> -			intel_runtime_pm_put(i915, wakeref);
> +			err = 0;
> +			with_intel_runtime_pm(i915, wakeref)
> +				err = gpu_fill(obj, ctx, engine, dw);
>   			if (err) {
>   				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n",
>   				       ndwords, dw, max_dwords(obj),
> @@ -683,9 +683,9 @@ static int igt_ctx_readonly(void *arg)
>   					i915_gem_object_set_readonly(obj);
>   			}
>   
> -			wakeref = intel_runtime_pm_get(i915);
> -			err = gpu_fill(obj, ctx, engine, dw);
> -			intel_runtime_pm_put(i915, wakeref);
> +			err = 0;
> +			with_intel_runtime_pm(i915, wakeref)
> +				err = gpu_fill(obj, ctx, engine, dw);
>   			if (err) {
>   				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n",
>   				       ndwords, dw, max_dwords(obj),
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> index 13d4dc5e8dfb..572016a45a59 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> @@ -506,9 +506,8 @@ static void disable_retire_worker(struct drm_i915_private *i915)
>   	if (!i915->gt.active_requests++) {
>   		intel_wakeref_t wakeref;
>   
> -		wakeref = intel_runtime_pm_get(i915);
> -		i915_gem_unpark(i915);
> -		intel_runtime_pm_put(i915, wakeref);
> +		with_intel_runtime_pm(i915, wakeref)
> +			i915_gem_unpark(i915);
>   	}
>   	mutex_unlock(&i915->drm.struct_mutex);
>   
> @@ -590,10 +589,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
>   			goto out;
>   		}
>   
> +		err = 0;
>   		mutex_lock(&i915->drm.struct_mutex);
> -		wakeref = intel_runtime_pm_get(i915);
> -		err = make_obj_busy(obj);
> -		intel_runtime_pm_put(i915, wakeref);
> +		with_intel_runtime_pm(i915, wakeref)
> +			err = make_obj_busy(obj);
>   		mutex_unlock(&i915->drm.struct_mutex);
>   		if (err) {
>   			pr_err("[loop %d] Failed to busy the object\n", loop);
> diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> index 0db63eb24dfc..e2b542c6bfb1 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> @@ -45,9 +45,9 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
>   	if (err)
>   		goto err_obj;
>   
> -	wakeref = intel_runtime_pm_get(engine->i915);
> -	rq = i915_request_alloc(engine, ctx);
> -	intel_runtime_pm_put(engine->i915, wakeref);
> +	rq = ERR_PTR(-ENODEV);
> +	with_intel_runtime_pm(engine->i915, wakeref)
> +		rq = i915_request_alloc(engine, ctx);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_pin;
> @@ -179,9 +179,9 @@ static int switch_to_scratch_context(struct intel_engine_cs *engine)
>   	if (IS_ERR(ctx))
>   		return PTR_ERR(ctx);
>   
> -	wakeref = intel_runtime_pm_get(engine->i915);
> -	rq = i915_request_alloc(engine, ctx);
> -	intel_runtime_pm_put(engine->i915, wakeref);
> +	rq = ERR_PTR(-ENODEV);
> +	with_intel_runtime_pm(engine->i915, wakeref)
> +		rq = i915_request_alloc(engine, ctx);
>   
>   	kernel_context_close(ctx);
>   	if (IS_ERR(rq))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx