[igt-dev] [PATCH i-g-t 1/4] tests/kms_color: Refactor invalid LUT size tests

[Ville Syrjälä]

On Fri, Sep 03, 2021 at 05:04:30AM +0000, Modem, Bhanuprakash wrote:
> > From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Ville
> > Syrjala
> > Sent: Friday, May 14, 2021 6:57 PM
> > To: igt-dev at lists.freedesktop.org
> > Subject: [igt-dev] [PATCH i-g-t 1/4] tests/kms_color: Refactor invalid LUT
> > size tests
> > 
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Reuse the same code for all invalid LUT size tests.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  tests/kms_color_helper.c | 81 +++++++++++++---------------------------
> >  1 file changed, 26 insertions(+), 55 deletions(-)
> > 
> > diff --git a/tests/kms_color_helper.c b/tests/kms_color_helper.c
> > index 5f223a88129f..f9fde340f4b7 100644
> > --- a/tests/kms_color_helper.c
> > +++ b/tests/kms_color_helper.c
> > @@ -277,77 +277,48 @@ pipe_set_property_blob(igt_pipe_t *pipe,
> >  				       COMMIT_ATOMIC : COMMIT_LEGACY);
> >  }
> > 
> > -void
> > -invalid_gamma_lut_sizes(data_t *data)
> > +static void
> > +invalid_lut_sizes(data_t *data, enum igt_atomic_crtc_properties prop, int
> > size)
> >  {
> >  	igt_display_t *display = &data->display;
> >  	igt_pipe_t *pipe = &display->pipes[0];
> > -	size_t gamma_lut_size = data->gamma_lut_size *
> > -				sizeof(struct drm_color_lut);
> > -	struct drm_color_lut *gamma_lut;
> > +	struct drm_color_lut *lut;
> > +	size_t lut_size = size * sizeof(lut[0]);
> > 
> > -	igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_GAMMA_LUT));
> > +	igt_require(igt_pipe_obj_has_prop(pipe, prop));
> > 
> > -	gamma_lut = malloc(gamma_lut_size * 2);
> > +	lut = malloc(lut_size * 2);
> > 
> >  	igt_display_commit2(display,
> >  			    display->is_atomic ?
> >  			    COMMIT_ATOMIC : COMMIT_LEGACY);
> > 
> > -	igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT,
> > -					     gamma_lut, 1), -EINVAL);
> > -	igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT,
> > -					     gamma_lut, gamma_lut_size + 1),
> > -					     -EINVAL);
> > -	igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT,
> > -					     gamma_lut, gamma_lut_size - 1),
> > -					     -EINVAL);
> > -	igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT,
> > -					     gamma_lut, gamma_lut_size +
> > -					     sizeof(struct drm_color_lut)),
> > -					     -EINVAL);
> > -	igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_GAMMA_LUT,
> > -		      pipe->crtc_id), -EINVAL);
> > -	igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_GAMMA_LUT,
> > -		      4096 * 4096), -EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob(pipe, prop, lut,
> > +					     1), -EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob(pipe, prop, lut,
> > +					     lut_size + 1), -EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob(pipe, prop, lut,
> > +					     lut_size - 1), -EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob(pipe, prop, lut,
> > +					     lut_size + sizeof(struct drm_color_lut)), -
> > EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob_id(pipe, prop,
> > +						pipe->crtc_id), -EINVAL);
> > +	igt_assert_eq(pipe_set_property_blob_id(pipe, prop,
> > +						4096 * 4096), -EINVAL);
> > 
> > -	free(gamma_lut);
> > +	free(lut);
> > +}
> > +
> > +void
> > +invalid_gamma_lut_sizes(data_t *data)
> > +{
> > +	return invalid_lut_sizes(data, IGT_CRTC_GAMMA_LUT, data-
> > >gamma_lut_size);
>  
> data->gamma_lut_size can be NULL.
> 
> gamma_lut_size in struct data_t is not initialized before calling the
> invalid_gamma_lut_sizes() 
> 
> igt_runner will run gamma test first which will cache gamma_lut_size &
> then try to run the size tests, so we didn't see any issue.
> If we try to run the size tests alone, we may get improper result.
> 
> Anyway it is not related this patch, but we must fix this at caller.

Hmm. Right. I think what we want to do is run the invalid LUT size tests
for each pipe since each pipe can have different LUT sizes. Currently we
just run them for pipe A.

-- 
Ville Syrjälä
Intel