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

Anshuman Gupta
2019-09-13T09:09:57Z

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
>