diff mbox series

drm/i915: Enable eLLC caching of display buffers for SKL+

Message ID 20190415141641.29055-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Enable eLLC caching of display buffers for SKL+ | expand

Commit Message

Ville Syrjälä April 15, 2019, 2:16 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since SKL the eLLC has been sitting on the far side of the system
agent, meaning the display engine can utilize it. Let's enable that.

I chose WB for the caching mode, because my numbers are indicating
that WT might actually be WB and WC might actually be UC. I'm not
100% sure that is indeed the case but at least my simple rendercopy
based benchmark didn't see any difference in performance.

Also if I configure things to do LLCeLLC+WT I still get cache dirt
on my screen, suggesting that is in fact operating in WB mode
anyway. This is also the reason I had to fix the MOCS target cache
to really say PTE rather than LLC+eLLC.

Caveat: I've not benchmarked any real workloads. IIRC Eero did
benchmark an earlier version, but that didn't have the PTE vs.
LLC+eLLC MOCS fix so it wasn't actually doing the right thing
most likely.

Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 3 +--
 drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++--
 drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
 drivers/gpu/drm/i915/intel_mocs.c   | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

Comments

Chris Wilson April 15, 2019, 4:03 p.m. UTC | #1 
Quoting Patchwork (2019-04-15 16:21:30)
> == Series Details ==
> 
> Series: drm/i915: Enable eLLC caching of display buffers for SKL+
> URL   : https://patchwork.freedesktop.org/series/59502/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5934 -> Patchwork_12799
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.

How good is our testing for detecting cacheline dirt from rendering?

Now, it's pretty easy for us to test, you fire up a desktop and wiggle a
few windows, but how well automated is it in CI?
-chris
Ville Syrjälä April 15, 2019, 4:20 p.m. UTC | #2 
On Mon, Apr 15, 2019 at 05:03:09PM +0100, Chris Wilson wrote:
> Quoting Patchwork (2019-04-15 16:21:30)
> > == Series Details ==
> > 
> > Series: drm/i915: Enable eLLC caching of display buffers for SKL+
> > URL   : https://patchwork.freedesktop.org/series/59502/
> > State : success
> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_5934 -> Patchwork_12799
> > ====================================================
> > 
> > Summary
> > -------
> > 
> >   **SUCCESS**
> > 
> >   No regressions found.
> 
> How good is our testing for detecting cacheline dirt from rendering?
> 
> Now, it's pretty easy for us to test, you fire up a desktop and wiggle a
> few windows, but how well automated is it in CI?

Not well. Rendercopy sets MOCS to 0 which means UC, so it won't test
this at all. We should probably change that.
Eero Tamminen April 16, 2019, 2:28 p.m. UTC | #3 
Hi,

Based on quick tests with the patch:

* Results in GfxBench and Unigine (Valley/Heaven) tests were within 
daily variation on the tested SKL machines

* SKL GT4e (128MB eLLC) / Wayland / Weston:
   +15-20% SynMark TexMem512 (512MB of textures)
    +4-6% SynMark TerrainFly*, CSCloth, ShMapVsm
   -5-10% SynMark TexMem128 (128MB of textures)

* SKL GT3e (64MB eLLC) / Xorg / Unity:
   +4-8% GpuTest Triangle fullscreen (FullHD)
  -5-10% GpuTest Triangle windowed (1/2 screen)

* SKL GT2 (no eLLC) / Xorg / Unity:
   * Some of the higher FPS SynMark pixel and vertex shader tests
     are few percent higher, more than daily variance
   => Do you see any reason why this machine would be impacted
      although it doesn't eLLC?

(I built it against drm-tip and compared results against previous and 
next day unpatched drm-tip results that I had otherwise.)


	- Eero

On 15.4.2019 17.16, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since SKL the eLLC has been sitting on the far side of the system
> agent, meaning the display engine can utilize it. Let's enable that.
> 
> I chose WB for the caching mode, because my numbers are indicating
> that WT might actually be WB and WC might actually be UC. I'm not
> 100% sure that is indeed the case but at least my simple rendercopy
> based benchmark didn't see any difference in performance.
> 
> Also if I configure things to do LLCeLLC+WT I still get cache dirt
> on my screen, suggesting that is in fact operating in WB mode
> anyway. This is also the reason I had to fix the MOCS target cache
> to really say PTE rather than LLC+eLLC.
> 
> Caveat: I've not benchmarked any real workloads. IIRC Eero did
> benchmark an earlier version, but that didn't have the PTE vs.
> LLC+eLLC MOCS fix so it wasn't actually doing the right thing
> most likely.
> 
> Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h     | 3 +--
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++--
>   drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
>   drivers/gpu/drm/i915/intel_mocs.c   | 2 +-
>   4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 35d0782c077e..2a4f33fa2bba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2517,8 +2517,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>   #define HAS_LLC(dev_priv)	(INTEL_INFO(dev_priv)->has_llc)
>   #define HAS_SNOOP(dev_priv)	(INTEL_INFO(dev_priv)->has_snoop)
>   #define HAS_EDRAM(dev_priv)	((dev_priv)->edram_size_mb)
> -#define HAS_WT(dev_priv)	((IS_HASWELL(dev_priv) || \
> -				 IS_BROADWELL(dev_priv)) && HAS_EDRAM(dev_priv))
> +#define HAS_WT(dev_priv)	HAS_EDRAM(dev_priv)
>   
>   #define HWS_NEEDS_PHYSICAL(dev_priv)	(INTEL_INFO(dev_priv)->hws_needs_physical)
>   
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8f460cc4cc1f..038fbf52a997 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3071,7 +3071,7 @@ static void cnl_setup_private_ppat(struct intel_ppat *ppat)
>   
>   	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);
>   	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
> -	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
> +	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE);
>   	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);
>   	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
>   	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
> @@ -3109,7 +3109,10 @@ static void bdw_setup_private_ppat(struct intel_ppat *ppat)
>   
>   	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);      /* for normal objects, no eLLC */
>   	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);  /* for something pointing to ptes? */
> -	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);  /* for scanout with eLLC */
> +	if (INTEL_GEN(ppat->i915) >= 9)
> +		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE); /* for scanout with eLLC */
> +	else
> +		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for scanout with eLLC */
>   	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);                      /* Uncached objects, mostly for scanout */
>   	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
>   	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f597f35b109b..47adc7268867 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -139,7 +139,7 @@ typedef u64 gen8_ppgtt_pml4e_t;
>   #define PPAT_UNCACHED			(_PAGE_PWT | _PAGE_PCD)
>   #define PPAT_CACHED_PDE			0 /* WB LLC */
>   #define PPAT_CACHED			_PAGE_PAT /* WB LLCeLLC */
> -#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT eLLC */
> +#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT LLCeLLC (HSW/BDW) or WB eLLC (SKL+) */
>   
>   #define CHV_PPAT_SNOOP			(1<<6)
>   #define GEN8_PPAT_AGE(x)		((x)<<4)
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 274ba78500c0..d984ccff94ef 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -115,7 +115,7 @@ struct drm_i915_mocs_table {
>   		   LE_1_UC | LE_TC_2_LLC_ELLC, \
>   		   L3_1_UC), \
>   	MOCS_ENTRY(I915_MOCS_PTE, \
> -		   LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
> +		   LE_0_PAGETABLE | LE_TC_0_PAGETABLE | LE_LRUM(3), \
>   		   L3_3_WB)
>   
>   static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
>
Ville Syrjälä April 16, 2019, 2:37 p.m. UTC | #4 
On Tue, Apr 16, 2019 at 05:28:57PM +0300, Eero Tamminen wrote:
> Hi,
> 
> Based on quick tests with the patch:
> 
> * Results in GfxBench and Unigine (Valley/Heaven) tests were within 
> daily variation on the tested SKL machines
> 
> * SKL GT4e (128MB eLLC) / Wayland / Weston:
>    +15-20% SynMark TexMem512 (512MB of textures)
>     +4-6% SynMark TerrainFly*, CSCloth, ShMapVsm
>    -5-10% SynMark TexMem128 (128MB of textures)

