[igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks

[Ville Syrjälä]

On Wed, Apr 17, 2019 at 09:52:21PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-17 21:35:44)
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > The earlier approach of checking the higher tiled stride
> > limit has backfired. All out blits are between tiled and
> > linear, but we only ever check this for the tiled fb. Thus
> > we are taking the blitter path even though the linear fb
> > exceeds the blitter limits. So let's just check the limits
> > as if we are operating on linear fbs.
> > 
> > And let's toss in some width/height checks, and let's do
> > the checks for all the color planes as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  lib/igt_fb.c | 26 +++++++++++++++++---------
> >  1 file changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 794f0eef04a3..65fc94f98a69 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1580,29 +1580,37 @@ struct fb_blit_upload {
> >         struct intel_batchbuffer *batch;
> >  };
> >  
> > -static int max_blitter_stride(int fd, uint64_t modifier)
> > +static bool blitter_ok(const struct igt_fb *fb)
> >  {
> > -       int stride = 32768;
> > -
> > -       if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
> > -           modifier != DRM_FORMAT_MOD_NONE)
> > -               stride *= 4;
> > +       for (int i = 0; i < fb->num_planes; i++) {
> > +               /*
> > +                * gen4+ stride limit is 4x this with tiling,
> > +                * but since our blits are always between tiled
> > +                * and linear and we do this check just for the
> > +                * tiled fb we must use the lower linear stride
> > +                * limit here.
> > +                */
> 
> I had to read this a couple of times before I realised you meant
> "but since our blits are always between tiled and linear surfaces,
> we must use the lower limit for linear surfaces here".
> 
> So the fun fact about tiled surfaces having their limit in dwords was a
> complete red herring. Sorry.

No worries. The code is admittedly confusing. Ideally we'd do the check
for both the linear and tile surfaces, but we don't have the linear one
when we do the first check. So we'd have to at least construct its
metadata ahead of time. Doable, but I decided to go with the more
minimal apporach for now.

> 
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel