Skip to contentSkip to conversation

lib/igt_kms: Set writeback connector capacity to fix VC4 testing

PATCH In review Paul Kocialkowski 3 participants 5 messages Nov 14, 2018 – Nov 15, 2018 Library code

lore
Share & tools
lore

Paul Kocialkowski

author
#0 lore raw

PATCH 1/1 lib/igt_kms: Set writeback connector capacity to fix VC4 testing

Support for writeback connectors was recently introduced in DRM, with an implementation in the VC4 DRM driver. This unfortunately broke IGT testing for the driver due to always-failing atomic commits.

After boot up, the writeback connector is attached to a CRTC, with a virtual encoder and a plane (that takes the same framebuffer as the primary plane).

Because IGT configures all the available planes, this plane is no longer associated with the writeback CRTC. The connector remains associated with the CRTC as the core does not automatically clean it up. IGT is not aware of the writeback connector (hidden by default), so it does not detach the CRTC. As a result, the atomic commit fails because the CRTC is moving to a disabled state with a connector still attached.

Fix this issue by setting the writeback connector cap before getting DRM resources so that the connector can be discovered and the CRTC can be detached by IGT. Since it requires atomic support, make it conditional and move the atomic capability before getting DRM resources.

The associated cap definitions are also updated to include the required writeback one.

Signed-off-by Paul Kocialkowski <[email protected]>
---
 include/drm-uapi/drm.h | 16 ++++++++++++++++
 lib/igt_kms.c          |  8 ++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)
