[igt-dev] [PATCH i-g-t v4 52/56] tests/kms_vblank: Adopt to use allocator

[Zbigniew Kempczyński]

On Fri, Aug 06, 2021 at 03:41:41PM +0200, Zbigniew Kempczyński wrote:
> From: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> 
> For newer gens kernel will reject relocations returning -EINVAL
> so we should just provide the allocator handle to inject the hang.
> 
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Petri Latvala <petri.latvala at intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> ---
>  tests/kms_vblank.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 885b2e2c4..7b7e1372e 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -118,6 +118,7 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
>  	igt_output_t *output = data->output;
>  	int fd = display->drm_fd;
>  	igt_hang_t hang;
> +	uint64_t ahnd = 0;
>  
>  	prepare_crtc(data, fd, output);
>  
> @@ -128,8 +129,11 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
>  		 igt_subtest_name(), kmstest_pipe_name(data->pipe),
>  		 igt_output_name(output));
>  
> -	if (!(data->flags & NOHANG))
> -		hang = igt_hang_ring(fd, I915_EXEC_DEFAULT);
> +	if (!(data->flags & NOHANG)) {
> +		if (is_i915_device(fd))
> +			ahnd = get_reloc_ahnd(fd, 0);
> +		hang = igt_hang_ring_with_ahnd(fd, I915_EXEC_DEFAULT, ahnd);
> +	}

What for is_i915_device(fd) check is introduced here? igt_hang_ring() is currently
valid only for i915 so ahnd = get_reloc_ahnd(fd, 0) can be called unconditionally
here. Am I missed something here?

>  
>  	if (data->flags & BUSY) {
>  		union drm_wait_vblank vbl;
> @@ -166,6 +170,9 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
>  	igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
>  		 igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
>  
> +	if (is_i915_device(fd))
> +		put_ahnd(ahnd);
> +

For non-i915 machines ahnd will be equal to 0, so put_ahnd() will just return
and no i915 dependand code will be called. I would rather expect: 

if (!(data->flags & NOHANG))
	put_ahnd(ahnd);

But as I said put_ahnd() with ahnd == 0 does nothing.

--
Zbigniew 


>  	/* cleanup what prepare_crtc() has done */
>  	cleanup_crtc(data, fd, output);
>  }
> -- 
> 2.26.0
>