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

[Antoine, Peter]

Anymore feedback on this?

-----Original Message-----
From: Antoine, Peter 
Sent: Thursday, May 5, 2016 8:17 AM
To: Ville Syrjälä <ville.syrjala at linux.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

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.
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.

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?

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