All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: Francisco Jerez <currojerez@riseup.net>, intel-gfx@lists.freedesktop.org
Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Subject: Re: [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table	definitions
Date: Tue, 12 Jul 2016 14:08:52 +0300	[thread overview]
Message-ID: <1468321732.12301.28.camel@intel.com> (raw)
In-Reply-To: <87eg7dyqsb.fsf@riseup.net>

On pe, 2016-07-01 at 14:47 -0700, Francisco Jerez wrote:
> Hi Imre,
> 
> Imre Deak <imre.deak@intel.com> writes:
> 
> > Use named struct initializers for clarity. Also fix the target
> > cache
> > definition to reflect its role in GEN9 onwards. On GEN8 a TC value
> > of 0
> > meant ELLC but on GEN9+ it means the TC and LRU controls are taken
> > from
> > the PTE.
> > 
> I vaguely recall bringing up this apparent inconsistency with the
> hardware spec during review of the original series of patches
> programming the MOCS tables.  IIRC TC value 0 behaved as ELLC-only on
> the simulator, but the hardware docs claimed it would take the target
> cache from the PTE controls, not sure what the real hardware actually
> does.

At least the documentation seems to be quite consistent on this. It's
true that you have to set the MOCS TC to 0 for an eLLC-only mapping,
but that depends on the (default) 0 of the PTE [3:2] field (override to
eLLC-only). So maybe during simulation PTE was at the default eLLC-
only, hence your observed behavior.

In any case we don't use TC PTE passthrough for any MOCS index atm, so
I would suggest keeping the code in line with the documentation for
now.

--Imre

> Maybe Peter can confirm whether this was intentional?
> 
> > No functional change, igt/gem_mocs_settings still passing after
> > this
> > change.
> > 
> > v2: (Chris)
> > - Add back the hexa literals for the entries.
> >   Add note that igt/gem_mocs_settings still passes.
> > 
> > CC: Rong R Yang <rong.r.yang@intel.com>
> > CC: Yakui Zhao <yakui.zhao@intel.com>
> > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_mocs.c | 88
> > +++++++++++++++++++++++++++------------
> >  1 file changed, 61 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c
> > b/drivers/gpu/drm/i915/intel_mocs.c
> > index 3c1482b..d36e609 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table {
> >  #define L3_WB			3
> >  
> >  /* Target cache */
> > -#define ELLC			0
> > -#define LLC			1
> > -#define LLC_ELLC		2
> > +#define LE_TC_PAGETABLE		0
> > +#define LE_TC_LLC		1
> > +#define LE_TC_LLC_ELLC		2
> > +#define LE_TC_LLC_ELLC_ALT	3
> >  
> >  /*
> >   * MOCS tables
> > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table {
> >   *       end.
> >   */
> >  static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> > -	/* { 0x00000009, 0x0010 } */
> > -	{ (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) |
> > LE_LRUM(0) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) },
> > -	/* { 0x00000038, 0x0030 } */
> > -	{ (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC)
> > | LE_LRUM(3) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) },
> > -	/* { 0x0000003b, 0x0030 } */
> > -	{ (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) |
> > LE_LRUM(3) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }
> > +	{ /* 0x00000009 */
> > +	  .control_value = LE_CACHEABILITY(LE_UC) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +
> > +	  /* 0x0010 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_UC),
> > +	},
> > +	{
> > +	  /* 0x00000038 */
> > +	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> > +	{
> > +	  /* 0x0000003b */
> > +	  .control_value = LE_CACHEABILITY(LE_WB) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +	  /* 0x0030 */
> > +	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> >  };
> >  
> >  /* NOTE: the LE_TGT_CACHE is not used on Broxton */
> >  static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> > -	/* { 0x00000009, 0x0010 } */
> > -	{ (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) |
> > LE_LRUM(0) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) },
> > -	/* { 0x00000038, 0x0030 } */
> > -	{ (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC)
> > | LE_LRUM(3) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) },
> > -	/* { 0x0000003b, 0x0030 } */
> > -	{ (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) |
> > LE_LRUM(3) |
> > -	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) |
> > LE_SCF(0)),
> > -	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }
> > +	{
> > +	  /* 0x00000009 */
> > +	  .control_value = LE_CACHEABILITY(LE_UC) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +
> > +	  /* 0x0010 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_UC),
> > +	},
> > +	{
> > +	  /* 0x00000038 */
> > +	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> > +	{
> > +	  /* 0x0000003b */
> > +	  .control_value = LE_CACHEABILITY(LE_WB) |
> > +			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> > +			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) |
> > LE_SCC(0) |
> > +			   LE_PFM(0) | LE_SCF(0),
> > +
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> >  };
> >  
> >  /**
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-07-12 11:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak
2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak
2016-07-01 21:23   ` Bish, Jim
2016-07-01 21:47   ` Francisco Jerez
2016-07-12 11:08     ` Imre Deak [this message]
2016-07-13  2:10   ` Zhao Yakui
2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak
2016-07-13  2:32   ` Zhao Yakui
2016-07-14  8:33   ` Yang, Rong R
2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak
2016-07-01 13:49   ` Chris Wilson
2016-07-01 13:56     ` Imre Deak
2016-07-01 14:32   ` [PATCH v4 " Imre Deak
2016-07-13  2:22   ` [PATCH v3 " Zhao Yakui
2016-07-13 10:04     ` Imre Deak
2016-07-14  1:38       ` Zhao Yakui
2016-07-01 14:54 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry Patchwork
2016-07-19 18:15   ` Imre Deak
2016-07-01 15:15 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) Patchwork
2016-07-18 14:28 ` [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1468321732.12301.28.camel@intel.com \
    --to=imre.deak@intel.com \
    --cc=currojerez@riseup.net \
    --cc=eero.t.tamminen@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.