public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
To: Christian König <christian.koenig@amd.com>,
	intel-xe@lists.freedesktop.org
Cc: Felix Kuehling <Felix.Kuehling@amd.com>,
	Alex Deucher	 <alexander.deucher@amd.com>,
	David Airlie <airlied@gmail.com>,
	Simona Vetter	 <simona@ffwll.ch>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Danilo Krummrich <dakr@kernel.org>,
	 Matthew Brost <matthew.brost@intel.com>,
	Alice Ryhl <aliceryhl@google.com>,
	Rob Clark	 <robin.clark@oss.qualcomm.com>,
	Dmitry Baryshkov <lumag@kernel.org>,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	Jessica Zhang <jesszhan0024@gmail.com>,
	Sean Paul	 <sean@poorly.run>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	Natalie Vock	 <natalie.vock@gmx.de>
Subject: Re: [PATCH v2 0/4] drm/exec: drm_exec polishing
Date: Thu, 21 May 2026 13:45:35 +0200	[thread overview]
Message-ID: <9266d1f9615d942c2472f5b9846247a71ab62dc7.camel@linux.intel.com> (raw)
In-Reply-To: <da49f6c2-5b6e-4bd6-8940-84206b5913f8@amd.com>

Hi,

On Wed, 2026-05-20 at 13:34 +0200, Christian König wrote:
> On 5/20/26 12:16, Thomas Hellström wrote:
> > During the work towards enabling exhaustive eviction using full
> > ww locking in TTM, Christian indicated that the path for the
> > drm_exec moving forward was to be a full drm_exec helper with
> > things like userptr validation rather than a WW transaction
> > abstraction. The idea was then briefly discussed to craft a
> > WW transaction helper and then subclass that with drm_exec
> > with the idea that the WW transaction helper could be used in
> > TTM for eviction and for other uses that didn't mandate a full
> > exec sequence.
> 
> BTW Natalie is now trying to polish my TTM patches for doing that
> because some test case for the SteamDeck is running
>  into issues with pinning buffers for display.

Thanks for the review, Christian.

FWIW I did a POC implementation moving drm_exec as a transaction helper
to the dma-buf layer, and then re-implemented drm_exec on top as a
wrapper here:

https://gitlab.freedesktop.org/thomash/xe-vibe/-/commits/ww_transaction?ref_type=heads

Since this would work on any object with a refererence count and a
dma_resv, and even mixing them, it would solve a couple of issues:

1) Decouple the ttm object refcount issue from the TTM exhaustive
eviction (Not saying we shouldn't get rid of the ttm object refcount
anyway).

2) Implement any eviction locking modes (like snapshotting, object
release after slow-lock and similar) separate from drm_exec, and keep
exec as a pure exec-buf helper.

3) Facilitate ww locking in dma_buf_map().

4) Directly lock VMs instead of vm objects.

I haven't posted the patches yet since I wanted to get some feedback
first of whether this is a reasonable direction.

Thanks,
Thomas

> 
> > Regardless whether that actually happens or not, this series
> > aims to clean up abuses of drm_exec internals in drivers
> > so that future development of drm_exec isn't blocked by
> > such driver usage.
> 
> Yeah completely agree, going to review those patches now.
> 
> Regards,
> Christian.
> 
> > 
> > Except for patch 2 which is a small cleanup only.
> > 
> > v2:
> > - An drm/msm patch was already merged.
> > - Use a unique id for the loop variable (patch 1) (Christian)
> > - Move assignment to declaration (patch 2) (Christian)
> > - Declare the retry pointer as void *const. (patch 2)
> > - Only allow if the drm_exec context is newly initialized. (patch
> > 3)
> >   (Christian)
> > - Fix amdgpu compile error (patch 4) (Intel CI)
> > - Update the commit message (patch 4)
> > 
> > Thomas Hellström (4):
> >   drm/exec: Remove the index parameter from
> >     drm_exec_for_each_locked_obj[_reverse]
> >   drm/exec: Make the drm_exec_until_all_locked() macro more
> > readable
> >   drm/exec, drm/xe: Avoid abusing the drm_exec retry pointer
> >   drm/exec, drm/xe, drm/amdgpu: Add an accessor for struct
> >     drm_exec::ticket
> > 
> >  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        | 15 ++--
> >  .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  3 +-
> >  drivers/gpu/drm/drm_exec.c                    |  9 +--
> >  drivers/gpu/drm/drm_gpuvm.c                   |  3 +-
> >  drivers/gpu/drm/xe/xe_validation.c            |  4 +-
> >  drivers/gpu/drm/xe/xe_validation.h            |  2 +-
> >  drivers/gpu/drm/xe/xe_vm.c                    |  3 +-
> >  include/drm/drm_exec.h                        | 71 ++++++++++++++-
> > ----
> >  9 files changed, 70 insertions(+), 44 deletions(-)
> > 

  reply	other threads:[~2026-05-21 11:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-20 10:16 [PATCH v2 0/4] drm/exec: drm_exec polishing Thomas Hellström
2026-05-20 10:16 ` [PATCH v2 1/4] drm/exec: Remove the index parameter from drm_exec_for_each_locked_obj[_reverse] Thomas Hellström
2026-05-20 11:32   ` Christian König
2026-05-25 11:59   ` Claude review: " Claude Code Review Bot
2026-05-20 10:16 ` [PATCH v2 2/4] drm/exec: Make the drm_exec_until_all_locked() macro more readable Thomas Hellström
2026-05-20 11:35   ` Christian König
2026-05-25 11:59   ` Claude review: " Claude Code Review Bot
2026-05-20 10:16 ` [PATCH v2 3/4] drm/exec, drm/xe: Avoid abusing the drm_exec retry pointer Thomas Hellström
2026-05-20 11:39   ` Christian König
2026-05-25 11:59   ` Claude review: " Claude Code Review Bot
2026-05-20 10:16 ` [PATCH v2 4/4] drm/exec, drm/xe, drm/amdgpu: Add an accessor for struct drm_exec::ticket Thomas Hellström
2026-05-20 11:42   ` Christian König
2026-05-25 11:59   ` Claude review: " Claude Code Review Bot
2026-05-20 11:34 ` [PATCH v2 0/4] drm/exec: drm_exec polishing Christian König
2026-05-21 11:45   ` Thomas Hellström [this message]
2026-05-25 11:59 ` Claude review: " Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9266d1f9615d942c2472f5b9846247a71ab62dc7.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=aliceryhl@google.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jesszhan0024@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marijn.suijten@somainline.org \
    --cc=matthew.brost@intel.com \
    --cc=mripard@kernel.org \
    --cc=natalie.vock@gmx.de \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox