[Mesa-dev] [PATCH] [RFC] i965/blit: bump some limits to 64k

[Nanley Chery]

On Thu, Jun 14, 2018 at 10:01:18AM -0700, Nanley Chery wrote:
> On Thu, Jun 14, 2018 at 04:18:30PM +0300, Martin Peres wrote:
> > This fixes screenshots using 8k+ wide display setups in modesetting.
> > 
> > Chris Wilson even recommended the changes in intel_mipmap_tree.c
> > should read 131072 instead of 65535, but I for sure got confused by
> > his explanation.
> > 
> > In any case, I would like to use this RFC patch as a forum to discuss
> > why the fallback path is broken[1], and as to what should be the
> > limits for HW-accelerated blits now that we got rid of the blitter
> > usage on recent platforms.
> > 
> 
> Hi,
> 
> My understanding is that the fallback path is broken because we silently
> ignore miptree_create_for_bo's request for a tiled miptree. This results
> in some parts of mesa treating the surface as tiled and other parts of
> treating the surface as linear.
> 
> I couldn't come up with a piglit test for this when I was working on a
> fix. Please let me know if you can think of any.
> 
> I think what the limits should be depends on which mesa branch you're
> working off of.
> 
> * On the master branch of mesa, which has some commits which reduce the
>   dependence on the BLT engine, we can remove these limits by using BLORP.
>   As much as I can tell, BLORP can handle images as wide as the surface
>   pitch limit in the RENDER_SURFACE_STATE packet will allow.
> 
>   I sent out a series [a] a couple weeks ago that removes the limits
>   imposed by the hardware blitter.
> 
> * On the stable branch however, we can modify some incorrect code to set
>   the correct BLT limits (as Chris has suggested). The BLT engine's pitch
>   field is a signed 16bit integer, whose unit changes depending on the
>   tiling of the surface. For linear surfaces, it's in units of bytes and
>   for non-linear surfaces, it's in units of dwords. This translates to
>   2^15-1 bytes or (2^15-1) * 4 bytes respectively.
>   
>   I made a branch [b] which does this already, but I think my rebasing +
>   testing strategy for stable branches on the CI might be incorrect.

I just rebased this branch onto master and tested it on the CI.
Everything passes except for SNB. I get 1 GPU hang and two test
failures:
* failure-gpu-hang-otc-gfxtest-snbgt1-01-snbm64.compile.error
* KHR-GL33.shaders.uniform_block.random.all_shared_buffer.3.snbm64
* dEQP-EGL.functional.color_clears.multi_context.gles3.rgba8888_pbuffer.snbm64

I'm not sure why this happens.

-Nanley

> 
> [a] https://patchwork.freedesktop.org/series/43971/
> [b] https://cgit.freedesktop.org/~nchery/mesa/log/?h=wip/stable/stop-retiling
> 
> > Tested-by: Martin Peres <martin.peres at linux.intel.com> # HSW
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > [1] https://fs.mupuf.org/corruption_8k%2B.png
> > ---
> >  src/mesa/drivers/dri/i965/intel_blit.c        | 2 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
> > index 90784c5b1958..458f8bd42857 100644
> > --- a/src/mesa/drivers/dri/i965/intel_blit.c
> > +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> > @@ -403,7 +403,7 @@ emit_miptree_blit(struct brw_context *brw,
> >      * for linear surfaces and DWords for tiled surfaces.  So the maximum
> >      * pitch is 32k linear and 128k tiled.
> >      */
> > -   if (blt_pitch(src_mt) >= 32768 || blt_pitch(dst_mt) >= 32768) {
> > +   if (blt_pitch(src_mt) >= 65536 || blt_pitch(dst_mt) >= 65536) {
> 
> This is too large for linear miptrees.
> 
> >        perf_debug("Falling back due to >= 32k/128k pitch\n");
> >        return false;
> >     }
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 6b89bf6848af..7347ea8b99d8 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -523,7 +523,7 @@ need_to_retile_as_linear(struct brw_context *brw, unsigned row_pitch,
> >     if (row_pitch < 64)
> >        return true;
> >  
> > -   if (ALIGN(row_pitch, 512) >= 32768) {
> > +   if (ALIGN(row_pitch, 512) >= 65536) {
> >        perf_debug("row pitch %u too large to blit, falling back to untiled",
> >                   row_pitch);
> >        return true;
> > @@ -3583,7 +3583,7 @@ can_blit_slice(struct intel_mipmap_tree *mt,
> >                 unsigned int level, unsigned int slice)
> >  {
> >     /* See intel_miptree_blit() for details on the 32k pitch limit. */
> > -   if (mt->surf.row_pitch >= 32768)
> > +   if (mt->surf.row_pitch >= 65536)
> 
> This is also too large for linear miptrees.
> 
> -Nanley
> 
> >  
> >     return true;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev