[Mesa-dev] [PATCH 1/3] radeonsi: assert that the DB format is valid

[Christian König]

On 15.11.2012 19:14, Marek Olšák wrote:
> On Thu, Nov 15, 2012 at 5:28 PM, Christian König
> <deathsimple at vodafone.de> wrote:
>> On 15.11.2012 17:00, alexdeucher at gmail.com wrote:
>>> From: Alex Deucher <alexander.deucher at amd.com>
>>>
>>> Rather than disabling the depth buffer.
>> I would still prefer to use *_INVALID as the invalid function return value
>> here.
>>
>> Just imagine a release build with an undetected bug, writing *_INVALID would
>> still result in well defined hw behavior, while ~0 usually maps to something
>> reserved. Of course additionally asserting on it still makes allot of sense.
> How about #define INVALID_FORMAT ~0? Using the hardware "INVALID"
> format means the buffer binding is *disabled*, not *invalid*, so there
> should be a distinction between disabled buffers (unbound) and invalid
> ones (bound with an unsupported format, we should never happen).

Well, there isn't any difference from the hardware point of view. 
Neither unbound buffers nor buffers with an invalid format shouldn't be 
touched by the hardware. The driver of course could and should still 
make a difference.

>
> If you're concerned about a release build, I guess we can just abort()
> if the format translation fails and there is no way to report the
> failure to the state tracker. I just think it's a serious bug and if
> it happens, we shouldn't hide it.

I agree that it is a serous bug and definitely shouldn't go unnoticed, 
but calling abort() seems a bit harsh to me.

The best thing we could probably do for a debug build is to assert and 
for a release build printing and error message and then continuing with 
a disabled buffer seems to make the most sense to me. In this way the 
application wouldn't abort, but just might show rendering glitches.

Christian.