<div class="gmail_quote">On Mon, May 3, 2010 at 1:57 PM, José Fonseca <span dir="ltr"><<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Mon, 2010-05-03 at 01:36 -0700, Xavier Chantry wrote:<br>
> I am trying to understand the s3tc init code as nv50 gallium has a<br>
> problem with that.<br>
><br>
> It looks like some drivers (r300g and llvmpipe) actually inits s3tc in<br>
> two places :<br>
> ./src/mesa/main/texcompress_s3tc.c _mesa_init_texture_s3tc()<br>
> ./src/gallium/auxiliary/util/u_format_s3tc.c util_format_s3tc_init()<br>
><br>
> Here is an extract of the backtrace calls while loading fgfs on llvmpipe :<br>
> driCreateScreen -> llvmpipe_create_screen -> util_format_s3tc_init<br>
> driCreateContext -> st_create_context -> _mesa_create_context_for_api<br>
> -> init_attrib_groups -> _mesa_init_texture_s3tc<br>
><br>
> These two functions seem to do more or less the same job, and<br>
> apparently, the classic mesa init is unused for a gallium driver.<br>
> So I suppose that init is not necessary, but it happens because<br>
> gallium and mesa are tightly tied together, and create context is<br>
> handled similarly, but it shouldn't hurt ?<br>
<br>
</div>Both inits are necessary. The same .so is used for both paths, but given<br>
that gallium and mesa do not depend on each other that's the way to do<br>
it. Gallium and mesa also have seperate format translation functions.<br>
<br>
At least until we start factoring code into a separate library. (I said<br>
I'd do it, but stuff came up and I couldn't do when I thought, and I<br>
don't know when I'll be able to do it...)<br>
<div><div></div><div class="h5"><br>
> Additionally r300g and llvmpipe added util_format_s3tc_init to their<br>
> create_screen functions, and util/u_format_s3tc.c apparently contains<br>
> all the functions that a gallium driver uses.<br>
> So I suppose that nvfx and nv50 should do the same ?<br>
><br>
> If that's correct, the patch below might not be completely wrong.<br>
><br>
> On Mon, May 3, 2010 at 12:44 AM, Xavier Chantry<br>
> <<a href="mailto:chantry.xavier@gmail.com">chantry.xavier@gmail.com</a>> wrote:<br>
> > flightgear now dies with :<br>
> > Mesa warning: external dxt library not available: texstore_rgba_dxt3<br>
> > util/u_format_s3tc.c:66:util_format_dxt3_rgba_fetch_stub: Assertion `0' failed.<br>
> ><br>
> > I don't really understand what these stubs are about, they were<br>
> > introduced by following commit :<br>
> > commit d96e87c3c513f8ed350ae24425edb74b6d6fcc13<br>
> > Author: José Fonseca <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>><br>
> > Date: Wed Apr 7 20:47:38 2010 +0100<br>
> ><br>
> > util: Use stubs for the dynamically loaded S3TC functions.<br>
> ><br>
> > Loosely based on Luca Barbieri's commit<br>
> > 52e9b990a192a9329006d5f7dd2ac222effea5a5.<br>
> ><br>
> > Looking at llvm and r300 code and trying to guess, I came up with the<br>
> > following patch that allows flightgear to start again. But I don't<br>
> > really understand that stuff so it could be wrong.<br>
> > nvfx is probably affected as well.<br>
> ><br>
> ><br>
> > diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c<br>
> > b/src/gallium/drivers/nouveau/nouveau_screen.c<br>
> > index 233a91a..a91b00b 100644<br>
> > --- a/src/gallium/drivers/nouveau/nouveau_screen.c<br>
> > +++ b/src/gallium/drivers/nouveau/nouveau_screen.c<br>
> > @@ -5,6 +5,7 @@<br>
> > #include "util/u_memory.h"<br>
> > #include "util/u_inlines.h"<br>
> > #include "util/u_format.h"<br>
> > +#include "util/u_format_s3tc.h"<br>
> ><br>
> > #include <stdio.h><br>
> > #include <errno.h><br>
> > @@ -248,6 +249,8 @@ nouveau_screen_init(struct nouveau_screen *screen,<br>
> > struct nouveau_device *dev)<br>
> > pscreen->fence_signalled = nouveau_screen_fence_signalled;<br>
> > pscreen->fence_finish = nouveau_screen_fence_finish;<br>
> ><br>
> > + util_format_s3tc_init();<br>
> > +<br>
> > return 0;<br>
> > }<br>
> ><br>
> > diff --git a/src/gallium/drivers/nv50/nv50_screen.c<br>
> > b/src/gallium/drivers/nv50/nv50_screen.c<br>
> > index 2dd1042..0d74c90 100644<br>
> > --- a/src/gallium/drivers/nv50/nv50_screen.c<br>
> > +++ b/src/gallium/drivers/nv50/nv50_screen.c<br>
> > @@ -20,6 +20,7 @@<br>
> > * SOFTWARE.<br>
> > */<br>
> ><br>
> > +#include "util/u_format_s3tc.h"<br>
> > #include "pipe/p_screen.h"<br>
> ><br>
> > #include "nv50_context.h"<br>
> > @@ -72,10 +73,6 @@ nv50_screen_is_format_supported(struct pipe_screen *pscreen,<br>
> > case PIPE_FORMAT_A8_UNORM:<br>
> > case PIPE_FORMAT_I8_UNORM:<br>
> > case PIPE_FORMAT_L8A8_UNORM:<br>
> > - case PIPE_FORMAT_DXT1_RGB:<br>
> > - case PIPE_FORMAT_DXT1_RGBA:<br>
> > - case PIPE_FORMAT_DXT3_RGBA:<br>
> > - case PIPE_FORMAT_DXT5_RGBA:<br>
> > case PIPE_FORMAT_S8_USCALED_Z24_UNORM:<br>
> > case PIPE_FORMAT_Z24_UNORM_S8_USCALED:<br>
> > case PIPE_FORMAT_Z32_FLOAT:<br>
> > @@ -85,6 +82,11 @@ nv50_screen_is_format_supported(struct pipe_screen *pscreen,<br>
> > case PIPE_FORMAT_R16G16_SNORM:<br>
> > case PIPE_FORMAT_R16G16_UNORM:<br>
> > return TRUE;<br>
> > + case PIPE_FORMAT_DXT1_RGB:<br>
> > + case PIPE_FORMAT_DXT1_RGBA:<br>
> > + case PIPE_FORMAT_DXT3_RGBA:<br>
> > + case PIPE_FORMAT_DXT5_RGBA:<br>
> > + return util_format_s3tc_enabled;<br>
> > default:<br>
> > break;<br>
> > }<br>
> ><br>
<br>
</div></div>You should only advertise PIPE_FORMAT_DXT* for BIND_SAMPLER_VIEW, or you<br>
may end up in very weird paths. Same for YUV.<br></blockquote></div><br>I think my hw supports rendering to YUV textures, it's been even implemented in r300g but not tested. I am not sure who's gonna use such a feature. Should I disable it then?<br>
<br>-Marek<br>