Re: [Mesa-dev] [PATCH 2/2] i915: fallback for NPOT cubemap texture
On Fri, Mar 02, 2012 at 01:52:00PM +0800, Yuanhan Liu wrote: On Thu, Mar 01, 2012 at 04:04:59PM +0800, Yuanhan Liu wrote: On Wed, Feb 29, 2012 at 11:44:59AM -0800, Eric Anholt wrote: On Wed, 29 Feb 2012 15:11:06 +0800, Yuanhan Liu yuanhan@linux.intel.com wrote: According to 3DSTATE_MAP_STATE at page of 104 in Bspec [snip]... + Pineview should be DevBLB derived, which doens't have so that requirement, Yeah, it's true according to Bspec: [DevLPT, DevCST and DevBLB]: If not a power of 2, cube maps must have all faces enabled. Well, we have all faces enabled in our code and it still failed. And it's ok when the height is power of 2. That's why I think we need do fallback for other platforms as well. so this doesn't really make sense to me. Have you looked at the rendering to see what's going on? Not much, and I'm tring now. Seems that the cube map texture layout is wrong. Making the width and height align of 4(although this doesn't make much sense) will fix this issue and also broke some others. Well, Bspec mentioned nothing about the layout alignment and layout for NPOT cube map. I guess the Bspec assume it's a POT cube map. Thus I don't know the _working_ texture layout of NPOT cube map texture layout. Thoughts? My thought is if we can't figure out the texture layout for NPOT cube map, I would like to do fallback as a workaround. Eric, does that make sense to you? Eric, does the following patch make sense to you? Well, honestlly speaking, I just added some more detailed comments. Thanks, Yuanhan Liu -- From 784b067924c05dc41c4ea56d1d9389aade0f076b Mon Sep 17 00:00:00 2001 From: Yuanhan Liu yuanhan@linux.intel.com Date: Wed, 29 Feb 2012 15:04:45 +0800 Subject: [PATCH] i915: fallback for NPOT cubemap texture Although some hardware support NPOT cubemap, but it seems we don't know the right layout for NPOT cubemap. Thus seems we need do fallback for other platforms as well. See comments inline the code for more detailed info. v2: give a more detailed info about why we need fallback for other platfroms as well. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=4 NOTE: This is a candidate for stable release branches. Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com --- src/mesa/drivers/dri/i915/i915_texstate.c | 22 ++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i915/i915_texstate.c b/src/mesa/drivers/dri/i915/i915_texstate.c index 9022548..fd63a69 100644 --- a/src/mesa/drivers/dri/i915/i915_texstate.c +++ b/src/mesa/drivers/dri/i915/i915_texstate.c @@ -319,6 +319,28 @@ i915_update_tex_unit(struct intel_context *intel, GLuint unit, GLuint ss3) ((wt != GL_CLAMP) (wt != GL_CLAMP_TO_EDGE return false; + /* + * According to 3DSTATE_MAP_STATE at page of 104 in Bspec + * Vol3d 3D Instructions: + * [DevGDG and DevAlv]: Must be a power of 2 for cube maps. + * [DevLPT, DevCST and DevBLB]: If not a power of 2, cube maps + * must have all faces enabled. + * + * But, as I tested on pineview(DevBLB derived), the rendering is + * bad(you will find the color isn't samplered right in some + * fragments). After checking, it seems that the texture layout is + * wrong: making the width and height align of 4(although this + * doesn't make much sense) will fix this issue and also broke some + * others. Well, Bspec mentioned nothing about the layout alignment + * and layout for NPOT cube map. I guess the Bspec just assume it's + * a POT cube map. + * + * Thus, I guess we need do this for other platforms as well. + */ + if (tObj-Target == GL_TEXTURE_CUBE_MAP_ARB + !is_power_of_two(firstImage-Height)) + return false; + state[I915_TEXREG_SS3] = ss3; /* SS3_NORMALIZED_COORDS */ state[I915_TEXREG_SS3] |= -- 1.7.7 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i915: fallback for NPOT cubemap texture
On Wed, Feb 29, 2012 at 11:44:59AM -0800, Eric Anholt wrote: On Wed, 29 Feb 2012 15:11:06 +0800, Yuanhan Liu yuanhan@linux.intel.com wrote: According to 3DSTATE_MAP_STATE at page of 104 in Bspec vol3d 3D Instructions: [DevGDG and DevAlv]: Must be a power of 2 for cube maps Well, it turned out to be that we need do this for other platforms as well, like pineview. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=4 NOTE: This is a candidate for stable release branches. Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com --- src/mesa/drivers/dri/i915/i915_texstate.c | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i915/i915_texstate.c b/src/mesa/drivers/dri/i915/i915_texstate.c index 0e500e2..e3ab50e 100644 --- a/src/mesa/drivers/dri/i915/i915_texstate.c +++ b/src/mesa/drivers/dri/i915/i915_texstate.c @@ -319,6 +319,18 @@ i915_update_tex_unit(struct intel_context *intel, GLuint unit, GLuint ss3) ((wt != GL_CLAMP) (wt != GL_CLAMP_TO_EDGE return false; + /* + * According to 3DSTATE_MAP_STATE at page of 104 in Bspec + * Vol3d 3D Instructions: + * [DevGDG and DevAlv]: Must be a power of 2 for cube maps. + * + * Well, it turned out to be that we need do this for other + * platforms as well, like pineview. + */ + if (tObj-Target == GL_TEXTURE_CUBE_MAP_ARB + !is_power_of_two(firstImage-Height)) + return false; + Pineview should be DevBLB derived, which doens't have so that requirement, Yeah, it's true according to Bspec: [DevLPT, DevCST and DevBLB]: If not a power of 2, cube maps must have all faces enabled. Well, we have all faces enabled in our code and it still failed. And it's ok when the height is power of 2. That's why I think we need do fallback for other platforms as well. so this doesn't really make sense to me. Have you looked at the rendering to see what's going on? Not much, and I'm tring now. Thanks, Yuanhan Liu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i915: fallback for NPOT cubemap texture
On Wed, 29 Feb 2012 15:11:06 +0800, Yuanhan Liu yuanhan@linux.intel.com wrote: According to 3DSTATE_MAP_STATE at page of 104 in Bspec vol3d 3D Instructions: [DevGDG and DevAlv]: Must be a power of 2 for cube maps Well, it turned out to be that we need do this for other platforms as well, like pineview. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=4 NOTE: This is a candidate for stable release branches. Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com --- src/mesa/drivers/dri/i915/i915_texstate.c | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i915/i915_texstate.c b/src/mesa/drivers/dri/i915/i915_texstate.c index 0e500e2..e3ab50e 100644 --- a/src/mesa/drivers/dri/i915/i915_texstate.c +++ b/src/mesa/drivers/dri/i915/i915_texstate.c @@ -319,6 +319,18 @@ i915_update_tex_unit(struct intel_context *intel, GLuint unit, GLuint ss3) ((wt != GL_CLAMP) (wt != GL_CLAMP_TO_EDGE return false; + /* + * According to 3DSTATE_MAP_STATE at page of 104 in Bspec + * Vol3d 3D Instructions: + * [DevGDG and DevAlv]: Must be a power of 2 for cube maps. + * + * Well, it turned out to be that we need do this for other + * platforms as well, like pineview. + */ + if (tObj-Target == GL_TEXTURE_CUBE_MAP_ARB + !is_power_of_two(firstImage-Height)) + return false; + Pineview should be DevBLB derived, which doens't have so that requirement, so this doesn't really make sense to me. Have you looked at the rendering to see what's going on? pgp9KHTDw3Bh6.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
