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

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

[Nirmoy Das]

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@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: <stable@vger.kernel.org> # v6.11+
Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@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"
 #include "xe_exec_queue.h"
 
 static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
@@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
 		}
 
 		if (!timeout) {
+			if (IS_LUNARLAKE(xe)) {
+				/*
+				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
+				 * worker in case of g2h response timeout")
+				 *
+				 * TODO: Drop this change once workqueue scheduling delay issue is
+				 * fixed on LNL Hybrid CPU.
+				 */
+				flush_workqueue(xe->ordered_wq);
+				err = do_compare(addr, args->value, args->mask, args->op);
+				if (err <= 0)
+					break;
+			}
 			err = -ETIME;
 			break;
 		}
-- 
2.46.0

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

[Nirmoy Das]

On 10/24/2024 5:18 PM, Nirmoy Das 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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: <stable@vger.kernel.org> # v6.11+
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@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 sent too soon. This is bit out of place. I will sort it and resend after sometime to accumulate reviews.


>  #include "xe_exec_queue.h"
>  
>  static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>  		}
>  
>  		if (!timeout) {
> +			if (IS_LUNARLAKE(xe)) {
> +				/*
> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
> +				 * worker in case of g2h response timeout")
> +				 *
> +				 * TODO: Drop this change once workqueue scheduling delay issue is
> +				 * fixed on LNL Hybrid CPU.
> +				 */
> +				flush_workqueue(xe->ordered_wq);
> +				err = do_compare(addr, args->value, args->mask, args->op);
> +				if (err <= 0)
> +					break;
> +			}
>  			err = -ETIME;
>  			break;
>  		}

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

[Jani Nikula]

On Thu, 24 Oct 2024, Nirmoy Das <nirmoy.das@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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: <stable@vger.kernel.org> # v6.11+
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@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

Although platform checks in xe code are generally discouraged.

BR,
Jani.



>  #include "xe_exec_queue.h"
>  
>  static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>  		}
>  
>  		if (!timeout) {
> +			if (IS_LUNARLAKE(xe)) {
> +				/*
> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
> +				 * worker in case of g2h response timeout")
> +				 *
> +				 * TODO: Drop this change once workqueue scheduling delay issue is
> +				 * fixed on LNL Hybrid CPU.
> +				 */
> +				flush_workqueue(xe->ordered_wq);
> +				err = do_compare(addr, args->value, args->mask, args->op);
> +				if (err <= 0)
> +					break;
> +			}
>  			err = -ETIME;
>  			break;
>  		}

-- 
Jani Nikula, Intel

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

[John Harrison]

On 10/24/2024 08:18, Nirmoy Das 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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: <stable@vger.kernel.org> # v6.11+
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@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"
>   #include "xe_exec_queue.h"
>   
>   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>   		}
>   
>   		if (!timeout) {
> +			if (IS_LUNARLAKE(xe)) {
> +				/*
> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
> +				 * worker in case of g2h response timeout")
> +				 *
> +				 * TODO: Drop this change once workqueue scheduling delay issue is
> +				 * fixed on LNL Hybrid CPU.
> +				 */
> +				flush_workqueue(xe->ordered_wq);
If we are having multiple instances of this workaround, can we wrap them 
up in as 'LNL_FLUSH_WORKQUEUE(q)' or some such? Put the IS_LNL check 
inside the macro and make it pretty obvious exactly where all the 
instances are by having a single macro name to search for.

John.

> +				err = do_compare(addr, args->value, args->mask, args->op);
> +				if (err <= 0)
> +					break;
> +			}
>   			err = -ETIME;
>   			break;
>   		}

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

[Matthew Brost]

On Thu, Oct 24, 2024 at 10:14:21AM -0700, John Harrison wrote:
> On 10/24/2024 08:18, Nirmoy Das 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@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: John Harrison <John.C.Harrison@Intel.com>
> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: <stable@vger.kernel.org> # v6.11+
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > Reviewed-by: Matthew Brost <matthew.brost@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"
> >   #include "xe_exec_queue.h"
> >   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> > @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
> >   		}
> >   		if (!timeout) {
> > +			if (IS_LUNARLAKE(xe)) {
> > +				/*
> > +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
> > +				 * worker in case of g2h response timeout")
> > +				 *
> > +				 * TODO: Drop this change once workqueue scheduling delay issue is
> > +				 * fixed on LNL Hybrid CPU.
> > +				 */
> > +				flush_workqueue(xe->ordered_wq);
> If we are having multiple instances of this workaround, can we wrap them up
> in as 'LNL_FLUSH_WORKQUEUE(q)' or some such? Put the IS_LNL check inside the
> macro and make it pretty obvious exactly where all the instances are by
> having a single macro name to search for.
> 

+1, I think Lucas is suggesting something similar to this on the chat to
make sure we don't lose track of removing these W/A when this gets
fixed.

Matt

> John.
> 
> > +				err = do_compare(addr, args->value, args->mask, args->op);
> > +				if (err <= 0)
> > +					break;
> > +			}
> >   			err = -ETIME;
> >   			break;
> >   		}
> 

✓ CI.Patch_applied: success for drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)

[Patchwork]

== Series Details ==

Series: drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)
URL   : https://patchwork.freedesktop.org/series/140386/
State : success

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 75eab7b9ee32 drm-tip: 2024y-10m-25d-01h-19m-09s UTC integration manifest
=== git am output follows ===
Applying: drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout

✗ CI.checkpatch: warning for drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)

[Patchwork]

== Series Details ==

Series: drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)
URL   : https://patchwork.freedesktop.org/series/140386/
State : warning

== Summary ==

+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
30ab6715fc09baee6cc14cb3c89ad8858688d474
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 26bd871db556b14abd07ed49de04cdb1e8e7a861
Author: Nirmoy Das <nirmoy.das@intel.com>
Date:   Thu Oct 24 17:18:15 2024 +0200

    drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout
    
    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@intel.com>
    Cc: Jani Nikula <jani.nikula@intel.com>
    Cc: Matthew Auld <matthew.auld@intel.com>
    Cc: John Harrison <John.C.Harrison@Intel.com>
    Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
    Cc: Lucas De Marchi <lucas.demarchi@intel.com>
    Cc: <stable@vger.kernel.org> # v6.11+
    Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
    Suggested-by: Matthew Brost <matthew.brost@intel.com>
    Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
    Reviewed-by: Matthew Brost <matthew.brost@intel.com>
+ /mt/dim checkpatch 75eab7b9ee32a121d95a42ac704b679097f1fac4 drm-intel
26bd871db556 drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout
-:60: WARNING:MISSING_FIXES_TAG: The commit message has 'stable@', perhaps it also needs a 'Fixes:' tag?

total: 0 errors, 1 warnings, 0 checks, 26 lines checked

✗ CI.KUnit: failure for drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)

[Patchwork]

== Series Details ==

Series: drm/xe/ufence: Flush xe ordered_wq in case of ufence timeout (rev2)
URL   : https://patchwork.freedesktop.org/series/140386/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
  156 | u64 ioread64_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
  163 | u64 ioread64_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
  170 | u64 ioread64be_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
  178 | u64 ioread64be_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
  264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
  272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
  280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
  288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
In file included from ../drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:16,
                 from ../drivers/gpu/drm/xe/xe_wait_user_fence.c:16:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:24:35: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   24 | intel_runtime_pm_suspended(struct xe_runtime_pm *pm)
      |                                   ^~~~~~~~~~~~~
In file included from ../include/linux/container_of.h:5,
                 from ../include/linux/list.h:5,
                 from ../include/drm/drm_device.h:4,
                 from ../drivers/gpu/drm/xe/xe_wait_user_fence.c:8:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_suspended’:
../include/linux/container_of.h:20:54: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:26:32: note: in expansion of macro ‘container_of’
   26 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:26:32: note: in expansion of macro ‘container_of’
   26 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
