[PATCH 2/2] [RFC] drm/i915: Unbind the vma in suspend if it was bound

[Srinivas, Vidya]

> -----Original Message-----
> From: Brian Geffon <bgeffon at google.com>
> Sent: 25 November 2024 16:54
> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Syrjala, Ville <ville.syrjala at intel.com>;
> ville.syrjala at linux.intel.com; Lee, Shawn C <shawn.c.lee at intel.com>
> Subject: Re: [PATCH 2/2] [RFC] drm/i915: Unbind the vma in suspend if it was
> bound
> 
> On Mon, Nov 25, 2024 at 3:51 PM Vidya Srinivas <vidya.srinivas at intel.com>
> wrote:
> >
> > In i915_ggtt_suspend_vm, if the vma was bound, the path to
> > unbind/evict is not hitting during suspend. This is causing issues
> > with DPT where DPT gets shrunk but the framebuffer is still on the
> > DPT's bound list. This causes system reboot in some scenarios of
> > suspend/resume where it tries to rewrite the PTEs via a stale mapping.
> >
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_ggtt.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index d60a6ca0cae5..8499aa12a787 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -157,13 +157,11 @@ void i915_ggtt_suspend_vm(struct
> i915_address_space *vm)
> >                         goto retry;
> >                 }
> >
> > -               if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) {
> > +               if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND))
> >                         i915_vma_wait_for_bind(vma);
> >
> > -                       __i915_vma_evict(vma, false);
> > -                       drm_mm_remove_node(&vma->node);
> > -               }
> > -
> > +               __i915_vma_evict(vma, false);
> > +               drm_mm_remove_node(&vma->node);
> >                 i915_gem_object_unlock(obj);
> >         }
> 
> I think all you need is in the else path of if (!i915_vma_is_bound(vma,
> I915_VMA_GLOBAL_BIND)) is a call to
> __i915_vma_unbind(vma)


Hello Brian, thank you so much. Will submit v2 version with review comments
addressed.

> 
> But we need Ville to confirm this makes sense?
Yes, need Ville to see if this might be the real cause of DPT issue.
Thank you.

Regards
Vidya

> 
> >
> > --
> > 2.34.1
> >