[Mesa-dev] [PATCH] winsys/radeon: fix a wrong NUM_TILE_PIPES value from the kernel

[Alexandre Demers]

On Tue, 9 Feb 2016 at 15:17 Alex Deucher <alexdeucher at gmail.com> wrote:

> On Tue, Feb 9, 2016 at 12:47 PM, Marek Olšák <maraeo at gmail.com> wrote:
> > On Tue, Feb 9, 2016 at 6:17 PM, Alexandre Demers
> > <alexandre.f.demers at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160209/7b590f18/attachment-0001.html>