On 10/13/2011 10:40 PM, Yuanhan Liu wrote:
On Thu, Oct 13, 2011 at 09:12:09PM -0700, Ian Romanick wrote:
On 10/12/2011 08:34 PM, Yuanhan Liu wrote:
The patch(based on the reading of the emulator) came from while I was
trying to fix the oglc pbo texImage.1PBODefaults fail. This case
generates a texture with the width and height equal to window's width
and height respectively, then try to texture it on the whole window.
So, it's exactly one texel for one pixel. And, the min filter and mag
filter are GL_LINEAR. It runs with swrast OK, as expected. But it failed
with i965 driver.
Well, you can't tell the difference from the screen, as the error is
quite tiny. From my digging, it seems that there are some tiny error
happened while getting tex address. This will break the one texel for
one pixel rule in this case. Thus the linear result is taken, with tiny
error.
This patch would fix several oglc pbo subcase fail on both ILK, SNB and
IVB.
I don't know about this patch, but this led me to notice something
else in the docs. Starting with Sandybridge, bit 27 in dword 0 is
"Min and Mag State Not Equal." This is ss0.min_mag_neq in our
structure. The hardware docs say that this bit must be set to 0 if
the min and mag filter state are not the same, min and mag U round
modes are not the same, min and mag V round modes are not the same,
or min and mag R round modes are not the same. I can't see that
this bit is set anywhere in the driver.
From my check, this bit was already set in our driver. It only use
the mag and min filter as the condition. But in this patch, the round
mode is set according to the mag and min filter. So, the results would
be the same. And, IVB don't have this bit.
I see it now. I don't know how I missed it before.
Does adding the following code at about line 227 of brw_wm_sampler.c
make any additional tests pass on SNB?
Sorry, I didn't make it clear. Here the word 'several' means all the
fails with the same issue. As the there are still about three fails that
not belong this issue in the pbo test suite. My bad.
You were clear. :) I was wondering if setting that bit would fix even
more tests. Since the bit is already set correctly, it shouldn't matter.
if (intel->gen>= 6)
sampler->ss0.min_mag_neq =
sampler->ss0.min_filter != sampler->ss0.mag_filter;
Signed-off-by: Yuanhan Liu<yuanhan....@linux.intel.com>
---
src/mesa/drivers/dri/i965/brw_defines.h | 7 +++++++
src/mesa/drivers/dri/i965/brw_wm_sampler_state.c | 9 +++++++++
src/mesa/drivers/dri/i965/gen7_sampler_state.c | 9 +++++++++
3 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index a111630..e4647ff 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -196,6 +196,13 @@
#define BRW_MIPFILTER_NEAREST 1
#define BRW_MIPFILTER_LINEAR 3
+#define BRW_ADDRESS_ROUNDING_ENABLE_U_MAG 0x20
+#define BRW_ADDRESS_ROUNDING_ENABLE_U_MIN 0x10
+#define BRW_ADDRESS_ROUNDING_ENABLE_V_MAG 0x08
+#define BRW_ADDRESS_ROUNDING_ENABLE_V_MIN 0x04
+#define BRW_ADDRESS_ROUNDING_ENABLE_R_MAG 0x02
+#define BRW_ADDRESS_ROUNDING_ENABLE_R_MIN 0x01
+
#define BRW_POLYGON_FRONT_FACING 0
#define BRW_POLYGON_BACK_FACING 1
diff --git a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
index 6834eba..9fc1c1a 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
@@ -312,6 +312,15 @@ static void brw_update_sampler_state(struct brw_context
*brw,
intel->batch.bo, brw->wm.sdc_offset[unit],
I915_GEM_DOMAIN_SAMPLER, 0);
}
+
+ if (sampler->ss0.min_filter != BRW_MAPFILTER_NEAREST)
+ sampler->ss3.address_round = BRW_ADDRESS_ROUNDING_ENABLE_U_MIN |
+ BRW_ADDRESS_ROUNDING_ENABLE_V_MIN |
+ BRW_ADDRESS_ROUNDING_ENABLE_R_MIN;
+ if (sampler->ss0.mag_filter != BRW_MAPFILTER_NEAREST)
+ sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG |
+ BRW_ADDRESS_ROUNDING_ENABLE_V_MAG |
+ BRW_ADDRESS_ROUNDING_ENABLE_R_MAG;
It looks weird that one of these cases use = and the other uses |=. It
would be better to assign 0 to ss3.address_round before the first case
and use |= in both cases.
Yes, will fix it later.
Thanks,
Yuanhan Liu
}
diff --git a/src/mesa/drivers/dri/i965/gen7_sampler_state.c
b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
index aee67c8..5fab91c 100644
--- a/src/mesa/drivers/dri/i965/gen7_sampler_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
@@ -167,6 +167,15 @@ gen7_update_sampler_state(struct brw_context *brw, int
unit,
upload_default_color(brw, gl_sampler, unit);
sampler->ss2.default_color_pointer = brw->wm.sdc_offset[unit]>> 5;
+
+ if (sampler->ss0.min_filter != BRW_MAPFILTER_NEAREST)
+ sampler->ss3.address_round = BRW_ADDRESS_ROUNDING_ENABLE_U_MIN |
+ BRW_ADDRESS_ROUNDING_ENABLE_V_MIN |
+ BRW_ADDRESS_ROUNDING_ENABLE_R_MIN;
+ if (sampler->ss0.mag_filter != BRW_MAPFILTER_NEAREST)
+ sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG |
+ BRW_ADDRESS_ROUNDING_ENABLE_V_MAG |
+ BRW_ADDRESS_ROUNDING_ENABLE_R_MAG;
Same comment here as above.
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
