<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Nov 20, 2018 at 12:08 AM Dave Airlie <<a href="mailto:airlied@gmail.com">airlied@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, 20 Nov 2018 at 14:42, Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>> wrote:<br>
><br>
> On Mon, Nov 19, 2018 at 7:15 PM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> wrote:<br>
>><br>
>> So I tried to test this with radv and got a bunch of crashes in CTS,<br>
>> mostly around 3d image support:<br>
>><br>
>> #3 0x00007ffff71a9396 in __assert_fail () from /usr/lib/libc.so.6<br>
>> #4 0x00007ffff69da3b4 in<br>
>> Addr::V2::Gfx9Lib::HwlGetPreferredSurfaceSetting (this=0x555557661b30,<br>
>> pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)<br>
>> at ../mesa/src/amd/addrlib/src/gfx9/gfx9addrlib.cpp:3684<br>
>> #5 0x00007ffff69cf331 in<br>
>> Addr::V2::Lib::Addr2GetPreferredSurfaceSetting (this=0x555557661b30,<br>
>> pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)<br>
>> at ../mesa/src/amd/addrlib/src/core/addrlib2.cpp:1742<br>
>> #6 0x00007ffff69c4e87 in Addr2GetPreferredSurfaceSetting<br>
>> (hLib=0x555557661b30, pIn=0x7fffffffd5f0, pOut=0x7fffffffd5d0)<br>
>> at ../mesa/src/amd/addrlib/src/addrinterface.cpp:1697<br>
>> #7 0x00007ffff69bf8d4 in gfx9_get_preferred_swizzle_mode<br>
>> (addrlib=0x555557661b30, in=0x7fffffffd690, is_fmask=false,<br>
>> flags=33555202, swizzle_mode=0x7fffffffd698)<br>
>><br>
>> It seems to be caused by the explicit swizzle mode override that we do with<br>
>><br>
>> commit b64b7125586ce48232658cd860f549a6139b6ddd<br>
>> Author: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank">marek.olsak@amd.com</a>><br>
>> Date: Mon Apr 2 12:54:52 2018 -0400<br>
>><br>
>> ac/surface/gfx9: request desired micro tile mode explicitly<br>
>><br>
>> Tested-by: Dieter Nützel <<a href="mailto:Dieter@nuetzel-hh.de" target="_blank">Dieter@nuetzel-hh.de</a>><br>
>><br>
>><br>
>> Since we never got a reason to have it (the commit message above is<br>
>> not descriptive and the patch not reviewed) and this is the second<br>
>> time already that this breaks stuff (The other was allowing S tiling<br>
>> for raven displayable surfaces, per 7eff8d7d3564), maybe revert it and<br>
>> let addrlib make the decision?<br>
><br>
><br>
> 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.<br>
><br>
> Secondly, past commits can't break future commits, so don't say it breaks stuff again. It's illogical.<br>
><br>
> 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.<br>
<br>
Marek,<br>
<br>
There is no way anybody could review this commit, the commit log<br>
contains 0 information on why or what the commit is doing or what it<br>
fixes, there is nothing to say what the reviewer is looking out for.<br>
<br>
So maybe in future if you are pushing unreviewed commits in you could<br>
add the multiple reasons to the commit log? clearly you wrote the<br>
patch for a reason, adding the reason to the changelog shouldn't be a<br>
major burden.<br></blockquote><div><br></div>Yes, I can try to do that.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Marek<br></div></div>