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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3FF7C433EF for ; Thu, 30 Sep 2021 09:11:35 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 368DB61439 for ; Thu, 30 Sep 2021 09:11:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 368DB61439 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=igalia.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 788038909C; Thu, 30 Sep 2021 09:11:34 +0000 (UTC) Received: from fanzine.igalia.com (fanzine.igalia.com [178.60.130.6]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8103889C3F for ; Thu, 30 Sep 2021 09:11:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:Cc:To:From:Subject:Message-ID; bh=PFWvrkSctZ6ypDPSfduYtJRRJk49WrcRXq5uvROqKpA=; b=AaA1xJP/hpWFFznBwrakpodc0I/ilCJw4YiU1e25q43TRO+p/YT+M8TsDwU8CUgewr2qTmYYJGPKLnatEsXz1NrmI4NUvwXmVZaXWmLiaQFAXT5Rk7m0VQOBwCiqLXuoOOaoyosXJPLLJmPvFjoKqaA95E9blzZvBzjvD4XGanR3SC28S8Z4xV+wQQnnFpasWWRU91UmxeTTMtSvePofN0cuolRiPsG97bgc7UtNsLkAdBoKUDAq6UIvuRT76FLHAXGRp8DS1iZKwB//Geoyv2Aq4aSSvkpY1hNYRbHGJBKx93SyzRVgB/XDS29hV5p29aZATm5DmZnG+Tja27m8cA==; Received: from 182.48.165.83.dynamic.reverse-mundo-r.com ([83.165.48.182] helo=zeus) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1mVs6F-0000ww-5d; Thu, 30 Sep 2021 11:11:31 +0200 Message-ID: Subject: Re: [PATCH v2 3/4] drm/v3d: add generic ioctl extension From: Iago Toral To: Melissa Wen , dri-devel@lists.freedesktop.org Cc: Emma Anholt , David Airlie , Daniel Vetter , Maxime Ripard , Boris Brezillon Date: Thu, 30 Sep 2021 11:11:21 +0200 In-Reply-To: <779e2cc57efd5fbc95f1267e7252de5f092045c5.1632905676.git.mwen@igalia.com> References: <779e2cc57efd5fbc95f1267e7252de5f092045c5.1632905676.git.mwen@igalia.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, 2021-09-29 at 10:44 +0100, Melissa Wen wrote: > Add support to attach generic extensions on job submission. This > patch > is third prep work to enable multiple syncobjs on job submission. > With > this work, when the job submission interface needs to be extended to > accomodate a new feature, we will use a generic extension struct > where > an id determines the data type to be pointed. The first application > is > to enable multiples in/out syncobj (next patch), but the base is > already done for future features. Therefore, to attach a new feature, > a specific extension struct should subclass drm_v3d_extension and > update the list of extensions in a job submission. > > v2: > - remove redundant elements to subclass struct (Daniel) > > Signed-off-by: Melissa Wen > --- > drivers/gpu/drm/v3d/v3d_drv.c | 4 +- > drivers/gpu/drm/v3d/v3d_gem.c | 71 > +++++++++++++++++++++++++++++++++-- > include/uapi/drm/v3d_drm.h | 31 +++++++++++++++ > 3 files changed, 100 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_drv.c > b/drivers/gpu/drm/v3d/v3d_drv.c > index c1deab2cf38d..3d6b9bcce2f7 100644 > --- a/drivers/gpu/drm/v3d/v3d_drv.c > +++ b/drivers/gpu/drm/v3d/v3d_drv.c > @@ -83,7 +83,6 @@ static int v3d_get_param_ioctl(struct drm_device > *dev, void *data, > return 0; > } > > - > switch (args->param) { > case DRM_V3D_PARAM_SUPPORTS_TFU: > args->value = 1; > @@ -147,7 +146,7 @@ v3d_postclose(struct drm_device *dev, struct > drm_file *file) > DEFINE_DRM_GEM_FOPS(v3d_drm_fops); > > /* DRM_AUTH is required on SUBMIT_CL for now, while we don't have > GMP > - * protection between clients. Note that render nodes would be be > + * protection between clients. Note that render nodes would be > * able to submit CLs that could access BOs from clients > authenticated > * with the master node. The TFU doesn't use the GMP, so it would > * need to stay DRM_AUTH until we do buffer size/offset validation. > @@ -219,7 +218,6 @@ static int v3d_platform_drm_probe(struct > platform_device *pdev) > u32 mmu_debug; > u32 ident1; > > - > v3d = devm_drm_dev_alloc(dev, &v3d_drm_driver, struct v3d_dev, > drm); > if (IS_ERR(v3d)) > return PTR_ERR(v3d); > diff --git a/drivers/gpu/drm/v3d/v3d_gem.c > b/drivers/gpu/drm/v3d/v3d_gem.c > index 9cfa6f8d4357..b912419027f7 100644 > --- a/drivers/gpu/drm/v3d/v3d_gem.c > +++ b/drivers/gpu/drm/v3d/v3d_gem.c > @@ -535,6 +535,33 @@ v3d_attach_fences_and_unlock_reservation(struct > drm_file *file_priv, > } > } > > +static int > +v3d_get_extensions(struct drm_file *file_priv, u64 ext_handles) > +{ > + struct drm_v3d_extension __user *user_ext; > + > + user_ext = u64_to_user_ptr(ext_handles); > + while(user_ext) { > + struct drm_v3d_extension ext; > + > + if (copy_from_user(&ext, user_ext, sizeof(ext))) { > + DRM_DEBUG("Failed to copy submit extension\n"); > + return -EFAULT; > + } > + > + switch (ext.id) { > + case 0: > + default: > + DRM_DEBUG_DRIVER("Unknown extension id: %d\n", > ext.id); > + return -EINVAL; > + } > + > + user_ext = u64_to_user_ptr(ext.next); > + } > + > + return 0; > +} > + > /** > * v3d_submit_cl_ioctl() - Submits a job (frame) to the V3D. > * @dev: DRM device > @@ -563,15 +590,24 @@ v3d_submit_cl_ioctl(struct drm_device *dev, > void *data, > > trace_v3d_submit_cl_ioctl(&v3d->drm, args->rcl_start, args- > >rcl_end); > > - if (args->pad != 0) > + if (args->pad) > return -EINVAL; > > - if (args->flags != 0 && > - args->flags != DRM_V3D_SUBMIT_CL_FLUSH_CACHE) { > + if (args->flags && > + args->flags & ~(DRM_V3D_SUBMIT_CL_FLUSH_CACHE | > + DRM_V3D_SUBMIT_EXTENSION)) { > DRM_INFO("invalid flags: %d\n", args->flags); > return -EINVAL; > } > > + if (args->flags & DRM_V3D_SUBMIT_EXTENSION) { > + ret = v3d_get_extensions(file_priv, args->extensions); > + if (ret) { > + DRM_DEBUG("Failed to get extensions.\n"); > + return ret; > + } > + } > + > ret = v3d_job_init(v3d, file_priv, (void *)&render, > sizeof(*render), > v3d_render_job_free, args->in_sync_bcl, > V3D_RENDER); > if (ret) > @@ -700,6 +736,19 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, > void *data, > > trace_v3d_submit_tfu_ioctl(&v3d->drm, args->iia); > > + if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) { > + DRM_DEBUG("invalid flags: %d\n", args->flags); > + return -EINVAL; > + } > + > + if (args->flags & DRM_V3D_SUBMIT_EXTENSION) { > + ret = v3d_get_extensions(file_priv, args->extensions); > + if (ret) { > + DRM_DEBUG("Failed to get extensions.\n"); > + return ret; > + } > + } > + > ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job), > v3d_job_free, args->in_sync, V3D_TFU); > if (ret) > @@ -784,11 +833,27 @@ v3d_submit_csd_ioctl(struct drm_device *dev, > void *data, > > trace_v3d_submit_csd_ioctl(&v3d->drm, args->cfg[5], args- > >cfg[6]); > > + if (args->pad) > + return -EINVAL; > + > if (!v3d_has_csd(v3d)) { > DRM_DEBUG("Attempting CSD submit on non-CSD > hardware\n"); > return -EINVAL; > } > > + if (args->flags && !(args->flags & DRM_V3D_SUBMIT_EXTENSION)) { > + DRM_INFO("invalid flags: %d\n", args->flags); > + return -EINVAL; > + } > + > + if (args->flags & DRM_V3D_SUBMIT_EXTENSION) { > + ret = v3d_get_extensions(file_priv, args->extensions); > + if (ret) { > + DRM_DEBUG("Failed to get extensions.\n"); > + return ret; > + } > + } > + > ret = v3d_job_init(v3d, file_priv, (void *)&job, sizeof(*job), > v3d_job_free, args->in_sync, V3D_CSD); > if (ret) > diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h > index 4104f22fb3d3..55b443ca6c0b 100644 > --- a/include/uapi/drm/v3d_drm.h > +++ b/include/uapi/drm/v3d_drm.h > @@ -58,6 +58,20 @@ extern "C" { > struct > drm_v3d_perfmon_get_values) > > #define DRM_V3D_SUBMIT_CL_FLUSH_CACHE 0x01 > +#define DRM_V3D_SUBMIT_EXTENSION 0x02 > + > +/* struct drm_v3d_extension - ioctl extensions > + * > + * Linked-list of generic extensions where the id identify which > struct is > + * pointed by ext_data. Therefore, DRM_V3D_EXT_ID_* is used on id to > identify > + * the extension type. > + */ > +struct drm_v3d_extension { > + __u64 next; > + __u32 id; > +#define DRM_V3D_EXT_ID_MULTI_SYNC 0x01 > + __u32 flags; /* mbz */ > +}; > > /** > * struct drm_v3d_submit_cl - ioctl argument for submitting commands > to the 3D > @@ -135,12 +149,16 @@ struct drm_v3d_submit_cl { > /* Number of BO handles passed in (size is that times 4). */ > __u32 bo_handle_count; > > + /* DRM_V3D_SUBMIT_* properties */ > __u32 flags; > > /* ID of the perfmon to attach to this job. 0 means no perfmon. > */ > __u32 perfmon_id; > > __u32 pad; > + > + /* Pointer to an array of ioctl extensions*/ > + __u64 extensions; > }; > > /** > @@ -248,6 +266,12 @@ struct drm_v3d_submit_tfu { > __u32 in_sync; > /* Sync object to signal when the TFU job is done. */ > __u32 out_sync; > + > + __u32 flags; > + > + /* Pointer to an array of ioctl extensions*/ > + __u64 extensions; We want __u64 fields aligned to 64-bit so we should swap the positions of flags and extensions. > + > }; > > /* Submits a compute shader for dispatch. This job will block on > any > @@ -276,6 +300,13 @@ struct drm_v3d_submit_csd { > > /* ID of the perfmon to attach to this job. 0 means no perfmon. > */ > __u32 perfmon_id; > + > + /* Pointer to an array of ioctl extensions*/ > + __u64 extensions; > + > + __u32 flags; > + > + __u32 pad; > }; > > enum {