[igt-dev] [v4 0/2] New subtest for VRR Flipline mode

[igt-dev] [v4 0/2] New subtest for VRR Flipline mode

[bhanuprakash.modem]

From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

To validate Adaptive-Sync's Flipline mode, make sure that flips happen at
flipline decision boundary.

Example: if monitor vrr range is 40 - 60Hz and

* flip at refresh_rate > 60Hz:
        Flip should happen at the flipline boundary & returned refresh rate
        would be 60Hz.
* flip at refresh_rate == 50Hz:
        Flip should happen right away so returned refresh rate is 50Hz.
* flip at refresh_rate < 40Hz:
        Flip should happen at the vmax so the returned refresh rate
        would be 40Hz.

Bhanuprakash Modem (2):
  tests/kms_vrr: Use atomic API for page flip
  tests/kms_vrr: Add new subtest to validate Flipline mode

 tests/kms_vrr.c | 181 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 117 insertions(+), 64 deletions(-)

--
2.20.1

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

[igt-dev] ✓ Fi.CI.BAT: success for New subtest for VRR Flipline mode (rev5)

[Patchwork]

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

== Series Details ==

Series: New subtest for VRR Flipline mode (rev5)
URL   : https://patchwork.freedesktop.org/series/77128/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9045 -> IGTPW_5008
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/index.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html

  * igt@i915_module_load@reload:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([i915#1982] / [k.org#205379])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-tgl-y/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-tgl-y/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-bsw-n3050:       [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       [PASS][7] -> [DMESG-WARN][8] ([i915#2203])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-kefka:       [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-u2:          [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_psr@primary_page_flip:
    - fi-tgl-y:           [PASS][13] -> [DMESG-WARN][14] ([i915#1982])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-tgl-y/igt@kms_psr@primary_page_flip.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-tgl-y/igt@kms_psr@primary_page_flip.html

  * igt@vgem_basic@unload:
    - fi-kbl-x1275:       [PASS][15] -> [DMESG-WARN][16] ([i915#62] / [i915#92])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-x1275/igt@vgem_basic@unload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-x1275/igt@vgem_basic@unload.html

  
#### Possible fixes ####

  * {igt@core_hotunplug@unbind-rebind}:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html

  * igt@debugfs_test@read_all_entries:
    - {fi-kbl-7560u}:     [INCOMPLETE][19] ([i915#2417]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-7560u/igt@debugfs_test@read_all_entries.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-7560u/igt@debugfs_test@read_all_entries.html

  * igt@gem_tiled_fence_blits@basic:
    - fi-tgl-y:           [DMESG-WARN][21] ([i915#402]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-tgl-y/igt@gem_tiled_fence_blits@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-tgl-y/igt@gem_tiled_fence_blits@basic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [DMESG-WARN][23] ([i915#1982]) -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-tgl-y:           [DMESG-WARN][25] ([i915#1982] / [i915#2411]) -> [DMESG-WARN][26] ([i915#2411])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-tgl-y/igt@i915_pm_rpm@basic-pci-d3-state.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-tgl-y/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][27] ([i915#62]) -> [DMESG-FAIL][28] ([i915#62] / [i915#95])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_flip@basic-flip-vs-dpms@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][29] ([i915#62] / [i915#92]) -> [DMESG-WARN][30] ([i915#62] / [i915#92] / [i915#95])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-dpms@a-dp1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-dpms@a-dp1.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][31] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][32] ([i915#62] / [i915#92]) +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

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

  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2417]: https://gitlab.freedesktop.org/drm/intel/issues/2417
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (46 -> 40)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5787 -> IGTPW_5008

  CI-20190529: 20190529
  CI_DRM_9045: c090e19126310a35b72cafa971bdbdfa1c5c87db @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5008: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/index.html
  IGT_5787: 0ec962017c8131de14e0cb038f7f76b1f17ed637 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_vrr@flipline

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/index.html

[-- Attachment #1.2: Type: text/html, Size: 10302 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

[igt-dev] ✓ Fi.CI.IGT: success for New subtest for VRR Flipline mode (rev5)

[Patchwork]

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

== Series Details ==

Series: New subtest for VRR Flipline mode (rev5)
URL   : https://patchwork.freedesktop.org/series/77128/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9045_full -> IGTPW_5008_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_5008_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_5008_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_5008_full:

### IGT changes ###

#### Possible regressions ####

  * {igt@kms_vrr@flipline} (NEW):
    - shard-tglb:         NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-tglb6/igt@kms_vrr@flipline.html
    - shard-iclb:         NOTRUN -> [SKIP][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb3/igt@kms_vrr@flipline.html

  
#### Warnings ####

  * igt@kms_ccs@pipe-d-crc-primary-basic:
    - shard-hsw:          [SKIP][3] ([fdo#109271]) -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-hsw2/igt@kms_ccs@pipe-d-crc-primary-basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-hsw1/igt@kms_ccs@pipe-d-crc-primary-basic.html

  
New tests
---------

  New tests have been introduced between CI_DRM_9045_full and IGTPW_5008_full:

### New IGT tests (1) ###

  * igt@kms_vrr@flipline:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_reloc@basic-many-active@rcs0:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([i915#1635] / [i915#2389])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-apl6/igt@gem_exec_reloc@basic-many-active@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-apl8/igt@gem_exec_reloc@basic-many-active@rcs0.html

  * igt@gem_exec_whisper@basic-fds-forked-all:
    - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk2/igt@gem_exec_whisper@basic-fds-forked-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk3/igt@gem_exec_whisper@basic-fds-forked-all.html

  * igt@kms_big_fb@yf-tiled-32bpp-rotate-270:
    - shard-apl:          [PASS][9] -> [DMESG-WARN][10] ([i915#1635] / [i915#1982]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-apl2/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-apl7/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html

  * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([i915#2122])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk8/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk4/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a1.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [PASS][13] -> [INCOMPLETE][14] ([i915#155])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_cursor@pipe-b-overlay-size-128:
    - shard-snb:          [PASS][15] -> [SKIP][16] ([fdo#109271])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-snb5/igt@kms_plane_cursor@pipe-b-overlay-size-128.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-snb1/igt@kms_plane_cursor@pipe-b-overlay-size-128.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +4 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vblank@pipe-b-wait-busy:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([i915#1982])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-kbl1/igt@kms_vblank@pipe-b-wait-busy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-kbl6/igt@kms_vblank@pipe-b-wait-busy.html

  * igt@kms_vblank@pipe-c-wait-busy:
    - shard-tglb:         [PASS][23] -> [DMESG-WARN][24] ([i915#1982])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-tglb8/igt@kms_vblank@pipe-c-wait-busy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-tglb2/igt@kms_vblank@pipe-c-wait-busy.html

  
#### Possible fixes ####

  * igt@gem_exec_whisper@basic-queues-priority:
    - shard-glk:          [DMESG-WARN][25] ([i915#118] / [i915#95]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk7/igt@gem_exec_whisper@basic-queues-priority.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk5/igt@gem_exec_whisper@basic-queues-priority.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][27] ([i915#1899]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb3/igt@i915_pm_dc@dc6-psr.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb6/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][29] ([i915#72]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk4/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][31] ([i915#2122]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [DMESG-WARN][33] ([i915#180]) -> [PASS][34] +3 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@b-vga1:
    - shard-snb:          [INCOMPLETE][35] ([i915#82]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-snb1/igt@kms_flip@flip-vs-suspend@b-vga1.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-snb4/igt@kms_flip@flip-vs-suspend@b-vga1.html

  * igt@kms_flip_event_leak:
    - shard-glk:          [INCOMPLETE][37] -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-glk7/igt@kms_flip_event_leak.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-glk9/igt@kms_flip_event_leak.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [DMESG-WARN][39] ([i915#1982]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-tglb:         [DMESG-WARN][41] ([i915#1982]) -> [PASS][42] +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][43] ([fdo#109441]) -> [PASS][44] +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb8/igt@kms_psr@psr2_primary_page_flip.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  * igt@perf@polling-small-buf:
    - shard-iclb:         [FAIL][45] ([i915#1722]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb3/igt@perf@polling-small-buf.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb3/igt@perf@polling-small-buf.html

  
#### Warnings ####

  * igt@kms_content_protection@atomic:
    - shard-apl:          [TIMEOUT][47] ([i915#1319] / [i915#1635] / [i915#1958]) -> [FAIL][48] ([fdo#110321] / [fdo#110336] / [i915#1635])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-apl7/igt@kms_content_protection@atomic.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-apl7/igt@kms_content_protection@atomic.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][49] ([fdo#109349]) -> [DMESG-WARN][50] ([i915#1226])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-iclb6/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-hsw:          [INCOMPLETE][51] -> [SKIP][52] ([fdo#109271])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-hsw8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-hsw8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-apl:          [FAIL][53] ([fdo#108145] / [i915#1635] / [i915#265]) -> [DMESG-FAIL][54] ([fdo#108145] / [i915#1635] / [i915#1982])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9045/shard-apl1/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/shard-apl7/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1226]: https://gitlab.freedesktop.org/drm/intel/issues/1226
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1722]: https://gitlab.freedesktop.org/drm/intel/issues/1722
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1899]: https://gitlab.freedesktop.org/drm/intel/issues/1899
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 8)
------------------------------

  Missing    (3): pig-skl-6260u pig-glk-j5005 pig-icl-1065g7 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5787 -> IGTPW_5008
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_9045: c090e19126310a35b72cafa971bdbdfa1c5c87db @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5008: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5008/index.html
  IGT_5787: 0ec962017c8131de14e0cb038f7f76b1f17ed637 @ 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_5008/index.html

[-- Attachment #1.2: Type: text/html, Size: 16157 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

[igt-dev] [v4 1/2] tests/kms_vrr: Use atomic API for page flip

[bhanuprakash.modem]

From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

We should avoid using drmModePageFlip as it'll only be used for
legacy drivers, instead, use igt_display_commit_atomic() API to
page flip for atomic display code path.

v2:
* Look for the page flip event not for the vblank event (Nicholas)
* Fix to flip with different FBs (Bhanu)
v3:
* s/get_vblank_event_ns/get_kernel_event_ns/ (Manasi)
* Add a comment to capture the flip event (Manasi)
* Make sure we are reading valid event (Bhanu)
* Test clean-up (Bhanu)
v4:
* s/*vblank_ns/*event_ns/ (Manasi)

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_vrr.c | 76 +++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index a0db90006d..471d765dec 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -47,6 +47,7 @@ typedef struct range {
 typedef struct data {
 	igt_display_t display;
 	int drm_fd;
+	igt_plane_t *primary;
 	igt_fb_t fb0;
 	igt_fb_t fb1;
 } data_t;
@@ -60,15 +61,18 @@ static uint64_t timespec_to_ns(struct timespec *ts)
 }
 
 /*
- * Gets a vblank event from DRM and returns its timestamp in nanoseconds.
+ * Gets an event from DRM and returns its timestamp in nanoseconds.
+ * Asserts if the event from DRM is not matched with requested one.
+ *
  * This blocks until the event is received.
  */
-static uint64_t get_vblank_event_ns(data_t *data)
+static uint64_t get_kernel_event_ns(data_t *data, uint32_t event)
 {
 	struct drm_event_vblank ev;
 
-	igt_set_timeout(1, "Waiting for vblank event\n");
+	igt_set_timeout(1, "Waiting for an event\n");
 	igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
+	igt_assert_eq(ev.base.type, event);
 	igt_reset_timeout();
 
 	return ev.tv_sec * NSECS_PER_SEC + ev.tv_usec * 1000ull;
@@ -126,11 +130,11 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
 }
 
 /* Returns a suitable vrr test frequency. */
-static uint32_t get_test_rate_ns(data_t *data, igt_output_t *output)
+static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
 {
 	drmModeModeInfo *mode = igt_output_get_mode(output);
 	range_t range;
-	uint32_t vtest;
+	uint64_t vtest;
 
 	/*
 	 * The frequency with the fastest convergence speed should be
@@ -165,7 +169,6 @@ static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
 static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 {
 	drmModeModeInfo mode = *igt_output_get_mode(output);
-	igt_plane_t *primary;
 	cairo_t *cr;
 
 	/* Reset output */
@@ -189,8 +192,8 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	igt_put_cairo_ctx(cr);
 
 	/* Take care of any required modesetting before the test begins. */
-	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	igt_plane_set_fb(primary, &data->fb0);
+	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(data->primary, &data->fb0);
 
 	igt_display_commit_atomic(&data->display,
 				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
@@ -207,35 +210,28 @@ wait_for_vblank(data_t *data, enum pipe pipe)
 	vbl.request.sequence = 1;
 	drmWaitVBlank(data->drm_fd, &vbl);
 
-	return get_vblank_event_ns(data);
+	return get_kernel_event_ns(data, DRM_EVENT_VBLANK);
 }
 
-/* Performs an asynchronous non-blocking page-flip on a pipe. */
-static int
-do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
+/* Performs an atomic non-blocking page-flip on a pipe. */
+static void
+do_flip(data_t *data, igt_fb_t *fb)
 {
-	igt_pipe_t *pipe = &data->display.pipes[pipe_id];
 	int ret;
 
 	igt_set_timeout(1, "Scheduling page flip\n");
 
-	/*
-	 * Only the legacy flip ioctl supports async flips.
-	 * It's also non-blocking, but returns -EBUSY if flipping too fast.
-	 * 2x monitor tests will need async flips in the atomic API.
-	 */
+	igt_plane_set_fb(data->primary, fb);
+
 	do {
-		ret = drmModePageFlip(data->drm_fd, pipe->crtc_id,
-				      fb->fb_id,
-				      DRM_MODE_PAGE_FLIP_EVENT |
-				      DRM_MODE_PAGE_FLIP_ASYNC,
-				      data);
+		ret = igt_display_try_commit_atomic(&data->display,
+				  DRM_MODE_ATOMIC_NONBLOCK |
+				  DRM_MODE_PAGE_FLIP_EVENT,
+				  data);
 	} while (ret == -EBUSY);
 
 	igt_assert_eq(ret, 0);
 	igt_reset_timeout();
-
-	return 0;
 }
 
 /*
@@ -246,34 +242,34 @@ do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
  * can arbitrarily restrict the bounds further than the absolute
  * min and max range. But VRR is really about extending the flip
  * to prevent stuttering or to match a source content rate.
- *
- * The only way to "present" at a fixed rate like userspace in a vendor
- * neutral manner is to do it with async flips. This avoids the need
- * to wait for next vblank and it should eventually converge at the
- * desired rate.
  */
 static uint32_t
 flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		 uint64_t rate_ns, uint64_t duration_ns)
 {
-	uint64_t start_ns, last_vblank_ns;
+	uint64_t start_ns, last_event_ns;
 	uint32_t total_flip = 0, total_pass = 0;
 	bool front = false;
 
 	/* Align with the vblank region to speed up convergence. */
-	last_vblank_ns = wait_for_vblank(data, pipe);
+	last_event_ns = wait_for_vblank(data, pipe);
 	start_ns = get_time_ns();
 
 	for (;;) {
-		uint64_t now_ns, vblank_ns, wait_ns, target_ns;
+		uint64_t now_ns, event_ns, wait_ns, target_ns;
 		int64_t diff_ns;
 
 		front = !front;
-		do_flip(data, pipe, front ? &data->fb1 : &data->fb0);
+		do_flip(data, front ? &data->fb1 : &data->fb0);
 
-		vblank_ns = get_vblank_event_ns(data);
-		diff_ns = rate_ns - (vblank_ns - last_vblank_ns);
-		last_vblank_ns = vblank_ns;
+		/* We need to cpture flip event instead of vblank event,
+		 * because vblank is triggered after each frame, but depending
+		 * on the vblank evasion time flip might or might not happen in
+		 * that same frame.
+		 */
+		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
+		diff_ns = rate_ns - (event_ns - last_event_ns);
+		last_event_ns = event_ns;
 
 		total_flip += 1;
 
@@ -359,6 +355,12 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 		     "Target VRR off threshold exceeded, result was %u%%\n",
 		     result);
 
+	/* Clean-up */
+	igt_plane_set_fb(data->primary, NULL);
+	igt_output_set_pipe(output, PIPE_NONE);
+	igt_display_commit_atomic(&data->display,
+				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
 	igt_remove_fb(data->drm_fd, &data->fb1);
 	igt_remove_fb(data->drm_fd, &data->fb0);
 }
-- 
2.20.1

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

Re: [igt-dev] [v4 1/2] tests/kms_vrr: Use atomic API for page flip

[Navare, Manasi]

On Thu, Sep 24, 2020 at 08:18:01PM +0530, bhanuprakash.modem@intel.com wrote:
> From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> 
> We should avoid using drmModePageFlip as it'll only be used for
> legacy drivers, instead, use igt_display_commit_atomic() API to
> page flip for atomic display code path.
> 
> v2:
> * Look for the page flip event not for the vblank event (Nicholas)
> * Fix to flip with different FBs (Bhanu)
> v3:
> * s/get_vblank_event_ns/get_kernel_event_ns/ (Manasi)
> * Add a comment to capture the flip event (Manasi)
> * Make sure we are reading valid event (Bhanu)
> * Test clean-up (Bhanu)
> v4:
> * s/*vblank_ns/*event_ns/ (Manasi)
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Looks good to me, so like we discussed the set_vrr_on_pipe setting it to 0 after the test should clear it out
and then beginning of test we set it to 1 again. This should work when the test executes fully. If it fails in between
then thats a problem.

So I had suggested that before calling test_basic, we should startw ith setting vrr_on_pipe(0)

With that change:

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  tests/kms_vrr.c | 76 +++++++++++++++++++++++++------------------------
>  1 file changed, 39 insertions(+), 37 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index a0db90006d..471d765dec 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -47,6 +47,7 @@ typedef struct range {
>  typedef struct data {
>  	igt_display_t display;
>  	int drm_fd;
> +	igt_plane_t *primary;
>  	igt_fb_t fb0;
>  	igt_fb_t fb1;
>  } data_t;
> @@ -60,15 +61,18 @@ static uint64_t timespec_to_ns(struct timespec *ts)
>  }
>  
>  /*
> - * Gets a vblank event from DRM and returns its timestamp in nanoseconds.
> + * Gets an event from DRM and returns its timestamp in nanoseconds.
> + * Asserts if the event from DRM is not matched with requested one.
> + *
>   * This blocks until the event is received.
>   */
> -static uint64_t get_vblank_event_ns(data_t *data)
> +static uint64_t get_kernel_event_ns(data_t *data, uint32_t event)
>  {
>  	struct drm_event_vblank ev;
>  
> -	igt_set_timeout(1, "Waiting for vblank event\n");
> +	igt_set_timeout(1, "Waiting for an event\n");
>  	igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
> +	igt_assert_eq(ev.base.type, event);
>  	igt_reset_timeout();
>  
>  	return ev.tv_sec * NSECS_PER_SEC + ev.tv_usec * 1000ull;
> @@ -126,11 +130,11 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
>  }
>  
>  /* Returns a suitable vrr test frequency. */
> -static uint32_t get_test_rate_ns(data_t *data, igt_output_t *output)
> +static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
>  {
>  	drmModeModeInfo *mode = igt_output_get_mode(output);
>  	range_t range;
> -	uint32_t vtest;
> +	uint64_t vtest;
>  
>  	/*
>  	 * The frequency with the fastest convergence speed should be
> @@ -165,7 +169,6 @@ static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
>  static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>  {
>  	drmModeModeInfo mode = *igt_output_get_mode(output);
> -	igt_plane_t *primary;
>  	cairo_t *cr;
>  
>  	/* Reset output */
> @@ -189,8 +192,8 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>  	igt_put_cairo_ctx(cr);
>  
>  	/* Take care of any required modesetting before the test begins. */
> -	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	igt_plane_set_fb(primary, &data->fb0);
> +	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(data->primary, &data->fb0);
>  
>  	igt_display_commit_atomic(&data->display,
>  				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> @@ -207,35 +210,28 @@ wait_for_vblank(data_t *data, enum pipe pipe)
>  	vbl.request.sequence = 1;
>  	drmWaitVBlank(data->drm_fd, &vbl);
>  
> -	return get_vblank_event_ns(data);
> +	return get_kernel_event_ns(data, DRM_EVENT_VBLANK);
>  }
>  
> -/* Performs an asynchronous non-blocking page-flip on a pipe. */
> -static int
> -do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
> +/* Performs an atomic non-blocking page-flip on a pipe. */
> +static void
> +do_flip(data_t *data, igt_fb_t *fb)
>  {
> -	igt_pipe_t *pipe = &data->display.pipes[pipe_id];
>  	int ret;
>  
>  	igt_set_timeout(1, "Scheduling page flip\n");
>  
> -	/*
> -	 * Only the legacy flip ioctl supports async flips.
> -	 * It's also non-blocking, but returns -EBUSY if flipping too fast.
> -	 * 2x monitor tests will need async flips in the atomic API.
> -	 */
> +	igt_plane_set_fb(data->primary, fb);
> +
>  	do {
> -		ret = drmModePageFlip(data->drm_fd, pipe->crtc_id,
> -				      fb->fb_id,
> -				      DRM_MODE_PAGE_FLIP_EVENT |
> -				      DRM_MODE_PAGE_FLIP_ASYNC,
> -				      data);
> +		ret = igt_display_try_commit_atomic(&data->display,
> +				  DRM_MODE_ATOMIC_NONBLOCK |
> +				  DRM_MODE_PAGE_FLIP_EVENT,
> +				  data);
>  	} while (ret == -EBUSY);
>  
>  	igt_assert_eq(ret, 0);
>  	igt_reset_timeout();
> -
> -	return 0;
>  }
>  
>  /*
> @@ -246,34 +242,34 @@ do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
>   * can arbitrarily restrict the bounds further than the absolute
>   * min and max range. But VRR is really about extending the flip
>   * to prevent stuttering or to match a source content rate.
> - *
> - * The only way to "present" at a fixed rate like userspace in a vendor
> - * neutral manner is to do it with async flips. This avoids the need
> - * to wait for next vblank and it should eventually converge at the
> - * desired rate.
>   */
>  static uint32_t
>  flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>  		 uint64_t rate_ns, uint64_t duration_ns)
>  {
> -	uint64_t start_ns, last_vblank_ns;
> +	uint64_t start_ns, last_event_ns;
>  	uint32_t total_flip = 0, total_pass = 0;
>  	bool front = false;
>  
>  	/* Align with the vblank region to speed up convergence. */
> -	last_vblank_ns = wait_for_vblank(data, pipe);
> +	last_event_ns = wait_for_vblank(data, pipe);
>  	start_ns = get_time_ns();
>  
>  	for (;;) {
> -		uint64_t now_ns, vblank_ns, wait_ns, target_ns;
> +		uint64_t now_ns, event_ns, wait_ns, target_ns;
>  		int64_t diff_ns;
>  
>  		front = !front;
> -		do_flip(data, pipe, front ? &data->fb1 : &data->fb0);
> +		do_flip(data, front ? &data->fb1 : &data->fb0);
>  
> -		vblank_ns = get_vblank_event_ns(data);
> -		diff_ns = rate_ns - (vblank_ns - last_vblank_ns);
> -		last_vblank_ns = vblank_ns;
> +		/* We need to cpture flip event instead of vblank event,
> +		 * because vblank is triggered after each frame, but depending
> +		 * on the vblank evasion time flip might or might not happen in
> +		 * that same frame.
> +		 */
> +		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> +		diff_ns = rate_ns - (event_ns - last_event_ns);
> +		last_event_ns = event_ns;
>  
>  		total_flip += 1;
>  
> @@ -359,6 +355,12 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>  		     "Target VRR off threshold exceeded, result was %u%%\n",
>  		     result);
>  
> +	/* Clean-up */
> +	igt_plane_set_fb(data->primary, NULL);
> +	igt_output_set_pipe(output, PIPE_NONE);
> +	igt_display_commit_atomic(&data->display,
> +				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +
>  	igt_remove_fb(data->drm_fd, &data->fb1);
>  	igt_remove_fb(data->drm_fd, &data->fb0);
>  }
> -- 
> 2.20.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [v4 1/2] tests/kms_vrr: Use atomic API for page flip

[Navare, Manasi]

On Tue, Sep 29, 2020 at 12:41:43PM -0700, Navare, Manasi wrote:
> On Thu, Sep 24, 2020 at 08:18:01PM +0530, bhanuprakash.modem@intel.com wrote:
> > From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > 
> > We should avoid using drmModePageFlip as it'll only be used for
> > legacy drivers, instead, use igt_display_commit_atomic() API to
> > page flip for atomic display code path.
> > 
> > v2:
> > * Look for the page flip event not for the vblank event (Nicholas)
> > * Fix to flip with different FBs (Bhanu)
> > v3:
> > * s/get_vblank_event_ns/get_kernel_event_ns/ (Manasi)
> > * Add a comment to capture the flip event (Manasi)
> > * Make sure we are reading valid event (Bhanu)
> > * Test clean-up (Bhanu)
> > v4:
> > * s/*vblank_ns/*event_ns/ (Manasi)
> > 
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> Looks good to me, so like we discussed the set_vrr_on_pipe setting it to 0 after the test should clear it out
> and then beginning of test we set it to 1 again. This should work when the test executes fully. If it fails in between
> then thats a problem.
> 
> So I had suggested that before calling test_basic, we should startw ith setting vrr_on_pipe(0)
> 
> With that change:

Just saw that this is done as part of Patch 2 and I have some comments on patch 2.
But for this patch please consider my 

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com

Manasi

> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> Manasi
> 
> > ---
> >  tests/kms_vrr.c | 76 +++++++++++++++++++++++++------------------------
> >  1 file changed, 39 insertions(+), 37 deletions(-)
> > 
> > diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> > index a0db90006d..471d765dec 100644
> > --- a/tests/kms_vrr.c
> > +++ b/tests/kms_vrr.c
> > @@ -47,6 +47,7 @@ typedef struct range {
> >  typedef struct data {
> >  	igt_display_t display;
> >  	int drm_fd;
> > +	igt_plane_t *primary;
> >  	igt_fb_t fb0;
> >  	igt_fb_t fb1;
> >  } data_t;
> > @@ -60,15 +61,18 @@ static uint64_t timespec_to_ns(struct timespec *ts)
> >  }
> >  
> >  /*
> > - * Gets a vblank event from DRM and returns its timestamp in nanoseconds.
> > + * Gets an event from DRM and returns its timestamp in nanoseconds.
> > + * Asserts if the event from DRM is not matched with requested one.
> > + *
> >   * This blocks until the event is received.
> >   */
> > -static uint64_t get_vblank_event_ns(data_t *data)
> > +static uint64_t get_kernel_event_ns(data_t *data, uint32_t event)
> >  {
> >  	struct drm_event_vblank ev;
> >  
> > -	igt_set_timeout(1, "Waiting for vblank event\n");
> > +	igt_set_timeout(1, "Waiting for an event\n");
> >  	igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
> > +	igt_assert_eq(ev.base.type, event);
> >  	igt_reset_timeout();
> >  
> >  	return ev.tv_sec * NSECS_PER_SEC + ev.tv_usec * 1000ull;
> > @@ -126,11 +130,11 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
> >  }
> >  
> >  /* Returns a suitable vrr test frequency. */
> > -static uint32_t get_test_rate_ns(data_t *data, igt_output_t *output)
> > +static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
> >  {
> >  	drmModeModeInfo *mode = igt_output_get_mode(output);
> >  	range_t range;
> > -	uint32_t vtest;
> > +	uint64_t vtest;
> >  
> >  	/*
> >  	 * The frequency with the fastest convergence speed should be
> > @@ -165,7 +169,6 @@ static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
> >  static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
> >  {
> >  	drmModeModeInfo mode = *igt_output_get_mode(output);
> > -	igt_plane_t *primary;
> >  	cairo_t *cr;
> >  
> >  	/* Reset output */
> > @@ -189,8 +192,8 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
> >  	igt_put_cairo_ctx(cr);
> >  
> >  	/* Take care of any required modesetting before the test begins. */
> > -	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > -	igt_plane_set_fb(primary, &data->fb0);
> > +	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(data->primary, &data->fb0);
> >  
> >  	igt_display_commit_atomic(&data->display,
> >  				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > @@ -207,35 +210,28 @@ wait_for_vblank(data_t *data, enum pipe pipe)
> >  	vbl.request.sequence = 1;
> >  	drmWaitVBlank(data->drm_fd, &vbl);
> >  
> > -	return get_vblank_event_ns(data);
> > +	return get_kernel_event_ns(data, DRM_EVENT_VBLANK);
> >  }
> >  
> > -/* Performs an asynchronous non-blocking page-flip on a pipe. */
> > -static int
> > -do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
> > +/* Performs an atomic non-blocking page-flip on a pipe. */
> > +static void
> > +do_flip(data_t *data, igt_fb_t *fb)
> >  {
> > -	igt_pipe_t *pipe = &data->display.pipes[pipe_id];
> >  	int ret;
> >  
> >  	igt_set_timeout(1, "Scheduling page flip\n");
> >  
> > -	/*
> > -	 * Only the legacy flip ioctl supports async flips.
> > -	 * It's also non-blocking, but returns -EBUSY if flipping too fast.
> > -	 * 2x monitor tests will need async flips in the atomic API.
> > -	 */
> > +	igt_plane_set_fb(data->primary, fb);
> > +
> >  	do {
> > -		ret = drmModePageFlip(data->drm_fd, pipe->crtc_id,
> > -				      fb->fb_id,
> > -				      DRM_MODE_PAGE_FLIP_EVENT |
> > -				      DRM_MODE_PAGE_FLIP_ASYNC,
> > -				      data);
> > +		ret = igt_display_try_commit_atomic(&data->display,
> > +				  DRM_MODE_ATOMIC_NONBLOCK |
> > +				  DRM_MODE_PAGE_FLIP_EVENT,
> > +				  data);
> >  	} while (ret == -EBUSY);
> >  
> >  	igt_assert_eq(ret, 0);
> >  	igt_reset_timeout();
> > -
> > -	return 0;
> >  }
> >  
> >  /*
> > @@ -246,34 +242,34 @@ do_flip(data_t *data, enum pipe pipe_id, igt_fb_t *fb)
> >   * can arbitrarily restrict the bounds further than the absolute
> >   * min and max range. But VRR is really about extending the flip
> >   * to prevent stuttering or to match a source content rate.
> > - *
> > - * The only way to "present" at a fixed rate like userspace in a vendor
> > - * neutral manner is to do it with async flips. This avoids the need
> > - * to wait for next vblank and it should eventually converge at the
> > - * desired rate.
> >   */
> >  static uint32_t
> >  flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
> >  		 uint64_t rate_ns, uint64_t duration_ns)
> >  {
> > -	uint64_t start_ns, last_vblank_ns;
> > +	uint64_t start_ns, last_event_ns;
> >  	uint32_t total_flip = 0, total_pass = 0;
> >  	bool front = false;
> >  
> >  	/* Align with the vblank region to speed up convergence. */
> > -	last_vblank_ns = wait_for_vblank(data, pipe);
> > +	last_event_ns = wait_for_vblank(data, pipe);
> >  	start_ns = get_time_ns();
> >  
> >  	for (;;) {
> > -		uint64_t now_ns, vblank_ns, wait_ns, target_ns;
> > +		uint64_t now_ns, event_ns, wait_ns, target_ns;
> >  		int64_t diff_ns;
> >  
> >  		front = !front;
> > -		do_flip(data, pipe, front ? &data->fb1 : &data->fb0);
> > +		do_flip(data, front ? &data->fb1 : &data->fb0);
> >  
> > -		vblank_ns = get_vblank_event_ns(data);
> > -		diff_ns = rate_ns - (vblank_ns - last_vblank_ns);
> > -		last_vblank_ns = vblank_ns;
> > +		/* We need to cpture flip event instead of vblank event,
> > +		 * because vblank is triggered after each frame, but depending
> > +		 * on the vblank evasion time flip might or might not happen in
> > +		 * that same frame.
> > +		 */
> > +		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> > +		diff_ns = rate_ns - (event_ns - last_event_ns);
> > +		last_event_ns = event_ns;
> >  
> >  		total_flip += 1;
> >  
> > @@ -359,6 +355,12 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
> >  		     "Target VRR off threshold exceeded, result was %u%%\n",
> >  		     result);
> >  
> > +	/* Clean-up */
> > +	igt_plane_set_fb(data->primary, NULL);
> > +	igt_output_set_pipe(output, PIPE_NONE);
> > +	igt_display_commit_atomic(&data->display,
> > +				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > +
> >  	igt_remove_fb(data->drm_fd, &data->fb1);
> >  	igt_remove_fb(data->drm_fd, &data->fb0);
> >  }
> > -- 
> > 2.20.1
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline mode

[bhanuprakash.modem]

From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

Check flipline mode by making sure that flips happen at flipline
decision boundary.

Example: if monitor vrr range is 40 - 60Hz and

* flip at refresh_rate > 60Hz:
        Flip should happen at the flipline boundary & returned refresh rate
        would be 60Hz.
* flip at refresh_rate == 50Hz:
        Flip should happen right away so returned refresh rate is 50Hz.
* flip at refresh_rate < 40Hz:
        Flip should happen at the vmax so the returned refresh rate
        would be 40Hz.

v2:
* s/*vblank_ns/*event_ns/ (Manasi)
* Reset vrr_enabled state before enabling it (Manasi)

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_vrr.c | 113 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 82 insertions(+), 31 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 471d765dec..ac42165bef 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -37,6 +37,7 @@ enum {
 	TEST_NONE = 0,
 	TEST_DPMS = 1 << 0,
 	TEST_SUSPEND = 1 << 1,
+	TEST_FLIPLINE = 1 << 2,
 };
 
 typedef struct range {
@@ -52,6 +53,12 @@ typedef struct data {
 	igt_fb_t fb1;
 } data_t;
 
+typedef struct vtest_ns {
+	uint64_t min;
+	uint64_t mid;
+	uint64_t max;
+} vtest_ns_t;
+
 typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
 
 /* Converts a timespec structure to nanoseconds. */
@@ -104,13 +111,18 @@ static uint64_t rate_from_refresh(uint64_t refresh)
 	return NSECS_PER_SEC / refresh;
 }
 
-/* Returns the min and max vrr range from the connector debugfs. */
+/* Read min and max vrr range from the connector debugfs.
+ *  - min range should be less than the current mode vfreq
+ *  - if max range is grater than the current mode vfreq, consider
+ *       current mode vfreq as the max range.
+ */
 static range_t get_vrr_range(data_t *data, igt_output_t *output)
 {
 	char buf[256];
 	char *start_loc;
 	int fd, res;
 	range_t range;
+	drmModeModeInfo *mode = igt_output_get_mode(output);
 
 	fd = igt_debugfs_connector_dir(data->drm_fd, output->name, O_RDONLY);
 	igt_assert(fd >= 0);
@@ -122,32 +134,28 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
 
 	igt_assert(start_loc = strstr(buf, "Min: "));
 	igt_assert_eq(sscanf(start_loc, "Min: %u", &range.min), 1);
+	igt_require(mode->vrefresh > range.min);
 
 	igt_assert(start_loc = strstr(buf, "Max: "));
 	igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
 
+	range.max = (mode->vrefresh < range.max) ? mode->vrefresh : range.max;
+
 	return range;
 }
 
-/* Returns a suitable vrr test frequency. */
-static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
+/* Returns vrr test frequency for min, mid & max range. */
+static vtest_ns_t get_test_rate_ns(data_t *data, igt_output_t *output)
 {
-	drmModeModeInfo *mode = igt_output_get_mode(output);
 	range_t range;
-	uint64_t vtest;
+	vtest_ns_t vtest_ns;
 
-	/*
-	 * The frequency with the fastest convergence speed should be
-	 * the midpoint between the current mode vfreq and the min
-	 * supported vfreq.
-	 */
 	range = get_vrr_range(data, output);
-	igt_require(mode->vrefresh > range.min);
-
-	vtest = (mode->vrefresh - range.min) / 2 + range.min;
-	igt_require(vtest < mode->vrefresh);
+	vtest_ns.min = rate_from_refresh(range.min);
+	vtest_ns.mid = rate_from_refresh(((range.max + range.min) / 2));
+	vtest_ns.max = rate_from_refresh(range.max);
 
-	return rate_from_refresh(vtest);
+	return vtest_ns;
 }
 
 /* Returns true if an output supports VRR. */
@@ -195,6 +203,11 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(data->primary, &data->fb0);
 
+	/* Clear vrr_enabled state before enabling it, because
+	 * it might be left enabled if the previous test fails.
+	 */
+	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 0);
+
 	igt_display_commit_atomic(&data->display,
 				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
 }
@@ -250,6 +263,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 	uint64_t start_ns, last_event_ns;
 	uint32_t total_flip = 0, total_pass = 0;
 	bool front = false;
+	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
 
 	/* Align with the vblank region to speed up convergence. */
 	last_event_ns = wait_for_vblank(data, pipe);
@@ -268,10 +282,6 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		 * that same frame.
 		 */
 		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
-		diff_ns = rate_ns - (event_ns - last_event_ns);
-		last_event_ns = event_ns;
-
-		total_flip += 1;
 
 		/*
 		 * Check if the difference between the two flip timestamps
@@ -281,9 +291,19 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		 * difference between 144Hz and 143Hz which should give this
 		 * enough accuracy for most use cases.
 		 */
+		if ((rate_ns <= vtest_ns.min) && (rate_ns >= vtest_ns.max))
+			diff_ns = rate_ns - (event_ns - last_event_ns);
+		else if (rate_ns > vtest_ns.min)
+			diff_ns = vtest_ns.min - (event_ns - last_event_ns);
+		else if (rate_ns < vtest_ns.max)
+			diff_ns = vtest_ns.max - (event_ns - last_event_ns);
+
 		if (llabs(diff_ns) < 50000ll)
 			total_pass += 1;
 
+		last_event_ns = event_ns;
+		total_flip += 1;
+
 		now_ns = get_time_ns();
 		if (now_ns - start_ns > duration_ns)
 			break;
@@ -310,10 +330,13 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 static void
 test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 {
-	uint64_t rate;
 	uint32_t result;
+	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
+	range_t range = get_vrr_range(data, output);
+	uint64_t rate = vtest_ns.mid;
 
-	rate = get_test_rate_ns(data, output);
+	igt_info("VRR Test execution on %s, PIPE_%s\n",
+		 output->name, kmstest_pipe_name(pipe));
 
 	prepare_test(data, output, pipe);
 
@@ -339,21 +362,45 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 
-	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
-
-	set_vrr_on_pipe(data, pipe, 0);
+	/*
+	 * Check flipline mode by making sure that flips happen at flipline
+	 * decision boundary.
+	 *
+	 * Example: if range is 40 - 60Hz and
+	 * if refresh_rate > 60Hz:
+	 *      Flip should happen at the flipline boundary & returned refresh rate
+	 *      would be 60Hz.
+	 * if refresh_rate is 50Hz:
+	 *      Flip will happen right away so returned refresh rate is 50Hz.
+	 * if refresh_rate < 40Hz:
+	 *      Flip should happen at the vmax so the returned refresh rate
+	 *      would be 40Hz.
+	 */
+	if (flags & TEST_FLIPLINE) {
+		rate = rate_from_refresh(range.min - 5);
+		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+		igt_assert_f(result > 75,
+			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
+			     rate, result);
+
+		rate = rate_from_refresh(range.max + 5);
+		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+		igt_assert_f(result > 75,
+			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
+			     rate, result);
+	}
 
-	/* This check is delayed until after VRR is disabled so it isn't
-	 * left enabled if the test fails. */
+	rate = vtest_ns.mid;
+	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
 	igt_assert_f(result > 75,
-		     "Target VRR on threshold not reached, result was %u%%\n",
-		     result);
+		     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
+		     rate, result);
 
+	set_vrr_on_pipe(data, pipe, 0);
 	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
-
 	igt_assert_f(result < 10,
-		     "Target VRR off threshold exceeded, result was %u%%\n",
-		     result);
+		     "Refresh rate %"PRIu64"ns: Target VRR off threshold exceeded, result was %u%%\n",
+		     rate, result);
 
 	/* Clean-up */
 	igt_plane_set_fb(data->primary, NULL);
@@ -413,6 +460,10 @@ igt_main
 	igt_subtest("flip-suspend")
 		run_vrr_test(&data, test_basic, TEST_SUSPEND);
 
+	igt_describe("Make sure that flips happen at flipline decision boundary.");
+	igt_subtest("flipline")
+		run_vrr_test(&data, test_basic, TEST_FLIPLINE);
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.20.1

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

Re: [igt-dev] [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline mode

[Navare, Manasi]

On Thu, Sep 24, 2020 at 08:18:02PM +0530, bhanuprakash.modem@intel.com wrote:
> From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> 
> Check flipline mode by making sure that flips happen at flipline
> decision boundary.
> 
> Example: if monitor vrr range is 40 - 60Hz and
> 
> * flip at refresh_rate > 60Hz:
>         Flip should happen at the flipline boundary & returned refresh rate
>         would be 60Hz.
> * flip at refresh_rate == 50Hz:
>         Flip should happen right away so returned refresh rate is 50Hz.
> * flip at refresh_rate < 40Hz:
>         Flip should happen at the vmax so the returned refresh rate
>         would be 40Hz.
> 
> v2:
> * s/*vblank_ns/*event_ns/ (Manasi)
> * Reset vrr_enabled state before enabling it (Manasi)

> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  tests/kms_vrr.c | 113 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 82 insertions(+), 31 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 471d765dec..ac42165bef 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -37,6 +37,7 @@ enum {
>  	TEST_NONE = 0,
>  	TEST_DPMS = 1 << 0,
>  	TEST_SUSPEND = 1 << 1,
> +	TEST_FLIPLINE = 1 << 2,
>  };
>  
>  typedef struct range {
> @@ -52,6 +53,12 @@ typedef struct data {
>  	igt_fb_t fb1;
>  } data_t;
>  
> +typedef struct vtest_ns {
> +	uint64_t min;
> +	uint64_t mid;
> +	uint64_t max;
> +} vtest_ns_t;
> +
>  typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
>  
>  /* Converts a timespec structure to nanoseconds. */
> @@ -104,13 +111,18 @@ static uint64_t rate_from_refresh(uint64_t refresh)
>  	return NSECS_PER_SEC / refresh;
>  }
>  
> -/* Returns the min and max vrr range from the connector debugfs. */
> +/* Read min and max vrr range from the connector debugfs.
> + *  - min range should be less than the current mode vfreq
> + *  - if max range is grater than the current mode vfreq, consider
> + *       current mode vfreq as the max range.
> + */
>  static range_t get_vrr_range(data_t *data, igt_output_t *output)
>  {
>  	char buf[256];
>  	char *start_loc;
>  	int fd, res;
>  	range_t range;
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
>  
>  	fd = igt_debugfs_connector_dir(data->drm_fd, output->name, O_RDONLY);
>  	igt_assert(fd >= 0);
> @@ -122,32 +134,28 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
>  
>  	igt_assert(start_loc = strstr(buf, "Min: "));
>  	igt_assert_eq(sscanf(start_loc, "Min: %u", &range.min), 1);
> +	igt_require(mode->vrefresh > range.min);
>  
>  	igt_assert(start_loc = strstr(buf, "Max: "));
>  	igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
>  
> +	range.max = (mode->vrefresh < range.max) ? mode->vrefresh : range.max;
> +
>  	return range;
>  }
>  
> -/* Returns a suitable vrr test frequency. */
> -static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
> +/* Returns vrr test frequency for min, mid & max range. */
> +static vtest_ns_t get_test_rate_ns(data_t *data, igt_output_t *output)
>  {
> -	drmModeModeInfo *mode = igt_output_get_mode(output);
>  	range_t range;
> -	uint64_t vtest;
> +	vtest_ns_t vtest_ns;
>  
> -	/*
> -	 * The frequency with the fastest convergence speed should be
> -	 * the midpoint between the current mode vfreq and the min
> -	 * supported vfreq.
> -	 */
>  	range = get_vrr_range(data, output);
> -	igt_require(mode->vrefresh > range.min);
> -
> -	vtest = (mode->vrefresh - range.min) / 2 + range.min;
> -	igt_require(vtest < mode->vrefresh);
> +	vtest_ns.min = rate_from_refresh(range.min);
> +	vtest_ns.mid = rate_from_refresh(((range.max + range.min) / 2));
> +	vtest_ns.max = rate_from_refresh(range.max);
>  
> -	return rate_from_refresh(vtest);
> +	return vtest_ns;
>  }
>  
>  /* Returns true if an output supports VRR. */
> @@ -195,6 +203,11 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>  	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(data->primary, &data->fb0);
>  
> +	/* Clear vrr_enabled state before enabling it, because
> +	 * it might be left enabled if the previous test fails.
> +	 */
> +	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 0);

Like I said in my comment below this shouldhappen before calling
prepare_test()
Also the set_vrr_pipe(1) call should happen at the beginning of prepare_test() so that
VRR prop is set before doing a modeset with the desired fb.

What I have in my code for testing is:

test_basic()
{
	set_vrr_on_pipe(0)
	prepare_test()
	.
	.
	.
}

And in prepare_test()
{
	set_vrr_on_pipe(1);
	create_fb
	.
	.
	.
	atomic_commit();
}


> +
>  	igt_display_commit_atomic(&data->display,
>  				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>  }
> @@ -250,6 +263,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>  	uint64_t start_ns, last_event_ns;
>  	uint32_t total_flip = 0, total_pass = 0;
>  	bool front = false;
> +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
>  
>  	/* Align with the vblank region to speed up convergence. */
>  	last_event_ns = wait_for_vblank(data, pipe);
> @@ -268,10 +282,6 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>  		 * that same frame.
>  		 */
>  		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> -		diff_ns = rate_ns - (event_ns - last_event_ns);
> -		last_event_ns = event_ns;
> -
> -		total_flip += 1;
>  
>  		/*
>  		 * Check if the difference between the two flip timestamps
> @@ -281,9 +291,19 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>  		 * difference between 144Hz and 143Hz which should give this
>  		 * enough accuracy for most use cases.
>  		 */
> +		if ((rate_ns <= vtest_ns.min) && (rate_ns >= vtest_ns.max))
> +			diff_ns = rate_ns - (event_ns - last_event_ns);
> +		else if (rate_ns > vtest_ns.min)
> +			diff_ns = vtest_ns.min - (event_ns - last_event_ns);
> +		else if (rate_ns < vtest_ns.max)
> +			diff_ns = vtest_ns.max - (event_ns - last_event_ns);
> +
>  		if (llabs(diff_ns) < 50000ll)
>  			total_pass += 1;
>  
> +		last_event_ns = event_ns;
> +		total_flip += 1;
> +
>  		now_ns = get_time_ns();
>  		if (now_ns - start_ns > duration_ns)
>  			break;
> @@ -310,10 +330,13 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>  static void
>  test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>  {
> -	uint64_t rate;
>  	uint32_t result;
> +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> +	range_t range = get_vrr_range(data, output);
> +	uint64_t rate = vtest_ns.mid;
>  
> -	rate = get_test_rate_ns(data, output);
> +	igt_info("VRR Test execution on %s, PIPE_%s\n",
> +		 output->name, kmstest_pipe_name(pipe));
>
	The reseting of vrr_enabled prop should be done before prepare test so before we set the
desrired fb. I would recommend a call set_vrr_on_pipe(0) before calling prepare_test()
This is what I had to do for testing the kernel code. It should be a complete set_vrr_on_pipe() call
which will set the prop to 0 and commit

Manasi

  
>  	prepare_test(data, output, pipe);
>  
> @@ -339,21 +362,45 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  
> -	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> -
> -	set_vrr_on_pipe(data, pipe, 0);
> +	/*
> +	 * Check flipline mode by making sure that flips happen at flipline
> +	 * decision boundary.
> +	 *
> +	 * Example: if range is 40 - 60Hz and
> +	 * if refresh_rate > 60Hz:
> +	 *      Flip should happen at the flipline boundary & returned refresh rate
> +	 *      would be 60Hz.
> +	 * if refresh_rate is 50Hz:
> +	 *      Flip will happen right away so returned refresh rate is 50Hz.
> +	 * if refresh_rate < 40Hz:
> +	 *      Flip should happen at the vmax so the returned refresh rate
> +	 *      would be 40Hz.
> +	 */
> +	if (flags & TEST_FLIPLINE) {
> +		rate = rate_from_refresh(range.min - 5);
> +		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		igt_assert_f(result > 75,
> +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +			     rate, result);
> +
> +		rate = rate_from_refresh(range.max + 5);
> +		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		igt_assert_f(result > 75,
> +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +			     rate, result);
> +	}
>  
> -	/* This check is delayed until after VRR is disabled so it isn't
> -	 * left enabled if the test fails. */
> +	rate = vtest_ns.mid;
> +	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
>  	igt_assert_f(result > 75,
> -		     "Target VRR on threshold not reached, result was %u%%\n",
> -		     result);
> +		     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +		     rate, result);
>  
> +	set_vrr_on_pipe(data, pipe, 0);
>  	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> -
>  	igt_assert_f(result < 10,
> -		     "Target VRR off threshold exceeded, result was %u%%\n",
> -		     result);
> +		     "Refresh rate %"PRIu64"ns: Target VRR off threshold exceeded, result was %u%%\n",
> +		     rate, result);
>  
>  	/* Clean-up */
>  	igt_plane_set_fb(data->primary, NULL);
> @@ -413,6 +460,10 @@ igt_main
>  	igt_subtest("flip-suspend")
>  		run_vrr_test(&data, test_basic, TEST_SUSPEND);
>  
> +	igt_describe("Make sure that flips happen at flipline decision boundary.");
> +	igt_subtest("flipline")
> +		run_vrr_test(&data, test_basic, TEST_FLIPLINE);
> +
>  	igt_fixture {
>  		igt_display_fini(&data.display);
>  	}
> -- 
> 2.20.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline mode

[Modem, Bhanuprakash]

> -----Original Message-----
> From: Navare, Manasi <manasi.d.navare@intel.com>
> Sent: Wednesday, September 30, 2020 3:43 AM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Harry Wentland
> <harry.wentland@amd.com>; Nicholas Kazlauskas
> <nicholas.kazlauskas@amd.com>
> Subject: Re: [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline
> mode
> 
> On Thu, Sep 24, 2020 at 08:18:02PM +0530, bhanuprakash.modem@intel.com
> wrote:
> > From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> >
> > Check flipline mode by making sure that flips happen at flipline
> > decision boundary.
> >
> > Example: if monitor vrr range is 40 - 60Hz and
> >
> > * flip at refresh_rate > 60Hz:
> >         Flip should happen at the flipline boundary & returned refresh
> rate
> >         would be 60Hz.
> > * flip at refresh_rate == 50Hz:
> >         Flip should happen right away so returned refresh rate is 50Hz.
> > * flip at refresh_rate < 40Hz:
> >         Flip should happen at the vmax so the returned refresh rate
> >         would be 40Hz.
> >
> > v2:
> > * s/*vblank_ns/*event_ns/ (Manasi)
> > * Reset vrr_enabled state before enabling it (Manasi)
> 
> >
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > ---
> >  tests/kms_vrr.c | 113 +++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 82 insertions(+), 31 deletions(-)
> >
> > diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> > index 471d765dec..ac42165bef 100644
> > --- a/tests/kms_vrr.c
> > +++ b/tests/kms_vrr.c
> > @@ -37,6 +37,7 @@ enum {
> >  	TEST_NONE = 0,
> >  	TEST_DPMS = 1 << 0,
> >  	TEST_SUSPEND = 1 << 1,
> > +	TEST_FLIPLINE = 1 << 2,
> >  };
> >
> >  typedef struct range {
> > @@ -52,6 +53,12 @@ typedef struct data {
> >  	igt_fb_t fb1;
> >  } data_t;
> >
> > +typedef struct vtest_ns {
> > +	uint64_t min;
> > +	uint64_t mid;
> > +	uint64_t max;
> > +} vtest_ns_t;
> > +
> >  typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
> >
> >  /* Converts a timespec structure to nanoseconds. */
> > @@ -104,13 +111,18 @@ static uint64_t rate_from_refresh(uint64_t
> refresh)
> >  	return NSECS_PER_SEC / refresh;
> >  }
> >
> > -/* Returns the min and max vrr range from the connector debugfs. */
> > +/* Read min and max vrr range from the connector debugfs.
> > + *  - min range should be less than the current mode vfreq
> > + *  - if max range is grater than the current mode vfreq, consider
> > + *       current mode vfreq as the max range.
> > + */
> >  static range_t get_vrr_range(data_t *data, igt_output_t *output)
> >  {
> >  	char buf[256];
> >  	char *start_loc;
> >  	int fd, res;
> >  	range_t range;
> > +	drmModeModeInfo *mode = igt_output_get_mode(output);
> >
> >  	fd = igt_debugfs_connector_dir(data->drm_fd, output->name,
> O_RDONLY);
> >  	igt_assert(fd >= 0);
> > @@ -122,32 +134,28 @@ static range_t get_vrr_range(data_t *data,
> igt_output_t *output)
> >
> >  	igt_assert(start_loc = strstr(buf, "Min: "));
> >  	igt_assert_eq(sscanf(start_loc, "Min: %u", &range.min), 1);
> > +	igt_require(mode->vrefresh > range.min);
> >
> >  	igt_assert(start_loc = strstr(buf, "Max: "));
> >  	igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
> >
> > +	range.max = (mode->vrefresh < range.max) ? mode->vrefresh :
> range.max;
> > +
> >  	return range;
> >  }
> >
> > -/* Returns a suitable vrr test frequency. */
> > -static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
> > +/* Returns vrr test frequency for min, mid & max range. */
> > +static vtest_ns_t get_test_rate_ns(data_t *data, igt_output_t *output)
> >  {
> > -	drmModeModeInfo *mode = igt_output_get_mode(output);
> >  	range_t range;
> > -	uint64_t vtest;
> > +	vtest_ns_t vtest_ns;
> >
> > -	/*
> > -	 * The frequency with the fastest convergence speed should be
> > -	 * the midpoint between the current mode vfreq and the min
> > -	 * supported vfreq.
> > -	 */
> >  	range = get_vrr_range(data, output);
> > -	igt_require(mode->vrefresh > range.min);
> > -
> > -	vtest = (mode->vrefresh - range.min) / 2 + range.min;
> > -	igt_require(vtest < mode->vrefresh);
> > +	vtest_ns.min = rate_from_refresh(range.min);
> > +	vtest_ns.mid = rate_from_refresh(((range.max + range.min) / 2));
> > +	vtest_ns.max = rate_from_refresh(range.max);
> >
> > -	return rate_from_refresh(vtest);
> > +	return vtest_ns;
> >  }
> >
> >  /* Returns true if an output supports VRR. */
> > @@ -195,6 +203,11 @@ static void prepare_test(data_t *data, igt_output_t
> *output, enum pipe pipe)
> >  	data->primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> >  	igt_plane_set_fb(data->primary, &data->fb0);
> >
> > +	/* Clear vrr_enabled state before enabling it, because
> > +	 * it might be left enabled if the previous test fails.
> > +	 */
> > +	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED,
> 0);
> 
> Like I said in my comment below this shouldhappen before calling
> prepare_test()
> Also the set_vrr_pipe(1) call should happen at the beginning of
> prepare_test() so that
> VRR prop is set before doing a modeset with the desired fb.
> 
> What I have in my code for testing is:
> 
> test_basic()
> {
> 	set_vrr_on_pipe(0)
> 	prepare_test()
> 	.
> 	.
> 	.
> }
> 
> And in prepare_test()
> {
> 	set_vrr_on_pipe(1);
> 	create_fb
> 	.
> 	.
> 	.
> 	atomic_commit();
> }
> 
> 
[Bhanu] Why do we need to enable VRR before modeset? I think Driver will take care of it.

As you mentioned in your Engg changes, in atomic_check:
	if (!needs_modeset(new_crtc_state) && 	old_crtc_state->uapi.vrr_enabled == new_crtc_state->uapi.vrr_enabled)
		continue;

For first IGT execution:
	vrr_enabled status is 0
	create FB & do modeset
	set vrr_enabled to 1 & flip
	above check will fail & kernel will perform required tasks
	set vrr_enabled to 0 & flip
	above check will fail & kernel will perform required tasks

For some reason test got failed & vrr_enabled status kept 1.
In next IGT execution:
	vrr_enabled status is 1
	update to 0 (this patch will update vrr_enabled to 0 before modeset)
	remaining execution is same as above.
 
Is my interpretation correct? Also can you please help to test your code with this patch?
> > +
> >  	igt_display_commit_atomic(&data->display,
> >  				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> >  }
> > @@ -250,6 +263,7 @@ flip_and_measure(data_t *data, igt_output_t *output,
> enum pipe pipe,
> >  	uint64_t start_ns, last_event_ns;
> >  	uint32_t total_flip = 0, total_pass = 0;
> >  	bool front = false;
> > +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> >
> >  	/* Align with the vblank region to speed up convergence. */
> >  	last_event_ns = wait_for_vblank(data, pipe);
> > @@ -268,10 +282,6 @@ flip_and_measure(data_t *data, igt_output_t
> *output, enum pipe pipe,
> >  		 * that same frame.
> >  		 */
> >  		event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> > -		diff_ns = rate_ns - (event_ns - last_event_ns);
> > -		last_event_ns = event_ns;
> > -
> > -		total_flip += 1;
> >
> >  		/*
> >  		 * Check if the difference between the two flip timestamps
> > @@ -281,9 +291,19 @@ flip_and_measure(data_t *data, igt_output_t
> *output, enum pipe pipe,
> >  		 * difference between 144Hz and 143Hz which should give this
> >  		 * enough accuracy for most use cases.
> >  		 */
> > +		if ((rate_ns <= vtest_ns.min) && (rate_ns >= vtest_ns.max))
> > +			diff_ns = rate_ns - (event_ns - last_event_ns);
> > +		else if (rate_ns > vtest_ns.min)
> > +			diff_ns = vtest_ns.min - (event_ns - last_event_ns);
> > +		else if (rate_ns < vtest_ns.max)
> > +			diff_ns = vtest_ns.max - (event_ns - last_event_ns);
> > +
> >  		if (llabs(diff_ns) < 50000ll)
> >  			total_pass += 1;
> >
> > +		last_event_ns = event_ns;
> > +		total_flip += 1;
> > +
> >  		now_ns = get_time_ns();
> >  		if (now_ns - start_ns > duration_ns)
> >  			break;
> > @@ -310,10 +330,13 @@ flip_and_measure(data_t *data, igt_output_t
> *output, enum pipe pipe,
> >  static void
> >  test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t
> flags)
> >  {
> > -	uint64_t rate;
> >  	uint32_t result;
> > +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> > +	range_t range = get_vrr_range(data, output);
> > +	uint64_t rate = vtest_ns.mid;
> >
> > -	rate = get_test_rate_ns(data, output);
> > +	igt_info("VRR Test execution on %s, PIPE_%s\n",
> > +		 output->name, kmstest_pipe_name(pipe));
> >
> 	The reseting of vrr_enabled prop should be done before prepare test
> so before we set the
> desrired fb. I would recommend a call set_vrr_on_pipe(0) before calling
> prepare_test()
> This is what I had to do for testing the kernel code. It should be a
> complete set_vrr_on_pipe() call
> which will set the prop to 0 and commit
> 
> Manasi
> 
> 
> >  	prepare_test(data, output, pipe);
> >
> > @@ -339,21 +362,45 @@ test_basic(data_t *data, enum pipe pipe,
> igt_output_t *output, uint32_t flags)
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> >
> > -	result = flip_and_measure(data, output, pipe, rate,
> TEST_DURATION_NS);
> > -
> > -	set_vrr_on_pipe(data, pipe, 0);
> > +	/*
> > +	 * Check flipline mode by making sure that flips happen at flipline
> > +	 * decision boundary.
> > +	 *
> > +	 * Example: if range is 40 - 60Hz and
> > +	 * if refresh_rate > 60Hz:
> > +	 *      Flip should happen at the flipline boundary & returned
> refresh rate
> > +	 *      would be 60Hz.
> > +	 * if refresh_rate is 50Hz:
> > +	 *      Flip will happen right away so returned refresh rate is
> 50Hz.
> > +	 * if refresh_rate < 40Hz:
> > +	 *      Flip should happen at the vmax so the returned refresh rate
> > +	 *      would be 40Hz.
> > +	 */
> > +	if (flags & TEST_FLIPLINE) {
> > +		rate = rate_from_refresh(range.min - 5);
> > +		result = flip_and_measure(data, output, pipe, rate,
> TEST_DURATION_NS);
> > +		igt_assert_f(result > 75,
> > +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold
> not reached, result was %u%%\n",
> > +			     rate, result);
> > +
> > +		rate = rate_from_refresh(range.max + 5);
> > +		result = flip_and_measure(data, output, pipe, rate,
> TEST_DURATION_NS);
> > +		igt_assert_f(result > 75,
> > +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold
> not reached, result was %u%%\n",
> > +			     rate, result);
> > +	}
> >
> > -	/* This check is delayed until after VRR is disabled so it isn't
> > -	 * left enabled if the test fails. */
> > +	rate = vtest_ns.mid;
> > +	result = flip_and_measure(data, output, pipe, rate,
> TEST_DURATION_NS);
> >  	igt_assert_f(result > 75,
> > -		     "Target VRR on threshold not reached, result was %u%%\n",
> > -		     result);
> > +		     "Refresh rate %"PRIu64"ns: Target VRR on threshold not
> reached, result was %u%%\n",
> > +		     rate, result);
> >
> > +	set_vrr_on_pipe(data, pipe, 0);
> >  	result = flip_and_measure(data, output, pipe, rate,
> TEST_DURATION_NS);
> > -
> >  	igt_assert_f(result < 10,
> > -		     "Target VRR off threshold exceeded, result was %u%%\n",
> > -		     result);
> > +		     "Refresh rate %"PRIu64"ns: Target VRR off threshold
> exceeded, result was %u%%\n",
> > +		     rate, result);
> >
> >  	/* Clean-up */
> >  	igt_plane_set_fb(data->primary, NULL);
> > @@ -413,6 +460,10 @@ igt_main
> >  	igt_subtest("flip-suspend")
> >  		run_vrr_test(&data, test_basic, TEST_SUSPEND);
> >
> > +	igt_describe("Make sure that flips happen at flipline decision
> boundary.");
> > +	igt_subtest("flipline")
> > +		run_vrr_test(&data, test_basic, TEST_FLIPLINE);
> > +
> >  	igt_fixture {
> >  		igt_display_fini(&data.display);
> >  	}
> > --
> > 2.20.1
> >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline mode

[Navare, Manasi]

On Wed, Sep 30, 2020 at 08:34:10AM -0700, Modem, Bhanuprakash wrote:
> > -----Original Message-----
> > From: Navare, Manasi <manasi.d.navare@intel.com>
> > Sent: Wednesday, September 30, 2020 3:43 AM
> > To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> > Cc: igt-dev@lists.freedesktop.org; Harry Wentland
> > <harry.wentland@amd.com>; Nicholas Kazlauskas
> > <nicholas.kazlauskas@amd.com>
> > Subject: Re: [v4 2/2] tests/kms_vrr: Add new subtest to validate Flipline
> > mode
> >
> > On Thu, Sep 24, 2020 at 08:18:02PM +0530, bhanuprakash.modem@intel.com
> > wrote:
> > > From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > >
> > > Check flipline mode by making sure that flips happen at flipline
> > > decision boundary.
> > >
> > > Example: if monitor vrr range is 40 - 60Hz and
> > >
> > > * flip at refresh_rate > 60Hz:
> > >         Flip should happen at the flipline boundary & returned refresh
> > rate
> > >         would be 60Hz.
> > > * flip at refresh_rate == 50Hz:
> > >         Flip should happen right away so returned refresh rate is 50Hz.
> > > * flip at refresh_rate < 40Hz:
> > >         Flip should happen at the vmax so the returned refresh rate
> > >         would be 40Hz.
> > >
> > > v2:
> > > * s/*vblank_ns/*event_ns/ (Manasi)
> > > * Reset vrr_enabled state before enabling it (Manasi)
> >
> > >
> > > Cc: Harry Wentland <harry.wentland@amd.com>
> > > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > > ---
> > >  tests/kms_vrr.c | 113 +++++++++++++++++++++++++++++++++++-------------
> > >  1 file changed, 82 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> > > index 471d765dec..ac42165bef 100644
> > > --- a/tests/kms_vrr.c
> > > +++ b/tests/kms_vrr.c
> > > @@ -37,6 +37,7 @@ enum {
> > >  TEST_NONE = 0,
> > >  TEST_DPMS = 1 << 0,
> > >  TEST_SUSPEND = 1 << 1,
> > > +TEST_FLIPLINE = 1 << 2,
> > >  };
> > >
> > >  typedef struct range {
> > > @@ -52,6 +53,12 @@ typedef struct data {
> > >  igt_fb_t fb1;
> > >  } data_t;
> > >
> > > +typedef struct vtest_ns {
> > > +uint64_t min;
> > > +uint64_t mid;
> > > +uint64_t max;
> > > +} vtest_ns_t;
> > > +
> > >  typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
> > >
> > >  /* Converts a timespec structure to nanoseconds. */
> > > @@ -104,13 +111,18 @@ static uint64_t rate_from_refresh(uint64_t
> > refresh)
> > >  return NSECS_PER_SEC / refresh;
> > >  }
> > >
> > > -/* Returns the min and max vrr range from the connector debugfs. */
> > > +/* Read min and max vrr range from the connector debugfs.
> > > + *  - min range should be less than the current mode vfreq
> > > + *  - if max range is grater than the current mode vfreq, consider
> > > + *       current mode vfreq as the max range.
> > > + */
> > >  static range_t get_vrr_range(data_t *data, igt_output_t *output)
> > >  {
> > >  char buf[256];
> > >  char *start_loc;
> > >  int fd, res;
> > >  range_t range;
> > > +drmModeModeInfo *mode = igt_output_get_mode(output);
> > >
> > >  fd = igt_debugfs_connector_dir(data->drm_fd, output->name,
> > O_RDONLY);
> > >  igt_assert(fd >= 0);
> > > @@ -122,32 +134,28 @@ static range_t get_vrr_range(data_t *data,
> > igt_output_t *output)
> > >
> > >  igt_assert(start_loc = strstr(buf, "Min: "));
> > >  igt_assert_eq(sscanf(start_loc, "Min: %u", &range.min), 1);
> > > +igt_require(mode->vrefresh > range.min);
> > >
> > >  igt_assert(start_loc = strstr(buf, "Max: "));
> > >  igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
> > >
> > > +range.max = (mode->vrefresh < range.max) ? mode->vrefresh :
> > range.max;
> > > +
> > >  return range;
> > >  }
> > >
> > > -/* Returns a suitable vrr test frequency. */
> > > -static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
> > > +/* Returns vrr test frequency for min, mid & max range. */
> > > +static vtest_ns_t get_test_rate_ns(data_t *data, igt_output_t *output)
> > >  {
> > > -drmModeModeInfo *mode = igt_output_get_mode(output);
> > >  range_t range;
> > > -uint64_t vtest;
> > > +vtest_ns_t vtest_ns;
> > >
> > > -/*
> > > - * The frequency with the fastest convergence speed should be
> > > - * the midpoint between the current mode vfreq and the min
> > > - * supported vfreq.
> > > - */
> > >  range = get_vrr_range(data, output);
> > > -igt_require(mode->vrefresh > range.min);
> > > -
> > > -vtest = (mode->vrefresh - range.min) / 2 + range.min;
> > > -igt_require(vtest < mode->vrefresh);
> > > +vtest_ns.min = rate_from_refresh(range.min);
> > > +vtest_ns.mid = rate_from_refresh(((range.max + range.min) / 2));
> > > +vtest_ns.max = rate_from_refresh(range.max);
> > >
> > > -return rate_from_refresh(vtest);
> > > +return vtest_ns;
> > >  }
> > >
> > >  /* Returns true if an output supports VRR. */
> > > @@ -195,6 +203,11 @@ static void prepare_test(data_t *data, igt_output_t
> > *output, enum pipe pipe)
> > >  data->primary = igt_output_get_plane_type(output,
> > DRM_PLANE_TYPE_PRIMARY);
> > >  igt_plane_set_fb(data->primary, &data->fb0);
> > >
> > > +/* Clear vrr_enabled state before enabling it, because
> > > + * it might be left enabled if the previous test fails.
> > > + */
> > > +igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED,
> > 0);
> >
> > Like I said in my comment below this shouldhappen before calling
> > prepare_test()
> > Also the set_vrr_pipe(1) call should happen at the beginning of
> > prepare_test() so that
> > VRR prop is set before doing a modeset with the desired fb.
> >
> > What I have in my code for testing is:
> >
> > test_basic()
> > {
> > set_vrr_on_pipe(0)
> > prepare_test()
> > .
> > .
> > .
> > }
> >
> > And in prepare_test()
> > {
> > set_vrr_on_pipe(1);
> > create_fb
> > .
> > .
> > .
> > atomic_commit();
> > }
> >
> >
> [Bhanu] Why do we need to enable VRR before modeset? I think Driver will take care of it.
> 
> As you mentioned in your Engg changes, in atomic_check:
> if (!needs_modeset(new_crtc_state) && old_crtc_state->uapi.vrr_enabled == new_crtc_state->uapi.vrr_enabled)
> continue;
> 
> For first IGT execution:
> vrr_enabled status is 0
> create FB & do modeset
> set vrr_enabled to 1 & flip
> above check will fail & kernel will perform required tasks
> set vrr_enabled to 0 & flip
> above check will fail & kernel will perform required tasks
> 
> For some reason test got failed & vrr_enabled status kept 1.
> In next IGT execution:
> vrr_enabled status is 1
> update to 0 (this patch will update vrr_enabled to 0 before modeset)
> remaining execution is same as above.
> 
> Is my interpretation correct? Also can you please help to test your code with this patch?

Intially I thought that prepare test() since we are preparing for vrr test would
actually be the modeset where vrr goes from 0 to 1 but 
basically here we are using prepare test function to set everything clean
without VRR with vrr prop set to 0 and then set it to 1 before flip.
Yes I think this should work.

Also I do have the kernel patches now with the full enable/disable sequence
implemented and I will test this patch as well as send you my patches
to do the testing on your end.

But so far functionality wise this does look good.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> > > +
> > >  igt_display_commit_atomic(&data->display,
> > >    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > >  }
> > > @@ -250,6 +263,7 @@ flip_and_measure(data_t *data, igt_output_t *output,
> > enum pipe pipe,
> > >  uint64_t start_ns, last_event_ns;
> > >  uint32_t total_flip = 0, total_pass = 0;
> > >  bool front = false;
> > > +vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> > >
> > >  /* Align with the vblank region to speed up convergence. */
> > >  last_event_ns = wait_for_vblank(data, pipe);
> > > @@ -268,10 +282,6 @@ flip_and_measure(data_t *data, igt_output_t
> > *output, enum pipe pipe,
> > >   * that same frame.
> > >   */
> > >  event_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> > > -diff_ns = rate_ns - (event_ns - last_event_ns);
> > > -last_event_ns = event_ns;
> > > -
> > > -total_flip += 1;
> > >
> > >  /*
> > >   * Check if the difference between the two flip timestamps
> > > @@ -281,9 +291,19 @@ flip_and_measure(data_t *data, igt_output_t
> > *output, enum pipe pipe,
> > >   * difference between 144Hz and 143Hz which should give this
> > >   * enough accuracy for most use cases.
> > >   */
> > > +if ((rate_ns <= vtest_ns.min) && (rate_ns >= vtest_ns.max))
> > > +diff_ns = rate_ns - (event_ns - last_event_ns);
> > > +else if (rate_ns > vtest_ns.min)
> > > +diff_ns = vtest_ns.min - (event_ns - last_event_ns);
> > > +else if (rate_ns < vtest_ns.max)
> > > +diff_ns = vtest_ns.max - (event_ns - last_event_ns);
> > > +
> > >  if (llabs(diff_ns) < 50000ll)
> > >  total_pass += 1;
> > >
> > > +last_event_ns = event_ns;
> > > +total_flip += 1;
> > > +
> > >  now_ns = get_time_ns();
> > >  if (now_ns - start_ns > duration_ns)
> > >  break;
> > > @@ -310,10 +330,13 @@ flip_and_measure(data_t *data, igt_output_t
> > *output, enum pipe pipe,
> > >  static void
> > >  test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t
> > flags)
> > >  {
> > > -uint64_t rate;
> > >  uint32_t result;
> > > +vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> > > +range_t range = get_vrr_range(data, output);
> > > +uint64_t rate = vtest_ns.mid;
> > >
> > > -rate = get_test_rate_ns(data, output);
> > > +igt_info("VRR Test execution on %s, PIPE_%s\n",
> > > + output->name, kmstest_pipe_name(pipe));
> > >
> > The reseting of vrr_enabled prop should be done before prepare test
> > so before we set the
> > desrired fb. I would recommend a call set_vrr_on_pipe(0) before calling
> > prepare_test()
> > This is what I had to do for testing the kernel code. It should be a
> > complete set_vrr_on_pipe() call
> > which will set the prop to 0 and commit
> >
> > Manasi
> >
> >
> > >  prepare_test(data, output, pipe);
> > >
> > > @@ -339,21 +362,45 @@ test_basic(data_t *data, enum pipe pipe,
> > igt_output_t *output, uint32_t flags)
> > >  igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > >        SUSPEND_TEST_NONE);
> > >
> > > -result = flip_and_measure(data, output, pipe, rate,
> > TEST_DURATION_NS);
> > > -
> > > -set_vrr_on_pipe(data, pipe, 0);
> > > +/*
> > > + * Check flipline mode by making sure that flips happen at flipline
> > > + * decision boundary.
> > > + *
> > > + * Example: if range is 40 - 60Hz and
> > > + * if refresh_rate > 60Hz:
> > > + *      Flip should happen at the flipline boundary & returned
> > refresh rate
> > > + *      would be 60Hz.
> > > + * if refresh_rate is 50Hz:
> > > + *      Flip will happen right away so returned refresh rate is
> > 50Hz.
> > > + * if refresh_rate < 40Hz:
> > > + *      Flip should happen at the vmax so the returned refresh rate
> > > + *      would be 40Hz.
> > > + */
> > > +if (flags & TEST_FLIPLINE) {
> > > +rate = rate_from_refresh(range.min - 5);
> > > +result = flip_and_measure(data, output, pipe, rate,
> > TEST_DURATION_NS);
> > > +igt_assert_f(result > 75,
> > > +     "Refresh rate %"PRIu64"ns: Target VRR on threshold
> > not reached, result was %u%%\n",
> > > +     rate, result);
> > > +
> > > +rate = rate_from_refresh(range.max + 5);
> > > +result = flip_and_measure(data, output, pipe, rate,
> > TEST_DURATION_NS);
> > > +igt_assert_f(result > 75,
> > > +     "Refresh rate %"PRIu64"ns: Target VRR on threshold
> > not reached, result was %u%%\n",
> > > +     rate, result);
> > > +}
> > >
> > > -/* This check is delayed until after VRR is disabled so it isn't
> > > - * left enabled if the test fails. */
> > > +rate = vtest_ns.mid;
> > > +result = flip_and_measure(data, output, pipe, rate,
> > TEST_DURATION_NS);
> > >  igt_assert_f(result > 75,
> > > -     "Target VRR on threshold not reached, result was %u%%\n",
> > > -     result);
> > > +     "Refresh rate %"PRIu64"ns: Target VRR on threshold not
> > reached, result was %u%%\n",
> > > +     rate, result);
> > >
> > > +set_vrr_on_pipe(data, pipe, 0);
> > >  result = flip_and_measure(data, output, pipe, rate,
> > TEST_DURATION_NS);
> > > -
> > >  igt_assert_f(result < 10,
> > > -     "Target VRR off threshold exceeded, result was %u%%\n",
> > > -     result);
> > > +     "Refresh rate %"PRIu64"ns: Target VRR off threshold
> > exceeded, result was %u%%\n",
> > > +     rate, result);
> > >
> > >  /* Clean-up */
> > >  igt_plane_set_fb(data->primary, NULL);
> > > @@ -413,6 +460,10 @@ igt_main
> > >  igt_subtest("flip-suspend")
> > >  run_vrr_test(&data, test_basic, TEST_SUSPEND);
> > >
> > > +igt_describe("Make sure that flips happen at flipline decision
> > boundary.");
> > > +igt_subtest("flipline")
> > > +run_vrr_test(&data, test_basic, TEST_FLIPLINE);
> > > +
> > >  igt_fixture {
> > >  igt_display_fini(&data.display);
> > >  }
> > > --
> > > 2.20.1
> > >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev