[Intel-gfx] [PATCH] drm/i915/perf: Fix OA context id overlap with idle context id

[Lionel Landwerlin]

On 24/01/2020 03:37, Umesh Nerlige Ramappa wrote:
> Engine context pinned in perf OA was set to same context id as
> the idle context. Set the context id to an unused value.
>
> Clear the sw context id field in lrc descriptor before ORing with
> ce->tag (Chris)
>
> Fixes: https://gitlab.freedesktop.org/drm/intel/issues/756
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>   drivers/gpu/drm/i915/i915_perf.c    | 9 +++++++--
>   2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a13a8c4b65ab..cf6c43bd540a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1211,12 +1211,12 @@ __execlists_schedule_in(struct i915_request *rq)
>   	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
>   		execlists_check_context(ce, engine);
>   
> +	ce->lrc_desc &= ~GENMASK_ULL(47, 37);
>   	if (ce->tag) {
>   		/* Use a fixed tag for OA and friends */
>   		ce->lrc_desc |= (u64)ce->tag << 32;
>   	} else {
>   		/* We don't need a strict matching tag, just different values */
> -		ce->lrc_desc &= ~GENMASK_ULL(47, 37);
I guess you can remove the line just above.
>   		ce->lrc_desc |=
>   			(u64)(++engine->context_tag % NUM_CONTEXT_TAG) <<
>   			GEN11_SW_CTX_ID_SHIFT;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 3c4647054557..5bd878c64504 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1323,7 +1323,12 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
>   	case 12: {
>   		stream->specific_ctx_id_mask =
>   			((1U << GEN11_SW_CTX_ID_WIDTH) - 1) << (GEN11_SW_CTX_ID_SHIFT - 32);
> -		stream->specific_ctx_id = stream->specific_ctx_id_mask;
> +		/* Pick an unused context id
> +		 * 0 - (NUM_CONTEXT_TAG - 1) are used by other contexts
> +		 * GEN12_MAX_CONTEXT_HW_ID (0x7ff) is used by idle context
> +		 */
> +		stream->specific_ctx_id = (GEN12_MAX_CONTEXT_HW_ID - 1) << (GEN11_SW_CTX_ID_SHIFT - 32);
> +		BUILD_BUG_ON((GEN12_MAX_CONTEXT_HW_ID - 1) < NUM_CONTEXT_TAG);


Arg yeah, we can't use an id that has all bits to 1 because that matches 
the idle value in the OA reports :/

This also affects gen8-10 cases (afaik).


Thanks for spotting this!


-Lionel


>   		break;
>   	}
>   
> @@ -1331,7 +1336,7 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
>   		MISSING_CASE(INTEL_GEN(ce->engine->i915));
>   	}
>   
> -	ce->tag = stream->specific_ctx_id_mask;
> +	ce->tag = stream->specific_ctx_id;
>   
>   	DRM_DEBUG_DRIVER("filtering on ctx_id=0x%x ctx_id_mask=0x%x\n",
>   			 stream->specific_ctx_id,