Re: [igt-dev] [PATCH i-g-t 2/3] igt/kms_addfb_basic: Call igt_require_gem for gem specific tiling

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Deepak Singh Rawat (2019-01-16 16:34:39)
> On Wed, 2019-01-16 at 16:17 +0000, Chris Wilson wrote:
> > Quoting Deepak Singh Rawat (2019-01-16 16:11:16)
> > > On Wed, 2019-01-16 at 00:42 -0800, Chris Wilson wrote:
> > > > Quoting Deepak Rawat (2019-01-16 00:22:29)
> > > > > For tests using gem specific buffer tiling, add a call to
> > > > > igt_require_gem so that these tests don't fail at later stage.
> > > > 
> > > > KMS operates independently from GEM on i915. So long as you are
> > > > not
> > > > calling gem_execbuf, it should continue to work even if we
> > > > disable
> > > > the
> > > > GPU, i.e. igt_require_gem() should not be required here (afaik).
> > > > -Chris
> > > 
> > > gem_set_tiling() calls an i915 private IOCTL which I think would
> > > fail
> > > when GEM is disabled or on other drivers. So I guess either have a
> > > igt_require_gem() or perhaps GEM specific private IOCTL calls SKIP
> > > on
> > > other drivers.
> > 
> > Ah, you want igt_require(is_i915_device(fd)), or introduce something
> > like igt_require_i915(fd). Even better if such specifics are removed,
> > or
> > moved to a subtest group that are clearly device specific.
> > -Chris
> 
> I am not sure if i915 and GEM are independent of each other but yes you
> are right about kms_addfb_basic.c should not have device/GEM specific
> requires. Perhaps the better way would be something like (instead of
> gem_set_tiling()):
> 
> set_tiling() -> device_specific_tiling()
> 
> Also set_tiling() wrapper SKIP's for drivers which do not support
> tiling, which for now applies to all except i915.

It's not just testing a backend tiling format, but also the
corresponding I915_FORMAT_MOD. It won't generalise more than a starting
point for testing other backend specific DRM_FORMAT_MODIFIERS.

That addfb_expected_ret() is bogus. A different backend may reuse the
same value as I915_FORMAT_MOD_Y_TILED for a completely different
purpose, that ends up being valid for the given addfb.

My reading is addfb25_tests(); addfb25_ytile(); tiling_tests(); are all
i915-specific and should be placed in an i915 subtest group. And that we
should try to extract the (one?) generic addfb25 tests.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev