Re: [Intel-gfx] [PATCH] drm/i915/perf: Clear out entire reports after reading if not power of 2 size

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

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.ramappa@intel.com>
>Signed-off-by: Ashutosh Dixit <ashutosh.dixit@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.

lgtm,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Thanks,
Umesh


>+		}
> 	}
>
> 	if (start_offset != *offset) {
>-- 
>2.38.0
>