From: Matthew Brost <matthew.brost@intel.com>
To: Dave Airlie <airlied@gmail.com>
Cc: intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
"Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Subject: Re: [PATCH] drm/xe: don't store the xe device pointer inside xe_ttm_tt
Date: Wed, 4 Jun 2025 15:50:36 -0700 [thread overview]
Message-ID: <aEDNvCUTUdxgluzd@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <CAPM=9tzGqNQhGgZj-yNRcM751bR8bF4yigX1i26iH8pY28m5sA@mail.gmail.com>
On Thu, Jun 05, 2025 at 08:35:40AM +1000, Dave Airlie wrote:
> Just realised I hadn't cc'ed some people.
>
> ping,
>
> Dave.
>
> On Thu, 22 May 2025 at 05:48, Dave Airlie <airlied@gmail.com> wrote:
> >
> > From: Dave Airlie <airlied@redhat.com>
> >
> > This device pointer is nearly always available without storing
> > an extra copy for each tt in the system.
> >
> > Just noticed this while reading over the xe shrinker code.
> >
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
I'm not seeing a CI run for this on patchworks. Can you resend intel-xe?
You should have permission to trigger our CI. Assuming CI is clean, will
merge this one for you through drm-xe-next tomorrow.
Matt
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > ---
> > drivers/gpu/drm/xe/tests/xe_bo.c | 4 +--
> > drivers/gpu/drm/xe/xe_bo.c | 59 ++++++++++++++++----------------
> > 2 files changed, 32 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/tests/xe_bo.c b/drivers/gpu/drm/xe/tests/xe_bo.c
> > index 378dcd0fb414..77ca1ab527ec 100644
> > --- a/drivers/gpu/drm/xe/tests/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/tests/xe_bo.c
> > @@ -514,9 +514,9 @@ static int shrink_test_run_device(struct xe_device *xe)
> > * other way around, they may not be subject to swapping...
> > */
> > if (alloced < purgeable) {
> > - xe_ttm_tt_account_subtract(&xe_tt->ttm);
> > + xe_ttm_tt_account_subtract(xe, &xe_tt->ttm);
> > xe_tt->purgeable = true;
> > - xe_ttm_tt_account_add(&xe_tt->ttm);
> > + xe_ttm_tt_account_add(xe, &xe_tt->ttm);
> > bo->ttm.priority = 0;
> > spin_lock(&bo->ttm.bdev->lru_lock);
> > ttm_bo_move_to_lru_tail(&bo->ttm);
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index d99d91fe8aa9..4074e6f64fd0 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -336,15 +336,13 @@ static void xe_evict_flags(struct ttm_buffer_object *tbo,
> > /* struct xe_ttm_tt - Subclassed ttm_tt for xe */
> > struct xe_ttm_tt {
> > struct ttm_tt ttm;
> > - /** @xe - The xe device */
> > - struct xe_device *xe;
> > struct sg_table sgt;
> > struct sg_table *sg;
> > /** @purgeable: Whether the content of the pages of @ttm is purgeable. */
> > bool purgeable;
> > };
> >
> > -static int xe_tt_map_sg(struct ttm_tt *tt)
> > +static int xe_tt_map_sg(struct xe_device *xe, struct ttm_tt *tt)
> > {
> > struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> > unsigned long num_pages = tt->num_pages;
> > @@ -359,13 +357,13 @@ static int xe_tt_map_sg(struct ttm_tt *tt)
> > ret = sg_alloc_table_from_pages_segment(&xe_tt->sgt, tt->pages,
> > num_pages, 0,
> > (u64)num_pages << PAGE_SHIFT,
> > - xe_sg_segment_size(xe_tt->xe->drm.dev),
> > + xe_sg_segment_size(xe->drm.dev),
> > GFP_KERNEL);
> > if (ret)
> > return ret;
> >
> > xe_tt->sg = &xe_tt->sgt;
> > - ret = dma_map_sgtable(xe_tt->xe->drm.dev, xe_tt->sg, DMA_BIDIRECTIONAL,
> > + ret = dma_map_sgtable(xe->drm.dev, xe_tt->sg, DMA_BIDIRECTIONAL,
> > DMA_ATTR_SKIP_CPU_SYNC);
> > if (ret) {
> > sg_free_table(xe_tt->sg);
> > @@ -376,12 +374,12 @@ static int xe_tt_map_sg(struct ttm_tt *tt)
> > return 0;
> > }
> >
> > -static void xe_tt_unmap_sg(struct ttm_tt *tt)
> > +static void xe_tt_unmap_sg(struct xe_device *xe, struct ttm_tt *tt)
> > {
> > struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> >
> > if (xe_tt->sg) {
> > - dma_unmap_sgtable(xe_tt->xe->drm.dev, xe_tt->sg,
> > + dma_unmap_sgtable(xe->drm.dev, xe_tt->sg,
> > DMA_BIDIRECTIONAL, 0);
> > sg_free_table(xe_tt->sg);
> > xe_tt->sg = NULL;
> > @@ -400,24 +398,24 @@ struct sg_table *xe_bo_sg(struct xe_bo *bo)
> > * Account ttm pages against the device shrinker's shrinkable and
> > * purgeable counts.
> > */
> > -static void xe_ttm_tt_account_add(struct ttm_tt *tt)
> > +static void xe_ttm_tt_account_add(struct xe_device *xe, struct ttm_tt *tt)
> > {
> > struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> >
> > if (xe_tt->purgeable)
> > - xe_shrinker_mod_pages(xe_tt->xe->mem.shrinker, 0, tt->num_pages);
> > + xe_shrinker_mod_pages(xe->mem.shrinker, 0, tt->num_pages);
> > else
> > - xe_shrinker_mod_pages(xe_tt->xe->mem.shrinker, tt->num_pages, 0);
> > + xe_shrinker_mod_pages(xe->mem.shrinker, tt->num_pages, 0);
> > }
> >
> > -static void xe_ttm_tt_account_subtract(struct ttm_tt *tt)
> > +static void xe_ttm_tt_account_subtract(struct xe_device *xe, struct ttm_tt *tt)
> > {
> > struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> >
> > if (xe_tt->purgeable)
> > - xe_shrinker_mod_pages(xe_tt->xe->mem.shrinker, 0, -(long)tt->num_pages);
> > + xe_shrinker_mod_pages(xe->mem.shrinker, 0, -(long)tt->num_pages);
> > else
> > - xe_shrinker_mod_pages(xe_tt->xe->mem.shrinker, -(long)tt->num_pages, 0);
> > + xe_shrinker_mod_pages(xe->mem.shrinker, -(long)tt->num_pages, 0);
> > }
> >
> > static struct ttm_tt *xe_ttm_tt_create(struct ttm_buffer_object *ttm_bo,
> > @@ -436,7 +434,6 @@ static struct ttm_tt *xe_ttm_tt_create(struct ttm_buffer_object *ttm_bo,
> > return NULL;
> >
> > tt = &xe_tt->ttm;
> > - xe_tt->xe = xe;
> >
> > extra_pages = 0;
> > if (xe_bo_needs_ccs_pages(bo))
> > @@ -527,21 +524,23 @@ static int xe_ttm_tt_populate(struct ttm_device *ttm_dev, struct ttm_tt *tt,
> > return err;
> >
> > xe_tt->purgeable = false;
> > - xe_ttm_tt_account_add(tt);
> > + xe_ttm_tt_account_add(ttm_to_xe_device(ttm_dev), tt);
> >
> > return 0;
> > }
> >
> > static void xe_ttm_tt_unpopulate(struct ttm_device *ttm_dev, struct ttm_tt *tt)
> > {
> > + struct xe_device *xe = ttm_to_xe_device(ttm_dev);
> > +
> > if ((tt->page_flags & TTM_TT_FLAG_EXTERNAL) &&
> > !(tt->page_flags & TTM_TT_FLAG_EXTERNAL_MAPPABLE))
> > return;
> >
> > - xe_tt_unmap_sg(tt);
> > + xe_tt_unmap_sg(xe, tt);
> >
> > ttm_pool_free(&ttm_dev->pool, tt);
> > - xe_ttm_tt_account_subtract(tt);
> > + xe_ttm_tt_account_subtract(xe, tt);
> > }
> >
> > static void xe_ttm_tt_destroy(struct ttm_device *ttm_dev, struct ttm_tt *tt)
> > @@ -789,7 +788,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
> > /* Bo creation path, moving to system or TT. */
> > if ((!old_mem && ttm) && !handle_system_ccs) {
> > if (new_mem->mem_type == XE_PL_TT)
> > - ret = xe_tt_map_sg(ttm);
> > + ret = xe_tt_map_sg(xe, ttm);
> > if (!ret)
> > ttm_bo_move_null(ttm_bo, new_mem);
> > goto out;
> > @@ -812,7 +811,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
> > (!ttm && ttm_bo->type == ttm_bo_type_device);
> >
> > if (new_mem->mem_type == XE_PL_TT) {
> > - ret = xe_tt_map_sg(ttm);
> > + ret = xe_tt_map_sg(xe, ttm);
> > if (ret)
> > goto out;
> > }
> > @@ -973,7 +972,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
> > if (timeout < 0)
> > ret = timeout;
> >
> > - xe_tt_unmap_sg(ttm_bo->ttm);
> > + xe_tt_unmap_sg(xe, ttm_bo->ttm);
> > }
> >
> > return ret;
> > @@ -983,6 +982,7 @@ static long xe_bo_shrink_purge(struct ttm_operation_ctx *ctx,
> > struct ttm_buffer_object *bo,
> > unsigned long *scanned)
> > {
> > + struct xe_device *xe = ttm_to_xe_device(bo->bdev);
> > long lret;
> >
> > /* Fake move to system, without copying data. */
> > @@ -997,7 +997,7 @@ static long xe_bo_shrink_purge(struct ttm_operation_ctx *ctx,
> > if (lret)
> > return lret;
> >
> > - xe_tt_unmap_sg(bo->ttm);
> > + xe_tt_unmap_sg(xe, bo->ttm);
> > ttm_bo_move_null(bo, new_resource);
> > }
> >
> > @@ -1008,7 +1008,7 @@ static long xe_bo_shrink_purge(struct ttm_operation_ctx *ctx,
> > .allow_move = false});
> >
> > if (lret > 0)
> > - xe_ttm_tt_account_subtract(bo->ttm);
> > + xe_ttm_tt_account_subtract(xe, bo->ttm);
> >
> > return lret;
> > }
> > @@ -1039,7 +1039,7 @@ long xe_bo_shrink(struct ttm_operation_ctx *ctx, struct ttm_buffer_object *bo,
> > struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> > struct ttm_place place = {.mem_type = bo->resource->mem_type};
> > struct xe_bo *xe_bo = ttm_to_xe_bo(bo);
> > - struct xe_device *xe = xe_tt->xe;
> > + struct xe_device *xe = ttm_to_xe_device(bo->bdev);
> > bool needs_rpm;
> > long lret = 0L;
> >
> > @@ -1076,7 +1076,7 @@ long xe_bo_shrink(struct ttm_operation_ctx *ctx, struct ttm_buffer_object *bo,
> > xe_pm_runtime_put(xe);
> >
> > if (lret > 0)
> > - xe_ttm_tt_account_subtract(tt);
> > + xe_ttm_tt_account_subtract(xe, tt);
> >
> > out_unref:
> > xe_bo_put(xe_bo);
> > @@ -1377,7 +1377,8 @@ int xe_bo_dma_unmap_pinned(struct xe_bo *bo)
> > ttm_bo->sg = NULL;
> > xe_tt->sg = NULL;
> > } else if (xe_tt->sg) {
> > - dma_unmap_sgtable(xe_tt->xe->drm.dev, xe_tt->sg,
> > + dma_unmap_sgtable(ttm_to_xe_device(ttm_bo->bdev)->drm.dev,
> > + xe_tt->sg,
> > DMA_BIDIRECTIONAL, 0);
> > sg_free_table(xe_tt->sg);
> > xe_tt->sg = NULL;
> > @@ -2289,7 +2290,7 @@ int xe_bo_pin_external(struct xe_bo *bo)
> >
> > ttm_bo_pin(&bo->ttm);
> > if (bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
> > - xe_ttm_tt_account_subtract(bo->ttm.ttm);
> > + xe_ttm_tt_account_subtract(xe, bo->ttm.ttm);
> >
> > /*
> > * FIXME: If we always use the reserve / unreserve functions for locking
> > @@ -2337,7 +2338,7 @@ int xe_bo_pin(struct xe_bo *bo)
> >
> > ttm_bo_pin(&bo->ttm);
> > if (bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
> > - xe_ttm_tt_account_subtract(bo->ttm.ttm);
> > + xe_ttm_tt_account_subtract(xe, bo->ttm.ttm);
> >
> > /*
> > * FIXME: If we always use the reserve / unreserve functions for locking
> > @@ -2373,7 +2374,7 @@ void xe_bo_unpin_external(struct xe_bo *bo)
> >
> > ttm_bo_unpin(&bo->ttm);
> > if (bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
> > - xe_ttm_tt_account_add(bo->ttm.ttm);
> > + xe_ttm_tt_account_add(xe, bo->ttm.ttm);
> >
> > /*
> > * FIXME: If we always use the reserve / unreserve functions for locking
> > @@ -2405,7 +2406,7 @@ void xe_bo_unpin(struct xe_bo *bo)
> > }
> > ttm_bo_unpin(&bo->ttm);
> > if (bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
> > - xe_ttm_tt_account_add(bo->ttm.ttm);
> > + xe_ttm_tt_account_add(xe, bo->ttm.ttm);
> > }
> >
> > /**
> > --
> > 2.49.0
> >
next prev parent reply other threads:[~2025-06-04 22:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-21 19:48 [PATCH] drm/xe: don't store the xe device pointer inside xe_ttm_tt Dave Airlie
2025-05-21 19:54 ` ✓ CI.Patch_applied: success for " Patchwork
2025-05-21 19:55 ` ✓ CI.checkpatch: " Patchwork
2025-05-21 19:55 ` ✗ CI.KUnit: failure " Patchwork
2025-06-04 22:35 ` [PATCH] " Dave Airlie
2025-06-04 22:50 ` Matthew Brost [this message]
-- strict thread matches above, loose matches on Subject: below --
2025-06-05 6:21 Dave Airlie
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=aEDNvCUTUdxgluzd@lstrano-desk.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=thomas.hellstrom@linux.intel.com \
/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