Re: [PATCH i-g-t v2 1/4] tests/xe_oa: Drop TestOa from metric_set()

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

On Tue, 03 Sep 2024 06:24:33 -0700, José Roberto de Souza wrote:
>
> Some platforms don't provide TestOa and Renderbasic and ComputeBasic
> are better tests for render and compute engine and available in all
> platforms.
>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/intel/xe_oa.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index f2c6d53007..dac3fb6bd5 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -311,15 +311,21 @@ static struct intel_xe_perf_metric_set *metric_set(const struct drm_xe_engine_cl
>	struct intel_xe_perf_metric_set *metric_set_iter;
>	struct intel_xe_perf_metric_set *test_set = NULL;
>
> -	if (hwe->engine_class == DRM_XE_ENGINE_CLASS_RENDER ||
> -	    hwe->engine_class == DRM_XE_ENGINE_CLASS_COMPUTE)
> -		test_set_name = "TestOa";
> -	else if ((hwe->engine_class == DRM_XE_ENGINE_CLASS_VIDEO_DECODE ||
> -		  hwe->engine_class == DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE) &&
> -		 HAS_OAM(devid))
> -		test_set_name = "MediaSet1";
> -	else
> -		igt_assert(!"reached");
> +	switch (hwe->engine_class) {
> +	case DRM_XE_ENGINE_CLASS_RENDER:
> +		test_set_name = "RenderBasic";
> +		break;
> +	case DRM_XE_ENGINE_CLASS_COMPUTE:
> +		test_set_name = "ComputeBasic";
> +		break;
> +	case DRM_XE_ENGINE_CLASS_VIDEO_DECODE:
> +	case DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE:
> +		if (HAS_OAM(devid))
> +			test_set_name = "MediaSet1";
> +		break;

Drop this break so we fallthrough to default?

> +	default:
> +		igt_assert(!"missing");
> +	}
>
>	igt_list_for_each_entry(metric_set_iter, &intel_xe_perf->metric_sets, link) {
>		if (strcmp(metric_set_iter->symbol_name, test_set_name) == 0) {

For the record, grep'ing for testset name in xml's this is what I see:

oa-acmgt1.xml\0492:       symbol_name="TestOa"
oa-acmgt2.xml\0610:       symbol_name="TestOa"
oa-acmgt3.xml\0606:       symbol_name="TestOa"
oa-adl.xml\0564:       symbol_name="TestOa"
oa-dg1.xml\0571:       symbol_name="TestOa"
oa-mtlgt2.xml\0566:       symbol_name="TestOa"
oa-mtlgt3.xml\0580:       symbol_name="TestOa"
oa-pvc.xml\0553:       symbol_name="TestOa"
oa-rkl.xml\0577:       symbol_name="TestOa"
oa-tglgt1.xml\0573:       symbol_name="TestOa"
oa-tglgt2.xml\0567:       symbol_name="TestOa"

oa-acmgt1.xml\05:       symbol_name="RenderBasic"
oa-acmgt2.xml\05:       symbol_name="RenderBasic"
oa-acmgt3.xml\05:       symbol_name="RenderBasic"
oa-adl.xml\05:       symbol_name="RenderBasic"
oa-bmg.xml\05:       symbol_name="RenderBasic"
oa-dg1.xml\05:       symbol_name="RenderBasic"
oa-lnl.xml\05:       symbol_name="RenderBasic"
oa-mtlgt2.xml\05:       symbol_name="RenderBasic"
oa-mtlgt3.xml\05:       symbol_name="RenderBasic"
oa-rkl.xml\05:       symbol_name="RenderBasic"
oa-tglgt1.xml\05:       symbol_name="RenderBasic"
oa-tglgt2.xml\05:       symbol_name="RenderBasic"

So this patch is fine. With that this is:

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