Quoting Tvrtko Ursulin (2018-12-10 17:17:29)
>
> On 07/11/2018 15:16, Tomasz Lis wrote:
> > The table has been unified across OSes to minimize virtualization overhead.
> >
> > The MOCS table is now published as part of bspec, and versioned. Entries
> > are supposed to never be modified, but new ones can be added. Adding
> > entries increases table version. The patch includes version 1 entries.
> >
> > Meaning of each entry is now explained in bspec, and user mode clients
> > are expected to know what each entry means. The 3 entries used for previous
> > platforms are still compatible with their legacy definitions, but that is
> > not guaranteed to be true for future platforms.
> >
> > v2: Fixed SCC values, improved commit comment (Daniele)
> > v3: Improved MOCS table comment (Daniele)
> > v4: Moved new entries below gen9 ones. Put common entries into
> > definition to be used in multiple arrays. (Lucas)
> > v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
> > to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
> > v6: Removed definitions of reserved entries. (Michal)
> > Increased limit of entries sent to the hardware on gen11+.
> >
> > BSpec: 34007
> > BSpec: 560
> > Signed-off-by: Tomasz Lis <tomasz.lis@xxxxxxxxx>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> (v4)
>
> R-b upgrade needed here as well.
>
> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> > Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> > Cc: Zhi A Wang <zhi.a.wang@xxxxxxxxx>
> > Cc: Anuj Phogat <anuj.phogat@xxxxxxxxx>
> > Cc: Adam Cetnerowski <adam.cetnerowski@xxxxxxxxx>
> > Cc: Piotr Rozenfeld <piotr.rozenfeld@xxxxxxxxx>
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_mocs.c | 222 +++++++++++++++++++++++++++++++++-----
> > 1 file changed, 197 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> > index 8d08a7b..4eb05c6 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -44,6 +44,8 @@ struct drm_i915_mocs_table {
> > #define LE_SCC(value) ((value) << 8)
> > #define LE_PFM(value) ((value) << 11)
> > #define LE_SCF(value) ((value) << 14)
> > +#define LE_COS(value) ((value) << 15)
> > +#define LE_SSE(value) ((value) << 17)
> >
> > /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
> > #define L3_ESC(value) ((value) << 0)
> > @@ -52,6 +54,10 @@ struct drm_i915_mocs_table {
> >
> > /* Helper defines */
> > #define GEN9_NUM_MOCS_ENTRIES 62 /* 62 out of 64 - 63 & 64 are reserved. */
> > +#define GEN11_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */
> > +
> > +#define NUM_MOCS_ENTRIES(i915) \
> > + (INTEL_GEN(i915) < 11 ? GEN9_NUM_MOCS_ENTRIES : GEN11_NUM_MOCS_ENTRIES)
>
> I have a suggestion to make this a bit more elegant by avoiding a
> sprinkle of conditionals throughout the code - since I count ten
> non-debug call sites of this macros.
>
> Since the MOCS code seems nicely driven from struct drm_i915_mocs_table,
> the suggestion is to store both the used and maximum valid number of
> entries per platform in that structure.
>
> It's all nicely consolidated in get_mocs_settings, where all the sanity
> checks could be moved as well. Other bits of the code would then just
> trust the table.
>
> >
> > /* (e)LLC caching options */
> > #define LE_PAGETABLE 0
> > @@ -80,21 +86,21 @@ struct drm_i915_mocs_table {
> > * LNCFCMOCS0 - LNCFCMOCS32 registers.
> > *
> > * These tables are intended to be kept reasonably consistent across
> > - * platforms. However some of the fields are not applicable to all of
> > - * them.
> > + * HW platforms, and for ICL+, be identical across OSes. To achieve
> > + * that, for Icelake and above, list of entries is published as part
> > + * of bspec.
> > *
> > * Entries not part of the following tables are undefined as far as
> > - * userspace is concerned and shouldn't be relied upon. For the time
> > - * being they will be implicitly initialized to the strictest caching
> > - * configuration (uncached) to guarantee forwards compatibility with
> > - * userspace programs written against more recent kernels providing
> > - * additional MOCS entries.
> > + * userspace is concerned and shouldn't be relied upon.
> > + *
> > + * The last two entries are reserved by the hardware. For ICL+ they
> > + * should be initialized according to bspec and never used, for older
> > + * platforms they should never be written to.
> > *
> > - * NOTE: These tables MUST start with being uncached and the length
> > - * MUST be less than 63 as the last two registers are reserved
> > - * by the hardware. These tables are part of the kernel ABI and
> > - * may only be updated incrementally by adding entries at the
> > - * end.
> > + * NOTE: These tables are part of bspec and defined as part of hardware
> > + * interface for ICL+. For older platforms, they are part of kernel
> > + * ABI. It is expected that existing entries will remain constant
> > + * and the tables will only be updated by adding new entries.
> > */
> >
> > #define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> > @@ -147,6 +153,167 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> > #undef MOCS_CONTROL_VALUE
> > #undef MOCS_L3CC_VALUE
> >
> > +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf, cos, sse) \
> > + (LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> > + LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> > + LE_PFM(pfm) | LE_SCF(scf) | LE_COS(cos) | LE_SSE(sse))
> > +
> > +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> > + (L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
> > +
> > +#define GEN11_MOCS_ENTRIES \
> > + [0] = { \
> > + /* Base - Uncached (Deprecated) */ \
> > + .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> > + 0, 0, 0, 0, 0, 0, 0, 0), \
> > + .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
These indents don't follow the guidelines. Make sure checkpatch
doesn't complain.
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
