From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0F3D4C41513 for ; Tue, 17 Oct 2023 16:25:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A9D2A10E307; Tue, 17 Oct 2023 16:25:03 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 20C4210E307 for ; Tue, 17 Oct 2023 16:25:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1697559901; x=1729095901; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=ncQNbmpfJHWSNH6e4UZxO99kKPryp3hEm7vRi+Cqj4c=; b=TOtt8N/MPrknhIev+wlMPEUa3CybZIjTur7b8qJm9K4xwe8I9GwZyF98 mkBblSjG3kRjdZLRd5fYjNbcdaArQrI+JR9hVoOKii8k0G+1n4x1EWHqx hjBypx2yuCp4cdk4DrWAB8/AwXPikr+2zdIJ3v4ZsrBmz8ecLp0ro/VYn rLS7c+KmJ0+Ep0T1PXKns8u62LQV/nvK4kHNysb5DeVggcVNRlsOf7WN2 8XhQN0oTS3RIi+aBavP+cilfwJtdMf9GQAQNZo9wQDItpU5q8ubyc9ai5 gqHyOSXvOZnWCTOzC+V+3Z3QqrGDglCC86P9EYhqV6s15Ys1CMI3PlTNl A==; X-IronPort-AV: E=McAfee;i="6600,9927,10866"; a="4418718" X-IronPort-AV: E=Sophos;i="6.03,232,1694761200"; d="scan'208";a="4418718" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Oct 2023 09:25:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.03,232,1694761200"; d="scan'208";a="3978348" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.11.216]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Oct 2023 09:23:54 -0700 Date: Tue, 17 Oct 2023 09:24:37 -0700 Message-ID: <87zg0hcj3u.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Rodrigo Vivi In-Reply-To: References: <20231006095943.7-1-francois.dugast@intel.com> <20231006095943.7-11-francois.dugast@intel.com> <871qdtdzuc.wl-ashutosh.dixit@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-xe] [PATCH v5 10/20] drm/xe: Remove XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE from uAPI X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Francois Dugast , intel-xe@lists.freedesktop.org Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Tue, 17 Oct 2023 09:02:42 -0700, Rodrigo Vivi wrote: > > On Tue, Oct 17, 2023 at 08:37:47AM -0700, Dixit, Ashutosh wrote: > > On Fri, 06 Oct 2023 02:59:33 -0700, Francois Dugast wrote: > > > > > > > Hi Francois, > > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > > index ad21ba1d6e0b..2a9e04024723 100644 > > > --- a/include/uapi/drm/xe_drm.h > > > +++ b/include/uapi/drm/xe_drm.h > > > @@ -781,21 +781,14 @@ struct drm_xe_exec_queue_set_property { > > > /** @exec_queue_id: Exec queue ID */ > > > __u32 exec_queue_id; > > > > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY 0 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY 0 > > > #define XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE 1 > > > #define XE_EXEC_QUEUE_SET_PROPERTY_PREEMPTION_TIMEOUT 2 > > > - /* > > > - * Long running or ULLS engine mode. DMA fences not allowed in this > > > - * mode. Must match the value of DRM_XE_VM_CREATE_COMPUTE_MODE, serves > > > - * as a sanity check the UMD knows what it is doing. Can only be set at > > > - * engine create time. > > > - */ > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE 3 > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE 4 > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT 5 > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER 6 > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY 7 > > > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY 8 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE 3 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT 4 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER 5 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY 6 > > > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY 7 > > > /** @property: property to set */ > > > __u32 property; > > > > > > /** @value: property value */ > > > __u64 value; > > > > Mostly a nit, but I had a question about this because I am facing a similar > > decision elsewhere. Why do we have one property/value pair in this struct > > when all the rest of the property/value pairs are coming in via the > > extensions? Basically, how about removing this property/value pair from > > thus struct and let all the property/value pairs come in only via > > extensions? I think that would both simplify the code a bit and be more > > consistent. Thoughts? > > or maybe the other way around and do not have extensions on the first > version of the api? How would that work? Remove all these properties in v1 and replace by a struct and only have any later changes via extenstions? That would be non-uniform but of course doable. I have an identical situation with some OA uapi too so wanted to get a sense of what is preferable. So that we follow a common approach and don't do things differently for different portions of the uapi. Thanks. > just thinking out loud, but no hard feelings for either way... > > > > > Thanks. > > -- > > Ashutosh