[igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Jani Nikula]

Running the tests with i915.disable_display=1 leads to IGT errors. Skip
tests that need display.

References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

What's the best paradigm for this? There's loads of random and cargo
culted igt_requires for this stuff, with various checks on pipes > 0
etc.
---
 tests/kms_addfb_basic.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index 7d8852f02003..e7d3c0e298a3 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -543,9 +543,15 @@ int fd;
 
 igt_main
 {
-	igt_fixture
+	igt_display_t display;
+
+	igt_fixture {
 		fd = drm_open_driver_master(DRIVER_ANY);
 
+		igt_display_init(&display, fd);
+		igt_display_require_output(&display);
+	}
+
 	invalid_tests(fd);
 
 	pitch_tests(fd);
@@ -560,6 +566,8 @@ igt_main
 
 	prop_tests(fd);
 
-	igt_fixture
+	igt_fixture {
+		igt_display_fini(&display);
 		close(fd);
+	}
 }
-- 
2.11.0

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

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Maarten Lankhorst]

Op 28-06-18 om 11:56 schreef Jani Nikula:
> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
> tests that need display.
>
> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> What's the best paradigm for this? There's loads of random and cargo
> culted igt_requires for this stuff, with various checks on pipes > 0
> etc.
> ---
igt_display_require_output() is the easiest check to find if an output is connected, should be approximately same as display enabled. :)

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

>  tests/kms_addfb_basic.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> index 7d8852f02003..e7d3c0e298a3 100644
> --- a/tests/kms_addfb_basic.c
> +++ b/tests/kms_addfb_basic.c
> @@ -543,9 +543,15 @@ int fd;
>  
>  igt_main
>  {
> -	igt_fixture
> +	igt_display_t display;
> +
> +	igt_fixture {
>  		fd = drm_open_driver_master(DRIVER_ANY);
>  
> +		igt_display_init(&display, fd);
> +		igt_display_require_output(&display);
> +	}
> +
>  	invalid_tests(fd);
>  
>  	pitch_tests(fd);
> @@ -560,6 +566,8 @@ igt_main
>  
>  	prop_tests(fd);
>  
> -	igt_fixture
> +	igt_fixture {
> +		igt_display_fini(&display);
>  		close(fd);
> +	}
>  }


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

[igt-dev] ✓ Fi.CI.BAT: success for igt/kms_addfb_basic: require display

[Patchwork]

== Series Details ==

Series: igt/kms_addfb_basic: require display
URL   : https://patchwork.freedesktop.org/series/45567/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4373 -> IGTPW_1507 =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1507 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1507, 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/45567/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@kms_addfb_basic@unused-modifier:
      fi-kbl-guc:         PASS -> SKIP +36

    igt@kms_addfb_basic@unused-pitches:
      {fi-kbl-x1275}:     PASS -> SKIP +36

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

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

  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128


== Participating hosts (43 -> 39) ==

  Additional (1): fi-pnv-d510 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4529 -> IGTPW_1507

  CI_DRM_4373: be7193758db79443ad5dc45072a166746819ba7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1507: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1507/
  IGT_4529: 23d50a49413aff619d00ec50fc2e051e9b45baa5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1507/issues.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] igt/kms_addfb_basic: require display

[Daniel Vetter]

On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
> Op 28-06-18 om 11:56 schreef Jani Nikula:
> > Running the tests with i915.disable_display=1 leads to IGT errors. Skip
> > tests that need display.
> >
> > References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > ---
> >
> > What's the best paradigm for this? There's loads of random and cargo
> > culted igt_requires for this stuff, with various checks on pipes > 0
> > etc.
> > ---
> igt_display_require_output() is the easiest check to find if an output
> is connected, should be approximately same as display enabled. :)

Unfortunately not quite. I think we do want to run these tests here on
headless systems which have a display block, but no outputs connected.
Just for better test coverage and all that.

I think what we want is a new

