| Message ID | 20180914201310.19527-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,1/5] igt/kms_getfb: Check the iface exists before use | expand |
On Fri, Sep 14, 2018 at 09:13:07PM +0100, Chris Wilson wrote: > We try flipping a vgem surface onto a i915 scanout. However, if there > is no display we want to disable the kms interface, including the addfb > ioctl. On such systems the call to kms_addfb will naturally fail and the > test cannot be run. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > tests/prime_vgem.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c > index b821fbb8c..a76d3797b 100644 > --- a/tests/prime_vgem.c > +++ b/tests/prime_vgem.c > @@ -762,10 +762,13 @@ static void test_flip(int i915, int vgem, unsigned hang) > igt_assert(handle[i]); > close(fd); > > - do_or_die(__kms_addfb(i915, handle[i], > - bo[i].width, bo[i].height, bo[i].pitch, > - DRM_FORMAT_XRGB8888, I915_TILING_NONE, NULL, > - LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id[i])); > + /* May skip if i915 has no displays */ > + igt_require(__kms_addfb(i915, handle[i], > + bo[i].width, bo[i].height, bo[i].pitch, > + DRM_FORMAT_XRGB8888, > + I915_TILING_NONE, NULL, > + LOCAL_DRM_MODE_FB_MODIFIERS, > + &fb_id[i]) == 0); Hm, both here and in patch 1 I feel like this is super late to check for requirements. I think for these low-level tests a kms_require_display which checks for n_pipes > 0 && n_outputs > 0 would be good. Then we can sprinkle these early (and keep the do_or_die here), plus it won't need a comment about why we have the check since it's obvious from the name. -Daniel > igt_assert(fb_id[i]); > } > > -- > 2.19.0 > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev
Quoting Daniel Vetter (2018-10-01 09:49:07) > On Fri, Sep 14, 2018 at 09:13:07PM +0100, Chris Wilson wrote: > > We try flipping a vgem surface onto a i915 scanout. However, if there > > is no display we want to disable the kms interface, including the addfb > > ioctl. On such systems the call to kms_addfb will naturally fail and the > > test cannot be run. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > tests/prime_vgem.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c > > index b821fbb8c..a76d3797b 100644 > > --- a/tests/prime_vgem.c > > +++ b/tests/prime_vgem.c > > @@ -762,10 +762,13 @@ static void test_flip(int i915, int vgem, unsigned hang) > > igt_assert(handle[i]); > > close(fd); > > > > - do_or_die(__kms_addfb(i915, handle[i], > > - bo[i].width, bo[i].height, bo[i].pitch, > > - DRM_FORMAT_XRGB8888, I915_TILING_NONE, NULL, > > - LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id[i])); > > + /* May skip if i915 has no displays */ > > + igt_require(__kms_addfb(i915, handle[i], > > + bo[i].width, bo[i].height, bo[i].pitch, > > + DRM_FORMAT_XRGB8888, > > + I915_TILING_NONE, NULL, > > + LOCAL_DRM_MODE_FB_MODIFIERS, > > + &fb_id[i]) == 0); > > Hm, both here and in patch 1 I feel like this is super late to check for > requirements. This is earlier than the current check, interesting that check will segfault under this scenario. > I think for these low-level tests a kms_require_display > which checks for n_pipes > 0 && n_outputs > 0 would be good. Then we can > sprinkle these early (and keep the do_or_die here), plus it won't need a > comment about why we have the check since it's obvious from the name. The comment was to try and reinforce that this restriction wasn't to do with vgem itself. -Chris
diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c index b821fbb8c..a76d3797b 100644 --- a/tests/prime_vgem.c +++ b/tests/prime_vgem.c @@ -762,10 +762,13 @@ static void test_flip(int i915, int vgem, unsigned hang) igt_assert(handle[i]); close(fd); - do_or_die(__kms_addfb(i915, handle[i], - bo[i].width, bo[i].height, bo[i].pitch, - DRM_FORMAT_XRGB8888, I915_TILING_NONE, NULL, - LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id[i])); + /* May skip if i915 has no displays */ + igt_require(__kms_addfb(i915, handle[i], + bo[i].width, bo[i].height, bo[i].pitch, + DRM_FORMAT_XRGB8888, + I915_TILING_NONE, NULL, + LOCAL_DRM_MODE_FB_MODIFIERS, + &fb_id[i]) == 0); igt_assert(fb_id[i]); }
We try flipping a vgem surface onto a i915 scanout. However, if there is no display we want to disable the kms interface, including the addfb ioctl. On such systems the call to kms_addfb will naturally fail and the test cannot be run. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- tests/prime_vgem.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)