Re: [PATCH v3] drm/xe/xe_guc_ads: save/restore OA registers

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Oct 28, 2024 at 09:36:32AM -0700, Dixit, Ashutosh wrote:
>On Wed, 23 Oct 2024 13:07:15 -0700, Jonathan Cavitt wrote:
>>
>
>Hi Umesh,
>
>> @@ -748,6 +754,14 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads,
>>		}
>>	}
>>
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL0, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL1, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL2, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL3, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL4, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL5, count++);
>> +	guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL6, count++);
>
>I am trying to understand how this works. So these registers are
>saved/restored by GuC because they are not part of HW context image

correct.

>and that is why GuC needs to do the save/restore?

yes, only if GuC performs an engine reset

>Bspec 46458/56839 do seem to
>be saying that these registers are context saved/restored? If that is
>indeed true (though not sure), do they need to be here?

For pre-gen12 they were part of the engine context image, but not from 
gen12 onwards. From gen12, they are in the power context image.

These were added because users were seeing the EuStall and EuActive 
counters zeroed out during OA use case. GuC was doing an engine reset 
for some reason and that was resetting these registers. Once we added it 
here (so GuC would save restore these), the counters had correct values.

Regards,
Umesh

>
>Thanks.
>--
>Ashutosh