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 98FDFD11183 for ; Thu, 27 Nov 2025 10:34:08 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3693C10E81F; Thu, 27 Nov 2025 10:34:08 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="aDkLTVaD"; dkim-atps=neutral Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5C70210E825; Thu, 27 Nov 2025 10:34:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1764239645; bh=DHUeJy6RiFmbP61vJmBG5ZpHikoItVr2PujZhZ6Oa6I=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=aDkLTVaDbqdNjUwZGLcqcMRUsKcCTPGobPXaBfK1wqJx7O7sFgYKUDkU3X0vL+o5D CcE3UQzCAuESwDeodVwI7UIJpoTlSrYrBaL/6tKwWb/hy7xjbWZiR0SqK5q6Rlcj0U HGqtXTn1dZtTNSikPKQyOKzhYAxGJnYrGP/Vg1UEzXSm+I8M59bXO/M1/1AOTA8GJK T+6Y/HmgZzmNUJaZjCNc4QB3bZECacbhc77rxNMgY47VE1LffAJY04EQ+hQhg2V8KH NEJMSK/GPnASKPFhBJt9IHo/8Wao/yghRIUNYksS0x78yJJB1DesuOulWbfBVhPNOx 9uhVuyFyXVM6Q== Received: from fedora (unknown [IPv6:2a01:e0a:2c:6930:d919:a6e:5ea1:8a9f]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 0368F17E0360; Thu, 27 Nov 2025 11:34:04 +0100 (CET) Date: Thu, 27 Nov 2025 11:34:00 +0100 From: Boris Brezillon To: Thomas Zimmermann Cc: Steven Price , dri-devel@lists.freedesktop.org, Maarten Lankhorst , Maxime Ripard , David Airlie , Simona Vetter , Faith Ekstrand , Thierry Reding , Mikko Perttunen , Melissa Wen , =?UTF-8?B?TWHDrXJh?= Canal , Lucas De Marchi , Thomas =?UTF-8?B?SGVsbHN0csO2bQ==?= , Rodrigo Vivi , Frank Binns , Matt Coster , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , Alex Deucher , Christian =?UTF-8?B?S8O2bmln?= , amd-gfx@lists.freedesktop.org, kernel@collabora.com Subject: Re: [PATCH v6 01/16] drm/prime: Simplify life of drivers needing custom dma_buf_ops Message-ID: <20251127113400.0e522211@fedora> In-Reply-To: References: <20251126124455.3656651-1-boris.brezillon@collabora.com> <20251126124455.3656651-2-boris.brezillon@collabora.com> <2e789ff6-b79f-4577-bc69-f74dfed6acfa@suse.de> Organization: Collabora X-Mailer: Claws Mail 4.3.1 (GTK 3.24.51; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" On Thu, 27 Nov 2025 09:42:52 +0100 Thomas Zimmermann wrote: > Hi >=20 > Am 27.11.25 um 09:34 schrieb Thomas Zimmermann: > > Hi > > > > Am 26.11.25 um 13:44 schrieb Boris Brezillon: =20 > >> drm_gem_is_prime_exported_dma_buf() checks the dma_buf->ops against > >> drm_gem_prime_dmabuf_ops, which makes it impossible to use if the > >> driver implements custom dma_buf_ops. Instead of duplicating a bunch > >> of helpers to work around it, let's provide a way for drivers to > >> expose their custom dma_buf_ops so the core prime helpers can rely on > >> that instead of hardcoding &drm_gem_prime_dmabuf_ops. =20 > > > > This can't go in as-is. I've spent an awful amount of patches on=20 > > removing buffer callbacks from struct drm_driver. Let's please not go=20 > > back to that. > > =20 > >> > >> v5: > >> - New patch > >> > >> v6: > >> - Pass custom dma_buf_ops directly instead of through a getter > >> > >> Signed-off-by: Boris Brezillon > >> --- > >> =C2=A0 drivers/gpu/drm/drm_prime.c | 10 ++++++++-- > >> =C2=A0 include/drm/drm_drv.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2= =A0 8 ++++++++ > >> =C2=A0 2 files changed, 16 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > >> index 21809a82187b..86fd95f0c105 100644 > >> --- a/drivers/gpu/drm/drm_prime.c > >> +++ b/drivers/gpu/drm/drm_prime.c > >> @@ -904,6 +904,12 @@ unsigned long=20 > >> drm_prime_get_contiguous_size(struct sg_table *sgt) > >> =C2=A0 } > >> =C2=A0 EXPORT_SYMBOL(drm_prime_get_contiguous_size); > >> =C2=A0 +static const struct dma_buf_ops * > >> +drm_gem_prime_get_dma_buf_ops(struct drm_device *dev) > >> +{ > >> +=C2=A0=C2=A0=C2=A0 return dev->driver->dma_buf_ops ?: &drm_gem_prime_= dmabuf_ops; > >> +} > >> + > >> =C2=A0 /** > >> =C2=A0=C2=A0 * drm_gem_prime_export - helper library implementation of= the=20 > >> export callback > >> =C2=A0=C2=A0 * @obj: GEM object to export > >> @@ -920,7 +926,7 @@ struct dma_buf *drm_gem_prime_export(struct=20 > >> drm_gem_object *obj, > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct dma_buf_export_info exp_info =3D= { > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .exp_name =3D K= BUILD_MODNAME, /* white lie for debug */ > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .owner =3D dev-= >driver->fops->owner, > >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .ops =3D &drm_gem_prime_dm= abuf_ops, > >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .ops =3D drm_gem_prime_get= _dma_buf_ops(dev), =20 > > > > Rather provide a new function drm_gem_prime_export_with_ops() that=20 > > takes an additional dma_ops instance. The current=20 > > drm_gem_prime_export() would call it with &drm_gem_prime_dmabuf_ops. > > > > If this really does not work, you could add a pointer to dma_buf_ops=20 > > to drm_gem_object_funcs and fetch that from drm_gem_prime_export(). We= =20 > > already vm_ops there. > > > > Other drivers, such as amdgpu, would also benefit from such a change > > =20 > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .size =3D obj->= size, > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .flags =3D flag= s, > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .priv =3D obj, > >> @@ -947,7 +953,7 @@ bool drm_gem_is_prime_exported_dma_buf(struct=20 > >> drm_device *dev, > >> =C2=A0 { > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct drm_gem_object *obj =3D dma_buf-= >priv; > >> =C2=A0 -=C2=A0=C2=A0=C2=A0 return (dma_buf->ops =3D=3D &drm_gem_prime_= dmabuf_ops) &&=20 > >> (obj->dev =3D=3D dev); > >> +=C2=A0=C2=A0=C2=A0 return dma_buf->ops =3D=3D drm_gem_prime_get_dma_b= uf_ops(dev) &&=20 > >> obj->dev =3D=3D dev; =20 >=20 > On a second thought, we probably cannot be sure that dma_buf->priv=20 > really is a GEM object until we tested the ops field. :/=C2=A0 IIRC that'= s=20 > why the ops test goes first and the test for obj->dev goes second. So=20 > neither solution works. Hm, What do you mean by neither solution works? The original proposal never dereferences obj until it's sure it's a drm_gem_object, that's what dma_buf->ops =3D=3D drm_gem_prime_get_dma_buf_ops(dev) is for.