Re: [Intel-gfx] [PATCH] drm/i915/slpc: Fix inconsistent locked return
On Tue, 30 Aug 2022 12:35:37 -0700, Rodrigo Vivi wrote:
>
> Fix for intel_guc_slpc_set_min_freq() warn:
> inconsistent returns '>lock'.
>
> v2: Avoid with_intel_runtime_pm with the
> internal goto/return. (Ashutosh)
> Also standardize the 'ret' if this came from
> the efficient setup. And avoid the 'unlikely'.
Reviewed-by: Ashutosh Dixit
> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient
> frequency")
> Reported-by: kernel test robot
> Reported-by: Dan Carpenter
> Cc: Ashutosh Dixit
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 40 ++---
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 9d49ccef03bb..fdd895f73f9f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -467,33 +467,33 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
> *slpc, u32 val)
>
> /* Need a lock now since waitboost can be modifying min as well */
> mutex_lock(>lock);
> -
> - with_intel_runtime_pm(>runtime_pm, wakeref) {
> -
> - /* Ignore efficient freq if lower min freq is requested */
> - ret = slpc_set_param(slpc,
> - SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> - val < slpc->rp1_freq);
> - if (unlikely(ret)) {
> - i915_probe_error(i915, "Failed to toggle efficient freq
> (%pe)\n",
> - ERR_PTR(ret));
> - return ret;
> - }
> -
> - ret = slpc_set_param(slpc,
> - SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> - val);
> -
> - /* Return standardized err code for sysfs calls */
> - if (ret)
> - ret = -EIO;
> + wakeref = intel_runtime_pm_get(>runtime_pm);
> +
> + /* Ignore efficient freq if lower min freq is requested */
> + ret = slpc_set_param(slpc,
> + SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> + val < slpc->rp1_freq);
> + if (ret) {
> + i915_probe_error(i915, "Failed to toggle efficient freq
> (%pe)\n",
> + ERR_PTR(ret));
> + goto out;
> }
>
> + ret = slpc_set_param(slpc,
> + SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> + val);
> +
> if (!ret)
> slpc->min_freq_softlimit = val;
>
> +out:
> + intel_runtime_pm_put(>runtime_pm, wakeref);
> mutex_unlock(>lock);
>
> + /* Return standardized err code for sysfs calls */
> + if (ret)
> + ret = -EIO;
> +
> return ret;
> }
>
> --
> 2.37.2
>
[Intel-gfx] [PATCH] drm/i915/slpc: Fix inconsistent locked return
Fix for intel_guc_slpc_set_min_freq() warn:
inconsistent returns '>lock'.
v2: Avoid with_intel_runtime_pm with the
internal goto/return. (Ashutosh)
Also standardize the 'ret' if this came from
the efficient setup. And avoid the 'unlikely'.
Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Cc: Ashutosh Dixit
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 40 ++---
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 9d49ccef03bb..fdd895f73f9f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -467,33 +467,33 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
/* Need a lock now since waitboost can be modifying min as well */
mutex_lock(>lock);
-
- with_intel_runtime_pm(>runtime_pm, wakeref) {
-
- /* Ignore efficient freq if lower min freq is requested */
- ret = slpc_set_param(slpc,
-SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
-val < slpc->rp1_freq);
- if (unlikely(ret)) {
- i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
-ERR_PTR(ret));
- return ret;
- }
-
- ret = slpc_set_param(slpc,
-SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
-val);
-
- /* Return standardized err code for sysfs calls */
- if (ret)
- ret = -EIO;
+ wakeref = intel_runtime_pm_get(>runtime_pm);
+
+ /* Ignore efficient freq if lower min freq is requested */
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
+val < slpc->rp1_freq);
+ if (ret) {
+ i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
+ERR_PTR(ret));
+ goto out;
}
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+val);
+
if (!ret)
slpc->min_freq_softlimit = val;
+out:
+ intel_runtime_pm_put(>runtime_pm, wakeref);
mutex_unlock(>lock);
+ /* Return standardized err code for sysfs calls */
+ if (ret)
+ ret = -EIO;
+
return ret;
}
--
2.37.2
Re: [Intel-gfx] [PATCH] drm/i915/slpc: Fix inconsistent locked return
On Tue, 30 Aug 2022 08:02:29 -0700, Rodrigo Vivi wrote:
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 9d49ccef03bb..f8a2bbcdf14f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -477,7 +477,7 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
> *slpc, u32 val)
> if (unlikely(ret)) {
> i915_probe_error(i915, "Failed to toggle efficient freq
> (%pe)\n",
>ERR_PTR(ret));
> - return ret;
> + goto unlock;
I think leaking runtime_pm wakeref now...
> }
>
> ret = slpc_set_param(slpc,
> @@ -492,6 +492,7 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
> *slpc, u32 val)
> if (!ret)
> slpc->min_freq_softlimit = val;
>
> +unlock:
> mutex_unlock(>lock);
>
> return ret;
> --
> 2.37.2
>
[Intel-gfx] [PATCH] drm/i915/slpc: Fix inconsistent locked return
Fix for intel_guc_slpc_set_min_freq() warn: inconsistent returns '>lock'.
Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Cc: Ashutosh Dixit
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 9d49ccef03bb..f8a2bbcdf14f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -477,7 +477,7 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
if (unlikely(ret)) {
i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
ERR_PTR(ret));
- return ret;
+ goto unlock;
}
ret = slpc_set_param(slpc,
@@ -492,6 +492,7 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
if (!ret)
slpc->min_freq_softlimit = val;
+unlock:
mutex_unlock(>lock);
return ret;
--
2.37.2
