[Intel-gfx] [PATCH v5 04/10] drm/i915/perf: implement active wait for noa configurations

[Chris Wilson]

Quoting Lionel Landwerlin (2019-06-27 09:00:39)
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 2f1dc9be3bfb..b2f5ba87921c 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -410,6 +410,8 @@ static int alloc_oa_config_buffer(struct drm_i915_private *i915,
>         size_t config_length = 0;
>         u32 *cs;
>  
> +       lockdep_assert_held(&i915->drm.struct_mutex);

What inside *this* function depends on the struct_mutex? The noa_wait
vma is already pinned and I didn't spot anything else added that might
be contentious.

>         if (oa_config->mux_regs_len > 0) {
>                 config_length += DIV_ROUND_UP(oa_config->mux_regs_len,
>                                               MI_LOAD_REGISTER_IMM_MAX_REGS) * 4;
> @@ -425,7 +427,7 @@ static int alloc_oa_config_buffer(struct drm_i915_private *i915,
>                                               MI_LOAD_REGISTER_IMM_MAX_REGS) * 4;
>                 config_length += oa_config->flex_regs_len * 8;
>         }
> -       config_length += 4; /* MI_BATCH_BUFFER_END */
> +       config_length += 12; /* MI_BATCH_BUFFER_START into noa_wait loop */
>         config_length = ALIGN(config_length, I915_GTT_PAGE_SIZE);
>  
>         bo = i915_gem_object_create_shmem(i915, config_length);
> @@ -442,7 +444,12 @@ static int alloc_oa_config_buffer(struct drm_i915_private *i915,
>         cs = write_cs_mi_lri(cs, oa_config->b_counter_regs, oa_config->b_counter_regs_len);
>         cs = write_cs_mi_lri(cs, oa_config->flex_regs, oa_config->flex_regs_len);
>  
> -       *cs++ = MI_BATCH_BUFFER_END;
> +
> +       /* Jump into the NOA wait busy loop. */
> +       *cs++ = (INTEL_GEN(i915) < 8 ?
> +                MI_BATCH_BUFFER_START : MI_BATCH_BUFFER_START_GEN8);
> +       *cs++ = i915->perf.oa.noa_wait->node.start;

i915_ggtt_offset(i915->perf.oa.noa_wait);
double checks that you are in the ggtt and so assuming u32 is valid.

This is operating inside the user context right? (If I remember
correctly, you plug into the user dispatch) If so, where do you save and
restore the CS_GPR overwritten?
-Chris