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

[Marek Olšák]

int p = -1.
unsigned u = p;
int p2 = u;

p2 is -1.

Marek

On Tue, Jan 9, 2024, 03:26 Christian König <christian.koenig at amd.com> wrote:

> 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/dri-devel/attachments/20240109/1419a178/attachment.htm>