[Mesa-dev] [PATCH 2/2] amd/addrlib: update Mesa's copy of addrlib

[Marek Olšák]

On Tue, Nov 20, 2018 at 12:08 AM Dave Airlie <airlied at gmail.com> wrote:

> On Tue, 20 Nov 2018 at 14:42, Marek Olšák <maraeo at gmail.com> wrote:
> >
> > On Mon, Nov 19, 2018 at 7:15 PM Bas Nieuwenhuizen <
> bas at basnieuwenhuizen.nl> wrote:
> >>
> >> So I tried to test this with radv and got a bunch of crashes in CTS,
> >> mostly around 3d image support:
> >>
> >> #3  0x00007ffff71a9396 in __assert_fail () from /usr/lib/libc.so.6
> >> #4  0x00007ffff69da3b4 in
> >> Addr::V2::Gfx9Lib::HwlGetPreferredSurfaceSetting (this=0x555557661b30,
> >> pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)
> >>     at ../mesa/src/amd/addrlib/src/gfx9/gfx9addrlib.cpp:3684
> >> #5  0x00007ffff69cf331 in
> >> Addr::V2::Lib::Addr2GetPreferredSurfaceSetting (this=0x555557661b30,
> >> pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)
> >>     at ../mesa/src/amd/addrlib/src/core/addrlib2.cpp:1742
> >> #6  0x00007ffff69c4e87 in Addr2GetPreferredSurfaceSetting
> >> (hLib=0x555557661b30, pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)
> >>     at ../mesa/src/amd/addrlib/src/addrinterface.cpp:1697
> >> #7  0x00007ffff69bf8d4 in gfx9_get_preferred_swizzle_mode
> >> (addrlib=0x555557661b30, in=0x7fffffffd690, is_fmask=false,
> >> flags=33555202, swizzle_mode=0x7fffffffd698)
> >>
> >> It seems to be caused by the explicit swizzle mode override that we do
> with
> >>
> >> commit b64b7125586ce48232658cd860f549a6139b6ddd
> >> Author: Marek Olšák <marek.olsak at amd.com>
> >> Date:   Mon Apr 2 12:54:52 2018 -0400
> >>
> >>     ac/surface/gfx9: request desired micro tile mode explicitly
> >>
> >>     Tested-by: Dieter Nützel <Dieter at nuetzel-hh.de>
> >>
> >>
> >> Since we never got a reason to have it (the commit message above is
> >> not descriptive and the patch not reviewed) and this is the second
> >> time already that this breaks stuff (The other was allowing S tiling
> >> for raven displayable surfaces, per 7eff8d7d3564), maybe revert it and
> >> let addrlib make the decision?
> >
> >
> > Yes, my commits are mostly unreviewed. It's the norm now. Willing
> reviewers don't exist anymore. I don't really mind that my patches are not
> reviewed, but whoever complains that I push unreviewed commits should ask
> himself why he didn't review them in their review period. That applies to
> everybody. Either review regularly or accept that unreviewed commits are
> normal.
> >
> > Secondly, past commits can't break future commits, so don't say it
> breaks stuff again. It's illogical.
> >
> > There may be multiple reasons why the commit exists. As long as
> reverting it doesn't break piglit / radeonsi, I'm OK with the reverting.
>
> Marek,
>
> There is no way anybody could review this commit, the commit log
> contains 0 information on why or what the commit is doing or what it
> fixes, there is nothing to say what the reviewer is looking out for.
>
> So maybe in future if you are pushing unreviewed commits in you could
> add the multiple reasons to the commit log? clearly you wrote the
> patch for a reason, adding the reason to the changelog shouldn't be a
> major burden.
>

Yes, I can try to do that.

Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181120/2fc77ab6/attachment.html>