From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933840AbdKQTSe (ORCPT ); Fri, 17 Nov 2017 14:18:34 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:34585 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161705AbdKQTSG (ORCPT ); Fri, 17 Nov 2017 14:18:06 -0500 X-Google-Smtp-Source: AGs4zMZj/wOoPdtZ7XHLXK2bwg13BDlfQeH952mNRCDWMzUMSJ3D7ClM5ZMGsb0F8qX0VJOsNpPt5g== Reply-To: christian.koenig@amd.com Subject: Re: [git pull] drm for v4.15 To: Linus Torvalds , =?UTF-8?Q?Christian_K=c3=b6nig?= Cc: =?UTF-8?Q?Nicolai_H=c3=a4hnle?= , LKML , dri-devel References: <26729c32-cfd6-82e0-b370-a46ca951adea@gmail.com> <29dd45a7-4a58-ed71-e6d4-1cd4bfc69ee5@amd.com> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: Date: Fri, 17 Nov 2017 20:18:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 17.11.2017 um 19:55 schrieb Linus Torvalds: > On Fri, Nov 17, 2017 at 10:14 AM, Christian König > 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.