diff mbox

[v2] drm/i915/mocs: Program MOCS for all engines on init

Message ID 1457641378-20007-1-git-send-email-peter.antoine@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Antoine March 10, 2016, 8:22 p.m. UTC
Allow for the MOCS to be programmed for all engines.
Currently we program the MOCS when the first render batch
goes through. This works on most platforms but fails on
platforms that do not run a render batch early,
i.e. headless servers. The patch now programs all initialised engines
on init and the RCS is programmed again within the initial batch. This
is done for predictable consistency with regards to the hardware
context.

Hardware context loading sets the values of the MOCS for RCS
and L3CC. Programming them from within the batch makes sure that
the render context is valid, no matter what the previous state of
the saved-context was.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c   |   3 +
 drivers/gpu/drm/i915/intel_mocs.c | 138 ++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_mocs.h |   1 +
 3 files changed, 130 insertions(+), 12 deletions(-)

Comments

Chris Wilson March 10, 2016, 8:47 p.m. UTC | #1
On Thu, Mar 10, 2016 at 08:22:58PM +0000, Peter Antoine wrote:
> Allow for the MOCS to be programmed for all engines.
> Currently we program the MOCS when the first render batch
> goes through. This works on most platforms but fails on
> platforms that do not run a render batch early,
> i.e. headless servers. The patch now programs all initialised engines
> on init and the RCS is programmed again within the initial batch. This
> is done for predictable consistency with regards to the hardware
> context.
> 
> Hardware context loading sets the values of the MOCS for RCS
> and L3CC. Programming them from within the batch makes sure that
> the render context is valid, no matter what the previous state of
> the saved-context was.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c   |   3 +
>  drivers/gpu/drm/i915/intel_mocs.c | 138 ++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_mocs.h |   1 +
>  3 files changed, 130 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b854af2..05c9fcb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -32,6 +32,7 @@
>  #include "i915_vgpu.h"
>  #include "i915_trace.h"
>  #include "intel_drv.h"
> +#include "intel_mocs.h"
>  #include <linux/shmem_fs.h>
>  #include <linux/slab.h>
>  #include <linux/swap.h>
> @@ -4882,6 +4883,8 @@ i915_gem_init_hw(struct drm_device *dev)
>  			goto out;
>  	}
>  
> +	intel_program_mocs_engines(dev);

Registers to be written when enabling the engine are called from
engine->init_hw() in the line above. Please add this state there, which
can then be specialised via vfunc if so desired.
-Chris
Peter Antoine March 11, 2016, 2:02 p.m. UTC | #2
Hi Chris,

Just posted patch that moves to the engine->init_hw().

Have moved the L3CC registers programming out of the function and left in 
the same place as the original function was. These registers are shared 
with all engines.

Peter.

On Thu, 10 Mar 2016, Chris Wilson wrote:

> On Thu, Mar 10, 2016 at 08:22:58PM +0000, Peter Antoine wrote:
>> Allow for the MOCS to be programmed for all engines.
>> Currently we program the MOCS when the first render batch
>> goes through. This works on most platforms but fails on
>> platforms that do not run a render batch early,
>> i.e. headless servers. The patch now programs all initialised engines
>> on init and the RCS is programmed again within the initial batch. This
>> is done for predictable consistency with regards to the hardware
>> context.
>>
>> Hardware context loading sets the values of the MOCS for RCS
>> and L3CC. Programming them from within the batch makes sure that
>> the render context is valid, no matter what the previous state of
>> the saved-context was.
>>
>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_gem.c   |   3 +
>>  drivers/gpu/drm/i915/intel_mocs.c | 138 ++++++++++++++++++++++++++++++++++----
>>  drivers/gpu/drm/i915/intel_mocs.h |   1 +
>>  3 files changed, 130 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index b854af2..05c9fcb 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -32,6 +32,7 @@
>>  #include "i915_vgpu.h"
>>  #include "i915_trace.h"
>>  #include "intel_drv.h"
>> +#include "intel_mocs.h"
>>  #include <linux/shmem_fs.h>
>>  #include <linux/slab.h>
>>  #include <linux/swap.h>
>> @@ -4882,6 +4883,8 @@ i915_gem_init_hw(struct drm_device *dev)
>>  			goto out;
>>  	}
>>
>> +	intel_program_mocs_engines(dev);
>
> Registers to be written when enabling the engine are called from
> engine->init_hw() in the line above. Please add this state there, which
> can then be specialised via vfunc if so desired.
> -Chris
>
>

