On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
>
Hi Umesh,
Overall looks ok, just a couple of questions below. Splitting the patches
would be nice and easier to review, but I'm almost done with this one ;-)
> @@ -1876,7 +1875,13 @@ static int alloc_noa_wait(struct i915_perf_stream
> *stream)
> MI_PREDICATE_RESULT_2_ENGINE(base) :
>
> MI_PREDICATE_RESULT_1(RENDER_RING_BASE);
>
> - bo = i915_gem_object_create_internal(i915, 4096);
> + /*
> + * gt->scratch was being used to save/restore the GPR registers, but on
> + * MTL the scratch uses stolen lmem. An MI_SRM to this memory region
> + * causes an engine hang. Instead allocate an additional page here to
> + * save/restore GPR registers
> + */
> + bo = i915_gem_object_create_internal(i915, 8192);
Do we know how much space was used in stream->noa_wait originally? Anyway
allocating an additional page is not an issue so ok to skip the question.
> @@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf
> *perf)
> break;
>
> case INTEL_DG2:
> + case INTEL_METEORLAKE:
> oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
> oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
Do these formats also need to be added?
oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
oa_format_add(perf, I915_OAR_FORMAT_A36u64_B8_C8);
oa_format_add(perf, I915_OA_FORMAT_A38u64_R2u64_B8_C8);
Or these are not considered OAG formats?
> break;
Thanks.
--
Ashutosh
