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

[Matt Roper <matthew.d.roper@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx