From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Antoine Subject: Re: [PATCH] drm/i915/sysfs: Adding mocs_state Date: Fri, 6 May 2016 14:26:53 +0100 (BST) Message-ID: References: <1462367783-4342-1-git-send-email-peter.antoine@intel.com> <20160504132306.GB19266@nuc-i3427.alporthouse.com> <20160504134717.GD19266@nuc-i3427.alporthouse.com> <20160504143841.GR4329@intel.com> <20160504165533.GS4329@intel.com> <20160506111629.GY4329@intel.com> Mime-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="8323329-248305471-1462541215=:3052" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id EC8DE6EAA7 for ; Fri, 6 May 2016 13:26:55 +0000 (UTC) In-Reply-To: <20160506111629.GY4329@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: =?ISO-8859-15?Q?Ville_Syrj=E4l=E4?= Cc: "intel-gfx@lists.freedesktop.org" , "Widawsky, Benjamin" List-Id: intel-gfx@lists.freedesktop.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-248305471-1462541215=:3052 Content-Type: TEXT/PLAIN; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable On Fri, 6 May 2016, Ville Syrj=E4l=E4 wrote: > On Thu, May 05, 2016 at 07:17:02AM +0000, Antoine, Peter wrote: >> It's a little overkill? >> >> They just need to know if the cache tables have changed and to be able= to sync their indexes to the KMD. > > We already shot ourselves in the foot with this MOCS ABI stuff. This > sysfs stuff just feels like digging the hole deeper, as in more legacy > baggage when we eventually have to change the whole apporach anyway. > Given our track record here I have a feeling that will happen at some > point. The way the MOCS hardware works has pointed us into this direction. A=20 fixed table is all that works for all use-cases. Other platforms have=20 gotten around the hardware limitations by fixing the table in the "one=20 true" userspace library and have a matching table in the kernel/driver. We don't have the luxury of being able to change the table at will for=20 performance gains, security issues and have the userspace match-up. We=20 don't control both the userspace and the driver to update them both at=20 the same time and this is against the ABI. So to support the improved optimisation and handle backward compatibility= =20 we need to be able to allow the KMD and UMD to sync. The MOCS hardware was not designed from an ease of programming point of=20 view and compromises have had to be made in the current gen to make this=20 work. > >> Also you have to decode the L3CC (two 16 bit values in a 32 bit regist= er) and seems a bit unfriendly. Also you will need to know what the bits = mean to detect where the table ends (we fill with uncached entries). That= means they need to understand a lot of the workings of the hardware, whi= ch we are trying to hide as they don't need to know. It also may change a= nd then we have a support/maintenance issue which would be hidden behind = the sysfs. > > So you're worried that having to know the layout of the MOCS registers > is too much burden for userspace which already has to know pretty much > every other detail about the hardware? Yes. It's not just the layout of the registers but when they are valid an= d=20 not valid. Any work arounds that have been applied, availability in the=20 power saving modes and probably some other issues. Which registers do the= y=20 read? If they read the BSpec they will find registers for WiDi and GuC, w= e=20 don't program these. You are right, we could let the userspace work this out for themselves bu= t=20 I think this is going to cause more grief in the long term. > >> >> Also, this way future proofs the user-space from some of the changes t= hat may come in the future. >> >> I think the sysfs is nicer and easier to access for the users. >> >> On that note, what should the format of sysfs files be? > > If we would use sysfs then I think the only viable way might be to > dump out the entire table as a raw blob. And if you do that, userspace > will need to understand the contents, and at that point the whole thing > becomes pointless since you could just as well use SRM. Sorry, I don't understand why a raw blob has to be given. We list the=20 values that are required for use. This is enough for synchronisation and = any=20 performance tuning with regards to the current fixed KMD table. They then= =20 don't need to know the register layout just what each value means and=20 select which entry is best for them to use. Also, there are a set of MOCS values for each of the engines and a shared= =20 set (currently only used by RCS). As we currently ensure that all these=20 register sets (well for the engines that we use) are consistent with the=20 KMD table, it is simpler just to dump the table that we set. This is what= =20 the sysfs patch does. Ok, the format of the sysfs files have been argued=20 over it and needs to be settled on, but this seems the best currently=20 available solution to the problem. > > In any case, if you go to the trouble of enshrining a new ABI, then > maybe just go all the way and come up with a way for userspace to > reconfigure the MOCS table at will? Oh I wish so hard for this. The very first version of the MOCS almost a year ago was programmed this=20 way, it's the only sensible way that cache values for an application=20 should be programmed. But, it does not work. Due to context limitations=20 with the hardware and problems with usage for virtualisation - because of= =20 the context issues, a fixed table is the only acceptable way for all use=20 cases. Even this causes problems in virtualisation, but ones that can=20 be managed by the virtualiser (manual save/restore on non-saved engines). Hopefully in the future the sysfs and the MOCS table will be there only=20 for legacy support and won't need to change again. But for now we need to allow user space to sync with KMD. We can let them= =20 do it themselves and let the hilarity ensue or give them an easy table to= =20 ingest. I'd prefer the easy table to reduce the support costs. Peter. > >> Peter. >> -----Original Message----- >> From: Ville Syrj=E4l=E4 [mailto:ville.syrjala@linux.intel.com] >> Sent: Wednesday, May 4, 2016 5:56 PM >> To: Antoine, Peter >> Cc: Chris Wilson ; intel-gfx@lists.freedeskt= op.org; Widawsky, Benjamin >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/sysfs: Adding mocs_state >> >> On Wed, May 04, 2016 at 03:51:21PM +0100, Peter Antoine wrote: >>> >>> Sorry Ville, >>> >>> What is SRM? >> >> MI_STORE_REGISTER_MEM >> >>> >>> Peter. >>> >>> On Wed, 4 May 2016, Ville Syrj=E4l=E4 wrote: >>> >>>> On Wed, May 04, 2016 at 02:23:35PM +0000, Antoine, Peter wrote: >>>>> No, It's not debug. >>>>> It's for syncing and aligning (and validating) the open-source user= space with the kernel cache policy. >>>> >>>> Why doesn't userspace just use SRM to read registers? The spec gives >>>> me the impression that SRM doesn't care whether the register is >>>> privileged or not. >>>> >>>>> >>>>> As for the name being wrong, I'll change that. >>>>> >>>>> As for the sysfs, would you prefer the following structure: >>>>> >>>>> mocs/size >>>>> mocs/control_state >>>>> mocs/l3cc_state >>>>> >>>>> for the different tables? >>>>> >>>>> Peter. >>>>> >>>>> -----Original Message----- >>>>> From: Chris Wilson [mailto:chris@chris-wilson.co.uk] >>>>> Sent: Wednesday, May 4, 2016 2:47 PM >>>>> To: Antoine, Peter >>>>> Cc: intel-gfx@lists.freedesktop.org; Widawsky, Benjamin >>>>> >>>>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/sysfs: Adding mocs_state >>>>> >>>>> On Wed, May 04, 2016 at 02:32:53PM +0100, Peter Antoine wrote: >>>>>> Will wait for more comments, then will respin with a different >>>>>> commit message. Is the rest of the patch ok? >>>>> >>>>> No, you've put debug information into sysfs. (Also sysfs is one >>>>> value per >>>>> file.) sysfs does not match your goal of validation. And you export= ed an internal function (get_mocs...) without giving it a proper name. >>>>> -Chris >>>>> >>>>> -- >>>>> Chris Wilson, Intel Open Source Technology Centre >>>>> _______________________________________________ >>>>> Intel-gfx mailing list >>>>> Intel-gfx@lists.freedesktop.org >>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>>> >>>> >>> >>> -- >>> Peter Antoine (Android Graphics Driver Software Engineer) >>> -----------------------------------------------------------------= ---- >>> Intel Corporation (UK) Limited >>> Registered No. 1134945 (England) >>> Registered Office: Pipers Way, Swindon SN3 1RJ >>> VAT No: 860 2173 47 >> >> >> -- >> Ville Syrj=E4l=E4 >> Intel OTC > > -- Peter Antoine (Android Graphics Driver Software Engineer) --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 --8323329-248305471-1462541215=:3052 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --8323329-248305471-1462541215=:3052--