<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hi Lionel,<br>
    </p>
    <div class="moz-cite-prefix">On 4/19/2024 9:08 AM, Lionel Landwerlin
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:109eb32e-99c9-49a0-8166-42ed59e67662@intel.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">On 15/04/2024 17:52, Nirmoy Das
        wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:20240415145214.25641-8-nirmoy.das@intel.com">
        <pre class="moz-quote-pre" wrap="">Add a query flag for xe->info.has_device_atomics_on_smem
as this is platform dependent. This flag can be use to inform
whether DRM_XE_VM_BIND_FLAG_DEVICE_ATOMICS can be effectively
applied for a given platform.

Signed-off-by: Nirmoy Das <a class="moz-txt-link-rfc2396E"
        href="mailto:nirmoy.das@intel.com" moz-do-not-send="true"><nirmoy.das@intel.com></a>
---
 drivers/gpu/drm/xe/xe_query.c | 3 +++
 include/uapi/drm/xe_drm.h     | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
index df407d73e5f5..43272fdd5442 100644
--- a/drivers/gpu/drm/xe/xe_query.c
+++ b/drivers/gpu/drm/xe/xe_query.c
@@ -336,6 +336,9 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
        if (xe_device_get_root_tile(xe)->mem.vram.usable_size)
                config->info[DRM_XE_QUERY_CONFIG_FLAGS] =
                        DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM;
+       if (xe->info.has_device_atomics_on_smem)
+               config->info[DRM_XE_QUERY_CONFIG_FLAGS] |=
+                       DRM_XE_QUERY_CONFIG_FLAG_HAS_DEV_ATOMIC_ON_SMEM;
        config->info[DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT] =
                xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
        config->info[DRM_XE_QUERY_CONFIG_VA_BITS] = xe->info.va_bits;
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index ca4447e10ac9..80fff6fe3199 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -389,6 +389,8 @@ struct drm_xe_query_mem_regions {
  *
  *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
  *      has usable VRAM
+ *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_DEV_ATOMIC_ON_SMEM - Flag is set if the device
+ *      supports device atomics on system memory
  *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
  *    required by this device, typically SZ_4K or SZ_64K
  *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
@@ -404,7 +406,8 @@ struct drm_xe_query_config {
 
 #define DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID  0
 #define DRM_XE_QUERY_CONFIG_FLAGS                      1
-       #define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM       (1 << 0)
+       #define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM               (1 << 0)
+       #define DRM_XE_QUERY_CONFIG_FLAG_HAS_DEV_ATOMIC_ON_SMEM (1 << 1)</pre>
      </blockquote>
      <p><br>
      </p>
      <p>I find this flag a bit confusing, because it would imply that
        platforms that do not report this flag don't suppoort atomics on
        smem.</p>
      <p>But for DG2/MTL/TGL (if officially supported by Xe), that's not
        the case, yet they do support this.</p>
    </blockquote>
    <p>This query should return true for DG2/MTL/TGL but I missed
      setting <span style="white-space: pre-wrap">has_device_atomics_on_smem on those platform.</span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <blockquote type="cite"
      cite="mid:109eb32e-99c9-49a0-8166-42ed59e67662@intel.com">
      <p><br>
      </p>
      <p>Maybe renaming this to<span style="white-space: pre-wrap">DRM_XE_QUERY_CONFIG_FLAG_HAS_EXPLICIT_DEV_ATOMIC_ON_SMEM would make more sense.</span></p>
      <p><span style="white-space: pre-wrap">Meaning the platform requires a specific vm_bind flag to get atomics working on SMEM only BO.</span></p>
    </blockquote>
    <p>We could do that but we would need another query flag to indicate
      when a platform doesn't support dev atomics SMEM only BO at all
      like PVC. <br>
    </p>
    <p>So I think let's keep it <span style="white-space: pre-wrap">DRM_XE_QUERY_CONFIG_FLAG_HAS_DEV_ATOMIC_ON_SMEM which will return true for all platform but PVC at this moment.</span></p>
    <p><span style="white-space: pre-wrap">On platforms that doesn't have the new PTE bit, this flag will have no effect.  Let me know what do you think?</span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <p><span style="white-space: pre-wrap">Regards,</span></p>
    <p><span style="white-space: pre-wrap">Nirmoy
</span></p>
    <blockquote type="cite"
      cite="mid:109eb32e-99c9-49a0-8166-42ed59e67662@intel.com">
      <p><span style="white-space: pre-wrap">
</span></p>
      <p><span style="white-space: pre-wrap">-Lionel
</span></p>
      <p><br>
      </p>
      <blockquote type="cite"
        cite="mid:20240415145214.25641-8-nirmoy.das@intel.com">
        <pre class="moz-quote-pre" wrap=""> #define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT         2
 #define DRM_XE_QUERY_CONFIG_VA_BITS                    3
 #define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY    4
</pre>
      </blockquote>
      <p><br>
      </p>
    </blockquote>
  </body>
</html>