Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_scaling: Check supported format for rotation

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 04, 2019 at 01:20:47PM +0000, Kahola, Mika wrote:
> On Thu, 2019-04-04 at 14:57 +0200, Daniel Vetter wrote:
> > On Thu, Apr 04, 2019 at 01:37:02PM +0300, Mika Kahola wrote:
> > > Let's align with kernel and add checks for formats that are
> > > not allowed to do 90/270 rotation.
> > > 
> > > In dmesg we see for example, the following
> > > 
> > > "[drm:skl_plane_check [i915]] Unsupported pixel format Y210
> > > little-endian (0x30313259) for 90/270!"
> > > 
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > 
> > Can we try to use try_commit/TEST_ONLY to figure at least some of
> > this
> > out? Duplicating hard-coded tables in tests from the kernel isn't
> > great.
> I see what the problem here is that we need to keep up with the tables
> that are defined in kernel side also here in IGT side. We could do
> try_commit but then kernel returns with -EINVAL. What should we do
> then? We cannot be 100% sure that this was caused due to unsupported
> format and skip this error and pass the test.

I think the solution we kinda want is per-format+modifier subtests.
Then we could clearly see which formats were skipped, and could check
against the expected list of skips. The problem is that is a *lot*
of subtests.

Though for a test like this I'm not sure it makes sense to test every
pixel format anyway. Might want to just a pick one representive from
each set of similar formats (and hope the hw isn't nuts and implement
similar formats differently).

> 
> Maybe we could move all these tables into library routine instead? All
> table would be defined in one place and probably the maintenance effort
> would be less demanding?
> 
> Cheers,
> Mika
> > 
> > Note that the rotation support predates i915 atomic support, so back
> > then
> > we didn't have any other choice really. Now we do.
> > -Daniel
> > 
> > > ---
> > >  tests/kms_plane_scaling.c | 20 ++++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> > > index 3364073c..2d47c14b 100644
> > > --- a/tests/kms_plane_scaling.c
> > > +++ b/tests/kms_plane_scaling.c
> > > @@ -176,9 +176,25 @@ static const igt_rotation_t rotations[] = {
> > >  
> > >  static bool can_rotate(data_t *d, unsigned format)
> > >  {
> > > -	if (format == DRM_FORMAT_C8 ||
> > > -	    (intel_gen(d->devid) < 11 && format == DRM_FORMAT_RGB565))
> > > +	switch (format) {
> > > +	case DRM_FORMAT_RGB565:
> > > +		if (intel_gen(d->devid) >= 11)
> > > +			break;
> > > +	/* fall through */
> > > +	case DRM_FORMAT_C8:
> > > +	case DRM_FORMAT_XRGB16161616F:
> > > +	case DRM_FORMAT_XBGR16161616F:
> > > +	case DRM_FORMAT_ARGB16161616F:
> > > +	case DRM_FORMAT_ABGR16161616F:
> > > +	case DRM_FORMAT_Y210:
> > > +	case DRM_FORMAT_Y212:
> > > +	case DRM_FORMAT_Y216:
> > > +	case DRM_FORMAT_XVYU12_16161616:
> > > +	case DRM_FORMAT_XVYU16161616:
> > >  		return false;
> > > +	default:
> > > +		break;
> > > +	}
> > >  
> > >  	return true;
> > >  }
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev