Message-ID: <37ec0125-8d0b-7d87-321d-ed4c7c7b32a7@amd.com>
Date:   Mon, 20 Feb 2023 09:27:47 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v4 05/14] dma-buf/sync_file: Add SET_DEADLINE ioctl
Content-Language: en-US
To:     Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     freedreno@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <ckoenig.leichtzumerken@gmail.com>,
        =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel@daenzer.net>,
        Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        Pekka Paalanen <ppaalanen@gmail.com>,
        Simon Ser <contact@emersion.fr>,
        Rob Clark <robdclark@chromium.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Gustavo Padovan <gustavo@padovan.org>,
        "open list:SYNC FILE FRAMEWORK" <linux-media@vger.kernel.org>,
        "moderated list:DMA BUFFER SHARING FRAMEWORK" 
        <linaro-mm-sig@lists.linaro.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20230218211608.1630586-1-robdclark@gmail.com>
 <20230218211608.1630586-6-robdclark@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230218211608.1630586-6-robdclark@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T0ZEVC9NT2UwN1ltU0ROeDhEeWM4d2pvenQxemY4NndMV0NlRkd0MndJTEw3?=
 =?utf-8?B?K1llNm1DeTZmc0NOdDRWU1ZlVHEvZ3gvYmhGdWl6eCtyak9TbTdzS2MwQ1kz?=
 =?utf-8?B?N01qRGpndWhSYld1eVlIb2tGUm9VWmlXMDBXS3RyWjhxNDF6VTR0UEVnYlNp?=
 =?utf-8?B?UFlqUE9mckZ3L3JYdlN2amdRanU2ZzlPR0pyREdiWjhyVU5TVzA0dGdZUlpX?=
 =?utf-8?B?QWF5MnBkeXdHT25CQStaZ00xaHBRemk3UlZhQ0YvSGhEQ3I2V3Rua3Z4bkNM?=
 =?utf-8?B?ZGFIVURyRG5iM2VFTXdlTGdSSTlBU0ZMSWFLVmloOG42NW1yWmc4bTB6czFU?=
 =?utf-8?B?VDJBa2RhaXhsS3hxeThTQk12Uzd0QmJHbGd2RTJTbUFueFMyQzBhMkZvckM1?=
 =?utf-8?B?QzlFYmErc3R6SFNaZmVRWHppTmt0ejZuVHNEV1RocHVQOGFmb1hOOWduYzlC?=
 =?utf-8?B?bytuc3hiTEJyTlEvZWZvQ3hQazV2Nk5UaytwemJTb0dUbFFpU0RKd2VsSm1V?=
 =?utf-8?B?VE9NQ2NidjFDK2IzU3FwOStaNXJNZkVhR1F0eGdGMUxOS3BWbVFBMDN5aXZK?=
 =?utf-8?B?UmQvSjhnek9MY3FOWnloT0JxeWJXd0JwMmppQzl6bUNCcWliSldUV1NHeTc1?=
 =?utf-8?B?cFZ1KzJjb2hsNzV5VEFka0pBUzMwQTNqYnY0Zi8rc2QvRnJkOFg2MWltWWk5?=
 =?utf-8?B?ek1qRVROeG9QMFl2KzExVTEwakhrOExJb0xtNy9DaEV2VCs0SithVkFvQnkv?=
 =?utf-8?B?YSt5b011eHBQSGN0VEZNWVBHa0dTSFdRMFZ2WlROSThiM2NlaEFJVmJma1Ja?=
 =?utf-8?B?VHBjcHFUWnpIbGMvVkl2QUNmV2xkbnI1TUJ2amR6dGcvQ0IxaEJjL2JFaWJJ?=
 =?utf-8?B?aW4xb0VmTEh4ZDMwZjZqYXc2VDZSdGFXbjlzVXpJMldaTDNmM2pIamk0dDZ2?=
 =?utf-8?B?NExEaHcwbjkvclUzR2FmcHdxbjlJRUc2eVhqTmIyeUZld1BxeVVJb1JVSjMv?=
 =?utf-8?B?ZVhqSiszVU1CSzZ2WEw3TjdYNG94NDdXSlVNdkdhK3lONnZzbWhGbEt5VEVS?=
 =?utf-8?B?RkJCOG1JNFo3TFRhaTVhZTdmaSs1K3dmelM1aU1LQXhYMWMzSk9TWlZGSVgv?=
 =?utf-8?B?RnpmNzNpeWIwTnpOSTRWQkhQbFkvS1lTUUZOZWI3dEJvWVJiMDBqeUVLSmdI?=
 =?utf-8?B?SHVJYk9vaVNYS3RudEJ3WWxkekYyMVQ3QXg0Ly95VUdwWjVlTUVBSUVJVXZh?=
 =?utf-8?B?NXUvU1pOaG9KSjh6NEdtdFc5Z1Uwdi9QY25qaUgwQ3pQSTVvWUpnb20vZlJV?=
 =?utf-8?B?NXB1bHFaTnZiWXNRSzN6ekRsL0RRcklKMkFla1UrTWtIWCtFemtLSFYvL0E0?=
 =?utf-8?B?blZLR3FNcHRuU2hNTkswaFdPZkh4Z3VtRnI4OFlXNU5XR3lWN0poMkZ6dXNM?=
 =?utf-8?B?VmtFUklYeGVZempRMGZHQUx1SVhhWjdxZ3o2dTJxQllFQWF6K2xXSm9oaHR1?=
 =?utf-8?B?d09zSXVHK0lLMUwrT3R5S2xZbTJTV1JCSlpyRGF0by91YzhRRm5EUG5jdExU?=
 =?utf-8?B?eWY4eUUwbzBXR3lrREhDbFI3MGY4RlRnUmtyUlRncks1eXZUMWpUTktxTERS?=
 =?utf-8?B?ZGd2bEd0bUZxNVdFbXRRUG80K0FaaTgwRnBuWG05TklJZFpEc21FV0wwZm9r?=
 =?utf-8?B?K1VPZFN4N2JJMTdDS2ZnT0hFQnlFSGJGeEdaOWc0aStkMmlQZW5qVHdFdWR5?=
 =?utf-8?B?QUtWMjU2QW5HMUF1WE0yd0tSVUw0c3pIc1BZdmVUMityTGhqTE56b1IxY1gv?=
 =?utf-8?B?WkpEZWxTZzlzcHFTZnJ5MGVsRVZiblR2azV4RWczWjI0NFcveFloaHEvOUFy?=
 =?utf-8?B?MUk3YVhhdUVVd2gwdlY2Zlc4aVJFNWxjc0xoMzA0M3JQeTV5UUtpcm1DZGFr?=
 =?utf-8?B?bnBxK3E0ZkhFZDNmdGtES2J4b2Y1aVZlZm5SYlZjM3ptV05ZSEp2aDArb2tr?=
 =?utf-8?B?NnJhWXNMUWJnTmZ5ZnJ3Qld5VEVyNDJweVRrMjQ3RjI3SzZBRENnS1YyZXdr?=
 =?utf-8?B?VTBzZVZDRDRqTHU3UW9Tcnd5ZmYvdTdUQ3hTZW0zb0hWTDVlQTZsSnJ5cG1L?=
 =?utf-8?B?YURZVkh3UDhGaG9DdlFJOFhDaVozN005cktuQVRac2RIRFR1UlNpaEplYllW?=
 =?utf-8?Q?HjfNAqvEGO+ESL4duNmGYayMPWwpHzdurdQJnUL9Idpo?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7ffbabe9-32a5-4619-4b30-08db131c5d38
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 08:27:54.8914
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xNSiDsBtNdBeswBoyXxmR+fKFyhwZyb0yRHMYVNdg5aDFSLv9YsMYZryLNjwcLg2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4495
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 18.02.23 um 22:15 schrieb Rob Clark:
> From: Rob Clark <robdclark@chromium.org>
>
> The initial purpose is for igt tests, but this would also be useful for
> compositors that wait until close to vblank deadline to make decisions
> about which frame to show.
>
> The igt tests can be found at:
>
> https://gitlab.freedesktop.org/robclark/igt-gpu-tools/-/commits/fence-deadline
>
> v2: Clarify the timebase, add link to igt tests
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   drivers/dma-buf/sync_file.c    | 19 +++++++++++++++++++
>   include/uapi/linux/sync_file.h | 22 ++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index af57799c86ce..fb6ca1032885 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -350,6 +350,22 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>   	return ret;
>   }
>   
> +static int sync_file_ioctl_set_deadline(struct sync_file *sync_file,
> +					unsigned long arg)
> +{
> +	struct sync_set_deadline ts;
> +
> +	if (copy_from_user(&ts, (void __user *)arg, sizeof(ts)))
> +		return -EFAULT;
> +
> +	if (ts.pad)
> +		return -EINVAL;
> +
> +	dma_fence_set_deadline(sync_file->fence, ktime_set(ts.tv_sec, ts.tv_nsec));
> +
> +	return 0;
> +}
> +
>   static long sync_file_ioctl(struct file *file, unsigned int cmd,
>   			    unsigned long arg)
>   {
> @@ -362,6 +378,9 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd,
>   	case SYNC_IOC_FILE_INFO:
>   		return sync_file_ioctl_fence_info(sync_file, arg);
>   
> +	case SYNC_IOC_SET_DEADLINE:
> +		return sync_file_ioctl_set_deadline(sync_file, arg);
> +
>   	default:
>   		return -ENOTTY;
>   	}
> diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h
> index ee2dcfb3d660..c8666580816f 100644
> --- a/include/uapi/linux/sync_file.h
> +++ b/include/uapi/linux/sync_file.h
> @@ -67,6 +67,20 @@ struct sync_file_info {
>   	__u64	sync_fence_info;
>   };
>   
> +/**
> + * struct sync_set_deadline - set a deadline on a fence
> + * @tv_sec:	seconds elapsed since epoch
> + * @tv_nsec:	nanoseconds elapsed since the time given by the tv_sec
> + * @pad:	must be zero
> + *
> + * The timebase for the deadline is CLOCK_MONOTONIC (same as vblank)
> + */
> +struct sync_set_deadline {
> +	__s64	tv_sec;
> +	__s32	tv_nsec;
> +	__u32	pad;

IIRC struct timespec defined this as time_t/long (which is horrible for 
an UAPI because of the sizeof(long) dependency), one possible 
alternative is to use 64bit nanoseconds from CLOCK_MONOTONIC (which is 
essentially ktime).

Not 100% sure if there is any preferences documented, but I think the 
later might be better.

Either way the patch is Acked-by: Christian König 
<christian.koenig@amd.com> for this patch.

Regards,
Christian.

> +};
> +
>   #define SYNC_IOC_MAGIC		'>'
>   
>   /**
> @@ -95,4 +109,12 @@ struct sync_file_info {
>    */
>   #define SYNC_IOC_FILE_INFO	_IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info)
>   
> +
> +/**
> + * DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence
> + *
> + * Allows userspace to set a deadline on a fence, see dma_fence_set_deadline()
> + */
> +#define SYNC_IOC_SET_DEADLINE	_IOW(SYNC_IOC_MAGIC, 5, struct sync_set_deadline)
> +
>   #endif /* _UAPI_LINUX_SYNC_H */