Re: [PATCH] amdgpu: add context creation flags in CS IOCTL

[Sharma, Shashank]

On 8/8/2022 12:59 PM, Christian König wrote:
Am 02.08.22 um 15:55 schrieb Shashank Sharma:
This patch adds:
- A new input parameter "flags" in the amdgpu_ctx_create2 call.
- Some new flags defining workload type hints.
- Some change in the caller function of amdgpu_ctx_create2, to
   accomodate this new parameter.

The idea is to pass the workload hints while context creation,

Bad idea.

Please take AMDGPU_CTX_OP_SET_STABLE_PSTATE and 
AMDGPU_CTX_OP_GET_STABLE_PSTATE as blueprint for this and don't add 
extra flags to the context creation.

Regards,
Christian.

Hey Christian,
Noted, let me have a look at AMDGPU_CTX_OP_GET/SET_STABLE_PSTATE 
implementation.

- Shashank



  so
that kernel GPU scheduler can pass this information to GPU FW, which in
turn can adjust the GPU characterstics as per the workload type.

Signed-off-by: Shashank Sharma <shashank.sha...@amd.com>
Cc: Alex Deucher <alexander.deuc...@amd.com>
Cc: Marek Olsak <marek.ol...@amd.com>
Cc: Christian Koenig <christian.koe...@amd.com>
Cc: Amarnath Somalapuram <amaranath.somalapu...@amd.com>
---
  amdgpu/amdgpu.h          |  2 ++
  amdgpu/amdgpu_cs.c       |  5 ++++-
  include/drm/amdgpu_drm.h | 10 +++++++++-
  3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index b118dd48..1ebb46e6 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -874,6 +874,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle 
handle,
   *
   * \param   dev      - \c [in] Device handle. See 
#amdgpu_device_initialize()
   * \param   priority - \c [in] Context creation flags. See 
AMDGPU_CTX_PRIORITY_*
+ * \param   flags    - \c [in] Context flags. See AMDGPU_CTX_FLAGS_*
   * \param   context  - \c [out] GPU Context handle
   *
   * \return   0 on success\n
@@ -884,6 +885,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle 
handle,
  */
  int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
               uint32_t priority,
+             uint32_t flags,
               amdgpu_context_handle *context);
  /**
   * Create GPU execution Context
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index fad484bf..d4723ea5 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -44,12 +44,14 @@ static int 
amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);
   *
   * \param   dev      - \c [in] Device handle. See 
#amdgpu_device_initialize()
   * \param   priority - \c [in] Context creation flags. See 
AMDGPU_CTX_PRIORITY_*
+ * \param   flags    - \c [in] Context flags. See AMDGPU_CTX_FLAGS_*
   * \param   context  - \c [out] GPU Context handle
   *
   * \return  0 on success otherwise POSIX Error code
  */
  drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
                       uint32_t priority,
+                     uint32_t flags,
                       amdgpu_context_handle *context)
  {
      struct amdgpu_context *gpu_context;
@@ -74,6 +76,7 @@ drm_public int 
amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
      memset(&args, 0, sizeof(args));
      args.in.op = AMDGPU_CTX_OP_ALLOC_CTX;
      args.in.priority = priority;
+    args.in.flags = flags;
      r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_CTX, &args, 
sizeof(args));
      if (r)
@@ -97,7 +100,7 @@ error:
  drm_public int amdgpu_cs_ctx_create(amdgpu_device_handle dev,
                      amdgpu_context_handle *context)
  {
-    return amdgpu_cs_ctx_create2(dev, AMDGPU_CTX_PRIORITY_NORMAL, 
context);
+    return amdgpu_cs_ctx_create2(dev, AMDGPU_CTX_PRIORITY_NORMAL, 0, 
context);
  }
  /**
diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index 0cbd1540..d9fb1f20 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -238,10 +238,18 @@ union drm_amdgpu_bo_list {
  #define AMDGPU_CTX_PRIORITY_HIGH        512
  #define AMDGPU_CTX_PRIORITY_VERY_HIGH   1023
+/* GPU context workload hint bitmask */
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_MASK    0xFF
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_NONE    0
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_3D      (1 << 1)
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_VIDEO   (1 << 2)
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_VR      (1 << 3)
+#define AMDGPU_CTX_FLAGS_WORKLOAD_HINT_COMPUTE (1 << 4)
+
  struct drm_amdgpu_ctx_in {
      /** AMDGPU_CTX_OP_* */
      __u32    op;
-    /** For future use, no flags defined so far */
+    /** AMDGPU_CTX_FLAGS_* */
      __u32    flags;
      __u32    ctx_id;
      /** AMDGPU_CTX_PRIORITY_* */