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

[Paul Berry]

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'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.

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.


>           /* 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);
> --
> 1.8.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131126/1c8fe499/attachment-0001.html>