[Mesa-dev] [PATCH] i965: Disable guardband clipping on SNB unless workaround is present.

[Kenneth Graunke]

On 10/07/2012 06:34 PM, Kenneth Graunke wrote:
> On 10/07/2012 06:06 PM, Paul Berry wrote:
>> On 7 October 2012 15:38, Kenneth Graunke <kenneth at whitecape.org
>> <mailto:kenneth at whitecape.org>> wrote:
>>
>>     On 10/07/2012 02:40 PM, Daniel Vetter wrote:
>>
>>         On Fri, Oct 05, 2012 at 05:08:04PM -0700, Kenneth Graunke wrote:
>>
>>             According to internal bug reports, guardband clipping
>> sometimes
>>             incorrectly discards triangles unless bit 5 of 3D_CHICKEN3
>>             ("SF Disable
>>             fastclip optimization when culled") is set.
>>
>>             Experimentally, this appears to be true.  With guardband
>>             clipping
>>             enabled, Dante randomly loses triangles unless bit 5 is set.
>>
>>             The BSpec indicates that the default value of the register
>>             is supposed
>>             to be 0x20 (just bit 5), but on my Lenovo X220 it appears to
>>             be 0.
>>
>>             Daniel attempted to implement the workaround via a kernel
>>             patch named
>>                  drm/i915: implement w/a for incorrect guarband clipping
>>             but unfortunately, the write isn't taking effect for some
>>             reason.
>>
>>             Fixes clipping issues in Dante.
>>
>>             Bugzilla:
>>             https://bugs.freedesktop.org/__show_bug.cgi?id=53946
>>             <https://bugs.freedesktop.org/show_bug.cgi?id=53946>
>>             Bugzilla:
>>             https://bugs.freedesktop.org/__show_bug.cgi?id=55523
>>             <https://bugs.freedesktop.org/show_bug.cgi?id=55523>
>>             Cc: Oliver McFadden <oliver.mcfadden at linux.intel.__com
>>             <mailto:oliver.mcfadden at linux.intel.com>>
>>             Cc: nkalkhof at web.de <mailto:nkalkhof at web.de>
>>             Cc: Eric Anholt <eric at anholt.net <mailto:eric at anholt.net>>
>>             Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>>             <mailto:kenneth at whitecape.org>>
>>
>>
>>         Does this really work as advertised?
>>
>>         The reg_read_ioctl is brand-new, only merged into 3.7. And it
>> has a
>>         whitelist of registers that userspace is allowed to read, and
>>         CHICKEN_3 is
>>         not on it.
>>         -Daniel
>>
>>
>>     Oh.  Damn.  I neglected to check that it ever returned success.
>>
>>     The fact that it's new is fine, as the fix is newer.  If the ioctl
>>     didn't exist, it would simply turn off guardband clipping.
>>
>>     But you're right, since we can't read 3D_CHICKEN3, we'd just turn it
>>     off unilaterally.  That sucks.
>>
>>     I'm not sure what to do then.
>>
>>
>> Can we make a kernel patch that adds 3D_CHICKEN3 to the whitelist?  That
>> would at least allow guardband clipping to work with future kernels.
>
> That would definitely work.  It's a bit ironic though: assuming the
> kernel fix to apply the workaround went in before the whitelist change,
> we wouldn't even need to check the value returned.  We could simply
> check whether drm_intel_reg_read returned -EINVAL or 0, as the mere
> ability to read the register would prove that the kernel was new enough
> that we could safely use guardband clipping.  (One could argue that we
> *should* check the value to guard against future kernel bugs, but...)
>
> What do you think, Daniel?  Should we whitelist 3D_CHICKEN3 and check
> for bit 5 in Mesa?  Do you have a better idea?

On second thought, I don't know why I felt compelled to do anything 
about it in Mesa at all.  We've had much worse problems (GPU hangs in 
Google MapsGL) that have required kernel-side workarounds where we could 
have done Mesa-side workaround detection and disabled features 
accordingly (HiZ/separate stencil), and chose not to.

I guess we didn't have the register read mechanism then, but without the 
whitelist, we kind of don't now either.

Perhaps the best course of action is to simply:
1. Cc: stable at kernel.org the kernel fix and get it backported as far 
back as anybody cares.
2. Leave GB clipping disabled in 9.0, but keep it for 9.1.  That way, by 
the time it ships and non-developers/enthusiasts run it---in roughly 6 
months---people will likely also have received the updated kernel.

So far I like this plan best.  Just treat it as a kernel bug and 
backport the patch.  It's trivial to do anyway...one register write.

--Ken