--
    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
Peter Antoine March 11, 2016, 2:05 p.m. UTC | #3
Just seen a documentation error in program_mocs_l3cc_table() still 
documents a parameter that has been removed. Will fix later.

Peter.

On Fri, 11 Mar 2016, Peter Antoine wrote:

> Hi Chris,
>
> Just posted patch that moves to the engine->init_hw().
>
> Have moved the L3CC registers programming out of the function and left in the 
> same place as the original function was. These registers are shared with all 
> engines.
>
> Peter.
>
> On Thu, 10 Mar 2016, Chris Wilson wrote:
>
>> On Thu, Mar 10, 2016 at 08:22:58PM +0000, Peter Antoine wrote:
>>> Allow for the MOCS to be programmed for all engines.
>>> Currently we program the MOCS when the first render batch
>>> goes through. This works on most platforms but fails on
>>> platforms that do not run a render batch early,
>>> i.e. headless servers. The patch now programs all initialised engines
>>> on init and the RCS is programmed again within the initial batch. This
>>> is done for predictable consistency with regards to the hardware
>>> context.
>>> 
>>> Hardware context loading sets the values of the MOCS for RCS
>>> and L3CC. Programming them from within the batch makes sure that
>>> the render context is valid, no matter what the previous state of
>>> the saved-context was.
>>> 
>>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_gem.c   |   3 +
>>>  drivers/gpu/drm/i915/intel_mocs.c | 138 
>>> ++++++++++++++++++++++++++++++++++----
>>>  drivers/gpu/drm/i915/intel_mocs.h |   1 +
>>>  3 files changed, 130 insertions(+), 12 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>>> b/drivers/gpu/drm/i915/i915_gem.c
>>> index b854af2..05c9fcb 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -32,6 +32,7 @@
>>>  #include "i915_vgpu.h"
>>>  #include "i915_trace.h"
>>>  #include "intel_drv.h"
>>> +#include "intel_mocs.h"
>>>  #include <linux/shmem_fs.h>
>>>  #include <linux/slab.h>
>>>  #include <linux/swap.h>
>>> @@ -4882,6 +4883,8 @@ i915_gem_init_hw(struct drm_device *dev)
>>>  			goto out;
>>>  	}
>>> 
>>> +	intel_program_mocs_engines(dev);
>> 
>> Registers to be written when enabling the engine are called from
>> engine->init_hw() in the line above. Please add this state there, which
>> can then be specialised via vfunc if so desired.
>> -Chris
>> 
>> 
>
> --
>   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
>

--
    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
Peter Antoine March 11, 2016, 3:02 p.m. UTC | #4
And the program_mocs_control_table(ring) in logical_render_ring_init() is 
not required and can be removed.

Will update that as well after review.

Peter.

On Fri, 11 Mar 2016, Peter Antoine wrote:

