Re: [Intel-gfx] [PATCH 0/3] add guard padding around i915_vma

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Thomas,

> This has been on the list before (three times I think) and at that
> point it (the guard pages) was NAK'd by Daniel as yet another
> complication, and a VT-d
> scanout workaround was implemented and pushed using a different
> approach, initially outlined by Daniel.

I reckon that this is an old patch, but I've seen only reviews
and acks. I haven't seen anything against this patch.

So that as far as it concerns me, this patch should be good to
go, unless I missed something or there is any technical concern.

> Patch is 2ef6efa79fecd. Those suspend/resumes should now be fast.

This patch is not actually resolving the boot time delays, that
is the main concern coming from the users, and, just as a
post-mortem review, as Tvrtko has pointed out, this:

  +/* Intel Rapid Start Technology ACPI device name */
  +static const char irst_name[] = "INT3392";

is a bit scary because we are depending very much on the firmware
and whatever happens there is not under our control. It's an
hardcoded string that requires maintenance.

In any case, the two patches are somehow doing different things:
I don't see them conflicting and to me looks like a reasonable
optimization (otherwise I wouldn't have put my signature on it :))

Thanks again a lot for your thoughts and inputs,
Andi

> I then also discussed patch 1 separately with Dave Airlie and Daniel
> and since both me and Dave liked it, Daniel OK'd it, but it never made
> it upstream.
> 
> Just a short heads up on the history.
> 
> /Thomas
> 
> 
> On Wed, 2022-11-09 at 18:40 +0100, Andi Shyti wrote:
> > Hi,
> > 
> > This series adds guards around vma's but setting a pages at the
> > beginning and at the end that work as padding.
> > 
> > The first user of the vma guard are scanout objects which don't
> > need anymore to add scratch to all the unused ggtt's and speeding
> > up up considerably the boot and resume by several hundreds of
> > milliseconds up to over a full second in slower machines.
> > 
> > Andi
> > 
> > Chris Wilson (3):
> >   drm/i915: Wrap all access to i915_vma.node.start|size
> >   drm/i915: Introduce guard pages to i915_vma
> >   drm/i915: Refine VT-d scanout workaround
> > 
> >  drivers/gpu/drm/i915/display/intel_fbdev.c    |  2 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 13 ++++
> >  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 33 ++++++-----
> >  drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  2 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_tiling.c    |  4 +-
> >  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> >  .../i915/gem/selftests/i915_gem_client_blt.c  | 23 ++++----
> >  .../drm/i915/gem/selftests/i915_gem_context.c | 15 +++--
> >  .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
> >  .../drm/i915/gem/selftests/igt_gem_utils.c    |  7 ++-
> >  drivers/gpu/drm/i915/gt/gen7_renderclear.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_ggtt.c          | 39 ++++--------
> >  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c  |  3 +-
> >  drivers/gpu/drm/i915/gt/intel_renderstate.c   |  2 +-
> >  .../gpu/drm/i915/gt/intel_ring_submission.c   |  2 +-
> >  drivers/gpu/drm/i915/gt/selftest_engine_cs.c  |  8 +--
> >  drivers/gpu/drm/i915/gt/selftest_execlists.c  | 18 +++---
> >  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 15 ++---
> >  drivers/gpu/drm/i915/gt/selftest_lrc.c        | 16 ++---
> >  .../drm/i915/gt/selftest_ring_submission.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/selftest_rps.c        | 12 ++--
> >  .../gpu/drm/i915/gt/selftest_workarounds.c    |  8 +--
> >  drivers/gpu/drm/i915/i915_cmd_parser.c        |  4 +-
> >  drivers/gpu/drm/i915/i915_debugfs.c           |  2 +-
> >  drivers/gpu/drm/i915/i915_gem_gtt.h           |  3 +-
> >  drivers/gpu/drm/i915/i915_perf.c              |  2 +-
> >  drivers/gpu/drm/i915/i915_vma.c               | 59 +++++++++++++----
> > --
> >  drivers/gpu/drm/i915/i915_vma.h               | 52 +++++++++++++++-
> >  drivers/gpu/drm/i915/i915_vma_resource.c      |  4 +-
> >  drivers/gpu/drm/i915/i915_vma_resource.h      | 17 ++++--
> >  drivers/gpu/drm/i915/i915_vma_types.h         |  3 +-
> >  drivers/gpu/drm/i915/selftests/i915_request.c | 20 +++----
> >  drivers/gpu/drm/i915/selftests/igt_spinner.c  |  8 +--
> >  34 files changed, 246 insertions(+), 160 deletions(-)
> >