Re: [PATCH i-g-t] kms_plane_scaling: Don't try to use second scaler on pipe C

Matt Roper
2016-06-02T14:50:34Z

On Thu, Jun 02, 2016 at 11:35:08AM +0300, Ville Syrjälä wrote:
> On Wed, Jun 01, 2016 at 03:13:19PM -0700, Matt Roper wrote:
> > Gen9 has two scalers on pipes A & B, but only a single scaler on pipe C.
> > We should bail out of the test early on pipe C so that we don't ask the
> > kernel to use more scalers than we really have.
> >
> > Note that this test may still fail (on any pipe) if we're already using
> > one of the scalers as a panel fitter. But at least this is an
> > improvement over the existing state where the test is guaranteed to fail
> > if run with pipe C active.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92248
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > tests/kms_plane_scaling.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> > index ad5404d..2b17702 100644
> > --- a/tests/kms_plane_scaling.c
> > +++ b/tests/kms_plane_scaling.c
> > @@ -264,6 +264,9 @@ static void test_plane_scaling(data_t *d)
> > igt_display_commit2(display, COMMIT_UNIVERSAL);
> > }
> >
> > + if (pipe == PIPE_C)
> > + goto crtcdone;
> > +
>
> You you make assumptions like that, then you should probably make the
> test skip on !gen9.

It already does as far as I can see. It sets data.num_scalers to 0 for
pre-gen9 in the main function and then has a
igt_require(d->num_scalers) in the test function.

> The test is making a lot of assumptions already though, so probably
> would blow up on many other platforms anyway. It also makes some
> assumptions about display resolutions and whatnot, so might blow up on
> gen9 as well.

Yep, agreed with this. I think there's a lot of improvement that could
be done here, but at the moment I just wanted to fix one case of low
hanging fruit that would fail 100% of the time.

Matt

>
> Would be cool if someone could make this use CRCs as well, but that
> would probably be a little difficult since the hardware scaling
> algorithm is naturally undocumented :( How I long for the old video
> overlay... Hmm. Actually there does seem to be a way to force bilinear
> scaling at least. Matching that with software rendering might be doable.
>
> > /* Set up fb3->plane3 mapping. */
> > d->plane3 = igt_output_get_plane(output, IGT_PLANE_3);
> > igt_plane_set_fb(d->plane3, &d->fb3);
> > @@ -301,9 +304,11 @@ static void test_plane_scaling(data_t *d)
> > igt_display_commit2(display, COMMIT_UNIVERSAL);
> > }
> >
> > +crtcdone:
> > /* back to single plane mode */
> > igt_plane_set_fb(d->plane2, NULL);
> > - igt_plane_set_fb(d->plane3, NULL);
> > + if (pipe != PIPE_C)
> > + igt_plane_set_fb(d->plane3, NULL);
> > igt_display_commit2(display, COMMIT_UNIVERSAL);
> >
> > valid_tests++;
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC

--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795