[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 09:34 PM, Kenneth Graunke wrote:
> On 11/26/2013 05:34 PM, Chad Versace 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) {
>>            /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel > Pixel
>>             * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table "Color
>>             * Clear of Non-MultiSampled Render Target Restrictions":
>> @@ -275,8 +275,8 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct brw_context *brw,
>>             *   y_align values computed above are the relevant entries in the
>>             *   referred table.
>>             *
>> -          * Note: An older BSpec documented the above restriction for only
>> -          * HSW GT3.
>> +          * Note: An older BSpec documented the above restriction for only HSW
>> +          * GT3. However, the restriction also fixes corruption on HSW GT1.
>>             */
>>            x0 = ROUND_DOWN_TO(x0, 2 * x_align);
>>            y0 = ROUND_DOWN_TO(y0, 2 * y_align);
>>
>
> Personally I would drop the "Note: An older BSpec documented the above
> restriction for only HSW GT3." comment.  It wasn't in the public docs,
> and the older internal docs you're talking to have been revised.  So I
> don't think it'll be terribly interesting in the future.

Done. I removed the "Note".

> I would be fine with applying this to all Gen7 platforms, as Paul
> mentioned...it's documented as necessary, a minimal cost, and could save
> headaches in the future.  But it may also be superfluous, so I'm okay
> with your patch as is.

We can later apply it to all Gen7 as a follow-up patch. See my reply
to Paul for why I refrained.

> I would squash patches 1 and 2.  It makes sense to update the comments
> saying "Hey, this applies more broadly" at the same time you fix the
> code to apply it more broadly. :)

Done, squashed.