[Mesa-dev] [PATCH 0/8] gallium: create a generic way to validate fb params

Ilia Mirkin imirkin at alum.mit.edu
Fri Oct 4 09:39:33 PDT 2013


On Fri, Oct 4, 2013 at 10:02 AM, Marek Olšák <maraeo at gmail.com> wrote:
> Wouldn't it be more flexible for you to just change the depth buffer
> format internally by reallocating the depth buffer in the driver
> whenever it's incompatible with the colorbuffer? I think you should be
> able to do that easily without changing the pipe_resource structure.
> If a depth buffer cannot be used by set_framebuffer_state, just create
> a new one with an appropriate format, then blit from the old depth
> buffer to the new one, so that you won't lose its content, and delete
> the old one. For transfers, you just need to blit to convert the depth
> buffer to the format the state tracker expects.

It would indeed be more flexible. There are two reasons why I didn't
do it that way

(a) I'm very new to gallium, I'm not comfortable with such trickery.
(I don't even know how to blit between textures.) Also, wouldn't you
have to blit the new texture back to the old texture? Or is there
sufficient magic to take care of the format difference? What if the
requested zsbuf was 24-bit but the cbufs were 16-bit? Would you
suggest upgrading the cbufs or downgrading the zsbuf -- downgrading
the zsbuf could also cause some application weirdness, no?

(b) If that were the preferred approach, wouldn't the same approach
have been taken with MIXED_COLORBUFFER_FORMATS? The preference seemed
to be to reject things the driver didn't like rather than accepting
every possible input and converting back and forth. Perhaps the cbufs
are somehow different from depth in a way I don't understand though.

>
> I'm quite sure exposing the weird restrictions of your hardware to
> applications won't end well. The apps won't like it.

Indeed. I observed that e.g. supertuxkart crashes (in its own code)
due to it trying GL_RGBA8 with a 16-bit zsbuf. They've fixed the crash
in SVN (before I even got to report it!), but not yet in the latest
released version.

  -ilia

>
> Marek
>
> On Fri, Oct 4, 2013 at 10:34 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> NV30 is blessed with all manners of hardware restrictions. One of them is that
>> the render target format's color and depth outputs need to be the same
>> bit-ness, i.e. either both 16 or both 32 bits. (In addition to all color
>> attachments needing to be the same, and everything needing to be the same
>> size.)
>>
>> There is a PIPE_CAP_MIXED_COLORBUFFER_FORMATS that currently handles the
>> requirement of all the cbuf attachments having the same format. This series
>> creates a more generic mechanism to perform such validation which allows nv30
>> to also check the sizes of the cbuf/zsbuf formats. At the end, I remove that
>> PIPE_CAP as it is no longer necessary.
>>
>> I first tried to use a pipe_framebuffer_state as the parameter to this new
>> call, however I couldn't quite figure out how to populate it in case that the
>> depth output buffer was a texture (seems to map to a pipe_resource, not a
>> pipe_surface).
>>
>> This series is a follow-up to the discussion started in
>> http://lists.freedesktop.org/archives/mesa-dev/2013-September/044658.html
>>
>> Ilia Mirkin (8):
>>   mesa/st: create interface to verify fb format, reject bad fbs
>>   gallium: add helper to use as a replacement for the MIXED_COLOR cap
>>   i915: hook up is_fb_format_supported
>>   r300: hook up is_fb_format_supported
>>   softpipe: hook up is_fb_format_supported
>>   nv30: hook up is_fb_format_supported
>>   gallium: remove PIPE_CAP_MIXED_COLORBUFFER_FORMATS
>>   nv30: make sure that cbufs and zsbuf have the same depth
>>
>>  src/gallium/auxiliary/util/u_format.c            | 20 +++++++++++
>>  src/gallium/auxiliary/util/u_format.h            | 11 +++++++
>>  src/gallium/docs/source/screen.rst               |  2 --
>>  src/gallium/drivers/freedreno/freedreno_screen.c |  1 -
>>  src/gallium/drivers/i915/i915_screen.c           |  2 +-
>>  src/gallium/drivers/ilo/ilo_screen.c             |  2 --
>>  src/gallium/drivers/llvmpipe/lp_screen.c         |  2 --
>>  src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 20 ++++++++++-
>>  src/gallium/drivers/nouveau/nv50/nv50_screen.c   |  1 -
>>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   |  1 -
>>  src/gallium/drivers/r300/r300_screen.c           |  3 +-
>>  src/gallium/drivers/r600/r600_pipe.c             |  1 -
>>  src/gallium/drivers/radeonsi/radeonsi_pipe.c     |  1 -
>>  src/gallium/drivers/softpipe/sp_screen.c         |  3 +-
>>  src/gallium/drivers/svga/svga_screen.c           |  3 --
>>  src/gallium/include/pipe/p_defines.h             |  1 -
>>  src/gallium/include/pipe/p_screen.h              | 18 +++++++++-
>>  src/mesa/state_tracker/st_cb_fbo.c               | 42 ++++++++++++++----------
>>  18 files changed, 95 insertions(+), 39 deletions(-)
>>
>> --
>> 1.8.1.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list