[Mesa-dev] radeon/r200 clear() inconsistency

[Roland Scheidegger]

btw I think there's more wrong with clears, for instance fbo clears
(BUFFER_BIT_COLORx) will always lead to swrast fallback, which seems
unnecessary. Clearly if we can render to it we can also clear it. The
clear function is just meta_Clear actually so maybe the whole
r200/radeonClear functions could just go away at least unless hyperz is
revived... (or maybe still need to invoke swrast clear for accum buffer
or something).


Am 09.03.2012 17:07, schrieb Roland Scheidegger:
> Brian,
> 
> I think you're right, the test in r200_ioctl.c looks bogus
> (the idea is fastz clear can only be used if both depth and stencil are
> cleared, though the test isn't quite right as it certainly would be
> possible to use fastz clear if there's no stencil buffer at all).
> However, all code related to hyperz is unfortunately dead anyway, that
> got lost with the dri2 transition. The flags computed in those functions
> aren't actually used.
> I am wondering though how this used to work with that bogus test, I
> doubt anyone actually touched that piece since it got useless...
> 
> If it ever gets revived it would need to look like hyperz handling in
> r300 I think. hyperz in radeon/r200 was always a hack anyway, since it
> can only work with one depth buffer, if you switch buffers it will still
> use the same on-chip depth buffer tags and the code didn't care about
> that, which is the reason why it was never enabled by default - well
> that and the fact you couldn't read back compressed (or fastz-cleared)
> buffers. r300 solves this (the former problem, not sure about the
> latter) by allowing only one app to use a compressed depth buffer IIRC.
> 
> Roland
> 
> 
> Am 09.03.2012 15:33, schrieb Brian Paul:
>>
>> I happened to be looking at radeonClear() and r200Clear() in the legacy
>> DRI drivers.  I think there's a bug in one function or the other in the
>> hyperz/fastclear test:
>>
>> From radeon_ioctl.c:
>>
>>    if (rmesa->using_hyperz) {
>>       flags |= RADEON_USE_COMP_ZBUF;
>> /*      if (rmesa->radeon.radeonScreen->chipset & RADEON_CHIPSET_TCL)
>>          flags |= RADEON_USE_HIERZ; */
>>       if (((flags & RADEON_DEPTH) && (flags & RADEON_STENCIL) &&
>>         ((rmesa->radeon.state.stencil.clear & RADEON_STENCIL_WRITE_MASK)
>> == RADEON_STENCIL_WRITE_MASK))) {
>>       flags |= RADEON_CLEAR_FASTZ;
>>       }
>>    }
>>
>> From r200_ioctl.c:
>>
>>    if (rmesa->using_hyperz) {
>>       flags |= RADEON_USE_COMP_ZBUF;
>> /*      if (rmesa->radeon.radeonScreen->chip_family == CHIP_FAMILY_R200)
>>      flags |= RADEON_USE_HIERZ; */
>>       if (!((flags & RADEON_DEPTH) && (flags & RADEON_STENCIL) &&
>>         ((rmesa->radeon.state.stencil.clear & R200_STENCIL_WRITE_MASK)
>> == R200_STENCIL_WRITE_MASK))) {
>>       flags |= RADEON_CLEAR_FASTZ;
>>       }
>>    }
>>
>>
>> Note that the later version has a '!' operator in the second
>> if-statement that's not present in the former.  I suspect the ! should
>> be removed.
>>
>> Can someone familiar with this code take a look?
>>
>> -Brian
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev