Re: [igt-dev] [PATCH i-g-t 2/6] tests/kms_invalid_mode: Convert the max dotclock test into a subtest

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

On Tue, Oct 05, 2021 at 01:33:58PM +0300, Petri Latvala wrote:
> On Tue, Oct 05, 2021 at 03:43:50PM +0530, Karthik B S wrote:
> > On 9/16/2021 9:05 PM, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We want to add different subtests to kms_invalud_mode. Convert
> > 
> > Hi,
> > 
> > Typo: 'kms_invalud_mode'
> > 
> > > the current max dotclock test into a subtest.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >   tests/i915/kms_invalid_mode.c | 80 +++++++++++++++++++++++++----------
> > >   1 file changed, 57 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/tests/i915/kms_invalid_mode.c b/tests/i915/kms_invalid_mode.c
> > > index dc30e6bb5444..8b24993e40a3 100644
> > > --- a/tests/i915/kms_invalid_mode.c
> > > +++ b/tests/i915/kms_invalid_mode.c
> > > @@ -27,13 +27,16 @@
> > >   IGT_TEST_DESCRIPTION("Make sure all modesets are rejected when the requested mode is invalid");
> > > -typedef struct {
> > > +typedef struct _data data_t;
> > > +
> > > +struct _data {
> > >   	int drm_fd;
> > >   	igt_display_t display;
> > >   	igt_output_t *output;
> > >   	drmModeResPtr res;
> > >   	int max_dotclock;
> > > -} data_t;
> > > +	bool (*adjust_mode)(data_t *data, drmModeModeInfoPtr mode);
> > > +};
> > >   static bool has_scaling_mode_prop(data_t *data)
> > >   {
> > > @@ -63,13 +66,10 @@ can_bigjoiner(data_t *data)
> > >   	return false;
> > >   }
> > > -static int
> > > -test_output(data_t *data)
> > > +static bool
> > > +adjust_mode_clock_too_high(data_t *data, drmModeModeInfoPtr mode)
> > >   {
> > > -	igt_output_t *output = data->output;
> > > -	drmModeModeInfo mode;
> > > -	struct igt_fb fb;
> > > -	int i;
> > > +	igt_require(data->max_dotclock != 0);
> > >   	/*
> > >   	 * FIXME When we have a fixed mode, the kernel will ignore
> > > @@ -80,14 +80,28 @@ test_output(data_t *data)
> > >   	 * test on  any connector with a fixed mode.
> > >   	 */
> > >   	if (has_scaling_mode_prop(data))
> > > -		return 0;
> > > +		return false;
> > > +
> > > +	mode->clock = data->max_dotclock + 1;
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +static int
> > > +test_output(data_t *data)
> > > +{
> > > +	igt_output_t *output = data->output;
> > > +	drmModeModeInfo mode;
> > > +	struct igt_fb fb;
> > > +	int i;
> > >   	/*
> > >   	 * 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;
> > > +	if (!data->adjust_mode(data, &mode))
> > > +		return 0;
> > >   	/*
> > >   	 * Newer platforms can support modes higher than the maximum dot clock
> > > @@ -154,25 +168,45 @@ static int i915_max_dotclock(data_t *data)
> > >   	return max_dotclock;
> > >   }
> > > +static const struct {
> > > +	const char *name;
> > > +	bool (*adjust_mode)(data_t *data, drmModeModeInfoPtr mode);
> > > +} subtests[] = {
> > > +	{ .name = "clock-too-high",
> > > +	  .adjust_mode = adjust_mode_clock_too_high,
> > > +	},
> > > +};
> > > +
> > >   static data_t data;
> > > -igt_simple_main
> > > +igt_main
> > >   {
> > > -	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > > -	igt_require_intel(data.drm_fd);
> > > +	igt_fixture {
> > > +		igt_skip_on_simulation();
> > 
> > 'igt_skip_on_simulation()' is not used in any other kms tests now. Do we
> > still need this?
> 
> Drive-by commenting: No, please remove the call.

Ack. Just some old copy pasta.

Though why does the function even exist anymore?

-- 
Ville Syrjälä
Intel