On Thu, 16 Feb 2023 16:58:47 -0800, Umesh Nerlige Ramappa wrote:
>
> Now that OA formats come in flavor of 64 bit reports, the report header
> has 64 bit report-id, timestamp, context-id and gpu-ticks fields. When
> filtering these reports, use the right width for these fields.
>
> Note that upper dword of context id is reserved, so squash lower dword
> only.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_perf.c | 105 ++++++++++++++++++++-----
> drivers/gpu/drm/i915/i915_perf_types.h | 6 ++
> 2 files changed, 92 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 3306653c0b85..9715b964aa1e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -441,6 +441,75 @@ static u32 gen7_oa_hw_tail_read(struct i915_perf_stream *stream)
> return oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
> }
>
> +#define oa_report_header_64bit(__s) \
> + ((__s)->oa_buffer.format->header == HDR_64_BIT)
> +
> +static inline u64
> +oa_report_id(struct i915_perf_stream *stream, void *report)
> +{
> + return oa_report_header_64bit(stream) ? *(u64 *)report : *(u32 *)report;
> +}
> +
> +static inline u64
> +oa_report_reason(struct i915_perf_stream *stream, void *report)
> +{
> + return (oa_report_id(stream, report) >> OAREPORT_REASON_SHIFT) &
> + (GRAPHICS_VER(stream->perf->i915) == 12 ?
> + OAREPORT_REASON_MASK_EXTENDED :
> + OAREPORT_REASON_MASK);
> +}
> +
> +static inline void
> +oa_report_id_clear(struct i915_perf_stream *stream, u32 *report)
> +{
> + if (oa_report_header_64bit(stream))
> + *(u64 *)report = 0;
> + else
> + *report = 0;
> +}
> +
> +static inline bool
> +oa_report_ctx_invalid(struct i915_perf_stream *stream, void *report)
> +{
> + return !(oa_report_id(stream, report) &
> + stream->perf->gen8_valid_ctx_bit) &&
> + GRAPHICS_VER(stream->perf->i915) <= 11;
> +}
> +
> +static inline u64
> +oa_timestamp(struct i915_perf_stream *stream, void *report)
> +{
> + return oa_report_header_64bit(stream) ?
> + *((u64 *)report + 1) :
> + *((u32 *)report + 1);
> +}
> +
> +static inline void
> +oa_timestamp_clear(struct i915_perf_stream *stream, u32 *report)
> +{
> + if (oa_report_header_64bit(stream))
> + *(u64 *)&report[2] = 0;
> + else
> + report[1] = 0;
> +}
> +
> +static inline u32
> +oa_context_id(struct i915_perf_stream *stream, u32 *report)
> +{
> + u32 ctx_id = oa_report_header_64bit(stream) ? report[4] : report[2];
> +
> + return ctx_id & stream->specific_ctx_id_mask;
> +}
> +
> +static inline void
> +oa_context_id_squash(struct i915_perf_stream *stream, u32 *report)
> +{
> + if (oa_report_header_64bit(stream))
> + report[4] = INVALID_CTX_ID;
> + else
> + report[2] = INVALID_CTX_ID;
> +}
Let's drop 'inline' from all these functions, the convention is to let
compiler decide to inline.
> +
> /**
> * oa_buffer_check_unlocked - check for data and update tail ptr state
> * @stream: i915 stream instance
> @@ -521,9 +590,10 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
> * If not : (╯°□°）╯︵ ┻━┻
> */
Might as well fix up the comment above to say report_id and timestamp
instead of dword 0 and 1.
With the above changes, this is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
