Jason Ekstrand <ja...@jlekstrand.net> writes: > On Sat, Aug 8, 2015 at 2:58 AM, Francisco Jerez <curroje...@riseup.net> wrote: >> Jason Ekstrand <ja...@jlekstrand.net> writes: >> >>> I'm not a huge fan of this patch. Really, given how complicated 3-D >>> textures are on SKL, there really is no sensible horizontal slice >>> pitch. We could return 0 as an "invalid value" but I think I'd rather >>> keep it an assert. Code that is dealing with 3-D textures should no >>> better than to just blindly call this function on SKL. >>> --Jason >> >> How so? The sub-tile structure may indeed be more complicated on SKL, >> but the way how Z-slices of whole tiles are laid out in 2D memory for 3D >> textures is even simpler than on earlier generations -- Say a given >> tile-slice of LOD l starts at 2D coordinates (x, y), the next tile-slice >> will start at (x, y + qpitch), IOW the horizontal offset between >> tile-slices is zero which is what this patch does. We could probably >> keep the assertion I had but that would complicate >> update_texture_image_param() a bit more so I'd rather do what AFAIUI is >> the right thing here. > > Doing a little spelunking, it appears as if the code this patch is > modifying is only used for setting up image uniforms (i.e., it's dead > at the moment) and was pushed without review. *sigh* > > Given that, this patch certainly doesn't break anything and seems to > do what you claim it does so I'm fine with it. That said, please do a > follow-on patch that actually documents these new tex_layout > entrypoints. In particular, it should be documented what value they > compute/return. The above description, re-written in a form more > appropriate for a comment, is probably sufficient for > get_horizontal_slice_pitch. Assuming the follow-on will happen, >
There is some documentation about them in the intel_mipmap_tree.h header
file, let me know if it's not sufficient for you so I write the
follow-up.
> Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com>
>
> Go land things! You can also apply my R-B to the patch that enables
> the extension (I don't want dig through my e-mail so I can respond to
> the specific patch.)
>
Cool, thanks.
> --Jason
>
>>>
>>> On Wed, May 13, 2015 at 9:37 AM, Francisco Jerez <curroje...@riseup.net>
>>> wrote:
>>>> ---
>>>> src/mesa/drivers/dri/i965/brw_tex_layout.c | 4 +---
>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>>> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>>> index 72b02a2..6c6dc5c 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>>> @@ -281,9 +281,7 @@ brw_miptree_get_horizontal_slice_pitch(const struct
>>>> brw_context *brw,
>>>> const struct intel_mipmap_tree *mt,
>>>> unsigned level)
>>>> {
>>>> - assert(brw->gen < 9);
>>>> -
>>>> - if (mt->target == GL_TEXTURE_3D ||
>>>> + if ((brw->gen < 9 && mt->target == GL_TEXTURE_3D) ||
>>>> (brw->gen == 4 && mt->target == GL_TEXTURE_CUBE_MAP)) {
>>>> return ALIGN(minify(mt->physical_width0, level), mt->align_w);
>>>> } else {
>>>> --
>>>> 2.3.5
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
