[igt-dev] [PATCH i-g-t v1] tests/kms_pipe_crc_basic: Support custom CRC sources

[Petri Latvala]

On Thu, Jun 02, 2022 at 01:41:16PM -0700, Jessica Zhang wrote:
> 
> 
> On 6/2/2022 5:54 AM, Petri Latvala wrote:
> > On Tue, May 31, 2022 at 03:03:13PM -0700, Jessica Zhang wrote:
> > > Add a custom command line option that allows user to set a custom CRC
> > > source. This will allow different drivers to test their own CRC sources
> > > instead of just the "auto" option.
> > > 
> > > Example usage: `./kms_pipe_crc_basic -s intf`
> > > 
> > > Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> > > ---
> > >   tests/kms_pipe_crc_basic.c | 28 ++++++++++++++++++++++------
> > >   1 file changed, 22 insertions(+), 6 deletions(-)
> > 
> > While this sounds useful, why only kms_pipe_crc_basic?
> > 
> > How about checking an environment variable in igt_pipe_crc_new*() and
> > overriding the source string based on that, if the parameter is
> > "auto"? That would bring this capability to other tests as well.
> 
> Hey Petri,
> 
> The intention of this patch is to have *a* way to validate the custom CRC
> source. If we implement an environmental variable, we would have to run all
> the CRC-related tests manually for validation.

What's the difference though? Between running

./kms_pipe_crc_basic -s intf

vs.

IGT_OVERRIDE_CRC_METHOD=intf ./kms_pipe_crc_basic


Or do you mean you want to first validate whether intf works for you
at all?

-- 
Petri Latvala



> 
> Thanks,
> 
> Jessica Zhang
> 
> > 
> > 
> > -- 
> > Petri Latvala
> > 
> > 
> > > 
> > > diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> > > index 0861c46dbd9b..6a593aec4c6c 100644
> > > --- a/tests/kms_pipe_crc_basic.c
> > > +++ b/tests/kms_pipe_crc_basic.c
> > > @@ -36,6 +36,7 @@ typedef struct {
> > >   	int debugfs;
> > >   	igt_display_t display;
> > >   	struct igt_fb fb;
> > > +	char *crc_source;
> > >   } data_t;
> > >   static struct {
> > > @@ -104,7 +105,7 @@ static void test_read_crc(data_t *data, enum pipe pipe, unsigned flags)
> > >   		if (flags & TEST_NONBLOCK) {
> > >   			igt_pipe_crc_t *pipe_crc;
> > > -			pipe_crc = igt_pipe_crc_new_nonblock(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> > > +			pipe_crc = igt_pipe_crc_new_nonblock(data->drm_fd, pipe, data->crc_source);
> > >   			igt_wait_for_vblank(data->drm_fd, display->pipes[pipe].crtc_offset);
> > >   			igt_pipe_crc_start(pipe_crc);
> > > @@ -119,7 +120,7 @@ static void test_read_crc(data_t *data, enum pipe pipe, unsigned flags)
> > >   		} else {
> > >   			igt_pipe_crc_t *pipe_crc;
> > > -			pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> > > +			pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, data->crc_source);
> > >   			igt_pipe_crc_start(pipe_crc);
> > >   			n_crcs = igt_pipe_crc_get_crcs(pipe_crc, N_CRCS, &crcs);
> > > @@ -202,8 +203,7 @@ static void test_compare_crc(data_t *data, enum pipe pipe)
> > >   	igt_plane_set_fb(primary, &fb0);
> > >   	igt_display_commit(display);
> > > -	pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> > > -				    INTEL_PIPE_CRC_SOURCE_AUTO);
> > > +	pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, data->crc_source);
> > >   	igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
> > >   	/* Flip FB1 with the Primary plane & compare the CRC with ref CRC. */
> > > @@ -265,9 +265,25 @@ static void test_disable_crc_after_crtc(data_t *data, enum pipe pipe)
> > >   	igt_remove_fb(data->drm_fd, &data->fb);
> > >   }
> > > -data_t data = {0, };
> > > +data_t data = {0, .crc_source = "auto"};
> > > -igt_main
> > > +static int opt_handler(int opt, int opt_index, void *_data)
> > > +{
> > > +	switch (opt) {
> > > +	case 's':
> > > +		data.crc_source = optarg;
> > > +		igt_debug("Setting CRC source to %s\n", data.crc_source);
> > > +		break;
> > > +	}
> > > +
> > > +	return IGT_OPT_HANDLER_SUCCESS;
> > > +}
> > > +
> > > +static const char help_str[] =
> > > +	"  -s\tPass in a custom source for crc test (default is \"auto\")\n";
> > > +
> > > +
> > > +igt_main_args("s:", NULL, help_str, opt_handler, NULL)
> > >   {
> > >   	enum pipe pipe;
> > > -- 
> > > 2.31.0
> > >