Re: [Intel-gfx] [PATCH 16/19] drm/i915/perf: Apply Wa_18013179988

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Mon, 19 Sep 2022 14:21:07 -0700, Umesh Nerlige Ramappa wrote:
>
> On Fri, Sep 16, 2022 at 02:00:19PM -0700, Dixit, Ashutosh wrote:
> > On Fri, 16 Sep 2022 13:25:17 -0700, Umesh Nerlige Ramappa wrote:
> >>
> >> On Fri, Sep 16, 2022 at 12:57:19PM -0700, Dixit, Ashutosh wrote:
> >> > On Fri, 16 Sep 2022 11:56:04 -0700, Umesh Nerlige Ramappa wrote:
> >> >>
> >> >> On Thu, Sep 15, 2022 at 10:16:30PM -0700, Dixit, Ashutosh wrote:
> >> >> > On Tue, 23 Aug 2022 13:41:52 -0700, Umesh Nerlige Ramappa wrote:
> >> >> >>
> >> >> >
> >> >> > Hi Umesh,
> >> >> >
> >> >> >> OA reports in the OA buffer contain an OA timestamp field that helps
> >> >> >> user calculate delta between 2 OA reports. The calculation relies on the
> >> >> >> CS timestamp frequency to convert the timestamp value to nanoseconds.
> >> >> >> The CS timestamp frequency is a function of the CTC_SHIFT value in
> >> >> >> RPM_CONFIG0.
> >> >> >>
> >> >> >> In DG2, OA unit assumes that the CTC_SHIFT is 3, instead of using the
> >> >> >> actual value from RPM_CONFIG0. At the user level, this results in an
> >> >> >> error in calculating delta between 2 OA reports since the OA timestamp
> >> >> >> is not shifted in the same manner as CS timestamp.
> >> >> >>
> >> >> >> To resolve this, return actual OA timestamp frequency to the user in
> >> >> >> i915_getparam_ioctl.
> >> >> >
> >> >> > Rather than exposing actual OA timestamp frequency to userspace (with the
> >> >> > corresponding uapi change, specially if it's only DG2 and not all future
> >> >> > products) questions about a couple of other options:
> >> >> >
> >> >> > Option 1. Can we set CTC_SHIFT in RPM_CONFIG0 to 3, so change GT freq to be the
> >> >> >          same as OA freq :-)
> >> >> >
> >> >> >   The HSD seems to mention this:
> >> >> >   Is setting CTC SHIFT to 0b11 on driver init an acceptable W/A?
> >> >> >   Note: Changing the shift setting on live driver may break apps that are
> >> >> >   currently running (including desktop manager).
> >> >> >
> >> >> > Option 2. Is it possible to correct the timestamps in OA report headers to
> >> >> >          compensate for the difference between OA and GT frequencies (say when
> >> >> >          copying OA data to userspace)?
> >> >> >
> >> >> >	  Though not sure if this is preferable to having userspace do this.
> >> >>
> >> >> It does affect other platforms too. There's no guarantee on what the
> >> >> CTC_SHIFT value would be for different platforms, so user would have to at
> >> >> least query that somehow (maybe from i915). It's simpler for user to use
> >> >> the exported OA frequency since it is also backwards compatible.
> >> >
> >> > Is Option 2 above feasible since it would stop propagating the change to
> >> > various UMD's?
> >>
> >> Hmm, there is logic today that squashes context ids when doing oa buffer
> >> filtering, but it does that on selective reports (i.e. if a gem_context is
> >> passed).
> >>
> >> For this issue: for a 16MB OA buffer with 256 byte reports, that would be
> >> an additional write of 262144 in the kmd (to smem). For 20us sampled OA
> >> reports, it would be approx. 195 KB/s. Shouldn't be too much. Only 2
> >> concerns:
> >>
> >> - the mmapped use case may break, but I don't see that being upstreamed.
> >> We may have divergent solutions for upstream and internal.
> >> - blocking/polling tests in IGT will be sensitive to this change on some
> >> platforms and may need to be bolstered.
> >
> > If this correction/compensation in the kernel works out, even for internal
> > too we could do the following:
> >
> > * For non-mmaped case, do the correction in the kernel and expose OA freq
> >  == GT freq (in the getparam ioctl)
> > * For mmaped case expose the actual OA freq (!= GT freq)
> >
> > This will restrict the divergence only to the mmaped case (which we will
> > probably not be able to upstream).
> >
> >>
> >> I will give it a shot and get back,
>
> We cannot tweak this in the OA report header since that will be out of sync
> with the counters in the report. The other issue here is that the bug also
> applies to MI_REPORT_PERF_COUNT, and KMD cannot do anything to fix that. I
> would think this interface is the clean way to do this.

In that case this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

May still need a uapi change ack but that's separate.

>
> Thanks,
> Umesh
>
> >>
> >> Thanks,
> >> Umesh
> >>
> >> >
> >> >> https://patchwork.freedesktop.org/patch/498917/?series=107633&rev=3 is
> >> >> consumed by GPUvis. That reminds me, I should include the UMD links for the
> >> >> patches with uapi changes.
> >> >
> >> > I was thinking more about UMD's which analayze OA data and who till now are
> >> > probably assuming OA freq == GT freq and will now have to drop that
> >> > assumption. So not sure how widespread would be these changes in
> >> > the (multiple different?) UMD(s).
> >> >
> >> > Thanks.
> >> > --
> >> > Ashutosh