All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	 intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org
Cc: Matthew Auld <matthew.auld@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Brost Matthew <matthew.brost@intel.com>,
	 Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gt: Register the migrate contexts with their engines
Date: Thu, 26 Aug 2021 15:31:52 +0200	[thread overview]
Message-ID: <fe6ffb91c2f562f50c66bc02e993d993280d0456.camel@linux.intel.com> (raw)
In-Reply-To: <2722f768-f73c-1501-996f-c009eab660a1@linux.intel.com>

On Thu, 2021-08-26 at 14:04 +0100, Tvrtko Ursulin wrote:
> 
> On 26/08/2021 11:45, Thomas Hellström wrote:
> > Pinned contexts, like the migrate contexts need reset after resume
> > since their context image may have been lost. Also the GuC needs to
> > register pinned contexts.
> 
> So kernel context can get corrupt because we park the GPU with it 
> active. Blitter context for a different reason - which is that it is 
> used to copy itself over to smem, no?
> 
> If that is correct, then why bother copying the blitter context in
> the 
> first place and not just always re-create it on resume?
> 
> That would be along the lines of marking the backing store as
> "dontneed" 
> (however the exact mechanics of that look these days) so suspend can 
> skip them.

I think that is marking the object with I915_BO_ALLOC_VOLATILE. However
I assume this follows the rule of the internal backend objects:
Contents are valid while pinned (or locked), and these images are
indeed pinned on suspend so we need to come up with something else.
Perhaps I915_BO_ALLOC_PM_NOSAVE for the context images (and engine
status pages?) I915_BO_ALLOC_PM_MEMCPY for the migrate vm pagetables
only. The latter will come in handy also for supporting small apertures
where we need to pin these in the mappable area.

> 
> > Add a list to struct intel_engine_cs where we add all pinned
> > contexts on
> > creation, and traverse that list at resume time to reset the pinned
> > contexts.
> > 
> > This fixes the kms_pipe_crc_basic@suspend-read-crc-pipe-a selftest
> > for now,
> > but proper LMEM backup / restore is needed for full suspend
> > functionality.
> > However, note that even with full LMEM backup / restore it may be
> > desirable to keep the reset since backing up the migrate context
> > images
> > must happen using memcpy() after the migrate context has become
> > inactive,
> > and for performance- and other reasons we want to avoid memcpy()
> > from
> > LMEM.
> 
> Hm I guess this talks about the issue - so are these images migrated
> at 
> all today or not?

My current WIP backs them up. But with something like the above flags,
that's easily changed. Suggestions welcome.

/Thomas



WARNING: multiple messages have this Message-ID (diff)
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	 intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org
Cc: Matthew Auld <matthew.auld@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Brost Matthew <matthew.brost@intel.com>,
	 Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH v2] drm/i915/gt: Register the migrate contexts with their engines
Date: Thu, 26 Aug 2021 15:31:52 +0200	[thread overview]
Message-ID: <fe6ffb91c2f562f50c66bc02e993d993280d0456.camel@linux.intel.com> (raw)
In-Reply-To: <2722f768-f73c-1501-996f-c009eab660a1@linux.intel.com>

On Thu, 2021-08-26 at 14:04 +0100, Tvrtko Ursulin wrote:
> 
> On 26/08/2021 11:45, Thomas Hellström wrote:
> > Pinned contexts, like the migrate contexts need reset after resume
> > since their context image may have been lost. Also the GuC needs to
> > register pinned contexts.
> 
> So kernel context can get corrupt because we park the GPU with it 
> active. Blitter context for a different reason - which is that it is 
> used to copy itself over to smem, no?
> 
> If that is correct, then why bother copying the blitter context in
> the 
> first place and not just always re-create it on resume?
> 
> That would be along the lines of marking the backing store as
> "dontneed" 
> (however the exact mechanics of that look these days) so suspend can 
> skip them.

I think that is marking the object with I915_BO_ALLOC_VOLATILE. However
I assume this follows the rule of the internal backend objects:
Contents are valid while pinned (or locked), and these images are
indeed pinned on suspend so we need to come up with something else.
Perhaps I915_BO_ALLOC_PM_NOSAVE for the context images (and engine
status pages?) I915_BO_ALLOC_PM_MEMCPY for the migrate vm pagetables
only. The latter will come in handy also for supporting small apertures
where we need to pin these in the mappable area.

> 
> > Add a list to struct intel_engine_cs where we add all pinned
> > contexts on
> > creation, and traverse that list at resume time to reset the pinned
> > contexts.
> > 
> > This fixes the kms_pipe_crc_basic@suspend-read-crc-pipe-a selftest
> > for now,
> > but proper LMEM backup / restore is needed for full suspend
> > functionality.
> > However, note that even with full LMEM backup / restore it may be
> > desirable to keep the reset since backing up the migrate context
> > images
> > must happen using memcpy() after the migrate context has become
> > inactive,
> > and for performance- and other reasons we want to avoid memcpy()
> > from
> > LMEM.
> 
> Hm I guess this talks about the issue - so are these images migrated
> at 
> all today or not?

My current WIP backs them up. But with something like the above flags,
that's easily changed. Suggestions welcome.

/Thomas



  reply	other threads:[~2021-08-26 13:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 10:45 [Intel-gfx] [PATCH v2] drm/i915/gt: Register the migrate contexts with their engines Thomas Hellström
2021-08-26 10:45 ` Thomas Hellström
2021-08-26 12:44 ` [Intel-gfx] " Daniel Vetter
2021-08-26 13:59   ` Thomas Hellström
2021-08-26 14:59     ` Daniel Vetter
2021-08-26 13:04 ` Tvrtko Ursulin
2021-08-26 13:04   ` Tvrtko Ursulin
2021-08-26 13:31   ` Thomas Hellström [this message]
2021-08-26 13:31     ` Thomas Hellström
2021-08-26 14:51 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Register the migrate contexts with their engines (rev2) Patchwork
2021-08-26 22:35 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fe6ffb91c2f562f50c66bc02e993d993280d0456.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.