| Message ID | 1521125144-28614-3-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Thu, Mar 15, 2018 at 03:45:38PM +0100, Ulrich Hecht wrote: > Fixes false negatives on non-i915 platforms. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > tests/kms_panel_fitting.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c > index b3cee22..6d0be50 100644 > --- a/tests/kms_panel_fitting.c > +++ b/tests/kms_panel_fitting.c > @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t *display) > igt_set_module_param_int("fastboot", 1); > > igt_require(display->is_atomic); > + igt_require(is_i915_device(display->drm_fd)); > igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); > > for_each_pipe_with_valid_output(display, pipe, output) { > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Ulrich, Thank you for the patch. On Thursday, 15 March 2018 16:45:38 EET Ulrich Hecht wrote: > Fixes false negatives on non-i915 platforms. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > tests/kms_panel_fitting.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c > index b3cee22..6d0be50 100644 > --- a/tests/kms_panel_fitting.c > +++ b/tests/kms_panel_fitting.c > @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t *display) > igt_set_module_param_int("fastboot", 1); > > igt_require(display->is_atomic); > + igt_require(is_i915_device(display->drm_fd)); > igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); I'm fine with this patch as a quick fix, but what in this test is Intel- specific ? Can't we replace the Intel generation check with a different feature check ? > for_each_pipe_with_valid_output(display, pipe, output) {
On Tue, Mar 20, 2018 at 01:24:09PM +0200, Laurent Pinchart wrote: > Hi Ulrich, > > Thank you for the patch. > > On Thursday, 15 March 2018 16:45:38 EET Ulrich Hecht wrote: > > Fixes false negatives on non-i915 platforms. > > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > > --- > > tests/kms_panel_fitting.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c > > index b3cee22..6d0be50 100644 > > --- a/tests/kms_panel_fitting.c > > +++ b/tests/kms_panel_fitting.c > > @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t *display) > > igt_set_module_param_int("fastboot", 1); > > > > igt_require(display->is_atomic); > > + igt_require(is_i915_device(display->drm_fd)); > > igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); > > I'm fine with this patch as a quick fix, but what in this test is Intel- > specific ? Can't we replace the Intel generation check with a different > feature check ? There's some checks in there that we can do certain panel fitter mode changes without a modeset (throught ALLOW_MODESET for atomic commits). That's 100% encoding intel hw constraints: Our hw can disable the panel fitter without a modest (so going from upscaled -> native resolution), but not any of the other changes (native -> upscaled or 2 different upscaled versions). -Daniel > > > for_each_pipe_with_valid_output(display, pipe, output) { > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Daniel, On Wednesday, 21 March 2018 10:34:33 EET Daniel Vetter wrote: > On Tue, Mar 20, 2018 at 01:24:09PM +0200, Laurent Pinchart wrote: > > Hi Ulrich, > > > > Thank you for the patch. > > > > On Thursday, 15 March 2018 16:45:38 EET Ulrich Hecht wrote: > > > Fixes false negatives on non-i915 platforms. > > > > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > > > --- > > > > > > tests/kms_panel_fitting.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c > > > index b3cee22..6d0be50 100644 > > > --- a/tests/kms_panel_fitting.c > > > +++ b/tests/kms_panel_fitting.c > > > @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t > > > *display) > > > > > > igt_set_module_param_int("fastboot", 1); > > > > > > igt_require(display->is_atomic); > > > > > > + igt_require(is_i915_device(display->drm_fd)); > > > > > > igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); > > > > I'm fine with this patch as a quick fix, but what in this test is Intel- > > specific ? Can't we replace the Intel generation check with a different > > feature check ? > > There's some checks in there that we can do certain panel fitter mode > changes without a modeset (throught ALLOW_MODESET for atomic commits). > That's 100% encoding intel hw constraints: Our hw can disable the panel > fitter without a modest (so going from upscaled -> native resolution), but > not any of the other changes (native -> upscaled or 2 different upscaled > versions). OK, thank you for the information. Would it make sense to rename the Intel-specific test files to start with i915 (or any other Intel prefix) to make this clear ? > > > for_each_pipe_with_valid_output(display, pipe, output) {
On Wed, Mar 21, 2018 at 10:52:19AM +0200, Laurent Pinchart wrote: > Hi Daniel, > > On Wednesday, 21 March 2018 10:34:33 EET Daniel Vetter wrote: > > On Tue, Mar 20, 2018 at 01:24:09PM +0200, Laurent Pinchart wrote: > > > Hi Ulrich, > > > > > > Thank you for the patch. > > > > > > On Thursday, 15 March 2018 16:45:38 EET Ulrich Hecht wrote: > > > > Fixes false negatives on non-i915 platforms. > > > > > > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > > > > --- > > > > > > > > tests/kms_panel_fitting.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c > > > > index b3cee22..6d0be50 100644 > > > > --- a/tests/kms_panel_fitting.c > > > > +++ b/tests/kms_panel_fitting.c > > > > @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t > > > > *display) > > > > > > > > igt_set_module_param_int("fastboot", 1); > > > > > > > > igt_require(display->is_atomic); > > > > > > > > + igt_require(is_i915_device(display->drm_fd)); > > > > > > > > igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); > > > > > > I'm fine with this patch as a quick fix, but what in this test is Intel- > > > specific ? Can't we replace the Intel generation check with a different > > > feature check ? > > > > There's some checks in there that we can do certain panel fitter mode > > changes without a modeset (throught ALLOW_MODESET for atomic commits). > > That's 100% encoding intel hw constraints: Our hw can disable the panel > > fitter without a modest (so going from upscaled -> native resolution), but > > not any of the other changes (native -> upscaled or 2 different upscaled > > versions). > > OK, thank you for the information. > > Would it make sense to rename the Intel-specific test files to start with i915 > (or any other Intel prefix) to make this clear ? We prefer to not rename test names too much, fairly disruptive to our CI. We're working on some new infrastructure which should make this easier at least. But yeah if you bring a qualition, then we can make this happen. Need to coordinate with igt maintainers on #intel-gfx (since we also need to pull in CI folks, which hang out there too). -Daniel > > > > > for_each_pipe_with_valid_output(display, pipe, output) { > > -- > Regards, > > Laurent Pinchart >
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c index b3cee22..6d0be50 100644 --- a/tests/kms_panel_fitting.c +++ b/tests/kms_panel_fitting.c @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t *display) igt_set_module_param_int("fastboot", 1); igt_require(display->is_atomic); + igt_require(is_i915_device(display->drm_fd)); igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); for_each_pipe_with_valid_output(display, pipe, output) {
Fixes false negatives on non-i915 platforms. Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> --- tests/kms_panel_fitting.c | 1 + 1 file changed, 1 insertion(+)