On Wed, Mar 09, 2016 at 11:16:41AM +0100, Iago Toral wrote: > On Wed, 2016-03-09 at 11:42 +0200, Pohjolainen, Topi wrote: > > On Wed, Mar 09, 2016 at 11:05:17AM +0200, Pohjolainen, Topi wrote: > > > On Wed, Mar 09, 2016 at 10:03:08AM +0100, Iago Toral wrote: > > > > On Wed, 2016-03-09 at 10:53 +0200, Pohjolainen, Topi wrote: > > > > > On Wed, Mar 09, 2016 at 09:36:42AM +0100, Iago Toral wrote: > > > > > > On Wed, 2016-03-09 at 09:54 +0200, Pohjolainen, Topi wrote: > > > > > > > On Mon, Mar 07, 2016 at 10:48:49AM +0100, Samuel Iglesias > > > > > > > Gons?lvez wrote: > > > > > > > > Hello, > > > > > > > > > > > > > > > > There is only one patch from this series that has been reviewed > > > > > > > > (patch > > > > > > > > 1). > > > > > > > > > > > > > > > > Our plans is to start sending patches for adding fp64 support > > > > > > > > to i965 > > > > > > > > driver in the coming weeks but they depend on these patches. > > > > > > > > > > > > > > > > Can someone take a look at them? ;) > > > > > > > > > > > > > > > > Sam > > > > > > > > > > > > > > > > > > > > > > > > On Thu, 2015-12-17 at 14:44 +0100, Samuel Iglesias Gonsálvez > > > > > > > > wrote: > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > This patch series is a updated version of the one Iago sent > > > > > > > > > last > > > > > > > > > week [0] that includes patches for gen6 too, as suggested by > > > > > > > > > Jason. > > > > > > > > > > > > > > > > > > We checked the gen9 code paths that work with a horizontal > > > > > > > > > width of 4 > > > > > > > > > and we think there won't be any regression on gen9... but we > > > > > > > > > don't > > > > > > > > > have any gen9 machine to run piglit with these patches. Can > > > > > > > > > someone > > > > > > > > > check it? > > > > > > > > > > > > > > I rebased it and ran it through the test system, gen9 seems to be > > > > > > > fine, I > > > > > > > only got one regression, and that was on old g965: > > > > > > > > > > > > Awesome! would it be possible to run that test in g695 with the > > > > > > attached > > > > > > change? If this is a regression caused by our code it should break > > > > > > at > > > > > > the assert introduced with it. > > > > > > > > > > > > > /tmp/build_root/m64/lib/piglit/bin/ext_framebuffer_multisample-accuracy > > > > > > > all_samples srgb depthstencil -auto -fbo > > > > > > > Pixels that should be unlit > > > > > > > count = 236444 > > > > > > > RMS error = 0.025355 > > > > > > > Pixels that should be totally lit > > > > > > > count = 13308 > > > > > > > Perfect output > > > > > > > The error threshold for unlit and totally lit pixels test is > > > > > > > 0.016650 > > > > > > > Pixels that should be partially lit > > > > > > > count = 12392 > > > > > > > RMS error = 0.273876 > > > > > > > The error threshold for partially lit pixels is 0.333000 > > > > > > > Samples = 0, Result = fail > > > > > > > > > > > > > > > > > > > > > But I'm not sure if this is caused by your patches. > > > > > > > _______________________________________________ > > > > > > > mesa-dev mailing list > > > > > > > mesa-dev@lists.freedesktop.org > > > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > > > > > > > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c > > > > > > b/src/mesa/drivers/dri/i965/brw_eu_emit.c > > > > > > index 6f11f59..625447f 100644 > > > > > > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c > > > > > > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c > > > > > > @@ -203,6 +203,7 @@ brw_set_dest(struct brw_codegen *p, brw_inst > > > > > > *inst, struct brw_reg dest) > > > > > > * or 16 (SIMD16), as that's normally correct. However, when > > > > > > dealing with > > > > > > * small registers, we automatically reduce it to match the > > > > > > register size. > > > > > > */ > > > > > > + assert(dest.width != BRW_EXECUTE_4 || > > > > > > brw_inst_exec_size(devinfo, inst) == dest.width); > > > > > > if (dest.width < BRW_EXECUTE_8) > > > > > > brw_inst_set_exec_size(devinfo, inst, dest.width); > > > > > > } > > > > > > > > > > Hmm, on top of your series this looks: > > > > > > > > > > /* Generators should set a default exec_size of either 8 (SIMD4x2 > > > > > or SIMD8) > > > > > * or 16 (SIMD16), as that's normally correct. However, when > > > > > dealing with > > > > > * small registers, we automatically reduce it to match the > > > > > register size. > > > > > * > > > > > * In platforms that support fp64 we can emit instructions with a > > > > > width of > > > > > * 4 that need two SIMD8 registers and an exec_size of 8 or 16. In > > > > > these > > > > > * cases we need to make sure that these instructions have their > > > > > exec sizes > > > > > * set properly when they are emitted and we can't rely on this > > > > > code to fix > > > > > * it. > > > > > */ > > > > > bool fix_exec_size; > > > > > if (devinfo->gen >= 6) > > > > > fix_exec_size = dest.width < BRW_EXECUTE_4; > > > > > else > > > > > fix_exec_size = dest.width < BRW_EXECUTE_8; > > > > > > > > > > if (fix_exec_size) > > > > > brw_inst_set_exec_size(devinfo, inst, dest.width); > > > > > > > > > > Do you want the assertion before or after fixing? > > > > > > > > > > > > > Before, you can put it right after that comment. Thanks! > > > > > > That is what I thought. Hold on, I'll give it a spin. > > > > Okay, now the system got really mad, I have some 12000 regressions on > > g45, ilk and g965. > > Oh right, that was my mistake, for a moment I forgot that these changes > are only for gen6+, gen < 6 are not affected (as you can see above, we > only change the original code in brw_set_dst for gens >= 6), so for gens > < 6 it is expected that you hit the assert I provided. Sorry for wasting > your time :( > > The only way to check if this test is a real regression is to run it > with and without our series and see if it consistently fails and passes > respectively.
I ran using the commit just before your series as HEAD, and I didn't get any failures. Then I re-ran with your series and I got the same regression on g965. Unfortunately I don't have that hardware myself and therefore I can't debug it further. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
