[PATCH v2] drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout

[Lucas De Marchi]

On Fri, Oct 25, 2024 at 09:33:39PM +0200, Nirmoy Das wrote:
>
>On 10/25/2024 8:34 PM, Matthew Brost wrote:
>> On Fri, Oct 25, 2024 at 11:27:55AM -0700, John Harrison wrote:
>>> On 10/25/2024 09:03, Nirmoy Das wrote:
>>>> On 10/24/2024 6:32 PM, Jani Nikula wrote:
>>>>> On Thu, 24 Oct 2024, Nirmoy Das <nirmoy.das at intel.com> wrote:
>>>>>> Flush xe ordered_wq in case of ufence timeout which is observed
>>>>>> on LNL and that points to the recent scheduling issue with E-cores.
>>>>>>
>>>>>> This is similar to the recent fix:
>>>>>> commit e51527233804 ("drm/xe/guc/ct: Flush g2h worker in case of g2h
>>>>>> response timeout") and should be removed once there is E core
>>>>>> scheduling fix.
>>>>>>
>>>>>> v2: Add platform check(Himal)
>>>>>>      s/__flush_workqueue/flush_workqueue(Jani)
>>>>>>
>>>>>> Cc: Badal Nilawar <badal.nilawar at intel.com>
>>>>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>>>>> Cc: Matthew Auld <matthew.auld at intel.com>
>>>>>> Cc: John Harrison <John.C.Harrison at Intel.com>
>>>>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
>>>>>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>>>>> Cc: <stable at vger.kernel.org> # v6.11+
>>>>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>>>>> Suggested-by: Matthew Brost <matthew.brost at intel.com>
>>>>>> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>>>>>> Reviewed-by: Matthew Brost <matthew.brost at intel.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/xe/xe_wait_user_fence.c | 14 ++++++++++++++
>>>>>>   1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_wait_user_fence.c b/drivers/gpu/drm/xe/xe_wait_user_fence.c
>>>>>> index f5deb81eba01..78a0ad3c78fe 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_wait_user_fence.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_wait_user_fence.c
>>>>>> @@ -13,6 +13,7 @@
>>>>>>   #include "xe_device.h"
>>>>>>   #include "xe_gt.h"
>>>>>>   #include "xe_macros.h"
>>>>>> +#include "compat-i915-headers/i915_drv.h"
>>>>> Sorry, you just can't use this in xe core. At all. Not even a little
>>>>> bit. It's purely for i915 display compat code.
>>>>>
>>>>> If you need it for the LNL platform check, you need to use:
>>>>>
>>>>> 	xe->info.platform == XE_LUNARLAKE
>>>> Will do that. That macro looked odd but I didn't know a better way.
>>>>
>>>>> Although platform checks in xe code are generally discouraged.
>>>> This issue unfortunately depending on platform instead of graphics IP.
>>> But isn't this issue dependent upon the CPU platform not the graphics
>>> platform? As in, a DG2 card plugged in to a LNL host will also have this
>>> issue. So testing any graphics related value is technically incorrect.
>
>
>Haven't thought about. LNL only has x8 PCIe lanes shared between NVME and other IOs but thunderbolt based eGPU should be easily doable.
>
>I think I could do "if (boot_cpu_data.x86_vfm == INTEL_LUNARLAKE_M)" instead.
>
>>>
>> This is a good point, maybe for now we blindly do this regardless of
>> platform. It is basically harmless to do this after a timeout... Also a
>> warning message if we can detect this fixed the timeout for CI purposes.
>
>I am open to this as well. Please let me know which one should be a better solution here.

if it's a cheap thing without side-effects, go for the version without
the platform check and document it in commit message / source comment

Lucas De Marchi