On Wed, Jun 24, 2015 at 02:29:09PM +0100, Neil Roberts wrote:
> Ben Widawsky <b...@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 <b...@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@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
