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

["Sharma, Shashank" <shashank.sharma@amd.com>]

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.sharma@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Marek Olsak <marek.olsak@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Cc: Amarnath Somalapuram <amaranath.somalapuram@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_* */
>