[Intel-xe] [PATCH] drm/xe: Use Xe BO pin / unpin functions for FB pin

[Matthew Brost]

On Wed, Apr 05, 2023 at 02:19:35PM +0100, Matthew Auld wrote:
> On Wed, 5 Apr 2023 at 05:19, Matthew Brost <matthew.brost at intel.com> wrote:
> >
> > Need to use XE BO pin / unpin for suspend / resume to work properly.
> > Part this fix too is too allow xe_bo_pin to be called more than once.
> >
> > Should fix:
> > https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/244
> >
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> 
> Change makes sense to me, but I think we also need to update xe_bo_unpin()?
> 

Yep, thought that coded correctly but was looking at xe_bo_unpin_external().

> But I must be misunderstanding something here. AFAICT we can have
> various pinned VRAM objects, and xe_bo_pin() still adds them to
> pinned.kernel_bo_present, where during suspend we evict everything on
> that list to system memory. But as these are all still pinned, suspend
> is still going to fail with this patch, given the recent TTM changes.

What reference recent TTM changes? This all worked at one point in time.

Matt

> What am I missing?
> 
> > ---
> >  drivers/gpu/drm/xe/display/xe_fb_pin.c | 4 ++--
> >  drivers/gpu/drm/xe/xe_bo.c             | 5 +++--
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > index 65c0bc28a3d1..c7c849be288f 100644
> > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > @@ -205,7 +205,7 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
> >
> >         ret = xe_bo_validate(bo, NULL, true);
> >         if (!ret)
> > -               ttm_bo_pin(&bo->ttm);
> > +               xe_bo_pin(bo);
> >         ttm_bo_unreserve(&bo->ttm);
> >         if (ret)
> >                 goto err;
> > @@ -222,7 +222,7 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
> >
> >  err_unpin:
> >         ttm_bo_reserve(&bo->ttm, false, false, NULL);
> > -       ttm_bo_unpin(&bo->ttm);
> > +       xe_bo_unpin(bo);
> >         ttm_bo_unreserve(&bo->ttm);
> >  err:
> >         kfree(vma);
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index 3a482c61c3ec..25c696bd380c 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -1278,8 +1278,8 @@ int xe_bo_pin(struct xe_bo *bo)
> >          */
> >         XE_BUG_ON(bo->ttm.base.import_attach);
> >
> > -       /* We only expect at most 1 pin */
> > -       XE_BUG_ON(xe_bo_is_pinned(bo));
> > +       if (xe_bo_is_pinned(bo))
> > +               goto pin;
> >
> >         err = xe_bo_validate(bo, NULL, false);
> >         if (err)
> > @@ -1308,6 +1308,7 @@ int xe_bo_pin(struct xe_bo *bo)
> >                 }
> >         }
> >
> > +pin:
> >         ttm_bo_pin(&bo->ttm);
> >
> >         /*
> > --
> > 2.34.1
> >