On Tue, Dec 6, 2016 at 4:26 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote: > On Tue, Dec 6, 2016 at 10:58 AM, Alejandro Piñeiro <apinhe...@igalia.com> > wrote: >> The FIXME suggest that the check should be removed. >> > Only if we see any performance or feature benefits in doing that. > Last I checked I didn't see any performance benefits on Skylake. and that was more than 6 months back. Things might be different now.
> I also couldn't figure out the cause of random failure in a piglit test: > ./bin/texelFetch fs sampler2DMSArray 4 98x1x9-98x129x9 -auto -fbo > > I'm still seeing this failure when I tested your patch with Jenkins CI system. > Test passes when I run it locally on my system. > >> This change helps the following test: >> GL45-CTS.texture_cube_map_array.color_depth_attachments >> >> to pass consistently on Skylake GT3e. Without this patch, on >> Skylake GT3e that test has a ~76% pass rate, with some random >> intel_do_flush_locked errors now and then. >> > By removing this check you're actually enabling fast copy blit on SKL+ > for all the tiling formats. So, now the driver will use XY_FAST_COPY_BLT > in place of XY_SRC_COPY_BLT. If this change is fixing a test case for > you, that's not because this is the right fix, but that's because you're > avoiding to use XY_SRC_COPY_BLT. > XY_SRC_COPY_BLT might be causing intel_do_flush_locked errors > and the test failure. We need to dig in there to find the real cause. > >> It works fine on Skylake GT2, though. >> --- >> >> I didn't analyze too much the code. It was more git history analysis. >> >> When I started to work to solve that test, I remembered that there was >> a time in the past that worked, so I just did a git bisect. The more >> recent bad commit I found was df210f. In any case, that one just fixed >> that check, as became broken with commit 0c02d7. The one that added >> the check (and the FIXME) was commit 412c8c. >> >> As the commit message says, that FIXME seems to suggest that is a >> provisional change. Although I recognize that the failure is really >> specific (for a Skylake model, not failing always), removing the check >> gets the test pass consistently, and as far as I see, it didn't >> introduce any regression. >> >> src/mesa/drivers/dri/i965/intel_blit.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c >> b/src/mesa/drivers/dri/i965/intel_blit.c >> index 03a35ee..9f3b4d1 100644 >> --- a/src/mesa/drivers/dri/i965/intel_blit.c >> +++ b/src/mesa/drivers/dri/i965/intel_blit.c >> @@ -487,14 +487,6 @@ can_fast_copy_blit(struct brw_context *brw, >> if (brw->gen < 9) >> return false; >> >> - /* Enable fast copy blit only if the surfaces are Yf/Ys tiled. >> - * FIXME: Based on performance data, remove this condition later to >> - * enable for all types of surfaces. >> - */ >> - if (src_tr_mode == INTEL_MIPTREE_TRMODE_NONE && >> - dst_tr_mode == INTEL_MIPTREE_TRMODE_NONE) >> - return false; >> - >> if (logic_op != GL_COPY) >> return false; >> >> -- >> 2.9.3 >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
