Commit dfc53c5e authored by Michel Thierry's avatar Michel Thierry Committed by Jani Nikula

drm/i915: Consider HW CSB write pointer before resetting the sw read pointer

A previous commit resets the Context Status Buffer (CSB) read pointer in
ring init
    commit c0a03a2e ("drm/i915: Reset CSB read pointer in ring init")

This is generally correct, but this pointer is not reset after
suspend/resume in some platforms (cht). In this case, the driver should
read the register value instead of resetting the sw read counter to 0.
Otherwise we process old events, leading to unwanted pre-emptions or
something worse.

But in other platforms (bdw) and also during GPU reset or power up, the
CSBWP is reset to 0x7 (an invalid number), and in this case the read
pointer should be set to 5 (the interrupt code will increment this
counter one more time, and will start reading from CSB[0]).

v2: When the CSB registers are reset, the read pointer needs to be set
to 5, otherwise the first write (CSB[0]) won't be read (Mika).
Replace magic numbers with GEN8_CSB_ENTRIES (6) and GEN8_CSB_PTR_MASK
(0x07).

Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: stable@vger.kernel.org # v4.0+
Signed-off-by: default avatarLei Shen <lei.shen@intel.com>
Signed-off-by: default avatarDeepak S <deepak.s@intel.com>
Signed-off-by: default avatarMichel Thierry <michel.thierry@intel.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent bc5f2ab1
...@@ -484,18 +484,18 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) ...@@ -484,18 +484,18 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring)); status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
read_pointer = ring->next_context_status_buffer; read_pointer = ring->next_context_status_buffer;
write_pointer = status_pointer & 0x07; write_pointer = status_pointer & GEN8_CSB_PTR_MASK;
if (read_pointer > write_pointer) if (read_pointer > write_pointer)
write_pointer += 6; write_pointer += GEN8_CSB_ENTRIES;
spin_lock(&ring->execlist_lock); spin_lock(&ring->execlist_lock);
while (read_pointer < write_pointer) { while (read_pointer < write_pointer) {
read_pointer++; read_pointer++;
status = I915_READ(RING_CONTEXT_STATUS_BUF(ring) + status = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
(read_pointer % 6) * 8); (read_pointer % GEN8_CSB_ENTRIES) * 8);
status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) + status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
(read_pointer % 6) * 8 + 4); (read_pointer % GEN8_CSB_ENTRIES) * 8 + 4);
if (status & GEN8_CTX_STATUS_IDLE_ACTIVE) if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
continue; continue;
...@@ -521,10 +521,12 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) ...@@ -521,10 +521,12 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
spin_unlock(&ring->execlist_lock); spin_unlock(&ring->execlist_lock);
WARN(submit_contexts > 2, "More than two context complete events?\n"); WARN(submit_contexts > 2, "More than two context complete events?\n");
ring->next_context_status_buffer = write_pointer % 6; ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
I915_WRITE(RING_CONTEXT_STATUS_PTR(ring), I915_WRITE(RING_CONTEXT_STATUS_PTR(ring),
_MASKED_FIELD(0x07 << 8, ((u32)ring->next_context_status_buffer & 0x07) << 8)); _MASKED_FIELD(GEN8_CSB_PTR_MASK << 8,
((u32)ring->next_context_status_buffer &
GEN8_CSB_PTR_MASK) << 8));
} }
static int execlists_context_queue(struct drm_i915_gem_request *request) static int execlists_context_queue(struct drm_i915_gem_request *request)
...@@ -1422,6 +1424,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring) ...@@ -1422,6 +1424,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
{ {
struct drm_device *dev = ring->dev; struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
u8 next_context_status_buffer_hw;
I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask)); I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff); I915_WRITE(RING_HWSTAM(ring->mmio_base), 0xffffffff);
...@@ -1436,7 +1439,29 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring) ...@@ -1436,7 +1439,29 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
_MASKED_BIT_DISABLE(GFX_REPLAY_MODE) | _MASKED_BIT_DISABLE(GFX_REPLAY_MODE) |
_MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE)); _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
POSTING_READ(RING_MODE_GEN7(ring)); POSTING_READ(RING_MODE_GEN7(ring));
ring->next_context_status_buffer = 0;
/*
* Instead of resetting the Context Status Buffer (CSB) read pointer to
* zero, we need to read the write pointer from hardware and use its
* value because "this register is power context save restored".
* Effectively, these states have been observed:
*
* | Suspend-to-idle (freeze) | Suspend-to-RAM (mem) |
* BDW | CSB regs not reset | CSB regs reset |
* CHT | CSB regs not reset | CSB regs not reset |
*/
next_context_status_buffer_hw = (I915_READ(RING_CONTEXT_STATUS_PTR(ring))
& GEN8_CSB_PTR_MASK);
/*
* When the CSB registers are reset (also after power-up / gpu reset),
* CSB write pointer is set to all 1's, which is not valid, use '5' in
* this special case, so the first element read is CSB[0].
*/
if (next_context_status_buffer_hw == GEN8_CSB_PTR_MASK)
next_context_status_buffer_hw = (GEN8_CSB_ENTRIES - 1);
ring->next_context_status_buffer = next_context_status_buffer_hw;
DRM_DEBUG_DRIVER("Execlists enabled for %s\n", ring->name); DRM_DEBUG_DRIVER("Execlists enabled for %s\n", ring->name);
memset(&ring->hangcheck, 0, sizeof(ring->hangcheck)); memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
......
...@@ -25,6 +25,8 @@ ...@@ -25,6 +25,8 @@
#define _INTEL_LRC_H_ #define _INTEL_LRC_H_
#define GEN8_LR_CONTEXT_ALIGN 4096 #define GEN8_LR_CONTEXT_ALIGN 4096
#define GEN8_CSB_ENTRIES 6
#define GEN8_CSB_PTR_MASK 0x07
/* Execlists regs */ /* Execlists regs */
#define RING_ELSP(ring) ((ring)->mmio_base+0x230) #define RING_ELSP(ring) ((ring)->mmio_base+0x230)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment