[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <72a3926a-f707-361d-22dc-74657eec8914@amd.com>
Date: Tue, 29 Oct 2024 08:22:53 -0700
From: Lizhi Hou <lizhi.hou@....com>
To: Jeffrey Hugo <quic_jhugo@...cinc.com>, <ogabbay@...nel.org>,
<dri-devel@...ts.freedesktop.org>
CC: <linux-kernel@...r.kernel.org>, <min.ma@....com>, <max.zhen@....com>,
<sonal.santan@....com>, <king.tam@....com>
Subject: Re: [PATCH V5 07/10] accel/amdxdna: Add command execution
On 10/25/24 10:55, Jeffrey Hugo wrote:
> On 10/21/2024 10:19 AM, Lizhi Hou wrote:
>> diff --git a/include/uapi/drm/amdxdna_accel.h
>> b/include/uapi/drm/amdxdna_accel.h
>> index 3792750834b2..08f3ec7146ab 100644
>> --- a/include/uapi/drm/amdxdna_accel.h
>> +++ b/include/uapi/drm/amdxdna_accel.h
>> @@ -13,6 +13,7 @@
>> extern "C" {
>> #endif
>> +#define AMDXDNA_INVALID_CMD_HANDLE (~0UL)
>> #define AMDXDNA_INVALID_ADDR (~0UL)
>> #define AMDXDNA_INVALID_CTX_HANDLE 0
>> #define AMDXDNA_INVALID_BO_HANDLE 0
>> @@ -29,6 +30,8 @@ enum amdxdna_drm_ioctl_id {
>> DRM_AMDXDNA_CREATE_BO,
>> DRM_AMDXDNA_GET_BO_INFO,
>> DRM_AMDXDNA_SYNC_BO,
>> + DRM_AMDXDNA_EXEC_CMD,
>> + DRM_AMDXDNA_WAIT_CMD,
>> };
>> /**
>> @@ -201,6 +204,54 @@ struct amdxdna_drm_sync_bo {
>> __u64 size;
>> };
>> +enum amdxdna_cmd_type {
>> + AMDXDNA_CMD_SUBMIT_EXEC_BUF = 0,
>> + AMDXDNA_CMD_SUBMIT_DEPENDENCY,
>> + AMDXDNA_CMD_SUBMIT_SIGNAL,
>> +};
>> +
>> +/**
>> + * struct amdxdna_drm_exec_cmd - Execute command.
>> + * @ext: MBZ.
>> + * @ext_flags: MBZ.
>> + * @hwctx: Hardware context handle.
>> + * @type: One of command type in enum amdxdna_cmd_type.
>> + * @cmd_handles: Array of command handles or the command handle itself
>> + * in case of just one.
>> + * @args: Array of arguments for all command handles.
>> + * @cmd_count: Number of command handles in the cmd_handles array.
>> + * @arg_count: Number of arguments in the args array.
>> + * @seq: Returned sequence number for this command.
>> + */
>> +struct amdxdna_drm_exec_cmd {
>> + __u64 ext;
>> + __u64 ext_flags;
>> + __u32 hwctx;
>> + __u32 type;
>> + __u64 cmd_handles;
>> + __u64 args;
>> + __u32 cmd_count;
>> + __u32 arg_count;
>> + __u64 seq;
>> +};
>> +
>> +/**
>> + * struct amdxdna_drm_wait_cmd - Wait exectuion command.
>> + *
>> + * @hwctx: hardware context handle.
>> + * @timeout: timeout in ms, 0 implies infinite wait.
>> + * @seq: sequence number of the command returned by execute command.
>> + *
>> + * Wait a command specified by seq to be completed.
>> + * Using AMDXDNA_INVALID_CMD_HANDLE as seq means wait till there is
>> a free slot
>> + * to submit a new command.
>> + */
>> +struct amdxdna_drm_wait_cmd {
>> + __u32 hwctx;
>> + __u32 timeout;
>> + __u64 seq;
>> +};
>> +
>> #define DRM_IOCTL_AMDXDNA_CREATE_HWCTX \
>> DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDXDNA_CREATE_HWCTX, \
>> struct amdxdna_drm_create_hwctx)
>> @@ -225,6 +276,14 @@ struct amdxdna_drm_sync_bo {
>> DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDXDNA_SYNC_BO, \
>> struct amdxdna_drm_sync_bo)
>> +#define DRM_IOCTL_AMDXDNA_EXEC_CMD \
>> + DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDXDNA_EXEC_CMD, \
>> + struct amdxdna_drm_exec_cmd)
>> +
>> +#define DRM_IOCTL_AMDXDNA_WAIT_CMD \
>> + DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDXDNA_WAIT_CMD, \
>> + struct amdxdna_drm_wait_cmd)
>> +
>
> Nope. This looks like a driver private wait ioctl on a specific BO.
> That is not the modern way to do things per Vetter -
>
> https://lore.kernel.org/dri-devel/ZC75%2Fq34YnDDsGpB@phenom.ffwll.local/
>
> Skimming the implementation, it looks like you are already using
> fences and the drm scheduler, so plumbing in syncobjs is not much more
> than what you are already doing, I think.
Ok. I will remove the wait ioctl and use syncobj instead.
Thanks,
Lizhi
>
> -Jeff
Powered by blists - more mailing lists