[igt-dev] [PATCH i-g-t] lib/igt_kms: Set writeback connector capacity to fix VC4 testing

[igt-dev] [PATCH i-g-t] lib/igt_kms: Set writeback connector capacity to fix VC4 testing

[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).

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 <paul.kocialkowski@bootlin.com>
---
 include/drm-uapi/drm.h | 16 ++++++++++++++++
 lib/igt_kms.c          |  8 ++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h
index f0bd91de..85c685a2 100644
--- a/include/drm-uapi/drm.h
+++ b/include/drm-uapi/drm.h
@@ -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;
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1..4f356b8b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -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);
-- 
2.19.1

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

Re: [igt-dev] [PATCH i-g-t] lib/igt_kms: Set writeback connector capacity to fix VC4 testing

[Boris Brezillon]

Hi Paul,

On Wed, 14 Nov 2018 16:31:49 +0100
Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:

> 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.

> 
> 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 <paul.kocialkowski@bootlin.com>
> ---
>  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
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Patchwork]

== 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
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Patchwork]

== 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
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] lib/igt_kms: Set writeback connector capacity to fix VC4 testing

[Paul Kocialkowski]

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

Hi,

On Wed, 2018-11-14 at 16:50 +0100, Boris Brezillon wrote:
> Hi Paul,
> 
> On Wed, 14 Nov 2018 16:31:49 +0100
> Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:
> 
> > 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.

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

> > 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 <paul.kocialkowski@bootlin.com>
> > ---
> >  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/
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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