Re: [igt-dev] [PATCH i-g-t 8/8] igt/i915/i915_pm_dc: DC3CO Video Playback Case IGT test

[Anshuman Gupta <anshuman.gupta@intel.com>]

On 2019-09-12 at 13:46:38 +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  tests/i915/i915_pm_dc.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 134 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index c1b03a9..4fab152 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -38,9 +38,20 @@
>  #define CHECK_DC6	2
>  #define CHECK_DC3CO     4
>  
> +/*Number of Frames Video Playback*/
> +#define VIDEO_FRAMES 30
> +
>  /* PSR2 status 9th bit is the PSR2 idle frame indication */
>  #define PSR2_IDLE_FRMAE_STS_MASK (1 << 9)
>  
> +/*Internal */
> +typedef struct {
> +	double r, g, b;
> +} color_t;
> +
> +igt_fb_t fb1, fb2;
> +igt_plane_t *primary;
> +
>  typedef struct {
>  	int drm_fd;
>  	int msr_fd;
> @@ -124,6 +135,7 @@ static uint32_t get_psr2_status(int debugfs_fd)
>  	ret = sscanf(str + 6, "%*c%*c%*c%x%*c", &psr2_sts);
>  	igt_debug("psr2 status 0x%x\n", psr2_sts);
>  	igt_assert_eq(ret, 1);
> +	return psr2_sts;
could you please make return statement part of previous patch.
>  }
>  
>  static bool psr2_idle_wait_entry(int debugfs_fd)
> @@ -134,8 +146,6 @@ static bool psr2_idle_wait_entry(int debugfs_fd)
>  
>  static void cleanup_dc_psr(data_t *data)
>  {
> -	igt_plane_t *primary;
> -
>  	primary = igt_output_get_plane_type(data->output,
>  					    DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(primary, NULL);
> @@ -143,10 +153,30 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> -static void setup_primary(data_t *data)
> +static void paint_rectangles(data_t *data,
> +				drmModeModeInfo *mode,
> +				color_t *colors,
> +				igt_fb_t *fb)
>  {
> -	igt_plane_t *primary;
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
>  
> +	colors[i-1].r, colors[i-1].g, colors[i-1].b);
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
> +static void setup_primary(data_t *data)
> +{
>  	primary = igt_output_get_plane_type(data->output,
>  					    DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(primary, NULL);
> @@ -160,6 +190,53 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_rgb_fb(data_t *data)
> +{
> +	int fb_id1;
> +	color_t red_green_blue[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 0.0, 0.0, 1.0 },
> +	};
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_plane_set_fb(primary, NULL);
> +	fb_id1 = igt_create_fb(data->drm_fd,
> +				data->mode->hdisplay,
> +				data->mode->vdisplay,
> +				DRM_FORMAT_XRGB8888,
> +				LOCAL_DRM_FORMAT_MOD_NONE,
> +				&fb1);
> +	igt_assert(fb_id1);
> +	paint_rectangles(data, data->mode, red_green_blue, &fb1);
> +}
> +
> +static void create_rgr_fb(data_t *data)
> +{
> +	int fb_id2;
> +	color_t red_green_red[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	primary = igt_output_get_plane_type(data->output,
> +	DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_plane_set_fb(primary, NULL);
> +	fb_id2 = igt_create_fb(data->drm_fd,
> +				data->mode->hdisplay,
> +				data->mode->vdisplay,
> +				DRM_FORMAT_XRGB8888,
> +				LOCAL_DRM_FORMAT_MOD_NONE,
> +				&fb2);
> +
> +	igt_assert(fb_id2);
> +	paint_rectangles(data, data->mode, red_green_red, &fb2);
> +}
> +
create_rgr_fb and create_rgb_fb shares most part of code,
could you please create a single function like create_fb
and pass color and other different data to that function.
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -216,6 +293,49 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
> +{
> +	uint32_t dc3co_ctr_before_psr, dc3co_ctr_after_psr;
> +	uint32_t dc5_ctr_before, dc5_ctr_after;
> +	int i, delay;
> +
> +	setup_output(data);
> +
> +	create_rgb_fb(data);
> +	create_rgr_fb(data);
> +
> +	igt_plane_set_fb(primary, NULL);
> +
> +	dc3co_ctr_before_psr = read_dc_counter(data->drm_fd, dc_flag);
> +	dc5_ctr_before = read_dc_counter(data->drm_fd, CHECK_DC5);
> +	/*Calculate delay to generate idle frame*/
> +	delay = ((1000*1000*2)/data->mode->vrefresh);
> +
> +	for (i = 0; i < VIDEO_FRAMES; i++) {
> +		if (i % 2 == 0) {
> +			igt_plane_set_fb(primary, &fb1);
you can give a meaningfull name to fb1 like fb_rgr
> +			igt_display_commit(&data->display);
> +		} else {
> +			igt_plane_set_fb(primary, &fb2);
> +			igt_display_commit(&data->display);
> +		}
> +
> +	if (i == 0)
> +		igt_assert(psr2_idle_wait_entry(data->debugfs_fd));
> +
> +	usleep(delay);
> +	dc3co_ctr_after_psr = read_dc_counter(data->drm_fd, dc_flag);
> +	igt_assert_f(dc3co_ctr_before_psr <= dc3co_ctr_after_psr,
> +			"DC3CO counter got reset\n");
if both counters are equal then test will pass which is wrong, 
and message should be something like "DC3CO state is not achieved".
better to use check_dc_counter function, after loop ends
and pass the initial counter.
> +	dc3co_ctr_before_psr = dc3co_ctr_after_psr;
> +	}
> +
> +	dc5_ctr_after = read_dc_counter(data->drm_fd, CHECK_DC5);
> +	igt_assert_f(dc5_ctr_after == dc5_ctr_before,
> +			"System moved to DC5 state\n");
> +	cleanup_dc_psr(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -317,6 +437,16 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("DC3CO igt test for Video Playback Scenario");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		igt_require(IS_TIGERLAKE(data.devid));
> +		data.op_psr_mode = PSR_MODE_2;
> +		psr_enable(data.debugfs_fd, data.op_psr_mode);
> +		igt_require_f(edp_psr2_enabled(&data),
> +				"PSR2 is not enabled\n");
> +		test_dc3co_vpb_simulation(&data, CHECK_DC3CO);
> +	}
> +
>  	igt_describe("DC5 igt test with PSR Sleep state");
>  	igt_subtest("dc5-psr") {
>  		data.op_psr_mode = PSR_MODE_1;
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev