Re: [igt-dev] [i-g-t] tests: Remove kms_crtc_background_color test

["Laxminarayan Bharadiya, Pankaj" <pankaj.laxminarayan.bharadiya@intel.com>]

> -----Original Message-----
> From: Sharma, Swati2 <swati2.sharma@intel.com>
> Sent: 02 September 2020 13:59
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; igt-dev@lists.freedesktop.org;
> Martin Peres <martin.peres@linux.intel.com>
> Subject: Re: [igt-dev] [i-g-t] tests: Remove kms_crtc_background_color test
> 
> 
> 
> On 28-Jul-20 8:53 PM, Pankaj Bharadiya wrote:
> > BACKGROUND_COLOR property is not supported in kernel as of now.
> > Following patch attempted to add support but never got merged due to
> > lack of userspace support.
> >
> > https://patchwork.freedesktop.org/patch/333632/?series=67424&rev=1
> >
> > This test case is always skipped, as it does not find the
> > BACKGROUND_COLOR prop support hence remove it.
> 
> Why we have to remove the test? What if its supported in kernel in future?
> If test is skipping; we can add it to blacklist (listing it as expected
> skip)

IMO, first of all, this code should not have been added unless kernel supports it.
It's a dead code.

I don’t think there a plan to add BACKGROUND_COLOR property in kernel since
we don't have any reference user space implantation which uses/need this property.

Thanks,
Pankaj

