Re: [git pull] drm for v4.15

[Christian KÃnig]

Am 17.11.2017 um 19:55 schrieb Linus Torvalds:
> On Fri, Nov 17, 2017 at 10:14 AM, Christian KÃnig
> <christian.koenig@xxxxxxx> wrote:
> > Taking an example from the AMD headers why this automation is more tricky
> > than it sounds in the first place: Look at the
> > mmVM_CONTEXT*_PAGE_TABLE_BASE_ADDR registers for example.
> >
> > Register 0-7 are consecutive and so could be perfectly addressable with an
> > index, but register 8-15 aren't and so we always end with logic like if(i<8)
> > ... else ....
> >
> > The rational from the hardware guys is obvious that they initially had only
> > 8 and on a later hardware generation extended that to 16 registers.
> Heh. I don't disagree, but at the same time, that case is actually a
> wonderful example.
>
> Let's take the gmc_6_0 case, because it shows your irregularity, but
> it also shows another horrid example of nasty nasty automation:
>
>    mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR 0x054F
>    mmVM_CONTEXT10_PAGE_TABLE_BASE_ADDR 0x0510
>    mmVM_CONTEXT11_PAGE_TABLE_BASE_ADDR 0x0511
>    mmVM_CONTEXT12_PAGE_TABLE_BASE_ADDR 0x0512
>    mmVM_CONTEXT13_PAGE_TABLE_BASE_ADDR 0x0513
>    mmVM_CONTEXT14_PAGE_TABLE_BASE_ADDR 0x0514
>    mmVM_CONTEXT15_PAGE_TABLE_BASE_ADDR 0x0515
>    mmVM_CONTEXT1_PAGE_TABLE_BASE_ADDR 0x0550
>    mmVM_CONTEXT2_PAGE_TABLE_BASE_ADDR 0x0551
>    mmVM_CONTEXT3_PAGE_TABLE_BASE_ADDR 0x0552
>    mmVM_CONTEXT4_PAGE_TABLE_BASE_ADDR 0x0553
>    mmVM_CONTEXT5_PAGE_TABLE_BASE_ADDR 0x0554
>    mmVM_CONTEXT6_PAGE_TABLE_BASE_ADDR 0x0555
>    mmVM_CONTEXT7_PAGE_TABLE_BASE_ADDR 0x0556
>    mmVM_CONTEXT8_PAGE_TABLE_BASE_ADDR 0x050E
>    mmVM_CONTEXT9_PAGE_TABLE_BASE_ADDR 0x050F
>
> Oops. Those were clearly sorted automatically, and in entirely the wrong way.
>
> So automation has _really_ done something inexcusably stupid, and made
> the end result completely illegible in the process.

Yeah, but that is already the input we get from the hardware teams. E.g. 
in this case a list of registers sorted by their name or address (or 
even sometimes some hardware internal magic).

There isn't much we could do about that except for manual or semi 
manually cleaning up the mess.

> And yes, you'd be right that it's discontiguous at 8, but it's still
> arithmetic, ie you could easily have
>
>   #define  mmVM_PAGE_TABLE_BASE_ADDR(ctx) \
>          ((ctx)+0x054f-((ctx) & 8)*9-((ctx)&8)/8)
>
> and if "ctx" is a constant, then the end result is trivially a
> constant and can be used as such. And if it isn't, it's still a much
> cheaper operation than an "if" or "switch ()" statement (it's just a
> bitmask and two shifts).

Interesting approach, but it is not so performance critical. So I would 
still go with the "if" or "?" operator just for the improved readability.

> Now, seeing those patterns is likely not something that automation
> should do (although it's definitely possible - superoptimizers do that
> all the time), but automation could still *verify* the patterns once a
> human has made them up.

Well, this was just a rather simple example, the real problem is that 
some blocks have a dozen instances and >10k registers each.

Manual intervention is just completely out of question when applied to 
the general problem.

What we need is some automation, but as you wrote as well that is 
possible but far from easy.

> And it's quite possible that it would be a good idea to encode that
> pattern even in the original source code. In fact, it may *be* there
> somewhere (not as that arithmetic expression, but as the reverse
> decode logic, obviously).

The obvious alternative which we are working on for a few years now is 
to improve the input data we get from the hardware people.

In other words instead of getting a flat list of registers we want the 
information about where and how many times a hardware block was 
instantiated.

But getting that proved much more difficult than we thought and yes we 
are working on that for multiple years now.

Regards,
Christian.