[PATCH v8 08/13] drm/i915/guc: Add capture region into intel_guc_log
Matthew Brost
matthew.brost at intel.com
Tue Mar 8 18:51:15 UTC 2022
On Mon, Feb 28, 2022 at 09:39:27AM -0800, Alan Previn wrote:
> GuC log buffer regions for debug-log-events, crash-dumps and
> error-state-capture are all part of a single bo allocation that
> also includes the guc_log_buffer_state structures. Now that we
> support it, increase the size allocation for error-capture.
>
> Since the error-capture region is accessed at non-deterministic
> times (as part of GuC triggered context reset) while debug-log-
> events region is accessed as part of relay logging or during
> debugfs triggered dumps, move the mapping and unmapping of the
> shared buffer into intel_guc_log_create and intel_guc_log_destroy
> so that it's always mapped throughout life of GuC operation.
>
> Additionally, while here, update the guc log region layout
> diagram to follow the order according to the enum definition
> as per the GuC interface.
>
One nit, buf_addr should be updated to be a io_sys_map + use the
io_sys_map wrapper functions to access this memory. Not a blocker and
can be done in a follow up but calling it out as work that needs to be
done.
With that:
Reviewed-by: Matthew Brost <matthew.brost at intel.com>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 58 +++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.h | 3 +-
> 2 files changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index bf3abb7e69b0..2cc52f1eedf3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -25,7 +25,8 @@ static void guc_log_copy_debuglogs_for_relay(struct intel_guc_log *log);
> static int guc_action_flush_log_complete(struct intel_guc *guc)
> {
> u32 action[] = {
> - INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
> + INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE,
> + GUC_DEBUG_LOG_BUFFER
> };
>
> return intel_guc_send(guc, action, ARRAY_SIZE(action));
> @@ -136,7 +137,7 @@ static void guc_move_to_next_buf(struct intel_guc_log *log)
> smp_wmb();
>
> /* All data has been written, so now move the offset of sub buffer. */
> - relay_reserve(log->relay.channel, log->vma->obj->base.size);
> + relay_reserve(log->relay.channel, log->vma->obj->base.size - CAPTURE_BUFFER_SIZE);
>
> /* Switch to the next sub buffer */
> relay_flush(log->relay.channel);
> @@ -212,7 +213,7 @@ static void _guc_log_copy_debuglogs_for_relay(struct intel_guc_log *log)
> goto out_unlock;
>
> /* Get the pointer to shared GuC log buffer */
> - log_buf_state = src_data = log->relay.buf_addr;
> + log_buf_state = src_data = log->buf_addr;
>
> /* Get the pointer to local buffer to store the logs */
> log_buf_snapshot_state = dst_data = guc_get_write_buffer(log);
> @@ -232,7 +233,8 @@ static void _guc_log_copy_debuglogs_for_relay(struct intel_guc_log *log)
> src_data += PAGE_SIZE;
> dst_data += PAGE_SIZE;
>
> - for (type = GUC_DEBUG_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
> + /* For relay logging, we exclude error state capture */
> + for (type = GUC_DEBUG_LOG_BUFFER; type <= GUC_CRASH_DUMP_LOG_BUFFER; type++) {
> /*
> * Make a copy of the state structure, inside GuC log buffer
> * (which is uncached mapped), on the stack to avoid reading
> @@ -310,23 +312,17 @@ static void copy_debug_logs_work(struct work_struct *work)
>
> static int guc_log_relay_map(struct intel_guc_log *log)
> {
> - void *vaddr;
> -
> lockdep_assert_held(&log->relay.lock);
>
> - if (!log->vma)
> + if (!log->vma || !log->buf_addr)
> return -ENODEV;
>
> /*
> - * Create a WC (Uncached for read) vmalloc mapping of log
> - * buffer pages, so that we can directly get the data
> - * (up-to-date) from memory.
> + * WC vmalloc mapping of log buffer pages was done at
> + * GuC Log Init time, but lets keep a ref for book-keeping
> */
> - vaddr = i915_gem_object_pin_map_unlocked(log->vma->obj, I915_MAP_WC);
> - if (IS_ERR(vaddr))
> - return PTR_ERR(vaddr);
> -
> - log->relay.buf_addr = vaddr;
> + i915_gem_object_get(log->vma->obj);
> + log->relay.buf_in_use = true;
>
> return 0;
> }
> @@ -335,8 +331,8 @@ static void guc_log_relay_unmap(struct intel_guc_log *log)
> {
> lockdep_assert_held(&log->relay.lock);
>
> - i915_gem_object_unpin_map(log->vma->obj);
> - log->relay.buf_addr = NULL;
> + i915_gem_object_put(log->vma->obj);
> + log->relay.buf_in_use = false;
> }
>
> void intel_guc_log_init_early(struct intel_guc_log *log)
> @@ -442,6 +438,7 @@ int intel_guc_log_create(struct intel_guc_log *log)
> {
> struct intel_guc *guc = log_to_guc(log);
> struct i915_vma *vma;
> + void *vaddr;
> u32 guc_log_size;
> int ret;
>
> @@ -449,20 +446,21 @@ int intel_guc_log_create(struct intel_guc_log *log)
>
> /*
> * GuC Log buffer Layout
> + * (this ordering must follow "enum guc_log_buffer_type" definition)
> *
> * +===============================+ 00B
> - * | Crash dump state header |
> - * +-------------------------------+ 32B
> * | Debug state header |
> + * +-------------------------------+ 32B
> + * | Crash dump state header |
> * +-------------------------------+ 64B
> * | Capture state header |
> * +-------------------------------+ 96B
> * | |
> * +===============================+ PAGE_SIZE (4KB)
> - * | Crash Dump logs |
> - * +===============================+ + CRASH_SIZE
> * | Debug logs |
> * +===============================+ + DEBUG_SIZE
> + * | Crash Dump logs |
> + * +===============================+ + CRASH_SIZE
> * | Capture logs |
> * +===============================+ + CAPTURE_SIZE
> */
> @@ -476,6 +474,17 @@ int intel_guc_log_create(struct intel_guc_log *log)
> }
>
> log->vma = vma;
> + /*
> + * Create a WC (Uncached for read) vmalloc mapping up front immediate access to
> + * data from memory during critical events such as error capture
> + */
> + vaddr = i915_gem_object_pin_map_unlocked(log->vma->obj, I915_MAP_WC);
> + if (IS_ERR(vaddr)) {
> + ret = PTR_ERR(vaddr);
> + i915_vma_unpin_and_release(&log->vma, 0);
> + goto err;
> + }
> + log->buf_addr = vaddr;
>
> log->level = __get_default_log_level(log);
> DRM_DEBUG_DRIVER("guc_log_level=%d (%s, verbose:%s, verbosity:%d)\n",
> @@ -486,13 +495,14 @@ int intel_guc_log_create(struct intel_guc_log *log)
> return 0;
>
> err:
> - DRM_ERROR("Failed to allocate GuC log buffer. %d\n", ret);
> + DRM_ERROR("Failed to allocate or map GuC log buffer. %d\n", ret);
> return ret;
> }
>
> void intel_guc_log_destroy(struct intel_guc_log *log)
> {
> - i915_vma_unpin_and_release(&log->vma, 0);
> + log->buf_addr = NULL;
> + i915_vma_unpin_and_release(&log->vma, I915_VMA_RELEASE_MAP);
> }
>
> int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
> @@ -537,7 +547,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
>
> bool intel_guc_log_relay_created(const struct intel_guc_log *log)
> {
> - return log->relay.buf_addr;
> + return log->buf_addr;
> }
>
> int intel_guc_log_relay_open(struct intel_guc_log *log)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> index d7e1b6471fed..e1345fca7729 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> @@ -49,8 +49,9 @@ struct intel_guc;
> struct intel_guc_log {
> u32 level;
> struct i915_vma *vma;
> + void *buf_addr;
> struct {
> - void *buf_addr;
> + bool buf_in_use;
> bool started;
> struct work_struct flush_work;
> struct rchan *channel;
> --
> 2.25.1
>
More information about the Intel-gfx-trybot
mailing list