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 35F44C43458 for ; Mon, 13 Jul 2026 11:57:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB41810E934; Mon, 13 Jul 2026 11:57:53 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="hpiGrhQT"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.14]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2D46B10E92E; Mon, 13 Jul 2026 11:57:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1783943873; x=1815479873; h=message-id:subject:from:to:date:in-reply-to:references: content-transfer-encoding:mime-version; bh=cfUXpxVp+7cu4pl6gIthdexAErydheekjCls546oHYg=; b=hpiGrhQTq54RFfEIxUaCBY6mYuQZSihVUU1sA0T3vCBEYtv9LTouy52z 7uCxSKCturyAe7rZ8Ubd0Kl/jL2U06PGpvTwUT3Zwx5AgwnZsIv0v2OsI 3tNB5Vw7VkXeiFgX2uciKBRaTau63i6C3YuBP8p5TLPjV2JmMo+eobrsb BICAoWf33/xmigPzZ1b2Lj0YM5JKd/XP0SP++TL7SMVYHO8kUDUxsbrVU InXpdOP1PEWXjxsdh37+NQoTCcBodQXLtovHPxPOoUcdYoyADhY/+Wp3I 3Vlj6hJKFqOtljMz5m/hrGG1ejqDxW9U5OtiGtgNCcoYkOagV78sEKRgS A==; X-CSE-ConnectionGUID: rxDPvbwzS32al3ZLbT54xA== X-CSE-MsgGUID: sl4uMp1LQq+wodt1vX3CLQ== X-IronPort-AV: E=McAfee;i="6800,10657,11841"; a="88454027" X-IronPort-AV: E=Sophos;i="6.25,154,1779174000"; d="scan'208";a="88454027" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jul 2026 04:57:52 -0700 X-CSE-ConnectionGUID: /ixp9IyJSH2NFzO+DeuKBg== X-CSE-MsgGUID: lloBdu5TQF6BU8rf5JZBUg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,154,1779174000"; d="scan'208";a="253796075" Received: from mkosciow-mobl1.ger.corp.intel.com (HELO [10.245.245.123]) ([10.245.245.123]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jul 2026 04:57:49 -0700 Message-ID: <0cfc1913440ce8f19f715f5df7faa23dc1164755.camel@linux.intel.com> Subject: Re: [PATCH 10/12] drm/exec: add drm_exec_lock_resv function From: Thomas =?ISO-8859-1?Q?Hellstr=F6m?= To: christian.koenig@amd.com, dakr@kernel.org, ecourtney@nvidia.com, simona@ffwll.ch, matthew.brost@intel.com, nat@pixelcluster.dev, airlied@gmail.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Date: Mon, 13 Jul 2026 13:57:46 +0200 In-Reply-To: <20260710190752.2355-11-christian.koenig@amd.com> References: <20260710190752.2355-1-christian.koenig@amd.com> <20260710190752.2355-11-christian.koenig@amd.com> Organization: Intel Sweden AB, Registration Number: 556189-6027 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Fri, 2026-07-10 at 20:52 +0200, Christian K=C3=B6nig wrote: > Restructure the drm_exec object to work with dma_resv references > instead > of GEM object references. >=20 > Add the new function dma_exec_lock_resv() to lock individual dma_resv > objects and so allow higher level implementations to handle > contention > purely on dma_resv objects. >=20 > WIP! Don't commit like that! >=20 > Signed-off-by: Christian K=C3=B6nig Here, IMO we should move a dma-resv based implementation to dma-buf to facilitate passing it also through dma_buf_map(), And if wanting to avoid rewriting all users of drm_exec, Make drm_exec a thin wrapper on top. Thanks, Thomas > --- > =C2=A0drivers/gpu/drm/drm_exec.c | 75 ++++++++++++++++++++++-------------= - > -- > =C2=A0drivers/gpu/drm/drm_gem.c=C2=A0 |=C2=A0 2 + > =C2=A0include/drm/drm_exec.h=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 9 +++-- > =C2=A03 files changed, 50 insertions(+), 36 deletions(-) >=20 > diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c > index fa923852fae45..382bf7bcd5ff3 100644 > --- a/drivers/gpu/drm/drm_exec.c > +++ b/drivers/gpu/drm/drm_exec.c > @@ -58,8 +58,11 @@ static void drm_exec_unlock_all(struct drm_exec > *exec) > =C2=A0 drm_gem_object_put(obj); > =C2=A0 } > =C2=A0 > - drm_gem_object_put(exec->prelocked); > - exec->prelocked =3D NULL; > + if (exec->prelocked) { > + dma_resv_unlock(exec->prelocked); > + dma_resv_put(exec->prelocked); > + exec->prelocked =3D NULL; > + } > =C2=A0} > =C2=A0 > =C2=A0/** > @@ -101,7 +104,7 @@ void drm_exec_fini(struct drm_exec *exec) > =C2=A0 drm_exec_unlock_all(exec); > =C2=A0 kvfree(exec->objects); > =C2=A0 if (exec->contended !=3D DRM_EXEC_DUMMY) { > - drm_gem_object_put(exec->contended); > + dma_resv_put(exec->contended); > =C2=A0 ww_acquire_fini(&exec->ticket); > =C2=A0 } > =C2=A0} > @@ -158,50 +161,41 @@ static int drm_exec_obj_locked(struct drm_exec > *exec, > =C2=A0/* Make sure the contended object is locked first */ > =C2=A0static int drm_exec_lock_contended(struct drm_exec *exec) > =C2=A0{ > - struct drm_gem_object *obj =3D exec->contended; > + struct dma_resv *resv =3D exec->contended; > =C2=A0 int ret; > =C2=A0 > - if (likely(!obj)) > + if (likely(!resv)) > =C2=A0 return 0; > =C2=A0 > =C2=A0 /* Always cleanup the contention so that error handling can > kick in */ > =C2=A0 exec->contended =3D NULL; > =C2=A0 if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) { > - ret =3D dma_resv_lock_slow_interruptible(obj->resv, > - =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 &exec- > >ticket); > + ret =3D dma_resv_lock_slow_interruptible(resv, &exec- > >ticket); > =C2=A0 if (unlikely(ret)) > =C2=A0 goto error_dropref; > =C2=A0 } else { > - dma_resv_lock_slow(obj->resv, &exec->ticket); > + dma_resv_lock_slow(resv, &exec->ticket); > =C2=A0 } > =C2=A0 > - ret =3D drm_exec_obj_locked(exec, obj); > - if (unlikely(ret)) > - goto error_unlock; > - > - exec->prelocked =3D obj; > + exec->prelocked =3D resv; > =C2=A0 return 0; > =C2=A0 > -error_unlock: > - dma_resv_unlock(obj->resv); > - > =C2=A0error_dropref: > - drm_gem_object_put(obj); > + dma_resv_put(resv); > =C2=A0 return ret; > =C2=A0} > =C2=A0 > =C2=A0/** > - * drm_exec_lock_obj - lock a GEM object for use > + * drm_exec_lock_resv - lock a dma_resv object > =C2=A0 * @exec: the drm_exec object with the state > - * @obj: the GEM object to lock > + * @resv: the dma_resv object to lock > =C2=A0 * > - * Lock a GEM object for use and grab a reference to it. > + * Lock a dma_resv object for use or grab a reference to it on > contention. > =C2=A0 * > =C2=A0 * Returns: -EDEADLK if a contention is detected, -EALREADY when > object is > - * already locked (can be suppressed by setting the > DRM_EXEC_IGNORE_DUPLICATES > - * flag), -ENOMEM when memory allocation failed and zero for > success. > + * already locked, -ENOMEM when memory allocation failed and zero > for success. > =C2=A0 */ > -int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object > *obj) > +int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv *resv) > =C2=A0{ > =C2=A0 int ret; > =C2=A0 > @@ -209,22 +203,39 @@ int drm_exec_lock_obj(struct drm_exec *exec, > struct drm_gem_object *obj) > =C2=A0 if (unlikely(ret)) > =C2=A0 return ret; > =C2=A0 > - if (exec->prelocked =3D=3D obj) { > - drm_gem_object_put(exec->prelocked); > + if (exec->prelocked =3D=3D resv) { > + dma_resv_put(exec->prelocked); > =C2=A0 exec->prelocked =3D NULL; > =C2=A0 return 0; > =C2=A0 } > =C2=A0 > =C2=A0 if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) > - ret =3D dma_resv_lock_interruptible(obj->resv, &exec- > >ticket); > + ret =3D dma_resv_lock_interruptible(resv, &exec- > >ticket); > =C2=A0 else > - ret =3D dma_resv_lock(obj->resv, &exec->ticket); > + ret =3D dma_resv_lock(resv, &exec->ticket); > =C2=A0 > - if (unlikely(ret =3D=3D -EDEADLK)) { > - drm_gem_object_get(obj); > - exec->contended =3D obj; > - return -EDEADLK; > - } > + if (unlikely(ret =3D=3D -EDEADLK)) > + exec->contended =3D dma_resv_get(resv); > + return ret; > +} > +EXPORT_SYMBOL(drm_exec_lock_resv); > + > +/** > + * drm_exec_lock_obj - lock a GEM object for use > + * @exec: the drm_exec object with the state > + * @obj: the GEM object to lock > + * > + * Lock a GEM object for use and grab a reference to it. > + * > + * Returns: -EDEADLK if a contention is detected, -EALREADY when > object is > + * already locked (can be suppressed by setting the > DRM_EXEC_IGNORE_DUPLICATES > + * flag), -ENOMEM when memory allocation failed and zero for > success. > + */ > +int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object > *obj) > +{ > + int ret; > + > + ret =3D drm_exec_lock_resv(exec, obj->resv); > =C2=A0 > =C2=A0 if (unlikely(ret =3D=3D -EALREADY) && > =C2=A0 =C2=A0=C2=A0=C2=A0 exec->flags & DRM_EXEC_IGNORE_DUPLICATES) > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index bbcbd25f014f0..f5cf9ad596a67 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -229,6 +229,8 @@ void drm_gem_private_object_init(struct > drm_device *dev, > =C2=A0 obj->size =3D size; > =C2=A0 mutex_init(&obj->gpuva.lock); > =C2=A0 dma_resv_init(&obj->_resv); > + > + /* TODO: This needs to go away for drm_exec to work > correctly!!! */ > =C2=A0 if (!obj->resv) > =C2=A0 obj->resv =3D dma_resv_get(&obj->_resv); > =C2=A0 > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h > index 8725ba92ff916..9daedb676d7b1 100644 > --- a/include/drm/drm_exec.h > +++ b/include/drm/drm_exec.h > @@ -47,14 +47,14 @@ struct drm_exec { > =C2=A0 struct drm_gem_object **objects; > =C2=A0 > =C2=A0 /** > - * @contended: contended GEM object we backed off for > + * @contended: contended dma_resv object we backed off for > =C2=A0 */ > - struct drm_gem_object *contended; > + struct dma_resv *contended; > =C2=A0 > =C2=A0 /** > - * @prelocked: already locked GEM object due to contention > + * @prelocked: already locked dma_resv object due to > contention > =C2=A0 */ > - struct drm_gem_object *prelocked; > + struct dma_resv *prelocked; > =C2=A0}; > =C2=A0 > =C2=A0/** > @@ -175,6 +175,7 @@ static inline struct ww_acquire_ctx > *drm_exec_ticket(struct drm_exec *exec) > =C2=A0void drm_exec_init(struct drm_exec *exec, u32 flags, unsigned nr); > =C2=A0void drm_exec_fini(struct drm_exec *exec); > =C2=A0bool drm_exec_cleanup(struct drm_exec *exec); > +int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv > *resv); > =C2=A0int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object > *obj); > =C2=A0void drm_exec_unlock_obj(struct drm_exec *exec, struct > drm_gem_object *obj); > =C2=A0int drm_exec_prepare_obj(struct drm_exec *exec, struct > drm_gem_object *obj,