[Intel-xe] [PATCH] drm/xe: Fixup small bug in xe_de.h

[Lucas De Marchi]

On Mon, Apr 24, 2023 at 04:09:11PM +0200, Maarten Lankhorst wrote:
>
>On 2023-04-24 15:57, Lucas De Marchi wrote:
>>On Mon, Apr 24, 2023 at 03:05:17PM +0200, Maarten Lankhorst wrote:
>>>In display it's not called from irq context, so use xe_mmio_wait32 with
>>>atomic = false. This fixes a splat and the need for i915_utils.
>>>
>>>Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/245
>>
>>
>>this needs to be:
>>git commit --fixup=0838430682d761d0c62836455c5a792bd2836615 so it gets
>>squashed in next rebase in "fixup! drm/xe/display: Implement display 
>>support"
>Yeah that's fine, I was planning to change the commit message when 
>committing anyway.
>>>---
>>>drivers/gpu/drm/xe/display/xe_de.h | 5 +++--
>>>scripts/package/buildtar           | 2 +-
>>>2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/display/xe_de.h 
>>>b/drivers/gpu/drm/xe/display/xe_de.h
>>>index 000379f8702e..0c76b0d24d96 100644
>>>--- a/drivers/gpu/drm/xe/display/xe_de.h
>>>+++ b/drivers/gpu/drm/xe/display/xe_de.h
>>>@@ -70,8 +70,9 @@ __intel_de_wait_for_register(struct 
>>>drm_i915_private *i915, i915_reg_t reg,
>>>                 unsigned int fast_timeout_us,
>>>                 unsigned int slow_timeout_ms, u32 *out_value)
>>>{
>>>-    return wait_for_atomic(((*out_value = 
>>>xe_mmio_read32(to_gt(i915), reg.reg)) & mask) == value,
>>>-            slow_timeout_ms);
>>>+    return xe_mmio_wait32(to_gt(i915), reg.reg, value, mask,
>>>+                  fast_timeout_us + 1000 * slow_timeout_ms,
>>>+                  out_value, false);
>>>}
>>>
>>>static inline int
>>>diff --git a/scripts/package/buildtar b/scripts/package/buildtar
>>>index 65b4ea502962..fee5b9d3b171 100755
>>>--- a/scripts/package/buildtar
>>>+++ b/scripts/package/buildtar
>>>@@ -51,7 +51,7 @@ fi
>>>#
>>>cp -v -- "${objtree}/System.map" 
>>>"${tmpdir}/boot/System.map-${KERNELRELEASE}"
>>>cp -v -- "${KCONFIG_CONFIG}" "${tmpdir}/boot/config-${KERNELRELEASE}"
>>>-cp -v -- "${objtree}/vmlinux" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}"
>>>+#cp -v -- "${objtree}/vmlinux" 
>>>"${tmpdir}/boot/vmlinux-${KERNELRELEASE}"
>>
>>
>>leftover from debugging?
>
>Oops, yes. :)
>
>Can I get your r-b if I commit as a fixup?

ok... in i915 we have:

intel_de_wait_for_register()
   intel_wait_for_register()
     __intel_wait_for_register_fw()
     __wait_for()

with the __wait_for() only happening when slow_timeout != 0 (and
slow_timeout also passed to __intel_wait_for_register_fw()).

So this is always a might_sleep condition.

For direct users of __intel_de_wait_for_register(), it's harder to
verify as it depends on the value passed by the caller. Currently I see
2 callers:

	drivers/gpu/drm/i915/display/intel_dp_aux.c|47 col 8| ret = __intel_de_wait_for_register(i915, ch_ctl,
	drivers/gpu/drm/i915/display/intel_hdcp.c|350 col 8| ret = __intel_de_wait_for_register(dev_priv, HDCP_KEY_STATUS,

It seems they are only using the __intel* variant to be able to get the
out_value and not because of the different handling for the timeout.
since the are both passing a !0 slow_timeout.


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi