(Cc: dri-devel)
On Tue, Jun 10, 2025 at 03:05:34PM +0200, Christian König wrote:
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index 5a8bc6342222..6108a6f9dba7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -44,6 +44,22 @@
> > struct amdgpu_fence;
> > enum amdgpu_ib_pool_type;
> >
> > +/* Internal kernel job ids. (decreasing values, starting from U64_MAX). */
> > +#define AMDGPU_KERNEL_JOB_ID_VM_UPDATE
> > (18446744073709551615ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES
> > (18446744073709551614ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_RANGE
> > (18446744073709551613ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_VM_PT_CLEAR
> > (18446744073709551612ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_TTM_MAP_BUFFER
> > (18446744073709551611ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_TTM_ACCESS_MEMORY_SDMA
> > (18446744073709551610ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER
> > (18446744073709551609ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE
> > (18446744073709551608ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_MOVE_BLIT
> > (18446744073709551607ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_TTM_CLEAR_BUFFER
> > (18446744073709551606ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_CLEANER_SHADER
> > (18446744073709551605ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_FLUSH_GPU_TLB
> > (18446744073709551604ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_KFD_GART_MAP
> > (18446744073709551603ULL)
> > +#define AMDGPU_KERNEL_JOB_ID_VCN_RING_TEST
> > (18446744073709551602ULL)
Why not
(U64_MAX - {1, 2, ...})?
> Mhm, reiterating our internal discussion on the mailing list.
>
> I think it would be nicer if we could use negative values for the kernel
> submissions and positive for userspace. But as discussed internally we would
> need to adjust the scheduler trace points for that once more.
>
> @Philip and @Danilo any opinion on that?
Both, the U64_MAX and the positive-negative approach, are a bit hacky. I wonder
why we need client_id to be a u64, wouldn't a u32 not be enough?
Anyways, if client_id remains to be a u64, we could add a global DRM constant
instead, e.g.
#define DRM_CLIENT_ID_MAX 0x0fffffffffffffff
#define DRM_KERNEL_ID_BASE (DRM_CLIENT_ID_MAX + 1)
And in drm_file_alloc() fail if we're out of IDs.
I think this would be much cleaner.
