Re: [PATCH] drm/i915: Migrate stolen objects before hibernation

Daniel Vetter
2015-06-30T11:20:42Z

On Tue, Jun 30, 2015 at 12:03:44PM +0100, Chris Wilson wrote:
> On Tue, Jun 30, 2015 at 12:54:02PM +0200, Daniel Vetter wrote:
> > > + memset(&node, 0, sizeof(node));
> > > + ret = drm_mm_insert_node_in_range_generic(&i915->gtt.base.mm,
> > > + &node,
> > > + 4096, 0, I915_CACHE_NONE,
> > > + 0, i915->gtt.mappable_end,
> > > + DRM_MM_SEARCH_DEFAULT,
> > > + DRM_MM_CREATE_DEFAULT);
> >
> > Hm, I think the plan with stolen is to mostly use it for giant scanout
> > buffers where we never plan to access them with the gpu. Maybe go with a
> > per-page loop here instead? You have a low-level pte writing call below
> > anyway. Would mean we'd need a 1-entry onstack sg_table too, but that
> > won't hurt.
>
> I'm not understanding. This is a per-page loop (because we don't need to
> bind the entire stolen vma into GGTT for copying with the CPU and
> thereby increase the risk of failure). Speaking of failure, should
> hibernation be interruptible? I guess it is usually called from an
> interruptible process context.

I was blind and confused by the insert_entries we have in upstream, which
takes a sg_table and hence can only map the full view without some jumping
through hoops. Concern fully addressed already ;-)

Wrt uninterruptible: GPU should be idle already completely (and reset if
something went wrong) so no need for interruptible. Hm, thinking about
this: Do we handle a gpu death only detected in gpu_idle? Nasty igt:
- inject hang, but be very careful to not cause any wait at all
- suspend

BOOM or not?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation