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

[Matthew Auld]

On Wed, 5 Apr 2023 at 15:15, Matthew Brost <matthew.brost at intel.com> wrote:
>
> 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.

>From the bug report:

commit f87c1f0b7b79b7a4f165750d1884da1fd29ae93c
       drm/ttm: prevent moving of pinned BOs

The change came from a recent rebase on drm-tip. It now treats trying
to move pinned objects as an error.

>
> 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
> > >