On 11/26/2013 07:13 PM, Paul Berry wrote:
On 26 November 2013 17:34, Chad Versace <chad.vers...@linux.intel.com>wrote:

Pre-patch, the workaround was applied to only HSW GT3. However, the
workaround also fixes render corruption on the HSW GT1 Chromebook,
codenamed Falco.

CC: Anuj Phogat <anuj.pho...@gmail.com>
CC: Paul Berry <stereotype...@gmail.com>
OTC-Tracker: CHRMOS-812
Signed-off-by: Chad Versace <chad.vers...@linux.intel.com>
---
  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
index 63d83d7..2620ce6 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
@@ -265,7 +265,7 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct
brw_context *brw,
        x_align *= 16;
        y_align *= 32;

-      if (brw->is_haswell && brw->gt == 3) {
+      if (brw->is_haswell) {


Ok, I'll ask the obvious question: if the bspec says that this extra
rectangle alignment code is needed for IVB, VLVT, and HSW, why are we doing
it for just HSW?

I suspect that in truth, the extra rectangle alignment is only needed for
HSW (This is based in part on the fact that fast clears have been working
fine on IVB for a long time without this bug fix), so the patch will
probably work fine as written.  But the performance cost of applying the
extra alignment to IVB is minuscule, and if it saves us from having to
track down and re-fix this bug one more time, it will be worth it.

On the other hand, there's some appeal to limiting the scope of the bug fix
to just the hardware that's been experiencing problems.

I applied it only to Haswell because...

  1. I did not validate the workaround on Ivybridge. I don't like committing 
code
     that affects older gens unless I've validated it on that gen. Especially
     for a patch in train for the stable branch.

  2. I suspect, as you, that the alignment is needed only on Haswell. 
Accordingly,
     I didn't want to waste time validating it on Ivybridge.

I'll leave it up to you.  Either way, the series is:

Reviewed-by: Paul Berry <stereotype...@gmail.com>

Note: Personally I'd prefer to see the two patches squashed together, but I
won't be a stickler about that.

I'll squash them.

Oh, one other question: was it a deliberate decision not to mark this as a
candidate for cherry-picking back to the 10.0 and 9.2 branches?  At first
blush it seems worth cherry-picking to me.

I forgot. I'll CC it to stable.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to