All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
@ 2019-01-29 22:13 Mamta Shukla
  2019-01-29 22:16 ` [igt-dev] [PATCH i-g-t, v2, 2/2] tests/kms_cursor_crc.c: Remove framebuffer at the end of the test Mamta Shukla
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Mamta Shukla @ 2019-01-29 22:13 UTC (permalink / raw)
  To: igt-dev, daniel, harry.wentland, hamohammed.sa, rodrigosiqueiramelo

In the kms_cursor_crc, the subtest to check exterme alpha values failed due
to different CRCs observed in hardware and software rendering. This patch fixes
CRC mismatch error in cursor-alpha-opaque and cursor-alpha-transparent subtests
by improving the order in which cursor was rendered in hardware test. Changes
made in test_cursor_alpha() to ensure cursor rendering in hardware test and
acquire CRC value:
- Add drmModeSetCursor() to set cursor image.
- Remove igt_display_commit() to reduce the chance of getting crc values cleared
  before it is acquired.
- Add igt_remove_fb() to remove locally created framebuffer after disabling the
  cursor plane in HW test

  Also, aligned this function with test_cursor_size and after these changes
  got passing results for alpha blending support added in VKMS CRC API.

Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
---
changes in v2:
-Modify commit message
-Add spaces around '='

 tests/kms_cursor_crc.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 3c9856d9..144b44b0 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -411,26 +411,28 @@ static void test_cursor_alpha(data_t *data, double a)
 	igt_crc_t crc, ref_crc;
 	cairo_t *cr;
 	uint32_t fb_id;
-	int curw=data->curw;
-	int curh=data->curh;
+	int curw = data->curw;
+	int curh = data->curh;
+	int ret;
 
 	/*alpha cursor fb*/
-	fb_id = igt_create_color_fb(data->drm_fd, curw, curh,
+	fb_id = igt_create_fb(data->drm_fd, curw, curh,
 				    DRM_FORMAT_ARGB8888,
 				    LOCAL_DRM_FORMAT_MOD_NONE,
-				    1.0, 1.0, 1.0,
 				    &data->fb);
 	igt_assert(fb_id);
 	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
-	draw_cursor(cr, 0, 0, curw, curh, a);
+	igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
 	igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
 
 	/*Hardware Test*/
 	cursor_enable(data);
-	igt_display_commit(display);
+	ret = drmModeSetCursor(data->drm_fd, data->output->config.crtc->crtc_id, data->fb.gem_handle, curw, curh);
+	igt_assert_eq(ret, 0);
 	igt_wait_for_vblank(data->drm_fd, data->pipe);
 	igt_pipe_crc_collect_crc(pipe_crc, &crc);
 	cursor_disable(data);
+	igt_remove_fb(data->drm_fd, &data->fb);
 
 	/*Software Test*/
 	cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
@@ -447,7 +449,6 @@ static void test_cursor_alpha(data_t *data, double a)
 	igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
 			0.0, 0.0, 0.0);
 	igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
-	igt_remove_fb(data->drm_fd, &data->fb);
 }
 
 static void test_cursor_transparent(data_t *data)
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [igt-dev] [PATCH i-g-t, v2, 2/2] tests/kms_cursor_crc.c: Remove framebuffer at the end of the test
  2019-01-29 22:13 [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Mamta Shukla
@ 2019-01-29 22:16 ` Mamta Shukla
  2019-01-29 22:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Mamta Shukla @ 2019-01-29 22:16 UTC (permalink / raw)
  To: igt-dev, daniel, harry.wentland, hamohammed.sa, rodrigosiqueiramelo

Use igt_remove_fb(data->drm_fd, &data->fb) to remove locally created
framebuffer at the end of the hardware test.

Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
---
No changes in v2.

 tests/kms_cursor_crc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 144b44b0..fd74fda5 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -583,6 +583,7 @@ static void test_cursor_size(data_t *data)
 		igt_pipe_crc_collect_crc(pipe_crc, &crc[i]);
 	}
 	cursor_disable(data);