> Just seen a documentation error in program_mocs_l3cc_table() still documents 
> a parameter that has been removed. Will fix later.
>
> Peter.
>
> On Fri, 11 Mar 2016, Peter Antoine wrote:
>
>> Hi Chris,
>> 
>> Just posted patch that moves to the engine->init_hw().
>> 
>> Have moved the L3CC registers programming out of the function and left in 
>> the same place as the original function was. These registers are shared 
>> with all engines.
>> 
>> Peter.
>> 
>> On Thu, 10 Mar 2016, Chris Wilson wrote:
>> 
>>> On Thu, Mar 10, 2016 at 08:22:58PM +0000, Peter Antoine wrote:
>>>> Allow for the MOCS to be programmed for all engines.
>>>> Currently we program the MOCS when the first render batch
>>>> goes through. This works on most platforms but fails on
>>>> platforms that do not run a render batch early,
>>>> i.e. headless servers. The patch now programs all initialised engines
>>>> on init and the RCS is programmed again within the initial batch. This
>>>> is done for predictable consistency with regards to the hardware
>>>> context.
>>>> 
>>>> Hardware context loading sets the values of the MOCS for RCS
>>>> and L3CC. Programming them from within the batch makes sure that
>>>> the render context is valid, no matter what the previous state of
>>>> the saved-context was.
>>>> 
>>>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/i915_gem.c   |   3 +
>>>>  drivers/gpu/drm/i915/intel_mocs.c | 138 
>>>> ++++++++++++++++++++++++++++++++++----
>>>>  drivers/gpu/drm/i915/intel_mocs.h |   1 +
>>>>  3 files changed, 130 insertions(+), 12 deletions(-)
>>>> 
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>>>> b/drivers/gpu/drm/i915/i915_gem.c
>>>> index b854af2..05c9fcb 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>>> @@ -32,6 +32,7 @@
>>>>  #include "i915_vgpu.h"
>>>>  #include "i915_trace.h"
>>>>  #include "intel_drv.h"
>>>> +#include "intel_mocs.h"
>>>>  #include <linux/shmem_fs.h>
>>>>  #include <linux/slab.h>
>>>>  #include <linux/swap.h>
>>>> @@ -4882,6 +4883,8 @@ i915_gem_init_hw(struct drm_device *dev)
>>>>  			goto out;
>>>>  	}
>>>> 
>>>> +	intel_program_mocs_engines(dev);
>>> 
>>> Registers to be written when enabling the engine are called from
>>> engine->init_hw() in the line above. Please add this state there, which
>>> can then be specialised via vfunc if so desired.
>>> -Chris
>>> 
>>> 
>> 
>> --
>>   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
>> 
>
> --
>   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
>

--
    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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b854af2..05c9fcb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -32,6 +32,7 @@ 
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include "intel_mocs.h"
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
@@ -4882,6 +4883,8 @@  i915_gem_init_hw(struct drm_device *dev)
 			goto out;
 	}
 
