On Mon, 28 Oct 2024 13:38:29 -0700, Umesh Nerlige Ramappa wrote: > > 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. Hi Umesh, thanks for the explanation, yes let's just leave these here. Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