In file included from ../include/uapi/linux/posix_types.h:5,
                 from ../include/uapi/linux/types.h:14,
                 from ../include/linux/types.h:6,
                 from ../include/linux/kasan-checks.h:5,
                 from ../include/asm-generic/rwonce.h:26,
                 from ./arch/x86/include/generated/asm/rwonce.h:1,
                 from ../include/linux/compiler.h:317,
                 from ../include/linux/build_bug.h:5:
../include/linux/stddef.h:16:33: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:26:32: note: in expansion of macro ‘container_of’
   26 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: At top level:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:31:59: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   31 | static inline intel_wakeref_t intel_runtime_pm_get(struct xe_runtime_pm *pm)
      |                                                           ^~~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_get’:
../include/linux/container_of.h:20:54: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:33:32: note: in expansion of macro ‘container_of’
   33 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:33:32: note: in expansion of macro ‘container_of’
   33 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../include/linux/stddef.h:16:33: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:33:32: note: in expansion of macro ‘container_of’
   33 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: At top level:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:38:69: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   38 | static inline intel_wakeref_t intel_runtime_pm_get_if_in_use(struct xe_runtime_pm *pm)
      |                                                                     ^~~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_get_if_in_use’:
../include/linux/container_of.h:20:54: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:40:32: note: in expansion of macro ‘container_of’
   40 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:40:32: note: in expansion of macro ‘container_of’
   40 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../include/linux/stddef.h:16:33: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:40:32: note: in expansion of macro ‘container_of’
   40 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: At top level:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:45:68: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   45 | static inline intel_wakeref_t intel_runtime_pm_get_noresume(struct xe_runtime_pm *pm)
      |                                                                    ^~~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_get_noresume’:
../include/linux/container_of.h:20:54: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:47:32: note: in expansion of macro ‘container_of’
   47 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:47:32: note: in expansion of macro ‘container_of’
   47 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../include/linux/stddef.h:16:33: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:47:32: note: in expansion of macro ‘container_of’
   47 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: At top level:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:54:58: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   54 | static inline void intel_runtime_pm_put_unchecked(struct xe_runtime_pm *pm)
      |                                                          ^~~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_put_unchecked’:
../include/linux/container_of.h:20:54: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:56:32: note: in expansion of macro ‘container_of’
   56 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:56:32: note: in expansion of macro ‘container_of’
   56 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../include/linux/stddef.h:16:33: error: ‘struct xe_device’ has no member named ‘runtime_pm’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:56:32: note: in expansion of macro ‘container_of’
   56 |         struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
      |                                ^~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: At top level:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:61:48: warning: ‘struct xe_runtime_pm’ declared inside parameter list will not be visible outside of this definition or declaration
   61 | static inline void intel_runtime_pm_put(struct xe_runtime_pm *pm, intel_wakeref_t wakeref)
      |                                                ^~~~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h: In function ‘intel_runtime_pm_put’:
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:64:48: error: passing argument 1 of ‘intel_runtime_pm_put_unchecked’ from incompatible pointer type [-Werror=incompatible-pointer-types]
   64 |                 intel_runtime_pm_put_unchecked(pm);
      |                                                ^~
      |                                                |
      |                                                struct xe_runtime_pm *
../drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h:54:73: note: expected ‘struct xe_runtime_pm *’ but argument is of type ‘struct xe_runtime_pm *’
   54 | static inline void intel_runtime_pm_put_unchecked(struct xe_runtime_pm *pm)
      |                                                   ~~~~~~~~~~~~~~~~~~~~~~^~
../drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h: In function ‘to_i915’:
../include/linux/container_of.h:20:54: error: invalid use of undefined type ‘struct drm_i915_private’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                                                      ^~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:22:16: note: in expansion of macro ‘container_of’
   22 |         return container_of(dev, struct drm_i915_private, drm);
      |                ^~~~~~~~~~~~
./../include/linux/compiler_types.h:458:27: error: expression in static assertion is not an integer
  458 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
