@@ -551,6 +551,8 @@ static bool stop_ring(struct intel_engine_cs *ring)
I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
}
+ WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
+
return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0;
}
@@ -2260,17 +2262,11 @@ static int intel_init_ring_buffer(struct drm_device *dev,
void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv;
-
if (!intel_ring_initialized(ring))
return;
- dev_priv = to_i915(ring->dev);
-
if (ring->buffer) {
intel_stop_ring_buffer(ring);
- WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
-
intel_unpin_ringbuffer_obj(ring->buffer);
intel_ringbuffer_free(ring->buffer);
ring->buffer = NULL;
intel_cleanup_ring_buffer() contains one low-level register access, which is not really appropriate for its level of abstraction. It calls intel_stop_ring_buffer() which then calls stop_ring() -- which is the level that deals with h/w registers -- then reads a GEN-specific register to see whether the ring is actually now idle. It only prints a WARNING, though, and doesn't actually refrain from continuing even if the test fails! So, let's move the register-level check and WARNING down into the low-level function that's already doing register access. If we wanted to, we could pass a pass/fail status back, but since the high-level code continues anyway, there's no reason to at present. As a bonus, apart from fixing the lavering violation, moving the code lets us eliminate the implicitly-used local 'dev_priv' from the caller. Signed-off-by: Dave Gordon <david.s.gordon@intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)