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

[Srinivas, Vidya]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Monday, March 19, 2018 7:35 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 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?

Thank you. Will try implementing this in KMD.

The problem is, the results are now showing consistent.
As in, not always the NV12 src height  multiplier of 4 for height results in non-underruns.
When I used a 29x29 buffer, even though the final source width height was 28x28
It still showed fifo underrun. 

Regards
Vidya

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