From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 6E6FD16C850; Mon, 14 Apr 2025 10:01:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744624892; cv=none; b=OTYkOrmMBP93oINmjKq3DBCtO3mUWifx31cb8w4DhOPjMmA8F3p68dnLzA1c415thvwlPbItoQEKu2pDc4lhZznmAHlvPII9qHiZBZFOJUZsCZtsJZfdepqBZ0+ZSxxLlgZZLUIWgF2INRbOR8PWaXTiD1GjDCV3PsMNuAngdGg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744624892; c=relaxed/simple; bh=4cs1i9ZIHdoVtdyynUgKR2MbNdufCRR7516/JLREVsM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=DcwgCLDn7qHiQVYXRf0PzfKd0C1AapXF4sNTU5NoZhhf3ZoDpLfu0P9Ns3bjco0E5BMahFhR13D403tNLNGH0M/m/A9noSingpxpG/KeiE+w85vKljsauLYIqp4AYla1dR8KCcQ4zeTrz5acXOEHfYAk44js5YouzxxPx3MAijs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 21C3C1007; Mon, 14 Apr 2025 03:01:27 -0700 (PDT) Received: from [10.57.87.24] (unknown [10.57.87.24]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 44AC93F694; Mon, 14 Apr 2025 03:01:24 -0700 (PDT) Message-ID: <6d67aff0-7082-4966-acb2-d7985820b3ea@arm.com> Date: Mon, 14 Apr 2025 11:01:22 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 2/4] drm/panthor: Add driver IOCTL for setting BO labels To: =?UTF-8?Q?Adri=C3=A1n_Larumbe?= , "To : Boris Brezillon" , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org References: <20250411150357.3308921-1-adrian.larumbe@collabora.com> <20250411150357.3308921-3-adrian.larumbe@collabora.com> From: Steven Price Content-Language: en-GB In-Reply-To: <20250411150357.3308921-3-adrian.larumbe@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 11/04/2025 16:03, Adrián Larumbe wrote: > Allow UM to label a BO for which it possesses a DRM handle. > > Signed-off-by: Adrián Larumbe > Reviewed-by: Liviu Dudau > Reviewed-by: Boris Brezillon Reviewed-by: Steven Price Although very minor NITs below which you can consider. > --- > drivers/gpu/drm/panthor/panthor_drv.c | 42 ++++++++++++++++++++++++++- > drivers/gpu/drm/panthor/panthor_gem.h | 2 ++ > include/uapi/drm/panthor_drm.h | 23 +++++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c > index 06fe46e32073..983b24f1236c 100644 > --- a/drivers/gpu/drm/panthor/panthor_drv.c > +++ b/drivers/gpu/drm/panthor/panthor_drv.c > @@ -1331,6 +1331,44 @@ static int panthor_ioctl_vm_get_state(struct drm_device *ddev, void *data, > return 0; > } > > +static int panthor_ioctl_bo_set_label(struct drm_device *ddev, void *data, > + struct drm_file *file) > +{ > + struct drm_panthor_bo_set_label *args = data; > + struct drm_gem_object *obj; > + const char *label; > + int ret = 0; > + > + obj = drm_gem_object_lookup(file, args->handle); > + if (!obj) > + return -ENOENT; > + > + if (args->size && args->label) { > + if (args->size > PANTHOR_BO_LABEL_MAXLEN) { > + ret = -E2BIG; > + goto err_label; > + } > + > + label = strndup_user(u64_to_user_ptr(args->label), args->size); > + if (IS_ERR(label)) { > + ret = PTR_ERR(label); > + goto err_label; > + } > + } else if (args->size && !args->label) { > + ret = -EINVAL; > + goto err_label; > + } else { > + label = NULL; > + } > + > + panthor_gem_bo_set_label(obj, label); > + > +err_label: > + drm_gem_object_put(obj); > + > + return ret; > +} > + > static int > panthor_open(struct drm_device *ddev, struct drm_file *file) > { > @@ -1400,6 +1438,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = { > PANTHOR_IOCTL(TILER_HEAP_CREATE, tiler_heap_create, DRM_RENDER_ALLOW), > PANTHOR_IOCTL(TILER_HEAP_DESTROY, tiler_heap_destroy, DRM_RENDER_ALLOW), > PANTHOR_IOCTL(GROUP_SUBMIT, group_submit, DRM_RENDER_ALLOW), > + PANTHOR_IOCTL(BO_SET_LABEL, bo_set_label, DRM_RENDER_ALLOW), > }; > > static int panthor_mmap(struct file *filp, struct vm_area_struct *vma) > @@ -1509,6 +1548,7 @@ static void panthor_debugfs_init(struct drm_minor *minor) > * - 1.2 - adds DEV_QUERY_GROUP_PRIORITIES_INFO query > * - adds PANTHOR_GROUP_PRIORITY_REALTIME priority > * - 1.3 - adds DRM_PANTHOR_GROUP_STATE_INNOCENT flag > + * - 1.4 - adds DRM_IOCTL_PANTHOR_BO_SET_LABEL ioctl > */ > static const struct drm_driver panthor_drm_driver = { > .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ | > @@ -1522,7 +1562,7 @@ static const struct drm_driver panthor_drm_driver = { > .name = "panthor", > .desc = "Panthor DRM driver", > .major = 1, > - .minor = 3, > + .minor = 4, > > .gem_create_object = panthor_gem_create_object, > .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, > diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h > index af0d77338860..beba066b4974 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.h > +++ b/drivers/gpu/drm/panthor/panthor_gem.h > @@ -13,6 +13,8 @@ > > struct panthor_vm; > > +#define PANTHOR_BO_LABEL_MAXLEN PAGE_SIZE > + > /** > * struct panthor_gem_object - Driver specific GEM object. > */ > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > index 97e2c4510e69..12b1994499a9 100644 > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h > @@ -127,6 +127,9 @@ enum drm_panthor_ioctl_id { > > /** @DRM_PANTHOR_TILER_HEAP_DESTROY: Destroy a tiler heap. */ > DRM_PANTHOR_TILER_HEAP_DESTROY, > + > + /** @DRM_PANTHOR_BO_SET_LABEL: Label a BO. */ > + DRM_PANTHOR_BO_SET_LABEL, > }; > > /** > @@ -977,6 +980,24 @@ struct drm_panthor_tiler_heap_destroy { > __u32 pad; > }; > > +/** > + * struct drm_panthor_bo_set_label - Arguments passed to DRM_IOCTL_PANTHOR_BO_SET_LABEL > + */ > +struct drm_panthor_bo_set_label { > + /** @handle: Handle of the buffer object to label. */ > + __u32 handle; > + > + /** > + * @size: Length of the label, including the NULL terminator. > + * > + * Cannot be greater than the OS page size. > + */ > + __u32 size; > + > + /** @label: User pointer to a NULL-terminated string */ > + __u64 label; > +}; First very minor NIT: * NULL is a pointer, i.e. (void*)0 * NUL is the ASCII code point '\0'. So it's a NUL-terminated string. Second NIT: We don't actually need 'size' - since the string is NUL-terminated we can just strndup_user(__user_pointer__, PAGE_SIZE). As things stand we validate that strlen(label) < size <= PAGE_SIZE - which is a little odd (user space might as well just pass PAGE_SIZE rather than calculate the actual length). Thanks, Steve > + > /** > * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number > * @__access: Access type. Must be R, W or RW. > @@ -1019,6 +1040,8 @@ enum { > DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create), > DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY = > DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy), > + DRM_IOCTL_PANTHOR_BO_SET_LABEL = > + DRM_IOCTL_PANTHOR(WR, BO_SET_LABEL, bo_set_label), > }; > > #if defined(__cplusplus)