[igt-dev] [PATCH i-g-t 10/31] i915/perf: Add OAM format type

[Kamil Konieczny]

Hi Umesh,

On 2023-02-14 at 16:46:27 -0800, Umesh Nerlige Ramappa wrote:
> Don't check for B and C counters sanity for OAM formats.

Please improve description here, like:

Add new OAM format, where OAM stands for ObservAbility of Media.
Also while at it, don't check for B and C counters sanity for
OAM formats.

Note: it is my guess what OAM means so please correct me, btw
by media do we mean video (image) stream ?
It would also help if you write why you don't check that counters
here or maybe in code comment before if condition.

> 
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> ---
>  tests/i915/perf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/perf.c b/tests/i915/perf.c
> index c3c5663b..e9613dc9 100644
> --- a/tests/i915/perf.c
> +++ b/tests/i915/perf.c
> @@ -104,6 +104,7 @@ struct accumulator {

It would be nice if you could add here description of the meaning
of OAG, OAR and OAM.

With improved comments you can add:
Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Regards,
Kamil

>  enum {
>  	OAG,
>  	OAR,
> +	OAM,
>  
>  	MAX_OA_TYPE,
>  };
> @@ -1035,7 +1036,7 @@ gen8_sanity_check_test_oa_reports(const uint32_t *oa_report0,
>  	/* The TestOa metric set defines all B counters to be a
>  	 * multiple of the gpu clock
>  	 */
> -	if (format.n_b) {
> +	if (format.n_b && (format.oa_type == OAG || format.oa_type == OAR)) {
>  		if (clock_delta > 0) {
>  			b = rpt1_b[0] - rpt0_b[0];
>  			igt_debug("B0: delta = %"PRIu32"\n", b);
> -- 
> 2.36.1
>