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

[Srinivas, Vidya]

> -----Original Message-----
> From: Srinivas, Vidya
> Sent: 25 November 2024 17:01
> To: Brian Geffon <bgeffon at google.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
> 
> 
> 
> > -----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.
> 
Hello Ville - sorry to bother you again, can you kindly help check this patch and see if
this might be the cause of DPT failures? Thank you so much.

> Regards
> Vidya
> 
> >
> > >
> > > --
> > > 2.34.1
> > >