+	intel_program_mocs_engines(dev);
+
 	/* We can't enable contexts until all firmware is loaded */
 	if (HAS_GUC_UCODE(dev)) {
 		ret = intel_guc_ucode_load(dev);
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index fed7bea..cd01682 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -128,9 +128,9 @@  static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
 
 /**
  * get_mocs_settings()
- * @dev:        DRM device.
+ * @dev:	DRM device.
  * @table:      Output table that will be made to point at appropriate
- *              MOCS values for the device.
+ *	      MOCS values for the device.
  *
  * This function will return the values of the MOCS table that needs to
  * be programmed for the platform. It will return the values that need
@@ -179,6 +179,48 @@  static i915_reg_t mocs_register(enum intel_ring_id ring, int index)
 }
 
 /**
+ * program_mocs_control_table() - emit the mocs control table
+ * @req:	Request to set up the MOCS table for.
+ * @table:	The values to program into the control regs.
+ * @ring:	The engine for whom to emit the registers.
+ *
+ * This function simply emits a MI_LOAD_REGISTER_IMM command for the
+ * given table starting at the given address.
+ *
+ * Return: 0 on success, otherwise the error status.
+ */
+static int program_mocs_control_table(struct drm_device *dev,
+				   const struct drm_i915_mocs_table *table,
+				   enum intel_ring_id ring)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned int index;
+
+	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
+		return -ENODEV;
+
+	for (index = 0; index < table->size; index++) {
+		I915_WRITE(mocs_register(ring, index),
+			table->table[index].control_value);
+	}
+
+	/*
+	 * Ok, now set the unused entries to uncached. These entries
+	 * are officially undefined and no contract for the contents
+	 * and settings is given for these entries.
+	 *
+	 * Entry 0 in the table is uncached - so we are just writing
+	 * that value to all the used entries.
+	 */
+	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
+		I915_WRITE(mocs_register(ring, index),
+					table->table[0].control_value);
+	}
+
+	return 0;
+}
+
+/**
  * emit_mocs_control_table() - emit the mocs control table
  * @req:	Request to set up the MOCS table for.
  * @table:	The values to program into the control regs.
@@ -302,6 +344,84 @@  static int emit_mocs_l3cc_table(struct drm_i915_gem_request *req,
 }
 
 /**
+ * program_mocs_l3cc_table() - program the mocs control table
+ * @dev:      The the device to be programmed.
+ * @table:    The values to program into the control regs.
+ *
+ * This function simply programs the mocs registers for the given table
+ * starting at the given address. This register set is  programmed in pairs.
+ *
+ * These registers may get programmed more than once, it is simpler to
+ * re-program 32 registers than maintain the state of when they were programmed.
+ * We are always reprogramming with the same values and this only on context
+ * start.
+ *
+ * Return: Nothing.
+ */
+static void program_mocs_l3cc_table(struct drm_device *dev,
+			      struct drm_i915_mocs_table *table)
+{
+	unsigned int count;
+	unsigned int i;
+	u32 value;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 filler = (table->table[0].l3cc_value & 0xffff) |
+			((table->table[0].l3cc_value & 0xffff) << 16);
+
+	for (i = 0, count = 0; i < table->size / 2; i++, count += 2) {
+		value = (table->table[count].l3cc_value & 0xffff) |
+			((table->table[count + 1].l3cc_value & 0xffff) << 16);
+
+		I915_WRITE(GEN9_LNCFCMOCS(i), value);
+	}
+
+	if (table->size & 0x01) {
+		/* Odd table size - 1 left over */
+		value = (table->table[count].l3cc_value & 0xffff) |
+			((table->table[0].l3cc_value & 0xffff) << 16);
+	} else
+		value = filler;
+
+	/*
+	 * Now set the rest of the table to uncached - use entry 0 as this
+	 * will be uncached. Leave the last pair as initialised as they are
+	 * reserved by the hardware.
+	 */
+	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++) {
+		I915_WRITE(GEN9_LNCFCMOCS(i), value);
+		value = filler;
+	}
+}
+
+/*
+ * intel_program_mocs_engines() - program all the MOCS register.
+ *
+ * This function will program the MOCS registers with the correct values.
+ * For the initialised engines, this means that some of the MOCS hw registers
+ * will be left at the hardware defaults.
+ *
+ * Return: Nothing.
+ */
+void intel_program_mocs_engines(struct drm_device *dev)
+{
+	struct drm_i915_mocs_table t;
+
+	if (get_mocs_settings(dev, &t)) {
+		struct drm_i915_private *dev_priv = dev->dev_private;
+		struct intel_engine_cs *ring;
+		enum intel_ring_id ring_id;
+
+		/* Program the control registers */
+		for_each_ring(ring, dev_priv, ring_id) {
+			program_mocs_control_table(dev, &t, ring_id);
+		}
+
+		/* Now program the l3cc registers */
+		program_mocs_l3cc_table(dev, &t);
+	}
+}
+
+/**
  * intel_rcs_context_init_mocs() - program the MOCS register.
  * @req:	Request to set up the MOCS tables for.
  *
@@ -323,16 +443,10 @@  int intel_rcs_context_init_mocs(struct drm_i915_gem_request *req)
 	int ret;
 
 	if (get_mocs_settings(req->ring->dev, &t)) {
-		struct drm_i915_private *dev_priv = req->i915;
-		struct intel_engine_cs *ring;
-		enum intel_ring_id ring_id;
-
-		/* Program the control registers */
-		for_each_ring(ring, dev_priv, ring_id) {
-			ret = emit_mocs_control_table(req, &t, ring_id);
-			if (ret)
-				return ret;
-		}
+		/* Program the RCS control registers */
+		ret = emit_mocs_control_table(req, &t, RCS);
+		if (ret)
+			return ret;
 
 		/* Now program the l3cc registers */
 		ret = emit_mocs_l3cc_table(req, &t);
diff --git a/drivers/gpu/drm/i915/intel_mocs.h b/drivers/gpu/drm/i915/intel_mocs.h
index 76e45b1..d20e16d 100644
--- a/drivers/gpu/drm/i915/intel_mocs.h
+++ b/drivers/gpu/drm/i915/intel_mocs.h
@@ -53,5 +53,6 @@ 
 #include "i915_drv.h"
 
 int intel_rcs_context_init_mocs(struct drm_i915_gem_request *req);
+void intel_program_mocs_engines(struct drm_device *dev);
 
 #endif