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
