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

Ville Syrjälä
2016-06-02T16:56:13Z

On Thu, Jun 02, 2016 at 07:50:33AM -0700, Matt Roper wrote:
> 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.

Hmm. Seems like a rather roundabout way of writing "I want gen9".

>
> > 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

--
Ville Syrjälä
Intel OTC