../include/linux/container_of.h:20:9: note: in expansion of macro ‘static_assert’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
../include/linux/container_of.h:20:23: note: in expansion of macro ‘__same_type’
   20 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |                       ^~~~~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:22:16: note: in expansion of macro ‘container_of’
   22 |         return container_of(dev, struct drm_i915_private, drm);
      |                ^~~~~~~~~~~~
../include/linux/stddef.h:16:33: error: invalid use of undefined type ‘struct drm_i915_private’
   16 | #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
      |                                 ^~~~~~~~~~~~~~~~~~
../include/linux/container_of.h:23:28: note: in expansion of macro ‘offsetof’
   23 |         ((type *)(__mptr - offsetof(type, member))); })
      |                            ^~~~~~~~
../drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:22:16: note: in expansion of macro ‘container_of’
   22 |         return container_of(dev, struct drm_i915_private, drm);
      |                ^~~~~~~~~~~~
cc1: some warnings being treated as errors
make[7]: *** [../scripts/Makefile.build:229: drivers/gpu/drm/xe/xe_wait_user_fence.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:478: drivers/gpu/drm/xe] Error 2
make[5]: *** [../scripts/Makefile.build:478: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:478: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:478: drivers] Error 2
make[2]: *** [/kernel/Makefile:1936: .] Error 2
make[1]: *** [/kernel/Makefile:224: __sub-make] Error 2
make: *** [Makefile:224: __sub-make] Error 2

[01:52:31] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[01:52:35] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel

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

[Nirmoy Das]

On 10/24/2024 6:32 PM, Jani Nikula wrote:
> On Thu, 24 Oct 2024, Nirmoy Das <nirmoy.das@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@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.11+
>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Matthew Brost <matthew.brost@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.


Thanks,

Nirmoy

>
> BR,
> Jani.
>
>
>
>>  #include "xe_exec_queue.h"
>>  
>>  static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
>> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>>  		}
>>  
>>  		if (!timeout) {
>> +			if (IS_LUNARLAKE(xe)) {
>> +				/*
>> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
>> +				 * worker in case of g2h response timeout")
>> +				 *
>> +				 * TODO: Drop this change once workqueue scheduling delay issue is
>> +				 * fixed on LNL Hybrid CPU.
>> +				 */
>> +				flush_workqueue(xe->ordered_wq);
>> +				err = do_compare(addr, args->value, args->mask, args->op);
>> +				if (err <= 0)
>> +					break;
>> +			}
>>  			err = -ETIME;
>>  			break;
>>  		}

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

[Nirmoy Das]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]


On 10/24/2024 7:22 PM, Matthew Brost wrote:
> On Thu, Oct 24, 2024 at 10:14:21AM -0700, John Harrison wrote:
>> On 10/24/2024 08:18, Nirmoy Das 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@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>> Cc: <stable@vger.kernel.org> # v6.11+
>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>> Reviewed-by: Matthew Brost <matthew.brost@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"
>>>   #include "xe_exec_queue.h"
>>>   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
>>> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>>>   		}
>>>   		if (!timeout) {
>>> +			if (IS_LUNARLAKE(xe)) {
>>> +				/*
>>> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
>>> +				 * worker in case of g2h response timeout")
>>> +				 *
>>> +				 * TODO: Drop this change once workqueue scheduling delay issue is
>>> +				 * fixed on LNL Hybrid CPU.
>>> +				 */
>>> +				flush_workqueue(xe->ordered_wq);
>> If we are having multiple instances of this workaround, can we wrap them up
>> in as 'LNL_FLUSH_WORKQUEUE(q)' or some such? Put the IS_LNL check inside the
>> macro and make it pretty obvious exactly where all the instances are by
>> having a single macro name to search for.
>>
> +1, I think Lucas is suggesting something similar to this on the chat to
> make sure we don't lose track of removing these W/A when this gets
> fixed.
>
> Matt


Sounds good. I will add LNL_FLUSH_WORKQUEUE() and use that for all the places we need this WA.

Regards,

Nirmoy

>
>> John.
>>
>>> +				err = do_compare(addr, args->value, args->mask, args->op);
>>> +				if (err <= 0)
>>> +					break;
>>> +			}
>>>   			err = -ETIME;
>>>   			break;
>>>   		}

