Re: [igt-dev] [PATCH i-g-t 3/3] Revert "tests/kms_invalid_mode: Test Cleanup"

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Oct 07, 2022 at 09:19:58PM +0530, Modem, Bhanuprakash wrote:
> On Fri-07-10-2022 07:10 pm, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This reverts commit 6725767a84109e934ca138d94df0482386c44532.
> > 
> > This was no cleanup. It does fifteen different things and
> > in the process flat out breaks the test at least on all
> > non-atomic drivers/platforms.
> 
> will it be useful [1]?
> 
> [1]: https://patchwork.freedesktop.org/series/109484/

Useful to specify a framebuffer? Sure. Will if fix the
thing? No.

> 
> - Bhanu
> 
> > 
> > Cc: Nidhi Gupta <nidhi1.gupta@intel.com>
> > Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   tests/kms_invalid_mode.c | 39 ++++++++++++++++++++++++---------------
> >   1 file changed, 24 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> > index 21ee91abf54b..0287ee02c1c4 100644
> > --- a/tests/kms_invalid_mode.c
> > +++ b/tests/kms_invalid_mode.c
> > @@ -35,6 +35,7 @@ struct _data {
> >   	enum pipe pipe;
> >   	igt_display_t display;
> >   	igt_output_t *output;
> > +	drmModeResPtr res;
> >   	int max_dotclock;
> >   	bool (*adjust_mode)(data_t *data, drmModeModeInfoPtr mode);
> >   };
> > @@ -181,29 +182,35 @@ static void
> >   test_output(data_t *data)
> >   {
> >   	igt_output_t *output = data->output;
> > +	drmModeModeInfo mode;
> >   	struct igt_fb fb;
> >   	int ret;
> > -	drmModeModeInfo *mode;
> > +	uint32_t crtc_id;
> >   
> > -	igt_output_set_pipe(output, data->pipe);
> > +	/*
> > +	 * FIXME test every mode we have to be more
> > +	 * sure everything is really getting rejected?
> > +	 */
> > +	mode = *igt_output_get_mode(output);
> > +	igt_require(data->adjust_mode(data, &mode));
> >   
> > -	igt_create_fb(data->drm_fd, 512, 512, DRM_FORMAT_XRGB8888,
> > +	igt_create_fb(data->drm_fd,
> > +		      max_t(uint16_t, mode.hdisplay, 64),
> > +		      max_t(uint16_t, mode.vdisplay, 64),
> > +		      DRM_FORMAT_XRGB8888,
> >   		      DRM_FORMAT_MOD_LINEAR,
> >   		      &fb);
> >   
> > -	for_each_connector_mode(output) {
> > -		mode = &output->config.connector->modes[j__];
> > -		igt_require(data->adjust_mode(data, mode));
> > -		igt_output_override_mode(output, mode);
> > -		ret = igt_display_try_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> > -		igt_assert(ret == -EINVAL);
> > -	}
> > +	kmstest_unset_all_crtcs(data->drm_fd, data->res);
> > +
> > +	crtc_id = data->display.pipes[data->pipe].crtc_id;
> > +
> > +	ret = drmModeSetCrtc(data->drm_fd, crtc_id,
> > +			     fb.fb_id, 0, 0,
> > +			     &output->id, 1, &mode);
> > +	igt_assert_lt(ret, 0);
> >   
> >   	igt_remove_fb(data->drm_fd, &fb);
> > -	igt_output_override_mode(output, NULL);
> > -	/*unset_all_crtcs*/
> > -	igt_display_reset(&data->display);
> > -	igt_display_commit(&data->display);
> >   }
> >   
> >   static int i915_max_dotclock(data_t *data)
> > @@ -283,10 +290,11 @@ igt_main
> >   		kmstest_set_vt_graphics_mode();
> >   
> >   		igt_display_require(&data.display, data.drm_fd);
> > +		data.res = drmModeGetResources(data.drm_fd);
> > +		igt_assert(data.res);
> >   
> >   		data.max_dotclock = i915_max_dotclock(&data);
> >   		igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
> > -		igt_display_require_output(&data.display);
> >   	}
> >   
> >   	igt_describe("Make sure all modesets are rejected when the requested mode is invalid");
> > @@ -306,5 +314,6 @@ igt_main
> >   	igt_fixture {
> >   		igt_display_fini(&data.display);
> >   		igt_reset_connectors();
> > +		drmModeFreeResources(data.res);
> >   	}
> >   }

-- 
Ville Syrjälä
Intel