[Mesa-dev] [PATCH] i965/skl: Fix aligning mt->total_width to the block size

[Ben Widawsky]

On Wed, Jun 24, 2015 at 02:29:09PM +0100, Neil Roberts wrote:
> Ben Widawsky <ben at bwidawsk.net> writes:
> 
> > I think this is beginning to infringe upon the definition of align_w.
> > The total width is a function of it's miptree properties and not the
> > compressed block properties, right?
> >
> > In other words, if there is a case where align_w != bw, I think
> > total_width should be aligned to align_w, NOT bw.
> 
> I don't think it's so clear cut. In practice the mt->total_width doesn't
> really need to be aligned to anything because as far as I can tell it is
> only used to calculate the row stride. The row stride is separately
> aligned to whatever constraints necessary by libdrm so it doesn't really
> matter what we pick here.
> 
> The reason I think that the intention was to align it to the block width
> rather than the horizontal alignment is that in the non-compressed case
> the total width isn't aligned to anything at all.
> 
> It's probably not worth making too much of a fuss over this patch seeing
> as it doesn't make any practical difference. I'm happy to forget about
> it and pretend I never noticed the inconsistency.
> 
> Regards,
> - Neil
> 

I'm not opposed, I guess I just wanted a clear understanding of how it should
work. If you think this is the right thing to do, I trust you.

Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

> >
> > (I'm not opposed to the patch, just making sure I understand.)
> >
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_tex_layout.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 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 1e7d8a1..dbb6cef 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> >> @@ -366,9 +366,8 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> >>  
> >>     mt->total_width = mt->physical_width0;
> >>  
> >> -   if (mt->compressed) {
> >> -       mt->total_width = ALIGN(mt->physical_width0, mt->align_w);
> >> -   }
> >> +   if (mt->compressed)
> >> +       mt->total_width = ALIGN(mt->total_width, bw);
> >>  
> >>     /* May need to adjust width to accommodate the placement of
> >>      * the 2nd mipmap.  This occurs when the alignment
> >> -- 
> >> 1.9.3
> >> 
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Ben Widawsky, Intel Open Source Technology Center