Re: [PATCH] drm/i915/perf: Fix OA context id overlap with idle context id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/01/2020 03:37, Lionel Landwerlin wrote:
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

I think the Fixes: tag is to point to another commit. And I think you want to point to the commit that you're fixing.

Not sure what you should use gitlab MRs.

-Lionel

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
---
  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,


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux