Re: [igt-dev] [PATCH i-g-t v2] tests/i915/kms_dsc: add limited flag

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

On Mon, May 29, 2023 at 10:10:22PM +0530, Swati Sharma wrote:
> Add limited flag to restrict test execution to one valid pipe-output
> combination. By default all tests will run on all valid pipe-output
> combinations.
> 
> v2: -rename extended to limited (Kamil)
> 
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  tests/i915/kms_dsc.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/kms_dsc.c b/tests/i915/kms_dsc.c
> index 3ce28f84..9ddb441d 100644
> --- a/tests/i915/kms_dsc.c
> +++ b/tests/i915/kms_dsc.c
> @@ -51,6 +51,7 @@ typedef struct {
>  	int input_bpc;
>  	int disp_ver;
>  	enum pipe pipe;
> +	bool limited;
>  } data_t;
>  
>  static int output_format_list[] = {DSC_FORMAT_YCBCR420, DSC_FORMAT_YCBCR444};
> @@ -250,13 +251,34 @@ static void test_dsc(data_t *data, enum dsc_test_type test_type, int bpc,
>  
>  		igt_dynamic_f("pipe-%s-%s%s",  kmstest_pipe_name(data->pipe), data->output->name, name)
>  			update_display(data, test_type);
> +
> +		if (data->limited)
> +			break;
>  	}
>  }
>  
> -igt_main
> +static int opt_handler(int opt, int opt_index, void *_data)
>  {
> -	data_t data = {};

You don't need to introduce static data below, handler has pointer
to it, and functions are called with &data so keeping this on stack
is better for me (protects from global access in any place).

> +	data_t *data = _data;
> +
> +	switch (opt) {
> +	case 'l':
> +		data->limited = true;
> +		break;
> +	default:
> +		return IGT_OPT_HANDLER_ERROR;
> +	}
> +
> +	return IGT_OPT_HANDLER_SUCCESS;
> +}
> +
> +static const char help_str[] =
> +	"  --limited\t\tLimit execution to 1 valid pipe-output combo\n";

I see you're using short option '-l', just add this to line above like:

"  --limited\t-l\tLimit ..."

>  
> +static data_t data = {};

Keep this on stack. With this nit fixed:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

> +
> +igt_main_args("l", NULL, help_str, opt_handler, &data)
> +{
>  	igt_fixture {
>  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_XE);
>  		data.devid = intel_get_drm_devid(data.drm_fd);
> -- 
> 2.25.1
>