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

[Maarten Lankhorst]

On 2023-04-24 17:32, Lucas De Marchi wrote:
> 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 for r-b, I see that I forgot to add it when pushing, but I fixed 
up the leftover.