+	igt_remove_fb(data->drm_fd, &data->fb);
 	/* Software test loop */
 	for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++) {
 		/* Now render the same in software and collect crc */
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
  2019-01-29 22:13 [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Mamta Shukla
  2019-01-29 22:16 ` [igt-dev] [PATCH i-g-t, v2, 2/2] tests/kms_cursor_crc.c: Remove framebuffer at the end of the test Mamta Shukla
@ 2019-01-29 22:50 ` Patchwork
  2019-01-30  3:03 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2019-01-31 12:49 ` [igt-dev] [PATCH i-g-t, v2, 1/2] " Rodrigo Siqueira
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-01-29 22:50 UTC (permalink / raw)
  To: Mamta Shukla; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
URL   : https://patchwork.freedesktop.org/series/55944/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5504 -> IGTPW_2316
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55944/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2316 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - fi-ilk-650:         DMESG-WARN [fdo#106387] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +2

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       {SKIP} [fdo#109271] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (44 -> 39)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-glk-j4005 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4798 -> IGTPW_2316

  CI_DRM_5504: b282332c2611e7f2e578846aaf74ceb5475e42f0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2316: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2316/
  IGT_4798: 998e0a4aedf10fb5f7c271018cd80d874668bf55 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2316/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
  2019-01-29 22:13 [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Mamta Shukla
  2019-01-29 22:16 ` [igt-dev] [PATCH i-g-t, v2, 2/2] tests/kms_cursor_crc.c: Remove framebuffer at the end of the test Mamta Shukla
  2019-01-29 22:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Patchwork
@ 2019-01-30  3:03 ` Patchwork
  2019-01-31 12:49 ` [igt-dev] [PATCH i-g-t, v2, 1/2] " Rodrigo Siqueira
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-01-30  3:03 UTC (permalink / raw)
  To: Mamta Shukla; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
URL   : https://patchwork.freedesktop.org/series/55944/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5504_full -> IGTPW_2316_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55944/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2316_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566]

  * igt@i915_suspend@shrink:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#109244]

  * igt@kms_busy@basic-modeset-b:
    - shard-snb:          NOTRUN -> FAIL [fdo#109490]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_color@pipe-a-ctm-max:
    - shard-kbl:          PASS -> FAIL [fdo#108147]
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232] +3
    - shard-apl:          PASS -> FAIL [fdo#103232] +4

  * igt@kms_cursor_crc@cursor-64x64-random:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166] +7

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +6
    - shard-kbl:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS
    - shard-kbl:          FAIL [fdo#106641] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-b:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] / [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +4

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-kbl:          FAIL [fdo#109350] -> PASS +1

  * igt@kms_cursor_crc@cursor-alpha-transparent:
    - shard-hsw:          FAIL [fdo#109350] -> PASS +1
    - shard-glk:          FAIL [fdo#109350] -> PASS
    - shard-apl:          FAIL [fdo#109350] -> PASS
    - shard-snb:          FAIL [fdo#109350] -> PASS

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-glk:          FAIL [fdo#105010] -> PASS
    - shard-kbl:          FAIL [fdo#105010] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109490]: https://bugs.freedesktop.org/show_bug.cgi?id=109490
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4798 -> IGTPW_2316
    * Piglit: piglit_4509 -> None

  CI_DRM_5504: b282332c2611e7f2e578846aaf74ceb5475e42f0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2316: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2316/
  IGT_4798: 998e0a4aedf10fb5f7c271018cd80d874668bf55 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2316/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
  2019-01-29 22:13 [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Mamta Shukla
                   ` (2 preceding siblings ...)
  2019-01-30  3:03 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-01-31 12:49 ` Rodrigo Siqueira
  2019-01-31 16:27   ` Mamta Shukla
  3 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Siqueira @ 2019-01-31 12:49 UTC (permalink / raw)
  To: Mamta Shukla; +Cc: igt-dev, hamohammed.sa, daniel


[-- Attachment #1.1: Type: text/plain, Size: 3256 bytes --]

Hi,

Thanks for your patch! :)

The patch looks good for me.

I just have one question: why did you split the patch into two parts?
IMHO this series can be a single patch.

Best Regards
Rodrigo Siqueira

On 01/30, Mamta Shukla wrote:
> In the kms_cursor_crc, the subtest to check exterme alpha values failed due
> to different CRCs observed in hardware and software rendering. This patch fixes
> CRC mismatch error in cursor-alpha-opaque and cursor-alpha-transparent subtests
> by improving the order in which cursor was rendered in hardware test. Changes
> made in test_cursor_alpha() to ensure cursor rendering in hardware test and
> acquire CRC value:
> - Add drmModeSetCursor() to set cursor image.
> - Remove igt_display_commit() to reduce the chance of getting crc values cleared
>   before it is acquired.
> - Add igt_remove_fb() to remove locally created framebuffer after disabling the
>   cursor plane in HW test
> 
>   Also, aligned this function with test_cursor_size and after these changes
>   got passing results for alpha blending support added in VKMS CRC API.
> 
> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> ---
> changes in v2:
> -Modify commit message
> -Add spaces around '='
> 
>  tests/kms_cursor_crc.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 3c9856d9..144b44b0 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -411,26 +411,28 @@ static void test_cursor_alpha(data_t *data, double a)
>  	igt_crc_t crc, ref_crc;
>  	cairo_t *cr;
>  	uint32_t fb_id;
> -	int curw=data->curw;
> -	int curh=data->curh;
> +	int curw = data->curw;
> +	int curh = data->curh;
> +	int ret;
>  
>  	/*alpha cursor fb*/
> -	fb_id = igt_create_color_fb(data->drm_fd, curw, curh,
> +	fb_id = igt_create_fb(data->drm_fd, curw, curh,
>  				    DRM_FORMAT_ARGB8888,
>  				    LOCAL_DRM_FORMAT_MOD_NONE,
> -				    1.0, 1.0, 1.0,
>  				    &data->fb);
>  	igt_assert(fb_id);
>  	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> -	draw_cursor(cr, 0, 0, curw, curh, a);
> +	igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
>  	igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
>  
>  	/*Hardware Test*/
>  	cursor_enable(data);
> -	igt_display_commit(display);
> +	ret = drmModeSetCursor(data->drm_fd, data->output->config.crtc->crtc_id, data->fb.gem_handle, curw, curh);
> +	igt_assert_eq(ret, 0);
>  	igt_wait_for_vblank(data->drm_fd, data->pipe);
>  	igt_pipe_crc_collect_crc(pipe_crc, &crc);
>  	cursor_disable(data);
> +	igt_remove_fb(data->drm_fd, &data->fb);
>  
>  	/*Software Test*/
>  	cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> @@ -447,7 +449,6 @@ static void test_cursor_alpha(data_t *data, double a)
>  	igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
>  			0.0, 0.0, 0.0);
>  	igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> -	igt_remove_fb(data->drm_fd, &data->fb);
>  }
>  
>  static void test_cursor_transparent(data_t *data)
> -- 
> 2.17.1
> 

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
  2019-01-31 12:49 ` [igt-dev] [PATCH i-g-t, v2, 1/2] " Rodrigo Siqueira
@ 2019-01-31 16:27   ` Mamta Shukla
  2019-01-31 16:45     ` Wentland, Harry
  0 siblings, 1 reply; 7+ messages in thread
From: Mamta Shukla @ 2019-01-31 16:27 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: igt-dev, Haneen Mohammed, Daniel Vetter

Thank you for the feedback.
On Thu, Jan 31, 2019 at 6:19 PM Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> Hi,
>
> Thanks for your patch! :)
>
> The patch looks good for me.
>
> I just have one question: why did you split the patch into two parts?
> IMHO this series can be a single patch.
>
The second patch in the series includes addition of igt_remove_fb() to remove
framebuffer initialized earlier using igt_create_fb(). As this deals
with a separate
subtest i.e cursor-size-change. That's why created a separate patch.

Patch-[1] https://patchwork.freedesktop.org/patch/281891/

Should I include the name of the subtest as well in commit message to
make it  more clear?

> Best Regards
> Rodrigo Siqueira
>
> On 01/30, Mamta Shukla wrote:
> > In the kms_cursor_crc, the subtest to check exterme alpha values failed due
> > to different CRCs observed in hardware and software rendering. This patch fixes
> > CRC mismatch error in cursor-alpha-opaque and cursor-alpha-transparent subtests
> > by improving the order in which cursor was rendered in hardware test. Changes
> > made in test_cursor_alpha() to ensure cursor rendering in hardware test and
> > acquire CRC value:
> > - Add drmModeSetCursor() to set cursor image.
> > - Remove igt_display_commit() to reduce the chance of getting crc values cleared
> >   before it is acquired.
> > - Add igt_remove_fb() to remove locally created framebuffer after disabling the
> >   cursor plane in HW test
> >
> >   Also, aligned this function with test_cursor_size and after these changes
> >   got passing results for alpha blending support added in VKMS CRC API.
> >
> > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> > ---
> > changes in v2:
> > -Modify commit message
> > -Add spaces around '='
> >
> >  tests/kms_cursor_crc.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 3c9856d9..144b44b0 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -411,26 +411,28 @@ static void test_cursor_alpha(data_t *data, double a)
> >       igt_crc_t crc, ref_crc;
> >       cairo_t *cr;
> >       uint32_t fb_id;
> > -     int curw=data->curw;
> > -     int curh=data->curh;
> > +     int curw = data->curw;
> > +     int curh = data->curh;
> > +     int ret;
> >
> >       /*alpha cursor fb*/
> > -     fb_id = igt_create_color_fb(data->drm_fd, curw, curh,
> > +     fb_id = igt_create_fb(data->drm_fd, curw, curh,
> >                                   DRM_FORMAT_ARGB8888,
> >                                   LOCAL_DRM_FORMAT_MOD_NONE,
> > -                                 1.0, 1.0, 1.0,
> >                                   &data->fb);
> >       igt_assert(fb_id);
> >       cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> > -     draw_cursor(cr, 0, 0, curw, curh, a);
> > +     igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
> >       igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
> >
> >       /*Hardware Test*/
> >       cursor_enable(data);
> > -     igt_display_commit(display);
> > +     ret = drmModeSetCursor(data->drm_fd, data->output->config.crtc->crtc_id, data->fb.gem_handle, curw, curh);
> > +     igt_assert_eq(ret, 0);
> >       igt_wait_for_vblank(data->drm_fd, data->pipe);
> >       igt_pipe_crc_collect_crc(pipe_crc, &crc);
> >       cursor_disable(data);
> > +     igt_remove_fb(data->drm_fd, &data->fb);
> >
> >       /*Software Test*/
> >       cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> > @@ -447,7 +449,6 @@ static void test_cursor_alpha(data_t *data, double a)
> >       igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
> >                       0.0, 0.0, 0.0);
> >       igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> > -     igt_remove_fb(data->drm_fd, &data->fb);
> >  }
> >
> >  static void test_cursor_transparent(data_t *data)
> > --
> > 2.17.1
> >
>
> --
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
Mamta Shukla
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha()
  2019-01-31 16:27   ` Mamta Shukla
@ 2019-01-31 16:45     ` Wentland, Harry
  0 siblings, 0 replies; 7+ messages in thread
From: Wentland, Harry @ 2019-01-31 16:45 UTC (permalink / raw)
  To: Mamta Shukla, Rodrigo Siqueira; +Cc: igt-dev, Haneen Mohammed, Daniel Vetter

On 2019-01-31 11:27 a.m., Mamta Shukla wrote:
> Thank you for the feedback.
> On Thu, Jan 31, 2019 at 6:19 PM Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
>>
>> Hi,
>>
>> Thanks for your patch! :)
>>
>> The patch looks good for me.
>>
>> I just have one question: why did you split the patch into two parts?
>> IMHO this series can be a single patch.
>>
> The second patch in the series includes addition of igt_remove_fb() to remove
> framebuffer initialized earlier using igt_create_fb(). As this deals
> with a separate
> subtest i.e cursor-size-change. That's why created a separate patch.
> 
> Patch-[1] https://patchwork.freedesktop.org/patch/281891/
> 
> Should I include the name of the subtest as well in commit message to
> make it  more clear?
> 

I think it's fine like this, although more detail usually doesn't hurt.

Either way, both patches are
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

>> Best Regards
>> Rodrigo Siqueira
>>
>> On 01/30, Mamta Shukla wrote:
>>> In the kms_cursor_crc, the subtest to check exterme alpha values failed due
>>> to different CRCs observed in hardware and software rendering. This patch fixes
>>> CRC mismatch error in cursor-alpha-opaque and cursor-alpha-transparent subtests
>>> by improving the order in which cursor was rendered in hardware test. Changes
>>> made in test_cursor_alpha() to ensure cursor rendering in hardware test and
>>> acquire CRC value:
>>> - Add drmModeSetCursor() to set cursor image.
>>> - Remove igt_display_commit() to reduce the chance of getting crc values cleared
>>>   before it is acquired.
>>> - Add igt_remove_fb() to remove locally created framebuffer after disabling the
>>>   cursor plane in HW test
>>>
>>>   Also, aligned this function with test_cursor_size and after these changes
>>>   got passing results for alpha blending support added in VKMS CRC API.
>>>
>>> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
>>> ---
>>> changes in v2:
>>> -Modify commit message
>>> -Add spaces around '='
>>>
>>>  tests/kms_cursor_crc.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
>>> index 3c9856d9..144b44b0 100644
>>> --- a/tests/kms_cursor_crc.c
>>> +++ b/tests/kms_cursor_crc.c
>>> @@ -411,26 +411,28 @@ static void test_cursor_alpha(data_t *data, double a)
>>>       igt_crc_t crc, ref_crc;
>>>       cairo_t *cr;
>>>       uint32_t fb_id;
>>> -     int curw=data->curw;
>>> -     int curh=data->curh;
>>> +     int curw = data->curw;
>>> +     int curh = data->curh;
>>> +     int ret;
>>>
>>>       /*alpha cursor fb*/
>>> -     fb_id = igt_create_color_fb(data->drm_fd, curw, curh,
>>> +     fb_id = igt_create_fb(data->drm_fd, curw, curh,
>>>                                   DRM_FORMAT_ARGB8888,
>>>                                   LOCAL_DRM_FORMAT_MOD_NONE,
>>> -                                 1.0, 1.0, 1.0,
>>>                                   &data->fb);
>>>       igt_assert(fb_id);
>>>       cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
>>> -     draw_cursor(cr, 0, 0, curw, curh, a);
>>> +     igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
>>>       igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
>>>
>>>       /*Hardware Test*/
>>>       cursor_enable(data);
>>> -     igt_display_commit(display);
>>> +     ret = drmModeSetCursor(data->drm_fd, data->output->config.crtc->crtc_id, data->fb.gem_handle, curw, curh);
>>> +     igt_assert_eq(ret, 0);
>>>       igt_wait_for_vblank(data->drm_fd, data->pipe);
>>>       igt_pipe_crc_collect_crc(pipe_crc, &crc);
>>>       cursor_disable(data);
>>> +     igt_remove_fb(data->drm_fd, &data->fb);
>>>
>>>       /*Software Test*/
>>>       cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
>>> @@ -447,7 +449,6 @@ static void test_cursor_alpha(data_t *data, double a)
>>>       igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
>>>                       0.0, 0.0, 0.0);
>>>       igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
>>> -     igt_remove_fb(data->drm_fd, &data->fb);
>>>  }
>>>
>>>  static void test_cursor_transparent(data_t *data)
>>> --
>>> 2.17.1
>>>
>>
>> --
>> Rodrigo Siqueira
>> https://siqueira.tech
>> Graduate Student
>> Department of Computer Science
>> University of São Paulo
> 
> 
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-31 16:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-29 22:13 [igt-dev] [PATCH i-g-t, v2, 1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Mamta Shukla
2019-01-29 22:16 ` [igt-dev] [PATCH i-g-t, v2, 2/2] tests/kms_cursor_crc.c: Remove framebuffer at the end of the test Mamta Shukla
2019-01-29 22:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/2] tests/kms_cursor_crc.c: Improve test_cursor_alpha() Patchwork
2019-01-30  3:03 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-01-31 12:49 ` [igt-dev] [PATCH i-g-t, v2, 1/2] " Rodrigo Siqueira
2019-01-31 16:27   ` Mamta Shukla
2019-01-31 16:45     ` Wentland, Harry

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.