These seem mostly good. The 128MB case regression seems
understandable since we don't quite fit into the eLLC
anymore.

> 
> * SKL GT3e (64MB eLLC) / Xorg / Unity:
>    +4-8% GpuTest Triangle fullscreen (FullHD)
>   -5-10% GpuTest Triangle windowed (1/2 screen)

Not quite sure why the windowed case would suffer here :/

> 
> * SKL GT2 (no eLLC) / Xorg / Unity:
>    * Some of the higher FPS SynMark pixel and vertex shader tests
>      are few percent higher, more than daily variance
>    => Do you see any reason why this machine would be impacted
>       although it doesn't eLLC?

Can't think of a reason for that. All display buffers should still
be UC on such a machine.

> 
> (I built it against drm-tip and compared results against previous and 
> next day unpatched drm-tip results that I had otherwise.)
> 
> 
> 	- Eero
> 
> On 15.4.2019 17.16, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Since SKL the eLLC has been sitting on the far side of the system
> > agent, meaning the display engine can utilize it. Let's enable that.
> > 
> > I chose WB for the caching mode, because my numbers are indicating
> > that WT might actually be WB and WC might actually be UC. I'm not
> > 100% sure that is indeed the case but at least my simple rendercopy
> > based benchmark didn't see any difference in performance.
> > 
> > Also if I configure things to do LLCeLLC+WT I still get cache dirt
> > on my screen, suggesting that is in fact operating in WB mode
> > anyway. This is also the reason I had to fix the MOCS target cache
> > to really say PTE rather than LLC+eLLC.
> > 
> > Caveat: I've not benchmarked any real workloads. IIRC Eero did
> > benchmark an earlier version, but that didn't have the PTE vs.
> > LLC+eLLC MOCS fix so it wasn't actually doing the right thing
> > most likely.
> > 
> > Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h     | 3 +--
> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++--
> >   drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
> >   drivers/gpu/drm/i915/intel_mocs.c   | 2 +-
> >   4 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 35d0782c077e..2a4f33fa2bba 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2517,8 +2517,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >   #define HAS_LLC(dev_priv)	(INTEL_INFO(dev_priv)->has_llc)
> >   #define HAS_SNOOP(dev_priv)	(INTEL_INFO(dev_priv)->has_snoop)
> >   #define HAS_EDRAM(dev_priv)	((dev_priv)->edram_size_mb)
> > -#define HAS_WT(dev_priv)	((IS_HASWELL(dev_priv) || \
> > -				 IS_BROADWELL(dev_priv)) && HAS_EDRAM(dev_priv))
> > +#define HAS_WT(dev_priv)	HAS_EDRAM(dev_priv)
> >   
> >   #define HWS_NEEDS_PHYSICAL(dev_priv)	(INTEL_INFO(dev_priv)->hws_needs_physical)
> >   
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 8f460cc4cc1f..038fbf52a997 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -3071,7 +3071,7 @@ static void cnl_setup_private_ppat(struct intel_ppat *ppat)
> >   
> >   	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);
> >   	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
> > -	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
> > +	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE);
> >   	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);
> >   	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
> >   	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
> > @@ -3109,7 +3109,10 @@ static void bdw_setup_private_ppat(struct intel_ppat *ppat)
> >   
> >   	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);      /* for normal objects, no eLLC */
> >   	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);  /* for something pointing to ptes? */
> > -	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);  /* for scanout with eLLC */
> > +	if (INTEL_GEN(ppat->i915) >= 9)
> > +		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE); /* for scanout with eLLC */
> > +	else
> > +		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for scanout with eLLC */
> >   	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);                      /* Uncached objects, mostly for scanout */
> >   	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
> >   	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index f597f35b109b..47adc7268867 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -139,7 +139,7 @@ typedef u64 gen8_ppgtt_pml4e_t;
> >   #define PPAT_UNCACHED			(_PAGE_PWT | _PAGE_PCD)
> >   #define PPAT_CACHED_PDE			0 /* WB LLC */
> >   #define PPAT_CACHED			_PAGE_PAT /* WB LLCeLLC */
> > -#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT eLLC */
> > +#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT LLCeLLC (HSW/BDW) or WB eLLC (SKL+) */
> >   
> >   #define CHV_PPAT_SNOOP			(1<<6)
> >   #define GEN8_PPAT_AGE(x)		((x)<<4)
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> > index 274ba78500c0..d984ccff94ef 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -115,7 +115,7 @@ struct drm_i915_mocs_table {
> >   		   LE_1_UC | LE_TC_2_LLC_ELLC, \
> >   		   L3_1_UC), \
> >   	MOCS_ENTRY(I915_MOCS_PTE, \
> > -		   LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
> > +		   LE_0_PAGETABLE | LE_TC_0_PAGETABLE | LE_LRUM(3), \
> >   		   L3_3_WB)
> >   
> >   static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> >
Chris Wilson April 17, 2019, 7:09 a.m. UTC | #5 
Quoting Ville Syrjala (2019-04-15 15:16:41)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since SKL the eLLC has been sitting on the far side of the system
> agent, meaning the display engine can utilize it. Let's enable that.
> 
> I chose WB for the caching mode, because my numbers are indicating
> that WT might actually be WB and WC might actually be UC. I'm not
> 100% sure that is indeed the case but at least my simple rendercopy
> based benchmark didn't see any difference in performance.
> 
> Also if I configure things to do LLCeLLC+WT I still get cache dirt
> on my screen, suggesting that is in fact operating in WB mode
> anyway. This is also the reason I had to fix the MOCS target cache
> to really say PTE rather than LLC+eLLC.

