[igt-dev] [RFC PATCH i-g-t] tests/kms_invalid_dotclock: Test every mode

[Almahallawy, Khaled]

On Wed, 2021-05-12 at 15:01 +0300, Ville Syrjälä wrote:
> On Tue, May 11, 2021 at 11:37:42PM -0700, Khaled Almahallawy wrote:
> > Implementing:
> > 
> > /*
> >  * FIXME test every mode we have to be more
> >  * sure everything is really getting rejected?
> >  */
> 
> I don't quite recall the history of that FIXME. I guess there
> should be at least no real harm in doing this.
> 
> However there are a bunch of other more extensive tests here:
> https://patchwork.freedesktop.org/series/69531/
> Would be nice to get those in. Can't remember if they would conflict
> with this stuff...

Sure, I will be happy to work to get this series in. I will rebase if
necessarily and test.
 
But I'm a little confused about the process here. If everything works
for me, Should I resend your entire series  to the same root-mesg-id (
20191115131520.9872-1-ville.syrjala at linux.intel.com ) from your
original series or should I not resend your series?
 
Thank you for your review

~Khaled
> 
> > Output of the test  will look like the following:
> > ...
> > Checking pipe A connector DP-2 with mode  720x400 70 720 738 846
> > 900 400 412 414 449 0x6 0x40 1305601
> > Checking pipe B connector DP-2 with mode  720x400 70 720 738 846
> > 900 400 412 414 449 0x6 0x40 1305601
> > Checking pipe C connector DP-2 with mode  720x400 70 720 738 846
> > 900 400 412 414 449 0x6 0x40 1305601
> > Checking pipe D connector DP-2 with mode  720x400 70 720 738 846
> > 900 400 412 414 449 0x6 0x40 1305601
> > SUCCESS (0.743s)
> > 
> > Signed-off-by: Khaled Almahallawy <khaled.almahallawy at intel.com>
> > ---
> >  tests/kms_invalid_dotclock.c | 54 +++++++++++++++++---------------
> > ----
> >  1 file changed, 26 insertions(+), 28 deletions(-)
> > 
> > diff --git a/tests/kms_invalid_dotclock.c
> > b/tests/kms_invalid_dotclock.c
> > index 402629ab097f..aab765136d0e 100644
> > --- a/tests/kms_invalid_dotclock.c
> > +++ b/tests/kms_invalid_dotclock.c
> > @@ -48,9 +48,10 @@ static int
> >  test_output(data_t *data)
> >  {
> >  	igt_output_t *output = data->output;
> > -	drmModeModeInfo mode;
> > +	drmModeConnector *connector = output->config.connector;
> > +	drmModeModeInfo *mode;
> >  	struct igt_fb fb;
> > -	int i;
> > +	int i, j;
> >  
> >  	/*
> >  	 * FIXME When we have a fixed mode, the kernel will ignore
> > @@ -63,33 +64,30 @@ test_output(data_t *data)
> >  	if (has_scaling_mode_prop(data))
> >  		return 0;
> >  
> > -	/*
> > -	 * FIXME test every mode we have to be more
> > -	 * sure everything is really getting rejected?
> > -	 */
> > -	mode = *igt_output_get_mode(output);
> > -	mode.clock = data->max_dotclock + 1;
> > -
> > -	igt_create_fb(data->drm_fd,
> > -		      mode.hdisplay, mode.vdisplay,
> > -		      DRM_FORMAT_XRGB8888,
> > -		      LOCAL_DRM_FORMAT_MOD_NONE,
> > -		      &fb);
> > -
> > -	for (i = 0; i < data->res->count_crtcs; i++) {
> > -		int ret;
> > -
> > -		igt_info("Checking pipe %c connector %s with mode
> > %s\n",
> > -			 'A'+i, output->name, mode.name);
> > -
> > -		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
> > -				     fb.fb_id, 0, 0,
> > -				     &output->id, 1, &mode);
> > -		igt_assert_lt(ret, 0);
> > +	for (j = 0; j < connector->count_modes; j++) {
> > +		mode = &connector->modes[j];
> > +		mode->clock = data->max_dotclock + 1;
> > +
> > +		igt_create_fb(data->drm_fd,
> > +			mode->hdisplay, mode->vdisplay,
> > +			DRM_FORMAT_XRGB8888,
> > +			LOCAL_DRM_FORMAT_MOD_NONE,
> > +			&fb);
> > +
> > +		for (i = 0; i < data->res->count_crtcs; i++) {
> > +			int ret;
> > +
> > +			igt_info("Checking pipe %c connector %s with
> > mode",
> > +				'A'+i, output->name);
> > +			kmstest_dump_mode(mode);
> > +			ret = drmModeSetCrtc(data->drm_fd, data->res-
> > >crtcs[i],
> > +					fb.fb_id, 0, 0,
> > +					&output->id, 1, mode);
> > +			igt_assert_lt(ret, 0);
> > +		}
> > +
> > +		igt_remove_fb(data->drm_fd, &fb);
> >  	}
> > -
> > -	igt_remove_fb(data->drm_fd, &fb);
> > -
> >  	return 1;
> >  }
> >  
> > -- 
> > 2.25.1