igt_require_display(int fd)
{
	igt_display_t display;
	igt_display_init(&display, fd);
	igt_require(display->n_pipes > 0, "drm driver without display
	hardware\n");
}

gtkdoc plus wiring it up needed too ofc.

We could also patch up igt_display_init() to check for this (and probably
should, not sure about that), but a very simply igt_require helper that
only takes the fd seems useful to me for all the low-level tests like
kms_addfb_basic here which don't want/need a full igt_display_t
initialized.
-Daniel

> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> >  tests/kms_addfb_basic.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> > index 7d8852f02003..e7d3c0e298a3 100644
> > --- a/tests/kms_addfb_basic.c
> > +++ b/tests/kms_addfb_basic.c
> > @@ -543,9 +543,15 @@ int fd;
> >  
> >  igt_main
> >  {
> > -	igt_fixture
> > +	igt_display_t display;
> > +
> > +	igt_fixture {
> >  		fd = drm_open_driver_master(DRIVER_ANY);
> >  
> > +		igt_display_init(&display, fd);
> > +		igt_display_require_output(&display);
> > +	}
> > +
> >  	invalid_tests(fd);
> >  
> >  	pitch_tests(fd);
> > @@ -560,6 +566,8 @@ igt_main
> >  
> >  	prop_tests(fd);
> >  
> > -	igt_fixture
> > +	igt_fixture {
> > +		igt_display_fini(&display);
> >  		close(fd);
> > +	}
> >  }
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for igt/kms_addfb_basic: require display

[Patchwork]

== Series Details ==

Series: igt/kms_addfb_basic: require display
URL   : https://patchwork.freedesktop.org/series/45567/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4529_full -> IGTPW_1507_full =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-render:
      {shard-glk9}:       NOTRUN -> FAIL (fdo#103158) +1

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@2x-flip-vs-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      {shard-glk9}:       NOTRUN -> FAIL (fdo#100368)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip_tiling@flip-to-y-tiled:
      {shard-glk9}:       NOTRUN -> FAIL (fdo#104724)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc:
      {shard-glk9}:       NOTRUN -> FAIL (fdo#103167, fdo#104724)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925, fdo#104724)

    igt@kms_sysfs_edid_timing:
      {shard-glk9}:       NOTRUN -> WARN (fdo#100047)

    igt@pm_rpm@system-suspend:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@prime_busy@basic-before-default:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      shard-apl:          DMESG-FAIL (fdo#106947, fdo#106560) -> PASS

    igt@gem_exec_params@rs-invalid-on-bsd-ring:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

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

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@dpms-vs-vblank-race:
      shard-glk:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#104724) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render:
      shard-snb:          FAIL (fdo#103167, fdo#104724) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      shard-glk:          DMESG-WARN (fdo#106247) -> PASS

    igt@kms_rotation_crc@primary-rotation-180:
      shard-hsw:          FAIL (fdo#103925, fdo#104724) -> PASS

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

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

  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Additional (1): shard-glk9 


== Build changes ==

    * IGT: IGT_4529 -> IGTPW_1507
    * Linux: CI_DRM_4371 -> CI_DRM_4373

  CI_DRM_4371: 9094e9d97a6e13db8c1a444d08c0988adad9a002 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4373: be7193758db79443ad5dc45072a166746819ba7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1507: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1507/
  IGT_4529: 23d50a49413aff619d00ec50fc2e051e9b45baa5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1507/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] igt/kms_addfb_basic: require display

[Maarten Lankhorst]

Op 28-06-18 om 13:54 schreef Daniel Vetter:
> On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
>> Op 28-06-18 om 11:56 schreef Jani Nikula:
>>> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
>>> tests that need display.
>>>
>>> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> ---
>>>
>>> What's the best paradigm for this? There's loads of random and cargo
>>> culted igt_requires for this stuff, with various checks on pipes > 0
>>> etc.
>>> ---
>> igt_display_require_output() is the easiest check to find if an output
>> is connected, should be approximately same as display enabled. :)
> Unfortunately not quite. I think we do want to run these tests here on
> headless systems which have a display block, but no outputs connected.
> Just for better test coverage and all that.
>
> I think what we want is a new
>
> igt_require_display(int fd)
> {
> 	igt_display_t display;
> 	igt_display_init(&display, fd);
> 	igt_require(display->n_pipes > 0, "drm driver without display
> 	hardware\n");
> }
>
> gtkdoc plus wiring it up needed too ofc.
>
> We could also patch up igt_display_init() to check for this (and probably
> should, not sure about that), but a very simply igt_require helper that
> only takes the fd seems useful to me for all the low-level tests like
> kms_addfb_basic here which don't want/need a full igt_display_t
> initialized.
But igt_display is not without side effects. It sets the atomic cap, which would be surprising in a require statement.

i915 without pipes should probably unset DRIVER_MODESET.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Daniel Vetter]

On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst wrote:
> Op 28-06-18 om 13:54 schreef Daniel Vetter:
> > On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
> >> Op 28-06-18 om 11:56 schreef Jani Nikula:
> >>> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
> >>> tests that need display.
> >>>
> >>> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>>
> >>> ---
> >>>
> >>> What's the best paradigm for this? There's loads of random and cargo
> >>> culted igt_requires for this stuff, with various checks on pipes > 0
> >>> etc.
> >>> ---
> >> igt_display_require_output() is the easiest check to find if an output
> >> is connected, should be approximately same as display enabled. :)
> > Unfortunately not quite. I think we do want to run these tests here on
> > headless systems which have a display block, but no outputs connected.
> > Just for better test coverage and all that.
> >
> > I think what we want is a new
> >
> > igt_require_display(int fd)
> > {
> > 	igt_display_t display;
> > 	igt_display_init(&display, fd);
> > 	igt_require(display->n_pipes > 0, "drm driver without display
> > 	hardware\n");
> > }
> >
> > gtkdoc plus wiring it up needed too ofc.
> >
> > We could also patch up igt_display_init() to check for this (and probably
> > should, not sure about that), but a very simply igt_require helper that
> > only takes the fd seems useful to me for all the low-level tests like
> > kms_addfb_basic here which don't want/need a full igt_display_t
> > initialized.
> But igt_display is not without side effects. It sets the atomic cap, which would be surprising in a require statement.

Hm, then maybe we need to open-coude the drmgetresources call + checking
for numCrtcs. Not that many more lines really.

> i915 without pipes should probably unset DRIVER_MODESET.

Too late by that point, and we're not the only driver doing this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Maarten Lankhorst]

Op 28-06-18 om 14:10 schreef Daniel Vetter:
> On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst wrote:
>> Op 28-06-18 om 13:54 schreef Daniel Vetter:
>>> On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
>>>> Op 28-06-18 om 11:56 schreef Jani Nikula:
>>>>> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
>>>>> tests that need display.
>>>>>
>>>>> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>>
>>>>> ---
>>>>>
>>>>> What's the best paradigm for this? There's loads of random and cargo
>>>>> culted igt_requires for this stuff, with various checks on pipes > 0
>>>>> etc.
>>>>> ---
>>>> igt_display_require_output() is the easiest check to find if an output
>>>> is connected, should be approximately same as display enabled. :)
>>> Unfortunately not quite. I think we do want to run these tests here on
>>> headless systems which have a display block, but no outputs connected.
>>> Just for better test coverage and all that.
>>>
>>> I think what we want is a new
>>>
>>> igt_require_display(int fd)
>>> {
>>> 	igt_display_t display;
>>> 	igt_display_init(&display, fd);
>>> 	igt_require(display->n_pipes > 0, "drm driver without display
>>> 	hardware\n");
>>> }
>>>
>>> gtkdoc plus wiring it up needed too ofc.
>>>
>>> We could also patch up igt_display_init() to check for this (and probably
>>> should, not sure about that), but a very simply igt_require helper that
>>> only takes the fd seems useful to me for all the low-level tests like
>>> kms_addfb_basic here which don't want/need a full igt_display_t
>>> initialized.
>> But igt_display is not without side effects. It sets the atomic cap, which would be surprising in a require statement.
> Hm, then maybe we need to open-coude the drmgetresources call + checking
> for numCrtcs. Not that many more lines really.
>
>> i915 without pipes should probably unset DRIVER_MODESET.
> Too late by that point, and we're not the only driver doing this.
Well drmGetResources could work, but I wouldn't put it in core unless other stuff cares about it.
Just a function in kms_addfb_basic.c is probably good enough. :)

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

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Daniel Vetter]

On Thu, Jun 28, 2018 at 2:22 PM, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 28-06-18 om 14:10 schreef Daniel Vetter:
>> On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst wrote:
>>> Op 28-06-18 om 13:54 schreef Daniel Vetter:
>>>> On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
>>>>> Op 28-06-18 om 11:56 schreef Jani Nikula:
>>>>>> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
>>>>>> tests that need display.
>>>>>>
>>>>>> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> What's the best paradigm for this? There's loads of random and cargo
>>>>>> culted igt_requires for this stuff, with various checks on pipes > 0
>>>>>> etc.
>>>>>> ---
>>>>> igt_display_require_output() is the easiest check to find if an output
>>>>> is connected, should be approximately same as display enabled. :)
>>>> Unfortunately not quite. I think we do want to run these tests here on
>>>> headless systems which have a display block, but no outputs connected.
>>>> Just for better test coverage and all that.
>>>>
>>>> I think what we want is a new
>>>>
>>>> igt_require_display(int fd)
>>>> {
>>>>     igt_display_t display;
>>>>     igt_display_init(&display, fd);
>>>>     igt_require(display->n_pipes > 0, "drm driver without display
>>>>     hardware\n");
>>>> }
>>>>
>>>> gtkdoc plus wiring it up needed too ofc.
>>>>
>>>> We could also patch up igt_display_init() to check for this (and probably
>>>> should, not sure about that), but a very simply igt_require helper that
>>>> only takes the fd seems useful to me for all the low-level tests like
>>>> kms_addfb_basic here which don't want/need a full igt_display_t
>>>> initialized.
>>> But igt_display is not without side effects. It sets the atomic cap, which would be surprising in a require statement.
>> Hm, then maybe we need to open-coude the drmgetresources call + checking
>> for numCrtcs. Not that many more lines really.
>>
>>> i915 without pipes should probably unset DRIVER_MODESET.
>> Too late by that point, and we're not the only driver doing this.
> Well drmGetResources could work, but I wouldn't put it in core unless other stuff cares about it.
> Just a function in kms_addfb_basic.c is probably good enough. :)

There's other testcase which want this too, this was just an example
... There's a subtest in vgem_prime, and then there's more outside of
BAT most likely.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Maarten Lankhorst]

Op 28-06-18 om 15:12 schreef Daniel Vetter:
> On Thu, Jun 28, 2018 at 2:22 PM, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> Op 28-06-18 om 14:10 schreef Daniel Vetter:
>>> On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst wrote:
>>>> Op 28-06-18 om 13:54 schreef Daniel Vetter:
>>>>> On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst wrote:
>>>>>> Op 28-06-18 om 11:56 schreef Jani Nikula:
>>>>>>> Running the tests with i915.disable_display=1 leads to IGT errors. Skip
>>>>>>> tests that need display.
>>>>>>>
>>>>>>> References: http://patchwork.freedesktop.org/patch/msgid/20180608124057.6889-1-jani.nikula@intel.com
>>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> What's the best paradigm for this? There's loads of random and cargo
>>>>>>> culted igt_requires for this stuff, with various checks on pipes > 0
>>>>>>> etc.
>>>>>>> ---
>>>>>> igt_display_require_output() is the easiest check to find if an output
>>>>>> is connected, should be approximately same as display enabled. :)
>>>>> Unfortunately not quite. I think we do want to run these tests here on
>>>>> headless systems which have a display block, but no outputs connected.
>>>>> Just for better test coverage and all that.
>>>>>
>>>>> I think what we want is a new
>>>>>
>>>>> igt_require_display(int fd)
>>>>> {
>>>>>     igt_display_t display;
>>>>>     igt_display_init(&display, fd);
>>>>>     igt_require(display->n_pipes > 0, "drm driver without display
>>>>>     hardware\n");
>>>>> }
>>>>>
>>>>> gtkdoc plus wiring it up needed too ofc.
>>>>>
>>>>> We could also patch up igt_display_init() to check for this (and probably
>>>>> should, not sure about that), but a very simply igt_require helper that
>>>>> only takes the fd seems useful to me for all the low-level tests like
>>>>> kms_addfb_basic here which don't want/need a full igt_display_t
>>>>> initialized.
>>>> But igt_display is not without side effects. It sets the atomic cap, which would be surprising in a require statement.
>>> Hm, then maybe we need to open-coude the drmgetresources call + checking
>>> for numCrtcs. Not that many more lines really.
>>>
>>>> i915 without pipes should probably unset DRIVER_MODESET.
>>> Too late by that point, and we're not the only driver doing this.
>> Well drmGetResources could work, but I wouldn't put it in core unless other stuff cares about it.
>> Just a function in kms_addfb_basic.c is probably good enough. :)
> There's other testcase which want this too, this was just an example
> ... There's a subtest in vgem_prime, and then there's more outside of
> BAT most likely.
> -Daniel

Since this doesn't operate on igt_display, probably needs to be kmstest_require_pipe()?

With that + drmModeGetResources, I'm happy. :)

~Maarten

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

Re: [igt-dev] [PATCH i-g-t] igt/kms_addfb_basic: require display

[Souza, Jose]

On Thu, 2018-06-28 at 15:21 +0200, Maarten Lankhorst wrote:
> Op 28-06-18 om 15:12 schreef Daniel Vetter:
> > On Thu, Jun 28, 2018 at 2:22 PM, Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com> wrote:
> > > Op 28-06-18 om 14:10 schreef Daniel Vetter:
> > > > On Thu, Jun 28, 2018 at 02:06:49PM +0200, Maarten Lankhorst
> > > > wrote:
> > > > > Op 28-06-18 om 13:54 schreef Daniel Vetter:
> > > > > > On Thu, Jun 28, 2018 at 12:19:57PM +0200, Maarten Lankhorst
> > > > > > wrote:
> > > > > > > Op 28-06-18 om 11:56 schreef Jani Nikula:
> > > > > > > > Running the tests with i915.disable_display=1 leads to
> > > > > > > > IGT errors. Skip
> > > > > > > > tests that need display.
> > > > > > > > 
> > > > > > > > References: http://patchwork.freedesktop.org/patch/msgi
> > > > > > > > d/20180608124057.6889-1-jani.nikula@intel.com
> > > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.co
> > > > > > > > m>
> > > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > > > > > > 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > What's the best paradigm for this? There's loads of
> > > > > > > > random and cargo
> > > > > > > > culted igt_requires for this stuff, with various checks
> > > > > > > > on pipes > 0
> > > > > > > > etc.
> > > > > > > > ---
> > > > > > > 
> > > > > > > igt_display_require_output() is the easiest check to find
> > > > > > > if an output
> > > > > > > is connected, should be approximately same as display
> > > > > > > enabled. :)
> > > > > > 
> > > > > > Unfortunately not quite. I think we do want to run these
> > > > > > tests here on
> > > > > > headless systems which have a display block, but no outputs
> > > > > > connected.
> > > > > > Just for better test coverage and all that.
> > > > > > 
> > > > > > I think what we want is a new
> > > > > > 
> > > > > > igt_require_display(int fd)
> > > > > > {
> > > > > >     igt_display_t display;
> > > > > >     igt_display_init(&display, fd);
> > > > > >     igt_require(display->n_pipes > 0, "drm driver without
> > > > > > display
> > > > > >     hardware\n");
> > > > > > }
> > > > > > 
> > > > > > gtkdoc plus wiring it up needed too ofc.
> > > > > > 
> > > > > > We could also patch up igt_display_init() to check for this
> > > > > > (and probably
> > > > > > should, not sure about that), but a very simply igt_require
> > > > > > helper that
> > > > > > only takes the fd seems useful to me for all the low-level
> > > > > > tests like
> > > > > > kms_addfb_basic here which don't want/need a full
> > > > > > igt_display_t
> > > > > > initialized.
> > > > > 
> > > > > But igt_display is not without side effects. It sets the
> > > > > atomic cap, which would be surprising in a require statement.
> > > > 
> > > > Hm, then maybe we need to open-coude the drmgetresources call +
> > > > checking
> > > > for numCrtcs. Not that many more lines really.
> > > > 
> > > > > i915 without pipes should probably unset DRIVER_MODESET.
> > > > 
> > > > Too late by that point, and we're not the only driver doing
> > > > this.
> > > 
> > > Well drmGetResources could work, but I wouldn't put it in core
> > > unless other stuff cares about it.
> > > Just a function in kms_addfb_basic.c is probably good enough. :)
> > 
> > There's other testcase which want this too, this was just an
> > example
> > ... There's a subtest in vgem_prime, and then there's more outside
> > of
> > BAT most likely.
> > -Daniel
> 
> Since this doesn't operate on igt_display, probably needs to be
> kmstest_require_pipe()?
> 
> With that + drmModeGetResources, I'm happy. :)

I just sent a patchset(https://patchwork.freedesktop.org/series/46630/)
unseting DRIVER_MODESET when display is disabled also added the lines
needed to check if a drm driver have modeset capability.

So what about test like this:

void igt_require_display(int fd)
{
	drmModeRes *resources;
	int pipes;
	uint64_t cap = 0;

	drmGetCap(fd, DRM_CAP_MODESET, &cap);
	igt_require_f(cap, "drm driver do not support modeset\n");

	resources = drmModeGetResources(fd);
	igt_assert(resources);
	pipes = resources->count_crtcs;
	drmModeFreeResources(resources);

	igt_require_f(pipes > 0, "drm driver without display
hardware\n");
}

I also have a patch calling it from all tests that needs display.

> 
> ~Maarten
> 
> _______________________________________________
> 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