On 12/29/2014 03:32 PM, Christian König wrote:
> Am 29.12.2014 um 14:17 schrieb Oded Gabbay:
>>
>>
>> On 12/29/2014 03:06 PM, Christian König wrote:
>>> Am 29.12.2014 um 13:42 schrieb Oded Gabbay:
>>>> This patch reformats the ioctl definitions in kfd_ioctl.h to be similar to
>>>> the
>>>> drm ioctls definition style.
>>>>
>>>> Signed-off-by: Oded Gabbay <oded.gabbay at amd.com>
>>>
>>> You are aware that this is a kernel API breakage?
>> Yes, but as long as 3.19 isn't released yet, I'm allowed to do this, no ?
>
> Strictly speaking only if you fix a bug with it, but since we are still pretty
> early in the cycle it's ok with we (especially since it looks like a valid
> cleanup). I'm just not sure what Dave/Linus think of this.
>
> Christian.
Well, Dave said I should remove the error prints from the ioctl function and
generally make it similar to drm ioctl handling. And as you said, this looks
like a valid cleanup so I assume it would be ok from his POV to merge it for
-rc2/3.
Oded
>
>> and I want this patch-set to be inserted to 3.19 so there won't be API
>> breakage afterwards.
>>
>>>
>>>> ---
>>>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +++++++------
>>>> include/uapi/linux/kfd_ioctl.h | 37
>>>> +++++++++++++++++++-------------
>>>> 2 files changed, 30 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> index 5460ad2..390385f 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> @@ -524,35 +524,36 @@ static long kfd_ioctl(struct file *filep, unsigned
>>>> int
>>>> cmd, unsigned long arg)
>>>> switch (cmd) {
>>>> - case KFD_IOC_GET_VERSION:
>>>> + case AMDKFD_IOC_GET_VERSION:
>>>> retcode = kfd_ioctl_get_version(filep, process, kdata);
>>>> break;
>>>> - case KFD_IOC_CREATE_QUEUE:
>>>> +
>>>> + case AMDKFD_IOC_CREATE_QUEUE:
>>>> retcode = kfd_ioctl_create_queue(filep, process,
>>>> kdata);
>>>> break;
>>>> - case KFD_IOC_DESTROY_QUEUE:
>>>> + case AMDKFD_IOC_DESTROY_QUEUE:
>>>> retcode = kfd_ioctl_destroy_queue(filep, process,
>>>> kdata);
>>>> break;
>>>> - case KFD_IOC_SET_MEMORY_POLICY:
>>>> + case AMDKFD_IOC_SET_MEMORY_POLICY:
>>>> retcode = kfd_ioctl_set_memory_policy(filep, process,
>>>> kdata);
>>>> break;
>>>> - case KFD_IOC_GET_CLOCK_COUNTERS:
>>>> + case AMDKFD_IOC_GET_CLOCK_COUNTERS:
>>>> retcode = kfd_ioctl_get_clock_counters(filep, process,
>>>> kdata);
>>>> break;
>>>> - case KFD_IOC_GET_PROCESS_APERTURES:
>>>> + case AMDKFD_IOC_GET_PROCESS_APERTURES:
>>>> retcode = kfd_ioctl_get_process_apertures(filep, process,
>>>> kdata);
>>>> break;
>>>> - case KFD_IOC_UPDATE_QUEUE:
>>>> + case AMDKFD_IOC_UPDATE_QUEUE:
>>>> retcode = kfd_ioctl_update_queue(filep, process,
>>>> kdata);
>>>> break;
>>>> diff --git a/include/uapi/linux/kfd_ioctl.h
>>>> b/include/uapi/linux/kfd_ioctl.h
>>>> index 7acef41..05b53f6 100644
>>>> --- a/include/uapi/linux/kfd_ioctl.h
>>>> +++ b/include/uapi/linux/kfd_ioctl.h
>>>> @@ -128,27 +128,34 @@ struct kfd_ioctl_get_process_apertures_args {
>>>> uint32_t pad;
>>>> };
>>>> -#define KFD_IOC_MAGIC 'K'
>>>> +#define AMDKFD_IOCTL_BASE 'K'
>>>> +#define AMDKFD_IO(nr) _IO(AMDKFD_IOCTL_BASE, nr)
>>>> +#define AMDKFD_IOR(nr, type) _IOR(AMDKFD_IOCTL_BASE, nr, type)
>>>> +#define AMDKFD_IOW(nr, type) _IOW(AMDKFD_IOCTL_BASE, nr, type)
>>>> +#define AMDKFD_IOWR(nr, type) _IOWR(AMDKFD_IOCTL_BASE, nr, type)
>>>> -#define KFD_IOC_GET_VERSION \
>>>> - _IOR(KFD_IOC_MAGIC, 1, struct kfd_ioctl_get_version_args)
>>>> +#define AMDKFD_IOC_GET_VERSION \
>>>> + AMDKFD_IOR(0x01, struct kfd_ioctl_get_version_args)
>>>> -#define KFD_IOC_CREATE_QUEUE \
>>>> - _IOWR(KFD_IOC_MAGIC, 2, struct kfd_ioctl_create_queue_args)
>>>> +#define AMDKFD_IOC_CREATE_QUEUE \
>>>> + AMDKFD_IOWR(0x02, struct kfd_ioctl_create_queue_args)
>>>> -#define KFD_IOC_DESTROY_QUEUE \
>>>> - _IOWR(KFD_IOC_MAGIC, 3, struct kfd_ioctl_destroy_queue_args)
>>>> +#define AMDKFD_IOC_DESTROY_QUEUE \
>>>> + AMDKFD_IOWR(0x03, struct kfd_ioctl_destroy_queue_args)
>>>> -#define KFD_IOC_SET_MEMORY_POLICY \
>>>> - _IOW(KFD_IOC_MAGIC, 4, struct kfd_ioctl_set_memory_policy_args)
>>>> +#define AMDKFD_IOC_SET_MEMORY_POLICY \
>>>> + AMDKFD_IOW(0x04, struct kfd_ioctl_set_memory_policy_args)
>>>> -#define KFD_IOC_GET_CLOCK_COUNTERS \
>>>> - _IOWR(KFD_IOC_MAGIC, 5, struct kfd_ioctl_get_clock_counters_args)
>>>> +#define AMDKFD_IOC_GET_CLOCK_COUNTERS \
>>>> + AMDKFD_IOWR(0x05, struct kfd_ioctl_get_clock_counters_args)
>>>> -#define KFD_IOC_GET_PROCESS_APERTURES \
>>>> - _IOR(KFD_IOC_MAGIC, 6, struct kfd_ioctl_get_process_apertures_args)
>>>> +#define AMDKFD_IOC_GET_PROCESS_APERTURES \
>>>> + AMDKFD_IOR(0x06, struct kfd_ioctl_get_process_apertures_args)
>>>> -#define KFD_IOC_UPDATE_QUEUE \
>>>> - _IOW(KFD_IOC_MAGIC, 7, struct kfd_ioctl_update_queue_args)
>>>> +#define AMDKFD_IOC_UPDATE_QUEUE \
>>>> + AMDKFD_IOW(0x07, struct kfd_ioctl_update_queue_args)
>>>> +
>>>> +#define KFD_COMMAND_START 0x01
>>>> +#define KFD_COMMAND_END 0x08
>>>
>>> If you rename everything to AMDKFD_* you probably want to do so as well for
>>> KFD_COMMAND_(START|END).
>>
>> Thanks, I'll do that.
>>
>> Oded
>>>
>>> Regards,
>>> Christian.
>>>
>>>> #endif
>>>
>
