[Intel-gfx] [PATCH v2 3/3] drm/i915/perf: enable OAR context save/restore of performance counters

[Umesh Nerlige Ramappa]

On Thu, Oct 17, 2019 at 07:30:18AM +0100, Chris Wilson wrote:
>Quoting Umesh Nerlige Ramappa (2019-10-17 07:11:06)
>> +static int gen12_emit_oar_config(struct intel_context *ce, bool enable)
>> +{
>> +       struct i915_request *rq;
>> +       u32 *cs;
>> +       int err = 0;
>> +
>> +       rq = i915_request_create(ce);
>> +       if (IS_ERR(rq))
>> +               return PTR_ERR(rq);
>> +
>> +       cs = intel_ring_begin(rq, 4);
>> +       if (IS_ERR(cs)) {
>> +               err = PTR_ERR(cs);
>> +               goto out;
>> +       }
>> +
>> +       *cs++ = MI_LOAD_REGISTER_IMM(1);
>> +       *cs++ = i915_mmio_reg_offset(RING_CONTEXT_CONTROL(ce->engine->mmio_base));
>> +       *cs++ = intel_lrc_make_ctx_control(ce->engine) |
>
>It's a masked update. It only changes the bit in the register identified
>by the mask.
>
>*cs++ = _MASKED_FIELD(GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE,
>		      enable ? GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE : 0);
>

got it. in that case changes related to intel_lrc_make_ctx_control are 
not needed.
>
>> +               (enable ?
>> +                _MASKED_BIT_ENABLE(GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE) :
>> +                _MASKED_BIT_DISABLE(GEN12_CTX_CTRL_OAR_CONTEXT_ENABLE));
>> +       *cs++ = MI_NOOP;
>> +
>> +       intel_ring_advance(rq, cs);
>> +
>> +out:
>> +       i915_request_add(rq);
>> +
>> +       return err;
>> +}
>
>>  /*
>>   * Manages updating the per-context aspects of the OA stream
>>   * configuration across all contexts.
>> @@ -2316,6 +2375,17 @@ static int lrc_configure_all_contexts(struct i915_perf_stream *stream,
>>                         return err;
>>                 }
>>
>> +               /*
>> +                * For Gen12, performance counters are context
>> +                * saved/restored. Only enable it for the context that
>> +                * requested this.
>> +                */
>> +               if (ctx == stream->ctx && IS_GEN(i915, 12)) {
>> +                       err = gen12_configure_context_oar(ctx, oa_config != NULL);
>
>You have the intel_context pinned already as stream->pinned_ctx.

I see, that's simpler. I will call emit function directly on pinned 
context.

Thanks,
Umesh

>-Chris