Bug 57660 - [NVA3:NVC0] Turn on support for PIPE_CAP_CUBE_MAP_ARRAY
Summary: [NVA3:NVC0] Turn on support for PIPE_CAP_CUBE_MAP_ARRAY
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/nouveau (show other bugs)
Version: git
Hardware: All Linux (All)
: medium normal
Assignee: Nouveau Project
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-28 20:14 UTC by Hristo Venev
Modified: 2014-02-26 17:17 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to fix it (2.16 KB, text/plain)
2012-11-28 20:14 UTC, Hristo Venev
Details
Partial implementation (3.89 KB, text/plain)
2012-11-29 18:51 UTC, Hristo Venev
Details
cubemap array patch (1.86 KB, patch)
2014-02-07 22:48 UTC, Ilia Mirkin
Details | Splinter Review

Description Hristo Venev 2012-11-28 20:14:07 UTC
Created attachment 70747 [details]
Patch to fix it

Unhandled case PIPE_CAP_CUBE_MAP_ARRAY in nv??_screen.c
Comment 1 Maarten Lankhorst 2012-11-29 15:22:03 UTC
I think nouveau could probably easily get support for it, so I don't know if we should default to false, would be better to leave it open.
Comment 2 Hristo Venev 2012-11-29 16:31:32 UTC
Working on it.
Comment 3 Hristo Venev 2012-11-29 17:27:11 UTC
In nv50_ir_lowering_nv50.cpp in handleTEX, arguments to the texture instruction i are (x, y, z, face, something).
What do they need to be transformed to? Is it (x, y, 6*z+face, something)?
Comment 4 Hristo Venev 2012-11-29 18:51:20 UTC
Created attachment 70799 [details]
Partial implementation

This is a partial implementation. It is probably slow and renders incorrectly for some reason.
Comment 5 Maarten Lankhorst 2012-11-29 19:33:45 UTC
I think it's only available on nva3...nva8 and nvaf (no idea about nvaa/ac, they've always been special..), not for all of nv50 :)
Comment 6 Hristo Venev 2012-11-30 21:23:58 UTC
What's the problem with the older gpu's? Why can't it be implemented?
Comment 7 Ilia Mirkin 2013-08-20 06:37:51 UTC
Looks like support for NVA3+ was added in f7599b2c3. However it's not actually turned on for pre-nvc0, the commit message says:

    NOTE: nv50 support not enabled, someone with nva3/8 please fix.

I have no idea what it is that's left to do/fix there, but I don't have a nva3/5/8/f (which is what I think the list of cards with the NVA3_3D class is).
Comment 8 Ilia Mirkin 2013-08-20 15:00:36 UTC
Sounds like what's left to be done is to test it. Just need to apply the below patch and see if it works (looks like there are a couple of piglit tests... ls bin/*cube_map_array* -- they ought to pass on the nva3+ hardware).

--- a/src/gallium/drivers/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nv50/nv50_screen.c
@@ -114,10 +114,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
    case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
       return 0;
    case PIPE_CAP_CUBE_MAP_ARRAY:
-      return 0;
-      /*
       return nv50_screen(pscreen)->tesla->oclass >= NVA3_3D_CLASS;
-      */
    case PIPE_CAP_TWO_SIDED_STENCIL:
    case PIPE_CAP_DEPTH_CLIP_DISABLE:
    case PIPE_CAP_POINT_SPRITE:
Comment 9 Martin Peres 2013-08-24 04:06:15 UTC
(In reply to comment #8)
> Sounds like what's left to be done is to test it. Just need to apply the
> below patch and see if it works (looks like there are a couple of piglit
> tests... ls bin/*cube_map_array* -- they ought to pass on the nva3+
> hardware).
> 
> --- a/src/gallium/drivers/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nv50/nv50_screen.c
> @@ -114,10 +114,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
>     case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
>        return 0;
>     case PIPE_CAP_CUBE_MAP_ARRAY:
> -      return 0;
> -      /*
>        return nv50_screen(pscreen)->tesla->oclass >= NVA3_3D_CLASS;
> -      */
>     case PIPE_CAP_TWO_SIDED_STENCIL:
>     case PIPE_CAP_DEPTH_CLIP_DISABLE:
>     case PIPE_CAP_POINT_SPRITE:

I tested this patch with piglit, it doesn't look good :D

* arb_texture_cube_map_array-get: PASS
* arb_texture_cube_map_array-teximage3d-invalid-values: PASS
* arb_texture_cube_map_array-cubemap: FAIL (http://codepad.org/sZpx3ajS)
* arb_texture_cube_map_array-cubemap-lod: FAIL (http://codepad.org/5E8EPmpx)
* arb_texture_cube_map_array-fbo-cubemap-array: FAIL (http://codepad.org/0QqPUEAE)
* arb_texture_cube_map_array-sampler-cube-array-shadow: FAIL (http://codepad.org/orzd5RoU)
Comment 10 Ilia Mirkin 2014-02-07 21:52:41 UTC
I think there are a bunch of places in the nv50 driver where things check for PIPE_TEXTURE_{1D,2D}_ARRAY but also need to check for PIPE_TEXTURE_CUBE_ARRAY. One such place is nv50_create_texture_view but there are probably a bunch of others.
Comment 11 Ilia Mirkin 2014-02-07 22:48:43 UTC
Created attachment 93636 [details] [review]
cubemap array patch

Well, this fixes the one place I found by copying what nvc0 did. But from Martin's test it seems like this probably won't be sufficient, since this just takes layer offsets into account.
Comment 12 Ilia Mirkin 2014-02-14 09:17:35 UTC
(In reply to comment #11)
> Created attachment 93636 [details] [review] [review]
> cubemap array patch
> 
> Well, this fixes the one place I found by copying what nvc0 did. But from
> Martin's test it seems like this probably won't be sufficient, since this
> just takes layer offsets into account.

In case there was any suspense about it, this doesn't work either. It seems like the texprep instruction doesn't actually adjust for the layer (or we're passing the layer in wrong) -- cubemap works for layer 0 but not layer 1. Will need some traces to figure out what's up.
Comment 13 Ilia Mirkin 2014-02-26 17:17:19 UTC
A working impl is in mesa-git now. http://cgit.freedesktop.org/mesa/mesa/commit/?id=0e71c65db0df86401f2caf26209ff73e3715443a

The sampler-cube-array-shadow test fails, but so does sampler-cube-shadow (on nva0+), so I'm guessing those two failures are highly related.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.