On Mon, 22 May 2023 14:34:18 -0700, Umesh Nerlige Ramappa wrote:
>
> On Mon, May 22, 2023 at 01:17:49PM -0700, Ashutosh Dixit wrote:
> > Clearing out report id and timestamp as means to detect unlanded reports
> > only works if report size is power of 2. That is, only when report size is
> > a sub-multiple of the OA buffer size can we be certain that reports will
> > land at the same place each time in the OA buffer (after rewind). If report
> > size is not a power of 2, we need to zero out the entire report to be able
> > to detect unlanded reports reliably.
> >
> > Cc: Umesh Nerlige Ramappa <umesh.nerlige.rama...@intel.com>
> > Signed-off-by: Ashutosh Dixit <ashutosh.di...@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c
> > b/drivers/gpu/drm/i915/i915_perf.c
> > index 19d5652300eeb..58284156428dc 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -877,12 +877,17 @@ static int gen8_append_oa_reports(struct
> > i915_perf_stream *stream,
> > stream->oa_buffer.last_ctx_id = ctx_id;
> > }
> >
> > - /*
> > - * Clear out the report id and timestamp as a means to detect
> > unlanded
> > - * reports.
> > - */
> > - oa_report_id_clear(stream, report32);
> > - oa_timestamp_clear(stream, report32);
> > + if (is_power_of_2(report_size)) {
> > + /*
> > + * Clear out the report id and timestamp as a means
> > + * to detect unlanded reports.
> > + */
> > + oa_report_id_clear(stream, report32);
> > + oa_timestamp_clear(stream, report32);
> > + } else {
> > + /* Zero out the entire report */
> > + memset(report32, 0, report_size);
>
> Indeed, this was a bug. For a minute, I started wondering if this is the
> issue I am running into with the other patch posted for DG2, but then I see
> the issue within the first fill of the OA buffer where chunks of the
> reports are zeroed out, so this is a new issue.
Yes I saw this while reviewing your patch. And also I thought your issue
was happening on DG2 with power of 2 report size, only on MTL OAM we
introduce non power of 2 report size.
> lgtm,
>
> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.rama...@intel.com>
Thanks.
--
Ashutosh
>
> > + }
> > }
> >
> > if (start_offset != *offset) {
> > --
> > 2.38.0
> >
