Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2

[Christian König]

Am 09.01.24 um 09:09 schrieb 李真能:
>
> Thanks!
>
> What about the second patch?
>
> The second patch:   amdgpu: change proirity value to be consistent 
> with kernel.
>
> As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module 
> drm-scheduler, if these two patches are not applyed,
>
> It can not pass LOW priority to drm-scheduler.
>
> Do you have any other suggestion?
>

Well what exactly is the problem? Just use AMD_PRIORITY=-512.

As far as I can see that is how it is supposed to be used.

Regards,
Christian.


>
>
>
>
>
>
> ----
>
>
>
>
> *主　题：*Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of 
> amdgpu_cs_ctx_create2
> *日　期：*2024-01-09 15:15
> *发件人：*Christian König
> *收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
>
> Am 09.01.24 um 02:50 schrieb 李真能:
>
> When the priority value is passed to the kernel, the kernel compares 
> it with the following values:
>
> #define AMDGPU_CTX_PRIORITY_VERY_LOW    -1023
> #define AMDGPU_CTX_PRIORITY_LOW         -512
> #define AMDGPU_CTX_PRIORITY_NORMAL      0
> #define AMDGPU_CTX_PRIORITY_HIGH        512
> #define AMDGPU_CTX_PRIORITY_VERY_HIGH   1023
>
> If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel 
> context priority,
>
>
> Well that's nonsense.
>
> How the kernel handles the values and how userspace handles them are 
> two separate things. You just need to make sure that it's always 32 bits.
>
> In other words if you have signed or unsigned data type in userspace 
> is irrelevant for the kernel.
>
> You can refer to the kernel function amdgpu_ctx_priority_permit, if 
> priority is greater
>
> than 0, and this process has not  CAP_SYS_NICE capibility or 
> DRM_MASTER permission,
>
> this process will be exited.
>
>
> Correct, that's intentional.
>
> Regards,
> Christian.
>
>
>
>
>
>
> ----
>
>
>
>
> *主　题：*Re: [PATCH libdrm 1/2] amdgpu: fix parameter of 
> amdgpu_cs_ctx_create2
> *日　期：*2024-01-09 00:28
> *发件人：*Christian König
> *收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
>
> Am 08.01.24 um 10:40 schrieb Zhenneng Li:
> > In order to pass the correct priority parameter to the kernel,
> > we must change priority type from uint32_t to int32_t.
>
> Hui what? Why should it matter if the parameter is signed or not?
>
> That doesn't seem to make sense.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Zhenneng Li
> > ---
> > amdgpu/amdgpu.h | 2 +-
> > amdgpu/amdgpu_cs.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> > index 9bdbf366..f46753f3 100644
> > --- a/amdgpu/amdgpu.h
> > +++ b/amdgpu/amdgpu.h
> > @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle 
> handle,
> > *
> > */
> > int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> > - uint32_t priority,
> > + int32_t priority,
> > amdgpu_context_handle *context);
> > /**
> > * Create GPU execution Context
> > diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> > index 49fc16c3..eb72c638 100644
> > --- a/amdgpu/amdgpu_cs.c
> > +++ b/amdgpu/amdgpu_cs.c
> > @@ -49,7 +49,7 @@ static int 
> amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);
> > * \return 0 on success otherwise POSIX Error code
> > */
> > drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> > - uint32_t priority,
> > + int32_t priority,
> > amdgpu_context_handle *context)
> > {
> > struct amdgpu_context *gpu_context;
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20240109/a3e3b44b/attachment.htm>