We also need to check with hybrid setups that supply buffers via prime,
and we may need to end up marking those as explicitly uncached.
-Chris
Ville Syrjälä April 17, 2019, 5:15 p.m. UTC | #6 
On Wed, Apr 17, 2019 at 08:09:07AM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-15 15:16:41)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Since SKL the eLLC has been sitting on the far side of the system
> > agent, meaning the display engine can utilize it. Let's enable that.
> > 
> > I chose WB for the caching mode, because my numbers are indicating
> > that WT might actually be WB and WC might actually be UC. I'm not
> > 100% sure that is indeed the case but at least my simple rendercopy
> > based benchmark didn't see any difference in performance.
> > 
> > Also if I configure things to do LLCeLLC+WT I still get cache dirt
> > on my screen, suggesting that is in fact operating in WB mode
> > anyway. This is also the reason I had to fix the MOCS target cache
> > to really say PTE rather than LLC+eLLC.
> 
> We also need to check with hybrid setups that supply buffers via prime,
> and we may need to end up marking those as explicitly uncached.

I think all memory access should be able to snoop the eLLC. But yeah,
this should be confirmed on actual hardware. Anyone have a prime setup
handy?
Ville Syrjälä April 26, 2019, 2:54 p.m. UTC | #7 
On Wed, Apr 17, 2019 at 08:15:43PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 17, 2019 at 08:09:07AM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2019-04-15 15:16:41)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Since SKL the eLLC has been sitting on the far side of the system
> > > agent, meaning the display engine can utilize it. Let's enable that.
> > > 
> > > I chose WB for the caching mode, because my numbers are indicating
> > > that WT might actually be WB and WC might actually be UC. I'm not
> > > 100% sure that is indeed the case but at least my simple rendercopy
> > > based benchmark didn't see any difference in performance.
> > > 
> > > Also if I configure things to do LLCeLLC+WT I still get cache dirt
> > > on my screen, suggesting that is in fact operating in WB mode
> > > anyway. This is also the reason I had to fix the MOCS target cache
> > > to really say PTE rather than LLC+eLLC.
> > 
> > We also need to check with hybrid setups that supply buffers via prime,
> > and we may need to end up marking those as explicitly uncached.
> 
> I think all memory access should be able to snoop the eLLC. But yeah,
> this should be confirmed on actual hardware. Anyone have a prime setup
> handy?