include/drm-uapi/drm.h +16 -0
index f0bd91de..85c685a2 100644
@@ -674,6 +674,22 @@ struct drm_get_cap {
  */
 #define DRM_CLIENT_CAP_ATOMIC	3
 
+/**
+ * DRM_CLIENT_CAP_ASPECT_RATIO
+ *
+ * If set to 1, the DRM core will provide aspect ratio information in modes.
+ */
+#define DRM_CLIENT_CAP_ASPECT_RATIO    4
+
+/**
+ * DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
+ *
+ * If set to 1, the DRM core will expose special connectors to be used for
+ * writing back to memory the scene setup in the commit. Depends on client
+ * also supporting DRM_CLIENT_CAP_ATOMIC
+ */
+#define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS	5
+
 /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
 struct drm_set_client_cap {
 	__u64 capability;
lib/igt_kms.c +6 -2
index d806ccc1..4f356b8b 100644
@@ -1882,6 +1882,12 @@ bool igt_display_init(igt_display_t *display, int drm_fd)
 
 	display->drm_fd = drm_fd;
 
+	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) {
+		display->is_atomic = 1;
+
+		drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+	}
+
 	resources = drmModeGetResources(display->drm_fd);
 	if (!resources)
 		goto out;

@@ -1895,8 +1901,6 @@ bool igt_display_init(igt_display_t *display, int drm_fd)
 	igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
 
 	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
-	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
-		display->is_atomic = 1;
 
 	plane_resources = drmModeGetPlaneResources(display->drm_fd);
 	igt_assert(plane_resources);
Signature · 6 lines
2.19.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
#1 lore raw

Hi Paul,

Quoted 7 lines · On Wed, 14 Nov 2018 16:31:49 +0100 Paul Kocialkowski…

Support for writeback connectors was recently introduced in DRM, with an implementation in the VC4 DRM driver. This unfortunately broke IGT testing for the driver due to always-failing atomic commits.

After boot up, the writeback connector is attached to a CRTC, with a virtual encoder and a plane (that takes the same framebuffer as the primary plane).

That's weird, the writeback connector should not be attached to its CRTC by default, unless someone explicitly requested that. Maybe something related to the fbdev emulation layer, but I'm not sure.

Quoted 13 lines

Because IGT configures all the available planes, this plane is no longer associated with the writeback CRTC. The connector remains associated with the CRTC as the core does not automatically clean it up. IGT is not aware of the writeback connector (hidden by default), so it does not detach the CRTC. As a result, the atomic commit fails because the CRTC is moving to a disabled state with a connector still attached.

Fix this issue by setting the writeback connector cap before getting DRM resources so that the connector can be discovered and the CRTC can be detached by IGT. Since it requires atomic support, make it conditional and move the atomic capability before getting DRM resources.

Quoted 7 lines

The associated cap definitions are also updated to include the required writeback one.

Signed-off-by Paul Kocialkowski <[email protected]>
---
 include/drm-uapi/drm.h | 16 ++++++++++++++++
 lib/igt_kms.c          |  8 ++++++--

Looks like [1] is doing pretty much the same thing.

[1]https://patchwork.freedesktop.org/patch/260289/
_______________________________________________
igt-dev mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

author
#2 lore raw

Hi,

Quoted 9 lines · Boris Brezillon

Hi Paul,

Quoted 4 lines · On Wed, 14 Nov 2018 16:31:49 +0100 Paul Kocialkowski…

Support for writeback connectors was recently introduced in DRM, with an implementation in the VC4 DRM driver. This unfortunately broke IGT testing for the driver due to always-failing atomic commits.

Quoted 7 lines
Quoted 3 lines

After boot up, the writeback connector is attached to a CRTC, with a virtual encoder and a plane (that takes the same framebuffer as the primary plane).

That's weird, the writeback connector should not be attached to its CRTC by default, unless someone explicitly requested that. Maybe something related to the fbdev emulation layer, but I'm not sure.

Thanks for the tip, this was definitely the cause of the writeback connector being attached to the CRTC at boot.

I have just sent out a patch to blacklist writeback connectors in fbdev.

I am not sure it's worth keeping this patch around if that issue is to be fixed in the kernel directly. It also conflicts with the upcoming writeback series in IGT.

Cheers,

Paul

Quoted 23 lines
Quoted 12 lines

Because IGT configures all the available planes, this plane is no longer associated with the writeback CRTC. The connector remains associated with the CRTC as the core does not automatically clean it up. IGT is not aware of the writeback connector (hidden by default), so it does not detach the CRTC. As a result, the atomic commit fails because the CRTC is moving to a disabled state with a connector still attached.

Fix this issue by setting the writeback connector cap before getting DRM resources so that the connector can be discovered and the CRTC can be detached by IGT. Since it requires atomic support, make it conditional and move the atomic capability before getting DRM resources.

Quoted 7 lines

The associated cap definitions are also updated to include the required writeback one.

Signed-off-by Paul Kocialkowski <[email protected]>
---
 include/drm-uapi/drm.h | 16 ++++++++++++++++
 lib/igt_kms.c          |  8 ++++++--

Looks like [1] is doing pretty much the same thing.

[1]https://patchwork.freedesktop.org/patch/260289/

Signature · 3 lines
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

CI
#3 lore raw

[igt-dev] ✓ Fi.CI.BAT: success for lib/igt_kms: Set writeback connector capacity to fix VC4 testing

In reply to Paul Kocialkowski
== Series Details ==

== Series Details ==

Series: lib/igt_kms: Set writeback connector capacity to fix VC4 testing
URL : https://patchwork.freedesktop.org/series/52493/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4714 -> IGTPW_2061 =

== Summary - WARNING ==

Minor unknown changes coming with IGTPW_2061 need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in IGTPW_2061, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

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

=== IGT changes ===

    ==== Warnings ====

    igt@kms_chamelium@hdmi-hpd-fast:
      fi-icl-u2:          SKIP -> PASS +3

    igt@kms_flip@basic-flip-vs-modeset:
      fi-kbl-x1275:       SKIP -> PASS +36

    igt@pm_rpm@basic-rte:
      fi-kbl-7567u:       PASS -> SKIP +4

    igt@prime_vgem@basic-fence-flip:
      fi-cfl-s3:          SKIP -> PASS

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload-inject:
      fi-kbl-7567u:       PASS -> DMESG-WARN (fdo#108529, fdo#105602) +1

    igt@drv_selftest@live_contexts:
      fi-icl-u:           NOTRUN -> INCOMPLETE (fdo#108315)

    igt@drv_selftest@live_hangcheck:
      fi-bwr-2160:        PASS -> DMESG-FAIL (fdo#108735)

    igt@kms_chamelium@common-hpd-after-suspend:
      fi-icl-u2:          SKIP -> DMESG-FAIL (fdo#108070, fdo#107732, fdo#103375)

    igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107732)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362) +1

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-kbl-7567u:       PASS -> DMESG-FAIL (fdo#105079)

    igt@pm_rpm@module-reload:
      fi-kbl-7567u:       PASS -> DMESG-WARN (fdo#108529)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_contexts:
      fi-icl-u2:          INCOMPLETE (fdo#108315) -> DMESG-FAIL (fdo#108569)

fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#105079 https://bugs.freedesktop.org/show_bug.cgi?id=105079
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107732 https://bugs.freedesktop.org/show_bug.cgi?id=107732
fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070
fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
fdo#108529 https://bugs.freedesktop.org/show_bug.cgi?id=108529
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735

== Participating hosts (53 -> 45) ==

Additional (1): fi-icl-u
Missing (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 fi-skl-iommu fi-pnv-d510

== Build changes ==

* IGT: IGT_4714 -> IGTPW_2061

CI_DRM_5106: 852b9329fbb6ea8bdbb3dac78328aae73d919305 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2061: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2061/
IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2061/issues.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

CI
#4 lore raw

[igt-dev] ✓ Fi.CI.IGT: success for lib/igt_kms: Set writeback connector capacity to fix VC4 testing

In reply to Paul Kocialkowski
== Series Details ==

== Series Details ==

Series: lib/igt_kms: Set writeback connector capacity to fix VC4 testing
URL : https://patchwork.freedesktop.org/series/52493/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4714_full -> IGTPW_2061_full =

== Summary - WARNING ==

Minor unknown changes coming with IGTPW_2061_full need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in IGTPW_2061_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://patchwork.freedesktop.org/api/1.0/series/52493/revisions/1/mbox/

== Possible new issues ==

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

=== IGT changes ===

    ==== Warnings ====

    igt@kms_concurrent@pipe-b:
      shard-snb:          PASS -> SKIP +1

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
      shard-kbl:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-glk:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_color@pipe-a-legacy-gamma:
      shard-apl:          PASS -> FAIL (fdo#104782, fdo#108145)

    igt@kms_cursor_crc@cursor-128x128-onscreen:
      shard-glk:          NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-128x42-onscreen:
      shard-apl:          PASS -> FAIL (fdo#103232) +3

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#102887)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          NOTRUN -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
      shard-glk:          PASS -> FAIL (fdo#103167) +6

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-kbl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
      shard-glk:          NOTRUN -> FAIL (fdo#108145) +5

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166) +2

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166) +2

    igt@kms_properties@connector-properties-atomic:
      shard-glk:          NOTRUN -> FAIL (fdo#108642)
      shard-hsw:          NOTRUN -> FAIL (fdo#108642)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_suspend@forcewake:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@drv_suspend@shrink:
      shard-snb:          INCOMPLETE (fdo#106886, fdo#105411) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023, fdo#106887) -> PASS

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-kbl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-glk:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_crc@cursor-64x21-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#105454, fdo#106509) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-kbl:          FAIL (fdo#103166) -> PASS

    igt@kms_universal_plane@universal-plane-pipe-a-functional:
      shard-glk:          FAIL (fdo#103166) -> PASS +2

    igt@kms_vblank@pipe-a-query-busy:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@perf_pmu@busy-start-vcs0:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108642 https://bugs.freedesktop.org/show_bug.cgi?id=108642
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

== Participating hosts (6 -> 5) ==

Missing (1): shard-skl

== Build changes ==

* IGT: IGT_4714 -> IGTPW_2061

CI_DRM_5106: 852b9329fbb6ea8bdbb3dac78328aae73d919305 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2061: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2061/
IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2061/shards.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev