<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, 9 Feb 2016 at 15:17 Alex Deucher <<a href="mailto:alexdeucher@gmail.com">alexdeucher@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, Feb 9, 2016 at 12:47 PM, Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>> wrote:<br>
> On Tue, Feb 9, 2016 at 6:17 PM, Alexandre Demers<br>
> <<a href="mailto:alexandre.f.demers@gmail.com" target="_blank">alexandre.f.demers@gmail.com</a>> wrote:<br>
>>> +            /* The kernel returns 12 for some cards for an unknown<br>
>>> reason.<br>
>>> +             * I thought this was supposed to be a power of two.<br>
>>> +             */<br>
>>> +            if (ws->gen == DRV_SI && ws->info.num_tile_pipes == 12)<br>
>>> +                ws->info.num_tile_pipes = 8;<br>
>>> +<br>
>><br>
>> I may be late in the conversation, but shouldn't we have a look at why the<br>
>> value reported by the kernel is wrong for "some" cards? Which ones and why<br>
>> should be identified. It seems to be limited to Southern Islands as far as<br>
>> we know for now, which limits the scope for now.<br>
>><br>
>> Also, about the patch itself, even if only some cards were reported to be<br>
>> problematic, why would we limit it to "ws->gen == DRV_SI"? Any cards<br>
>> reporting a wrong value should be treated the same way by mapping its value<br>
>> from 12 to 8, no?<br>
><br>
> No. Only one card is affected (Tahiti or Pitcairn, I don't remember<br>
> which one). No other card reports 12.<br>
><br>
> There is no point in looking into why the value is wrong and I haven't<br>
> been able to find where the value had come from. It's part of the<br>
> kernel ABI now anyway. Userspace won't use it anymore.<br>
<br>
I did it when I added SI support.  I think I get the value from tcore<br>
or some hw features header.  I don't remember the details.  Anyway, I<br>
think it may have been a hw value (related to the number of memory<br>
channels) whereas from a sw perspective, we'd use 8 for tiling<br>
computations.  E.g., when we set up rdev->config.si.tile_config which<br>
is what we used to use, we set it to 8.<br>
<br>
Alex<br></blockquote><div><br></div><div>Ok, I had a look at what you were pointing. There was even a comment in si_gpu_init() about tile_config for the case of the problematic value:<br><div><br>rdev->config.si.tile_config = 0;</div>switch (rdev->config.si.num_tile_pipes) {<br>[...]<br><div>case 8:</div><div>default:</div><div><span class="Apple-tab-span" style="white-space:pre">   </span>/* XXX what about 12? */</div><div><span class="Apple-tab-span" style="white-space:pre">     </span>rdev->config.si.tile_config |= (3 << 0);</div><span class="Apple-tab-span" style="white-space:pre">       </span>break;<br>[...]<br><br>Should we just clarify the comment in Mesa's committed patch according to Alex's explanation?<br>Alexandre Demers</div></div></div>