It occurred to me that finding a machine for this might be a little
difficult as most gt3e/gt4e chips are only available in laptops/nucs/etc.
IIRC there are some Xeons that would qualify, but I suppose those are
somewhat rare. Not sure if there are any other desktop parts that have
ellc.
Chris Wilson April 26, 2019, 3:01 p.m. UTC | #8 
Quoting Ville Syrjälä (2019-04-26 15:54:54)
> On Wed, Apr 17, 2019 at 08:15:43PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 17, 2019 at 08:09:07AM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2019-04-15 15:16:41)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Since SKL the eLLC has been sitting on the far side of the system
> > > > agent, meaning the display engine can utilize it. Let's enable that.
> > > > 
> > > > I chose WB for the caching mode, because my numbers are indicating
> > > > that WT might actually be WB and WC might actually be UC. I'm not
> > > > 100% sure that is indeed the case but at least my simple rendercopy
> > > > based benchmark didn't see any difference in performance.
> > > > 
> > > > Also if I configure things to do LLCeLLC+WT I still get cache dirt
> > > > on my screen, suggesting that is in fact operating in WB mode
> > > > anyway. This is also the reason I had to fix the MOCS target cache
> > > > to really say PTE rather than LLC+eLLC.
> > > 
> > > We also need to check with hybrid setups that supply buffers via prime,
> > > and we may need to end up marking those as explicitly uncached.
> > 
> > I think all memory access should be able to snoop the eLLC. But yeah,
> > this should be confirmed on actual hardware. Anyone have a prime setup
> > handy?
> 
> It occurred to me that finding a machine for this might be a little
> difficult as most gt3e/gt4e chips are only available in laptops/nucs/etc.
> IIRC there are some Xeons that would qualify, but I suppose those are
> somewhat rare. Not sure if there are any other desktop parts that have
> ellc.

For now at least. Would an ePCI be a fun mix of coherency problems? Not
that they are any more common.

Did you finish up the rendercopy tests? I think I saw that you were
working on something that looked like it could be used for verifying
rendering into the frontbuffer (or at least leaving cache dirty prior to
flips)?
-Chris
Ville Syrjälä April 26, 2019, 3:13 p.m. UTC | #9 
On Fri, Apr 26, 2019 at 04:01:02PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2019-04-26 15:54:54)
> > On Wed, Apr 17, 2019 at 08:15:43PM +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 17, 2019 at 08:09:07AM +0100, Chris Wilson wrote:
> > > > Quoting Ville Syrjala (2019-04-15 15:16:41)
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Since SKL the eLLC has been sitting on the far side of the system
> > > > > agent, meaning the display engine can utilize it. Let's enable that.
> > > > > 
> > > > > I chose WB for the caching mode, because my numbers are indicating
> > > > > that WT might actually be WB and WC might actually be UC. I'm not
> > > > > 100% sure that is indeed the case but at least my simple rendercopy
> > > > > based benchmark didn't see any difference in performance.
> > > > > 
> > > > > Also if I configure things to do LLCeLLC+WT I still get cache dirt
> > > > > on my screen, suggesting that is in fact operating in WB mode
> > > > > anyway. This is also the reason I had to fix the MOCS target cache
> > > > > to really say PTE rather than LLC+eLLC.
> > > > 
> > > > We also need to check with hybrid setups that supply buffers via prime,
> > > > and we may need to end up marking those as explicitly uncached.
> > > 
> > > I think all memory access should be able to snoop the eLLC. But yeah,
> > > this should be confirmed on actual hardware. Anyone have a prime setup
> > > handy?
> > 
> > It occurred to me that finding a machine for this might be a little
> > difficult as most gt3e/gt4e chips are only available in laptops/nucs/etc.
> > IIRC there are some Xeons that would qualify, but I suppose those are
> > somewhat rare. Not sure if there are any other desktop parts that have
> > ellc.
> 
> For now at least. Would an ePCI be a fun mix of coherency problems? Not
> that they are any more common.

Hmm. I keep forgetting what century we're in. Some kind of external
thunderbolt enclosure might do the trick. Never actually seen one but
I suppose it shouldn't be an impossible task to procure some.

> 
> Did you finish up the rendercopy tests? I think I saw that you were
> working on something that looked like it could be used for verifying
> rendering into the frontbuffer (or at least leaving cache dirty prior to
> flips)?

I just fixed up the mocs setup in rendercopy. I didn't write a specific
testase yet.
Chris Wilson May 24, 2019, 8:19 p.m. UTC | #10 
Quoting Ville Syrjala (2019-04-15 15:16:41)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since SKL the eLLC has been sitting on the far side of the system
> agent, meaning the display engine can utilize it. Let's enable that.
> 
> I chose WB for the caching mode, because my numbers are indicating
> that WT might actually be WB and WC might actually be UC. I'm not
> 100% sure that is indeed the case but at least my simple rendercopy
> based benchmark didn't see any difference in performance.
> 
> Also if I configure things to do LLCeLLC+WT I still get cache dirt
> on my screen, suggesting that is in fact operating in WB mode
> anyway. This is also the reason I had to fix the MOCS target cache
> to really say PTE rather than LLC+eLLC.
> 
> Caveat: I've not benchmarked any real workloads. IIRC Eero did
> benchmark an earlier version, but that didn't have the PTE vs.
> LLC+eLLC MOCS fix so it wasn't actually doing the right thing
> most likely.
> 
> Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 35d0782c077e..2a4f33fa2bba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2517,8 +2517,7 @@  IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_LLC(dev_priv)	(INTEL_INFO(dev_priv)->has_llc)
 #define HAS_SNOOP(dev_priv)	(INTEL_INFO(dev_priv)->has_snoop)
 #define HAS_EDRAM(dev_priv)	((dev_priv)->edram_size_mb)
-#define HAS_WT(dev_priv)	((IS_HASWELL(dev_priv) || \
-				 IS_BROADWELL(dev_priv)) && HAS_EDRAM(dev_priv))
+#define HAS_WT(dev_priv)	HAS_EDRAM(dev_priv)
 
 #define HWS_NEEDS_PHYSICAL(dev_priv)	(INTEL_INFO(dev_priv)->hws_needs_physical)
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8f460cc4cc1f..038fbf52a997 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3071,7 +3071,7 @@  static void cnl_setup_private_ppat(struct intel_ppat *ppat)
 
 	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);
 	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
-	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
+	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE);
 	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);
 	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
 	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
@@ -3109,7 +3109,10 @@  static void bdw_setup_private_ppat(struct intel_ppat *ppat)
 
 	__alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);      /* for normal objects, no eLLC */
 	__alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);  /* for something pointing to ptes? */
-	__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);  /* for scanout with eLLC */
+	if (INTEL_GEN(ppat->i915) >= 9)
+		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WB | GEN8_PPAT_ELLC_OVERRIDE); /* for scanout with eLLC */
+	else
+		__alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for scanout with eLLC */
 	__alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);                      /* Uncached objects, mostly for scanout */
 	__alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
 	__alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f597f35b109b..47adc7268867 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -139,7 +139,7 @@  typedef u64 gen8_ppgtt_pml4e_t;
 #define PPAT_UNCACHED			(_PAGE_PWT | _PAGE_PCD)
 #define PPAT_CACHED_PDE			0 /* WB LLC */
 #define PPAT_CACHED			_PAGE_PAT /* WB LLCeLLC */
-#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT eLLC */
+#define PPAT_DISPLAY_ELLC		_PAGE_PCD /* WT LLCeLLC (HSW/BDW) or WB eLLC (SKL+) */
 
 #define CHV_PPAT_SNOOP			(1<<6)
 #define GEN8_PPAT_AGE(x)		((x)<<4)
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 274ba78500c0..d984ccff94ef 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -115,7 +115,7 @@  struct drm_i915_mocs_table {
 		   LE_1_UC | LE_TC_2_LLC_ELLC, \
 		   L3_1_UC), \
 	MOCS_ENTRY(I915_MOCS_PTE, \
-		   LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
+		   LE_0_PAGETABLE | LE_TC_0_PAGETABLE | LE_LRUM(3), \
 		   L3_3_WB)
 
 static const struct drm_i915_mocs_entry skylake_mocs_table[] = {