[igt-dev] [PATCH i-g-t] [i-g-t] tests/kms_plane_scaling: fb height to be multiplier of 4 for YUV 420 planar

[Ville Syrjälä]

On Sat, Mar 17, 2018 at 09:15:48AM +0000, Srinivas, Vidya wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Friday, March 16, 2018 8:01 PM
> > To: Srinivas, Vidya <vidya.srinivas at intel.com>
> > Cc: igt-dev at lists.freedesktop.org; Kamath, Sunil <sunil.kamath at intel.com>;
> > Syrjala, Ville <ville.syrjala at intel.com>
> > Subject: Re: [igt-dev] [PATCH i-g-t] [i-g-t] tests/kms_plane_scaling: fb height
> > to be multiplier of 4 for YUV 420 planar
> > 
> > On Fri, Mar 16, 2018 at 06:50:27PM +0530, Vidya Srinivas wrote:
> > > For Gen9, GLK, CNL, GLV: Display WA 1106:
> > > Display corruption/color shift observed when using NV12 with
> > > 270 rotation or 90 rotation + horizontal flip.
> > > WA: NV12 with 270 rotation or 90 rotation + horizontal flip requires
> > > the programmed plane height to be a multiple of 4.
> > > Patch changes the NV12 buffer to 20x20 to maintain both fb > min fb
> > > and also multiplier of 4
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > > ---
> > >  tests/kms_plane_scaling.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> > > index 36fcfc0..b94d785 100644
> > > --- a/tests/kms_plane_scaling.c
> > > +++ b/tests/kms_plane_scaling.c
> > > @@ -132,7 +132,7 @@ static void check_scaling_pipe_plane_rot(data_t *d,
> > igt_plane_t *plane,
> > >  	/* create buffer in the range of  min and max source side limit.*/
> > >  	width = height = 9;
> > >  	if (pixel_format == DRM_FORMAT_NV12)
> > > -		width = height = 17;
> > > +		width = height = 20;
> > 
> > Why 20 and not 16? I thought 16 was the limit?
> 
> Yes, but when the NV12 17x17 buffer is flipped on the sprite, it gets adjusted to 16x16 in
> Intel_check_sprite_plane and further PLANE_SIZE gets written with 15x15.

I don't see what the -1 bias of the register values has to do with this.

> 
> So, I thought 20x20 would be better. However when I run the kms_plane_scaling with rotation
> About 100 times, I still see rare fifo underruns. This may be due to the WA1106
> Which says the plane height for NV12 in case of rotation needs to be multiplier of 4.
> Even with 20x20 buffer, after the adjustment it changes to 18x19 and this further reduces to 17x18 in skl_update_plane
> This causes 18 to be programmed as plane height which is not a multiplier of 4.

The bias has nothing to do with this. But the reduction is source size
to make it an exact multiple of the hscale/vscale can certainly be a
problem. That thing needs some rethinking for sure. Sadly the spec
doesn't document the scaling algorithm so it's hard to say what is the
best apporach.

But this does appear to highlight a problem in the nv12 patches. They
should be checking the final source coordinates against the hardware
minimum size, and reject the operation if we're trying to do something
bad. Perhaps we already have that same problem with non-nv12?

> 
> So this patch too is not good :( Sorry about pushing this.
> 
> Regards
> Vidya
> 
> > 
> > >  	igt_create_color_fb(display->drm_fd, width, height,
> > >  		       pixel_format, tiling, 0.0, 1.0, 0.0, &d->fb[0]);
> > >  	igt_plane_set_fb(plane, &d->fb[0]);
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > --
> > Ville Syrjälä
> > Intel OTC

-- 
Ville Syrjälä
Intel OTC