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