[-- Attachment #2: Type: text/html, Size: 4998 bytes --]

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

[Matthew Brost]

On Fri, Oct 25, 2024 at 06:06:47PM +0200, Nirmoy Das wrote:
>    On 10/24/2024 7:22 PM, Matthew Brost wrote:
> 
> On Thu, Oct 24, 2024 at 10:14:21AM -0700, John Harrison wrote:
> 
> On 10/24/2024 08:18, Nirmoy Das 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 [1]<badal.nilawar@intel.com>
> Cc: Jani Nikula [2]<jani.nikula@intel.com>
> Cc: Matthew Auld [3]<matthew.auld@intel.com>
> Cc: John Harrison [4]<John.C.Harrison@Intel.com>
> Cc: Himal Prasad Ghimiray [5]<himal.prasad.ghimiray@intel.com>
> Cc: Lucas De Marchi [6]<lucas.demarchi@intel.com>
> Cc: [7]<stable@vger.kernel.org> # v6.11+
> Link: [8]https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> Suggested-by: Matthew Brost [9]<matthew.brost@intel.com>
> Signed-off-by: Nirmoy Das [10]<nirmoy.das@intel.com>
> Reviewed-by: Matthew Brost [11]<matthew.brost@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_wai
> t_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"
>   #include "xe_exec_queue.h"
>   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *
> data,
>                 }
>                 if (!timeout) {
> +                       if (IS_LUNARLAKE(xe)) {
> +                               /*
> +                                * This is analogous to e51527233804 ("drm/xe/gu
> c/ct: Flush g2h
> +                                * worker in case of g2h response timeout")
> +                                *
> +                                * TODO: Drop this change once workqueue schedul
> ing delay issue is
> +                                * fixed on LNL Hybrid CPU.
> +                                */
> +                               flush_workqueue(xe->ordered_wq);
> 
> If we are having multiple instances of this workaround, can we wrap them up
> in as 'LNL_FLUSH_WORKQUEUE(q)' or some such? Put the IS_LNL check inside the
> macro and make it pretty obvious exactly where all the instances are by
> having a single macro name to search for.
> 
> 
> +1, I think Lucas is suggesting something similar to this on the chat to
> make sure we don't lose track of removing these W/A when this gets
> fixed.
> 
> Matt
> 
>    Sounds good. I will add LNL_FLUSH_WORKQUEUE() and use that for all the
>    places we need this WA.
> 

You will need 2 macros...

- LNL_FLUSH_WORKQUEUE() which accepts xe_device, workqueue_struct
- LNL_FLUSH_WORK() which accepts xe_device, work_struct

Matt

>    Regards,
> 
>    Nirmoy
> 
> 
> 
> John.
> 
> 
> +                               err = do_compare(addr, args->value, args->mask,
> args->op);
> +                               if (err <= 0)
> +                                       break;
> +                       }
>                         err = -ETIME;
>                         break;
>                 }
> 
> References
> 
>    1. mailto:badal.nilawar@intel.com
>    2. mailto:jani.nikula@intel.com
>    3. mailto:matthew.auld@intel.com
>    4. mailto:John.C.Harrison@Intel.com
>    5. mailto:himal.prasad.ghimiray@intel.com
>    6. mailto:lucas.demarchi@intel.com
>    7. mailto:stable@vger.kernel.org
>    8. https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>    9. mailto:matthew.brost@intel.com
>   10. mailto:nirmoy.das@intel.com
>   11. mailto:matthew.brost@intel.com

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

[John Harrison]

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@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@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>> Cc: <stable@vger.kernel.org> # v6.11+
>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>> Reviewed-by: Matthew Brost <matthew.brost@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.

John.

