[igt-dev] [PATCH i-g-t 1/2] tests/kms_ccs: Refactor code to add CCS modifiers.

[Dhinakaran Pandiyan]

On Wed, 2019-03-20 at 21:49 +0200, Ville Syrjälä wrote:
> On Wed, Mar 20, 2019 at 12:37:32PM -0700, Dhinakaran Pandiyan wrote:
> > Currently only Y tiled CCS modifier is tested, rewrite the
> > functions so
> > that we can add support for other CCS modifiers.
> > 
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  tests/kms_ccs.c | 52 +++++++++++++++++++++++++++++++++----------
> > ------
> >  1 file changed, 35 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> > index a74bd706..f780a01c 100644
> > --- a/tests/kms_ccs.c
> > +++ b/tests/kms_ccs.c
> > @@ -58,6 +58,7 @@ typedef struct {
> >  	enum test_flags flags;
> >  	igt_plane_t *plane;
> >  	igt_pipe_crc_t *pipe_crc;
> > +	uint64_t ccs_modifier;
> >  } data_t;
> >  
> >  static const struct {
> > @@ -69,6 +70,10 @@ static const struct {
> >  	{0.0, 1.0, 0.0}
> >  };
> >  
> > +static const uint64_t ccs_modifiers[] = {
> > +	LOCAL_I915_FORMAT_MOD_Y_TILED_CCS
> > +};
> > +
> >  /*
> >   * Limit maximum used sprite plane width so this test will not
> > mistakenly
> >   * fail on hardware limitations which are not interesting to this
> > test.
> > @@ -140,7 +145,8 @@ modifiers_ptr(struct
> > local_drm_format_modifier_blob *blob)
> >  	return (struct local_drm_format_modifier *)(((char *)blob) +
> > blob->modifiers_offset);
> >  }
> >  
> > -static bool plane_has_format_with_ccs(data_t *data, igt_plane_t
> > *plane, uint32_t format)
> > +static bool plane_has_format_with_ccs(data_t *data, igt_plane_t
> > *plane,
> > +				      uint32_t format)
> >  {
> >  	drmModePropertyBlobPtr blob;
> >  	struct local_drm_format_modifier_blob *blob_data;
> 
> Hmm. I thought I changed this to use the igt_kms thing. I guess it
> never
> got pushed.

I'll be happy to review if you can find and send it to the list. Thanks
for the reviews.

-DK
> 
> > @@ -178,7 +184,7 @@ static bool plane_has_format_with_ccs(data_t
> > *data, igt_plane_t *plane, uint32_t
> >  	last_mod = &modifiers[blob_data->count_modifiers];
> >  	igt_assert_lte(((char *) last_mod - (char *) blob_data), blob-
> > >length);
> >  	for (int i = 0; i < blob_data->count_modifiers; i++) {
> > -		if (modifiers[i].modifier !=
> > LOCAL_I915_FORMAT_MOD_Y_TILED_CCS)
> > +		if (modifiers[i].modifier != data->ccs_modifier)
> >  			continue;
> >  
> >  		if (modifiers[i].offset > fmt_idx ||
> > @@ -226,7 +232,7 @@ static void generate_fb(data_t *data, struct
> > igt_fb *fb,
> >  	 * available FIFO configurations.
> >  	 */
> >  	if (fb_flags & FB_COMPRESSED)
> > -		modifier = LOCAL_I915_FORMAT_MOD_Y_TILED_CCS;
> > +		modifier = data->ccs_modifier;
> >  	else if (!(fb_flags & FB_HAS_PLANE))
> >  		modifier = LOCAL_I915_FORMAT_MOD_Y_TILED;
> >  	else
> > @@ -313,6 +319,7 @@ static bool try_config(data_t *data, enum
> > test_fb_flags fb_flags,
> >  	if (data->plane && fb_flags & FB_COMPRESSED) {
> >  		if (!plane_has_format_with_ccs(data, data->plane,
> > DRM_FORMAT_XRGB8888))
> >  			return false;
> > +
> >  		generate_fb(data, &fb, min(MAX_SPRITE_PLANE_WIDTH,
> > drm_mode->hdisplay),
> >  			    drm_mode->vdisplay,
> >  			    (fb_flags & ~FB_COMPRESSED) |
> > FB_HAS_PLANE);
> > @@ -369,21 +376,10 @@ static bool try_config(data_t *data, enum
> > test_fb_flags fb_flags,
> >  	return true;
> >  }
> >  
> > -static int test_output(data_t *data)
> > -{
> > -	igt_display_t *display = &data->display;
> > +static int test_ccs(data_t *data)
> > +{	int valid_tests = 0;
> >  	igt_crc_t crc, ref_crc;
> >  	enum test_fb_flags fb_flags = 0;
> > -	int valid_tests = 0;
> > -
> > -	igt_display_require_output_on_pipe(display, data->pipe);
> > -
> > -	/* Sets data->output with a valid output. */
> > -	for_each_valid_output_on_pipe(display, data->pipe, data-
> > >output) {
> > -		break;
> > -	}
> > -
> > -	igt_output_set_pipe(data->output, data->pipe);
> >  
> >  	if (data->flags & TEST_CRC) {
> >  		data->pipe_crc = igt_pipe_crc_new(data->drm_fd, data-
> > >pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> > @@ -411,7 +407,29 @@ static int test_output(data_t *data)
> >  		valid_tests += try_config(data, fb_flags |
> > FB_COMPRESSED | FB_ZERO_AUX_STRIDE , NULL);
> >  	}
> >  
> > -	igt_output_set_pipe(data->output, PIPE_ANY);
> > +	return valid_tests;
> > +}
> > +
> > +static int test_output(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	int i, valid_tests = 0;
> > +
> > +	igt_display_require_output_on_pipe(display, data->pipe);
> > +
> > +	/* Sets data->output with a valid output. */
> > +	for_each_valid_output_on_pipe(display, data->pipe, data-
> > >output) {
> > +		break;
> > +	}
> 
> I think we have a nicer way to do this these days.
> 
> Patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> > +
> > +	igt_output_set_pipe(data->output, data->pipe);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ccs_modifiers); i++) {
> > +		data->ccs_modifier = ccs_modifiers[i];
> > +		valid_tests += test_ccs(data);
> > +	}
> > +
> > +	igt_output_set_pipe(data->output, PIPE_NONE);
> >  	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC
> > : COMMIT_LEGACY);
> >  
> >  	return valid_tests;
> > -- 
> > 2.17.1
> 
>