[Mesa-dev] [PATCH 2/2] i965/hsw: Apply non-msrt fast color clear w/a to all HSW GTs

[Chad Versace]

On 11/26/2013 07:13 PM, Paul Berry wrote:
> On 26 November 2013 17:34, Chad Versace <chad.versace at 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.phogat at gmail.com>
>> CC: Paul Berry <stereotype441 at gmail.com>
>> OTC-Tracker: CHRMOS-812
>> Signed-off-by: Chad Versace <chad.versace at 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 <stereotype441 at 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.