[Intel-gfx] [PATCH] drm/i915/sysfs: Adding mocs_state

[Ville Syrjälä]

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.

> Also you have to decode the L3CC (two 16 bit values in a 32 bit register) 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, which we are trying to hide as they don't need to know. It also may change and 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?

> 
> Also, this way future proofs the user-space from some of the changes that 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.

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?

> Peter.
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com] 
> Sent: Wednesday, May 4, 2016 5:56 PM
> To: Antoine, Peter <peter.antoine at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>; intel-gfx at lists.freedesktop.org; Widawsky, Benjamin <benjamin.widawsky at intel.com>
> 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älä 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 userspace 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 at chris-wilson.co.uk]
> > >> Sent: Wednesday, May 4, 2016 2:47 PM
> > >> To: Antoine, Peter <peter.antoine at intel.com>
> > >> Cc: intel-gfx at lists.freedesktop.org; Widawsky, Benjamin 
> > >> <benjamin.widawsky at intel.com>
> > >> 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 exported 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 at 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älä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC