[igt-dev] [PATCH i-g-t v3 1/2] lib/igt_fb: Don't use blitter for large buffers

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 08:49:30PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2019-04-09 13:25:55)
> > On Mon, Apr 08, 2019 at 06:20:50PM +0100, Chris Wilson wrote:
> > > Quoting Chris Wilson (2019-04-08 18:19:26)
> > > > Quoting Ville Syrjala (2019-04-08 18:13:06)
> > > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > 
> > > > > The blitter stride is limited to <32k. Fall back to gtt mmap or
> > > > > rendercopy if we're about to exceed that.
> > > > > 
> > > > > Not quite sure why we're not just using gtt mmap for Y tiling
> > > > > always. But let's keep it like that for now.
> > > > > 
> > > > > v2: Use rendercopy as the fallback for Yf
> > > > > v3: Deal with gen4+ tiled stride correctly (Chris)
> > > > > 
> > > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > ---
> > > > >  lib/igt_fb.c | 36 +++++++++++++++++++++++++++++-------
> > > > >  1 file changed, 29 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 6adf422228e8..d2f756dba2e9 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -1576,6 +1576,31 @@ struct fb_blit_upload {
> > > > >         struct intel_batchbuffer *batch;
> > > > >  };
> > > > >  
> > > > > +static int max_blitter_stride(int fd, uint64_t modifier)
> > > > > +{
> > > > > +       int stride = 32768;
> > > > > +
> > > > > +       if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
> > > > > +           modifier != DRM_FORMAT_MOD_NONE)
> > > > 
> > > > * fingers crossed that the modifier is the single truth
> > > > 
> > > > > +               stride *= 4;
> > > > > +
> > > > > +       return stride;
> > > > > +}
> > > > > +
> > > > > +static bool use_rendercopy(const struct igt_fb *fb)
> > > > > +{
> > > > > +       return is_ccs_modifier(fb->modifier) ||
> > > > > +               (fb->modifier == I915_FORMAT_MOD_Yf_TILED &&
> > > > > +                fb->strides[0] >= max_blitter_stride(fb->fd, fb->modifier));
> > > > > +}
> > > > > +
> > > > > +static bool use_blitter(const struct igt_fb *fb)
> > > > > +{
> > > > > +       return (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> > > > > +               fb->modifier == I915_FORMAT_MOD_Yf_TILED) &&
> > > 
> > > Oh but you would prefer to use rendercopy for Y_TILED (not just
> > > Yf_TILED). The blitter does work with Y_TILED, but it is quite slow.
> > 
> > Or we could just switch Y tiled to always use gtt mmap?
> 
> We could, just make sure to use memcpy_from_wc to speed it up. Although
> GTT is on the way out, I doubt it affects these use cases.

Right. I guess I'd like to keep the dependencies as minimal as
possible. Getting rendercopy updated for new hardware hasn't always
happened in the most timely manner. If we start depending on it too
much we may end up blocking early display testing. But as you say
gtt is on its way out so maybe we should think about minimizing its
use instead.

I suppose use_rendercopy() should check that rendercopy is actually
available. That way we could prefer rendercopy over the other methods
while still allowing graceful fallback. Well, assuming that our
rendercopy implementation and hw actually works.

-- 
Ville Syrjälä
Intel