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