On Fri, 11 Mar 2016, Chris Wilson wrote:
On Fri, Mar 11, 2016 at 02:00:22PM +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.
v2: posted correct version to the mailing list.
v3: moved programming to within engine->init_hw() (Chris Wilson)
Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 3 +
drivers/gpu/drm/i915/intel_lrc.c | 4 ++
drivers/gpu/drm/i915/intel_mocs.c | 120 +++++++++++++++++++++++++++++++++-----
drivers/gpu/drm/i915/intel_mocs.h | 2 +
4 files changed, 114 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b854af2..3e18cbd 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;
}
+ program_mocs_l3cc_table(dev);
intel_mocs_init_l3cc_table(dev_priv);
Changed.
+
/* 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_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 27c9ee3..2ef7a161 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1603,6 +1603,8 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
+ program_mocs_control_table(ring);
ret = intel_mocs_init_engine(ring);
Check and propagate the errors!
Ok.
+
return 0;
}
@@ -2151,6 +2153,8 @@ static int logical_render_ring_init(struct drm_device *dev)
ret);
}
+ program_mocs_control_table(ring);
Due to the splitting inside intel_lrc, it actually makes sense to do a
ret = engine->init_hw(engine) call here. But for the time being, adding
the setup here is redundant.
This call is not required and has been removed.
/**
+ * program_mocs_control_table() - emit the mocs control table
+ * @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.
+ */
+int program_mocs_control_table(struct intel_engine_cs *ring)
+{
+ struct drm_device *dev = ring->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_i915_mocs_table table;
+ unsigned int index;
+
+ if (get_mocs_settings(dev, &table)) {
if (!get_mocs_setttings())
return 0;
And then we can lose one level of indentation.
Ok. I like the indentation I find it easier to read. But will change.
+ if (WARN_ON(table.size > GEN9_NUM_MOCS_ENTRIES))
+ return -ENODEV;
+
+ for (index = 0; index < table.size; index++) {
+ I915_WRITE(mocs_register(ring->id, index),
+ table.table[index].control_value);
Whitespace makes this easier to read. Please align continuation lines
with the opening bracket.
Ok.
+ }
+
+ /*
+ * 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->id, index),
+ table.table[0].control_value);
Especially here.
done.
+ }
+ }
+
+ 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.
@@ -210,7 +252,8 @@ static int emit_mocs_control_table(struct drm_i915_gem_request *req,
MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES));
for (index = 0; index < table->size; index++) {
- intel_logical_ring_emit_reg(ringbuf, mocs_register(ring, index));
+ intel_logical_ring_emit_reg(ringbuf,
+ mocs_register(ring, index));
intel_logical_ring_emit(ringbuf,
table->table[index].control_value);
}
@@ -224,8 +267,10 @@ static int emit_mocs_control_table(struct drm_i915_gem_request *req,
* that value to all the used entries.
*/
for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
- intel_logical_ring_emit_reg(ringbuf, mocs_register(ring, index));
- intel_logical_ring_emit(ringbuf, table->table[0].control_value);
+ intel_logical_ring_emit_reg(ringbuf,
+ mocs_register(ring, index));
+ intel_logical_ring_emit(ringbuf,
+ table->table[0].control_value);
}
I'll pretend I never saw these spurious chunks.
removed (well un-removed)
intel_logical_ring_emit(ringbuf, MI_NOOP);
@@ -302,6 +347,57 @@ 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.
+ */
+void program_mocs_l3cc_table(struct drm_device *dev)
+{
+ unsigned int count;
+ unsigned int i;
+ u32 value;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_i915_mocs_table table;
+
+ if (get_mocs_settings(dev, &table)) {
Indentation, whitespace, passing around dev but using dev_priv
Get_mocs_settings() uses dev.
Done the rest.
+ u32 filler = (table.table[0].l3cc_value & 0xffff) |
+ ((table.table[0].l3cc_value & 0xffff) << 16);
l3cc_value is 16bit so the masking is garbage.
static inline u32 l3cc_combine(u16 lower, u16 upper)
{
return lower | upper << 16;
}
for (i = 0; i < table.size/2; i++)
I915_WRITE(GEN9_LNCFCMOCS(i),
l3cc_combine(table.table[2*i].l3cc_value,
table.table[2*i+1].l3cc_value));
if (table.size & 1) {
I915_WRITE(GEN9_LNCFCMOCS(i),
l3cc_combine(table.table[2*i].l3cc_value,
table.table[0].l3cc_value));
i++;
}
+ /*
+ * 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),
l3cc_combine(table.table[0].l3cc_value,
table.table[0].l3cc_value));
is easier on the eyes.
Have re-worked that function and the emit, might as well make them the
same. it does look better.
Peter.
-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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
