CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND is an int, defaulting to 250. When the wakeref is non-zero, it's either -1 or a dynamically allocated pointer, depending on CONFIG_DRM_I915_DEBUG_RUNTIME_PM. It's likely that the code works by coincidence with the bitwise AND, but with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, there's the off chance that the condition evaluates to false, and intel_wakeref_auto() doesn't get called. Switch to the intended logical AND.
Fixes: ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Anshuman Gupta anshuman.gupta@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 5c72462d1f57..c157ade48c39 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1131,7 +1131,7 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) GEM_WARN_ON(!i915_ttm_cpu_maps_iomem(bo->resource)); }
- if (wakeref & CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) + if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) intel_wakeref_auto(&to_i915(obj->base.dev)->runtime_pm.userfault_wakeref, msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
On 18/09/2024 12:17, Jani Nikula wrote:
CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND is an int, defaulting to 250. When the wakeref is non-zero, it's either -1 or a dynamically allocated pointer, depending on CONFIG_DRM_I915_DEBUG_RUNTIME_PM. It's likely that the code works by coincidence with the bitwise AND, but with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, there's the off chance that the condition evaluates to false, and intel_wakeref_auto() doesn't get called. Switch to the intended logical AND.
Fixes: ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Anshuman Gupta anshuman.gupta@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Matthew Auld matthew.auld@intel.com
Hi Jani,
On Wed, Sep 18, 2024 at 02:17:44PM +0300, Jani Nikula wrote:
CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND is an int, defaulting to 250. When the wakeref is non-zero, it's either -1 or a dynamically allocated pointer, depending on CONFIG_DRM_I915_DEBUG_RUNTIME_PM. It's likely that the code works by coincidence with the bitwise AND, but with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, there's the off chance that the condition evaluates to false, and intel_wakeref_auto() doesn't get called. Switch to the intended logical AND.
Fixes: ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Anshuman Gupta anshuman.gupta@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 5c72462d1f57..c157ade48c39 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1131,7 +1131,7 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) GEM_WARN_ON(!i915_ttm_cpu_maps_iomem(bo->resource)); }
- if (wakeref & CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
- if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
ops!
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
intel_wakeref_auto(&to_i915(obj->base.dev)->runtime_pm.userfault_wakeref, msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));2.39.2
On Wed, Sep 18, 2024 at 02:17:44PM +0300, Jani Nikula wrote:
CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND is an int, defaulting to 250. When the wakeref is non-zero, it's either -1 or a dynamically allocated pointer, depending on CONFIG_DRM_I915_DEBUG_RUNTIME_PM. It's likely that the code works by coincidence with the bitwise AND, but with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, there's the off chance that the condition evaluates to false, and intel_wakeref_auto() doesn't get called. Switch to the intended logical AND.
Fixes: ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Anshuman Gupta anshuman.gupta@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 5c72462d1f57..c157ade48c39 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1131,7 +1131,7 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) GEM_WARN_ON(!i915_ttm_cpu_maps_iomem(bo->resource)); }
- if (wakeref & CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
- if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
This is going to result in a clang warning:
drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: note: use '&' for a bitwise operation 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^~ | & drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: note: remove constant to silence this warning 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
Consider adding the explicit '!= 0' to make it clear this should be a boolean expression.
Cheers, Nathan
intel_wakeref_auto(&to_i915(obj->base.dev)->runtime_pm.userfault_wakeref, msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));2.39.2
On Wed, 18 Sep 2024, Nathan Chancellor nathan@kernel.org wrote:
On Wed, Sep 18, 2024 at 02:17:44PM +0300, Jani Nikula wrote:
CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND is an int, defaulting to 250. When the wakeref is non-zero, it's either -1 or a dynamically allocated pointer, depending on CONFIG_DRM_I915_DEBUG_RUNTIME_PM. It's likely that the code works by coincidence with the bitwise AND, but with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y, there's the off chance that the condition evaluates to false, and intel_wakeref_auto() doesn't get called. Switch to the intended logical AND.
Fixes: ad74457a6b5a ("drm/i915/dgfx: Release mmap on rpm suspend") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Anshuman Gupta anshuman.gupta@intel.com Cc: Andi Shyti andi.shyti@linux.intel.com Cc: stable@vger.kernel.org # v6.1+ Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 5c72462d1f57..c157ade48c39 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1131,7 +1131,7 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) GEM_WARN_ON(!i915_ttm_cpu_maps_iomem(bo->resource)); }
- if (wakeref & CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
- if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
This is going to result in a clang warning:
drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: note: use '&' for a bitwise operation 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^~ | & drivers/gpu/drm/i915/gem/i915_gem_ttm.c:1134:14: note: remove constant to silence this warning 1134 | if (wakeref && CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
Consider adding the explicit '!= 0' to make it clear this should be a boolean expression.
Thanks, will be fixed in v2.
BR, Jani.
Cheers, Nathan
intel_wakeref_auto(&to_i915(obj->base.dev)->runtime_pm.userfault_wakeref, msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));2.39.2
linux-stable-mirror@lists.linaro.org
-
Andi Shyti -
Jani Nikula -
Matthew Auld -
Nathan Chancellor