> 
> Adding Martin Peres.
> >
> > Signed-off-by: Pankaj Bharadiya
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > ---
> >   lib/igt_kms.c                     |   1 -
> >   lib/igt_kms.h                     |   3 +-
> >   tests/Makefile.sources            |   1 -
> >   tests/kms_crtc_background_color.c | 186 ------------------------------
> >   tests/meson.build                 |   1 -
> >   5 files changed, 1 insertion(+), 191 deletions(-)
> >   delete mode 100644 tests/kms_crtc_background_color.c
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c index f57972f19..003f6af7b
> > 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -398,7 +398,6 @@ const char * const
> igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> >   };
> >
> >   const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> > -	[IGT_CRTC_BACKGROUND] = "background_color",
> >   	[IGT_CRTC_CTM] = "CTM",
> >   	[IGT_CRTC_GAMMA_LUT] = "GAMMA_LUT",
> >   	[IGT_CRTC_GAMMA_LUT_SIZE] = "GAMMA_LUT_SIZE", diff --git
> > a/lib/igt_kms.h b/lib/igt_kms.h index 26dc9f5fb..954f7be52 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -96,8 +96,7 @@ void kmstest_restore_vt_mode(void);
> >   void kmstest_set_vt_text_mode(void);
> >
> >   enum igt_atomic_crtc_properties {
> > -       IGT_CRTC_BACKGROUND = 0,
> > -       IGT_CRTC_CTM,
> > +       IGT_CRTC_CTM = 0,
> >          IGT_CRTC_GAMMA_LUT,
> >          IGT_CRTC_GAMMA_LUT_SIZE,
> >          IGT_CRTC_DEGAMMA_LUT,
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
> > 93d7768c4..994011700 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -40,7 +40,6 @@ TESTS_progs = \
> >   	kms_ccs \
> >   	kms_concurrent \
> >   	kms_content_protection\
> > -	kms_crtc_background_color \
> >   	kms_cursor_crc \
> >   	kms_cursor_edge_walk \
> >   	kms_cursor_legacy \
> > diff --git a/tests/kms_crtc_background_color.c
> > b/tests/kms_crtc_background_color.c
> > deleted file mode 100644
> > index b4141b0df..000000000
> > --- a/tests/kms_crtc_background_color.c
> > +++ /dev/null
> > @@ -1,186 +0,0 @@
> > -/*
> > - * Copyright © 2013,2014 Intel Corporation
> > - *
> > - * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > - * copy of this software and associated documentation files (the
> > "Software"),
> > - * to deal in the Software without restriction, including without
> > limitation
> > - * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > - * and/or sell copies of the Software, and to permit persons to whom
> > the
> > - * Software is furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice (including
> > the next
> > - * paragraph) shall be included in all copies or substantial portions
> > of the
> > - * Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT
> > SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES
> > OR OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS
> > - * IN THE SOFTWARE.
> > - *
> > - */
> > -
> > -#include "igt.h"
> > -#include <math.h>
> > -
> > -
> > -IGT_TEST_DESCRIPTION("Test crtc background color feature");
> > -
> > -typedef struct {
> > -	int gfx_fd;
> > -	igt_display_t display;
> > -	struct igt_fb fb;
> > -	igt_crc_t ref_crc;
> > -	igt_pipe_crc_t *pipe_crc;
> > -} data_t;
> > -
> > -#define BLACK      0x000000           /* BGR 8bpc */
> > -#define CYAN       0xFFFF00           /* BGR 8bpc */
> > -#define PURPLE     0xFF00FF           /* BGR 8bpc */
> > -#define WHITE      0xFFFFFF           /* BGR 8bpc */
> > -
> > -#define BLACK64    0x000000000000     /* BGR 16bpc */
> > -#define CYAN64     0xFFFFFFFF0000     /* BGR 16bpc */
> > -#define PURPLE64   0xFFFF0000FFFF     /* BGR 16bpc */
> > -#define YELLOW64   0x0000FFFFFFFF     /* BGR 16bpc */
> > -#define WHITE64    0xFFFFFFFFFFFF     /* BGR 16bpc */
> > -#define RED64      0x00000000FFFF     /* BGR 16bpc */
> > -#define GREEN64    0x0000FFFF0000     /* BGR 16bpc */
> > -#define BLUE64     0xFFFF00000000     /* BGR 16bpc */
> > -
> > -static void
> > -paint_background(data_t *data, struct igt_fb *fb, drmModeModeInfo
> *mode,
> > -		uint32_t background, double alpha)
> > -{
> > -	cairo_t *cr;
> > -	int w, h;
> > -	double r, g, b;
> > -
> > -	w = mode->hdisplay;
> > -	h = mode->vdisplay;
> > -
> > -	cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
> > -
> > -	/* Paint with background color */
> > -	r = (double) (background & 0xFF) / 255.0;
> > -	g = (double) ((background & 0xFF00) >> 8) / 255.0;
> > -	b = (double) ((background & 0xFF0000) >> 16) / 255.0;
> > -	igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, alpha);
> > -
> > -	igt_put_cairo_ctx(cr);
> > -}
> > -
> > -static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
> > -			igt_plane_t *plane, int opaque_buffer, int plane_color,
> > -			uint64_t pipe_background_color)
> > -{
> > -	drmModeModeInfo *mode;
> > -	igt_display_t *display = &data->display;
> > -	int fb_id;
> > -	double alpha;
> > -
> > -	igt_output_set_pipe(output, pipe);
> > -
> > -	/* create the pipe_crc object for this pipe */
> > -	igt_pipe_crc_free(data->pipe_crc);
> > -	data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> > -
> > -	mode = igt_output_get_mode(output);
> > -
> > -	fb_id = igt_create_fb(data->gfx_fd,
> > -			mode->hdisplay, mode->vdisplay,
> > -			DRM_FORMAT_XRGB8888,
> > -			LOCAL_DRM_FORMAT_MOD_NONE, /* tiled */
> > -			&data->fb);
> > -	igt_assert(fb_id);
> > -
> > -	/* To make FB pixel win with background color, set alpha as full opaque
> */
> > -	igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND,
> pipe_background_color);
> > -	if (opaque_buffer)
> > -		alpha = 1.0;    /* alpha 1 is fully opque */
> > -	else
> > -		alpha = 0.0;    /* alpha 0 is fully transparent */
> > -	paint_background(data, &data->fb, mode, plane_color, alpha);
> > -
> > -	igt_plane_set_fb(plane, &data->fb);
> > -	igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -}
> > -
> > -static void cleanup_crtc(data_t *data, igt_output_t *output,
> > igt_plane_t *plane) -{
> > -	igt_display_t *display = &data->display;
> > -
> > -	igt_pipe_crc_free(data->pipe_crc);
> > -	data->pipe_crc = NULL;
> > -
> > -	igt_remove_fb(data->gfx_fd, &data->fb);
> > -
> > -	igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND,
> BLACK64);
> > -	igt_plane_set_fb(plane, NULL);
> > -	igt_output_set_pipe(output, PIPE_ANY);
> > -
> > -	igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -}
> > -
> > -static void test_crtc_background(data_t *data) -{
> > -	igt_display_t *display = &data->display;
> > -	igt_output_t *output;
> > -	enum pipe pipe;
> > -	int valid_tests = 0;
> > -
> > -	for_each_pipe_with_valid_output(display, pipe, output) {
> > -		igt_plane_t *plane;
> > -
> > -		igt_output_set_pipe(output, pipe);
> > -
> > -		plane = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> > -		igt_require(igt_pipe_has_prop(display, pipe,
> IGT_CRTC_BACKGROUND));
> > -
> > -		prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
> > -
> > -		/* Now set background without using a plane, i.e.,
> > -		 * Disable the plane to let hw background color win blend. */
> > -		igt_plane_set_fb(plane, NULL);
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, PURPLE64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		/* Try few other background colors */
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, CYAN64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, YELLOW64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, RED64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, GREEN64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, BLUE64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		igt_pipe_set_prop_value(display, pipe,
> IGT_CRTC_BACKGROUND, WHITE64);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -
> > -		valid_tests++;
> > -		cleanup_crtc(data, output, plane);
> > -	}
> > -	igt_require_f(valid_tests, "no valid crtc/connector combinations
> found\n");
> > -}
> > -
> > -igt_simple_main
> > -{
> > -	data_t data = {};
> > -
> > -	data.gfx_fd = drm_open_driver(DRIVER_INTEL);
> > -	igt_require_pipe_crc(data.gfx_fd);
> > -	igt_display_require(&data.display, data.gfx_fd);
> > -
> > -	test_crtc_background(&data);
> > -
> > -	igt_display_fini(&data.display);
> > -}
> > diff --git a/tests/meson.build b/tests/meson.build index
> > ca792ed86..a6e1b7fae 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -24,7 +24,6 @@ test_progs = [
> >   	'kms_ccs',
> >   	'kms_concurrent',
> >   	'kms_content_protection',
> > -	'kms_crtc_background_color',
> >   	'kms_cursor_crc',
> >   	'kms_cursor_edge_walk',
> >   	'kms_cursor_legacy',
> >
> 
> --
> ~Swati Sharma
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev