linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org,
	"Matt Roper" <matthew.d.roper@intel.com>,
	linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
	linux-kernel@vger.kernel.org,
	"Christian König" <christian.koenig@amd.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniele Ceraolo Spurio" <daniele.ceraolospurio@intel.com>,
	"David Airlie" <airlied@linux.ie>,
	"John Harrison" <John.C.Harrison@Intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Matthew Auld" <matthew.auld@intel.com>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>
Subject: [PATCH 00/19] drm/i915/guc: Refactor ADS access to use dma_buf_map
Date: Wed, 26 Jan 2022 12:36:43 -0800	[thread overview]
Message-ID: <20220126203702.1784589-1-lucas.demarchi@intel.com> (raw)

While porting i915 to arm64 we noticed some issues accessing lmem.
Some writes were getting corrupted and the final state of the buffer
didn't have exactly what we wrote. This became evident when enabling
GuC submission: depending on the number of engines the ADS struct was
being corrupted and GuC would reject it, refusin to initialize.

From Documentation/core-api/bus-virt-phys-mapping.rst:

	This memory is called "PCI memory" or "shared memory" or "IO memory" or
	whatever, and there is only one way to access it: the readb/writeb and
	related functions. You should never take the address of such memory, because
	there is really nothing you can do with such an address: it's not
	conceptually in the same memory space as "real memory" at all, so you cannot
	just dereference a pointer. (Sadly, on x86 it **is** in the same memory space,
	so on x86 it actually works to just deference a pointer, but it's not
	portable).

When reading or writing words directly to IO memory, in order to be portable
the Linux kernel provides the abstraction detailed in section "Differences
between I/O access functions" of Documentation/driver-api/device-io.rst.

This limits our ability to simply overlay our structs on top a buffer
and directly access it since that buffer may come from IO memory rather than
system memory. Hence the approach taken in intel_guc_ads.c needs to be
refactored. This is not the only place in i915 that neeed to be changed, but
the one causing the most problems, with a real reproducer. This first set of
patch focuses on fixing the gem object to pass the ADS

After the addition of a few helpers in the dma_buf_map API, most of
intel_guc_ads.c can be converted to use it. The exception is the regset
initialization: we'd incur into a lot of extra indirection when
reading/writting each register. So the regset is converted to use a
temporary buffer allocated on probe, which is then copied to its
final location when finishing the initialization or on gt reset.

Testing on some discrete cards, after this change we can correctly pass the
ADS struct to GuC and have it initialized correctly.

thanks
Lucas De Marchi

Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

Lucas De Marchi (19):
  dma-buf-map: Add read/write helpers
  dma-buf-map: Add helper to initialize second map
  drm/i915/gt: Add helper for shmem copy to dma_buf_map
  drm/i915/guc: Keep dma_buf_map of ads_blob around
  drm/i915/guc: Add read/write helpers for ADS blob
  drm/i915/guc: Convert golden context init to dma_buf_map
  drm/i915/guc: Convert policies update to dma_buf_map
  drm/i915/guc: Convert engine record to dma_buf_map
  dma-buf-map: Add wrapper over memset
  drm/i915/guc: Convert guc_ads_private_data_reset to dma_buf_map
  drm/i915/guc: Convert golden context prep to dma_buf_map
  drm/i915/guc: Replace check for golden context size
  drm/i915/guc: Convert mapping table to dma_buf_map
  drm/i915/guc: Convert capture list to dma_buf_map
  drm/i915/guc: Prepare for error propagation
  drm/i915/guc: Use a single pass to calculate regset
  drm/i915/guc: Convert guc_mmio_reg_state_init to dma_buf_map
  drm/i915/guc: Convert __guc_ads_init to dma_buf_map
  drm/i915/guc: Remove plain ads_blob pointer

 drivers/gpu/drm/i915/gt/shmem_utils.c         |  32 ++
 drivers/gpu/drm/i915/gt/shmem_utils.h         |   3 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |  14 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c    | 374 +++++++++++-------
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h    |   3 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  11 +-
 include/linux/dma-buf-map.h                   | 127 ++++++
 7 files changed, 405 insertions(+), 159 deletions(-)

-- 
2.35.0


             reply	other threads:[~2022-01-26 20:36 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 20:36 Lucas De Marchi [this message]
2022-01-26 20:36 ` [PATCH 01/19] dma-buf-map: Add read/write helpers Lucas De Marchi
2022-01-27  7:24   ` Christian König
2022-01-27  7:36     ` Matthew Brost
2022-01-27  7:59       ` Christian König
2022-01-27  9:02         ` [Intel-gfx] " Daniel Vetter
2022-01-27 14:26   ` Thomas Zimmermann
2022-01-27 16:34     ` Lucas De Marchi
2022-01-28  8:32       ` Thomas Zimmermann
2022-01-26 20:36 ` [PATCH 02/19] dma-buf-map: Add helper to initialize second map Lucas De Marchi
2022-01-27  7:27   ` Christian König
2022-01-27  7:57     ` Lucas De Marchi
2022-01-27  8:02       ` Christian König
2022-01-27  8:18         ` [Intel-gfx] " Lucas De Marchi
2022-01-27  8:55           ` Christian König
2022-01-27  9:12             ` Lucas De Marchi
2022-01-27  9:21               ` Christian König
2022-01-27  8:57         ` Daniel Vetter
2022-01-27  9:33           ` [Intel-gfx] " Lucas De Marchi
2022-01-27 10:00             ` Daniel Vetter
2022-01-27 10:21               ` Christian König
2022-01-27 11:16                 ` Daniel Vetter
2022-01-27 11:44                   ` [Linaro-mm-sig] " Christian König
2022-01-27 11:56                     ` Daniel Vetter
2022-01-27 16:13                     ` Lucas De Marchi
2022-01-27 14:52                 ` Thomas Zimmermann
2022-01-27 16:12                 ` Lucas De Marchi
2022-01-27 14:33   ` Thomas Zimmermann
2022-01-27 15:59     ` [Intel-gfx] " Lucas De Marchi
2022-01-28  8:15       ` Thomas Zimmermann
2022-01-28  8:34         ` Thomas Zimmermann
2022-01-26 20:36 ` [PATCH 09/19] dma-buf-map: Add wrapper over memset Lucas De Marchi
2022-01-27  7:28   ` Christian König
2022-01-27 14:54   ` Thomas Zimmermann
2022-01-27 15:38     ` [Intel-gfx] " Lucas De Marchi
2022-01-27 15:47       ` Thomas Zimmermann

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=20220126203702.1784589-1-lucas.demarchi@intel.com \
    --to=lucas.demarchi@intel.com \
    --cc=John.C.Harrison@Intel.com \
    --cc=airlied@linux.ie \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=sumit.semwal@linaro.org \
    --cc=thomas.hellstrom@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).