>
>
> Thanks,
>
> Nirmoy
>
>> BR,
>> Jani.
>>
>>
>>
>>>   #include "xe_exec_queue.h"
>>>   
>>>   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
>>> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>>>   		}
>>>   
>>>   		if (!timeout) {
>>> +			if (IS_LUNARLAKE(xe)) {
>>> +				/*
>>> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
>>> +				 * worker in case of g2h response timeout")
>>> +				 *
>>> +				 * TODO: Drop this change once workqueue scheduling delay issue is
>>> +				 * fixed on LNL Hybrid CPU.
>>> +				 */
>>> +				flush_workqueue(xe->ordered_wq);
>>> +				err = do_compare(addr, args->value, args->mask, args->op);
>>> +				if (err <= 0)
>>> +					break;
>>> +			}
>>>   			err = -ETIME;
>>>   			break;
>>>   		}

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

[Matthew Brost]

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@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@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > > Cc: John Harrison <John.C.Harrison@Intel.com>
> > > > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Cc: <stable@vger.kernel.org> # v6.11+
> > > > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
> > > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > > > Reviewed-by: Matthew Brost <matthew.brost@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.
> 

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.

Matt

> John.
> 
> > 
> > 
> > Thanks,
> > 
> > Nirmoy
> > 
> > > BR,
> > > Jani.
> > > 
> > > 
> > > 
> > > >   #include "xe_exec_queue.h"
> > > >   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
> > > > @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
> > > >   		}
> > > >   		if (!timeout) {
> > > > +			if (IS_LUNARLAKE(xe)) {
> > > > +				/*
> > > > +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
> > > > +				 * worker in case of g2h response timeout")
> > > > +				 *
> > > > +				 * TODO: Drop this change once workqueue scheduling delay issue is
> > > > +				 * fixed on LNL Hybrid CPU.
> > > > +				 */
> > > > +				flush_workqueue(xe->ordered_wq);
> > > > +				err = do_compare(addr, args->value, args->mask, args->op);
> > > > +				if (err <= 0)
> > > > +					break;
> > > > +			}
> > > >   			err = -ETIME;
> > > >   			break;
> > > >   		}
> 

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

[Nirmoy Das]

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@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@intel.com>
>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>>> Cc: <stable@vger.kernel.org> # v6.11+
>>>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>>> Reviewed-by: Matthew Brost <matthew.brost@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.


Regards,

Nirmoy

>
> Matt
>
>> John.
>>
>>>
>>> Thanks,
>>>
>>> Nirmoy
>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>
>>>>>   #include "xe_exec_queue.h"
>>>>>   static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
>>>>> @@ -155,6 +156,19 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>>>>>   		}
>>>>>   		if (!timeout) {
>>>>> +			if (IS_LUNARLAKE(xe)) {
>>>>> +				/*
>>>>> +				 * This is analogous to e51527233804 ("drm/xe/guc/ct: Flush g2h
>>>>> +				 * worker in case of g2h response timeout")
>>>>> +				 *
>>>>> +				 * TODO: Drop this change once workqueue scheduling delay issue is
>>>>> +				 * fixed on LNL Hybrid CPU.
>>>>> +				 */
>>>>> +				flush_workqueue(xe->ordered_wq);
>>>>> +				err = do_compare(addr, args->value, args->mask, args->op);
>>>>> +				if (err <= 0)
>>>>> +					break;
>>>>> +			}
>>>>>   			err = -ETIME;
>>>>>   			break;
>>>>>   		}

Re: [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@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@intel.com>
>>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>>>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>>>> Cc: <stable@vger.kernel.org> # v6.11+
>>>>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>>>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>>>> Reviewed-by: Matthew Brost <matthew.brost@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

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

[Nirmoy Das]

On 10/25/2024 9:56 PM, Lucas De Marchi wrote:
> 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@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@intel.com>
>>>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>>>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>>>>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>>>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>>>>> Cc: <stable@vger.kernel.org> # v6.11+
>>>>>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2754
>>>>>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>>>>> Reviewed-by: Matthew Brost <matthew.brost@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


That would be the previous rev. I will add the missing stable Cc and resend.



Thanks,

Nirmoy

>
> Lucas De Marchi