| Message ID | 1454095171-22475-2-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 29, 2016 at 07:19:26PM +0000, Dave Gordon wrote: > 1. Fix intel_cleanup_ring_buffer() to handle the error cleanup > case where the ringbuffer has been allocated but map-and-pin > failed. Unpin it iff it's previously been mapped-and-pinned. > > 2. Fix the error path in intel_init_ring_buffer(), which already > called intel_destroy_ringbuffer_obj(), but failed to free the > actual ringbuffer structure. Calling intel_ringbuffer_free() > instead does both in one go. > > 3. With the above change, intel_destroy_ringbuffer_obj() is only > called in one place (intel_ringbuffer_free()), so flatten it > into that function. > > 4. move low-level register accesses from intel_cleanup_ring_buffer() > (which calls intel_stop_ring_buffer(ring) which calls stop_ring()) > down into stop_ring() itself), which is already doing low-level > register accesses. Then, intel_cleanup_ring_buffer() no longer > needs 'dev_priv'. > > v3: > Make intel_unpin_ringbuffer_obj() return early if ringbuffer is > not mapped, simplifying matters for the caller. [Chris Wilson, > Daniel Vetter] > Don't bother to clear a pointer in an object about to be freed. > [Chris Wilson] > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 49 ++++++++++++++++----------------- > 1 file changed, 23 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 6f5b511..db02893 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -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); This warn was guarding the release of the backing pages, to verify that we had stopped the rings first. > + > return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0; > } > > @@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct intel_engine_cs *ring) > > void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > { > + if (!ringbuf->virtual_start) > + return; > + > if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) > vunmap(ringbuf->virtual_start); > else > @@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, > return 0; > } > > -static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > -{ > - drm_gem_object_unreference(&ringbuf->obj->base); > - ringbuf->obj = NULL; > -} > - > static int intel_alloc_ringbuffer_obj(struct drm_device *dev, > struct intel_ringbuffer *ringbuf) > { > @@ -2200,11 +2199,13 @@ struct intel_ringbuffer * > } > > void > -intel_ringbuffer_free(struct intel_ringbuffer *ring) > +intel_ringbuffer_free(struct intel_ringbuffer *ringbuf) > { > - intel_destroy_ringbuffer_obj(ring); > - list_del(&ring->link); > - kfree(ring); > + if (ringbuf->obj) > + drm_gem_object_unreference(&ringbuf->obj->base); No, let's not add duplicate code. If you worry, replace to_intel_bo() with static inline struct drm_i915_gem_object * to_intel_bo(struct drm_gem_object *gem_obj) { BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base)); return container_of(gem_obj, struct drm_i915_gem_object, base); } and we can begin erradicating the half-baked checks (some places assume, others do no-op conversions). Hindsight. > + > + list_del(&ringbuf->link); > + kfree(ringbuf); > } > > static int intel_init_ring_buffer(struct drm_device *dev, > @@ -2232,6 +2233,13 @@ static int intel_init_ring_buffer(struct drm_device *dev, > } > ring->buffer = ringbuf; > > + ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); > + if (ret) { > + DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", > + ring->name, ret); > + goto error; > + } > + > if (I915_NEED_GFX_HWS(dev)) { > ret = init_status_page(ring); > if (ret) > @@ -2243,14 +2251,6 @@ static int intel_init_ring_buffer(struct drm_device *dev, > goto error; > } > > - ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); > - if (ret) { > - DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", > - ring->name, ret); > - intel_destroy_ringbuffer_obj(ringbuf); > - goto error; > - } > - > ret = i915_cmd_parser_init_ring(ring); > if (ret) > goto error; > @@ -2264,19 +2264,16 @@ 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; > + struct intel_ringbuffer *ringbuf; > > if (!intel_ring_initialized(ring)) > return; > > - dev_priv = to_i915(ring->dev); > - > - if (ring->buffer) { > + ringbuf = ring->buffer; > + if (ringbuf) { > 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); > + intel_unpin_ringbuffer_obj(ringbuf); > + intel_ringbuffer_free(ringbuf); > ring->buffer = NULL; Conversions to a horrible name for no reason. ringbuf was a mistake, please no more -Chris
On 30/01/16 10:50, Chris Wilson wrote: > On Fri, Jan 29, 2016 at 07:19:26PM +0000, Dave Gordon wrote: >> 1. Fix intel_cleanup_ring_buffer() to handle the error cleanup >> case where the ringbuffer has been allocated but map-and-pin >> failed. Unpin it iff it's previously been mapped-and-pinned. >> >> 2. Fix the error path in intel_init_ring_buffer(), which already >> called intel_destroy_ringbuffer_obj(), but failed to free the >> actual ringbuffer structure. Calling intel_ringbuffer_free() >> instead does both in one go. >> >> 3. With the above change, intel_destroy_ringbuffer_obj() is only >> called in one place (intel_ringbuffer_free()), so flatten it >> into that function. >> >> 4. move low-level register accesses from intel_cleanup_ring_buffer() >> (which calls intel_stop_ring_buffer(ring) which calls stop_ring()) >> down into stop_ring() itself), which is already doing low-level >> register accesses. Then, intel_cleanup_ring_buffer() no longer >> needs 'dev_priv'. >> >> v3: >> Make intel_unpin_ringbuffer_obj() return early if ringbuffer is >> not mapped, simplifying matters for the caller. [Chris Wilson, >> Daniel Vetter] >> Don't bother to clear a pointer in an object about to be freed. >> [Chris Wilson] >> >> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 49 ++++++++++++++++----------------- >> 1 file changed, 23 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 6f5b511..db02893 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -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); > > This warn was guarding the release of the backing pages, to verify that > we had stopped the rings first. It's still effectively at the same point in the execution sequence, just not intruding into a function that has no business peeking at registers. It's now at the end of the called subfunctions, rather than just after the call at the top-level, where it was a layering violation. Or we could put it in a separate function, and say WARN_ON(!ring_is_idle(ring)); >> + >> return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0; >> } >> >> @@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct intel_engine_cs *ring) >> >> void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) >> { >> + if (!ringbuf->virtual_start) >> + return; >> + >> if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) >> vunmap(ringbuf->virtual_start); >> else >> @@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, >> return 0; >> } >> >> -static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf) >> -{ >> - drm_gem_object_unreference(&ringbuf->obj->base); >> - ringbuf->obj = NULL; >> -} >> - >> static int intel_alloc_ringbuffer_obj(struct drm_device *dev, >> struct intel_ringbuffer *ringbuf) >> { >> @@ -2200,11 +2199,13 @@ struct intel_ringbuffer * >> } >> >> void >> -intel_ringbuffer_free(struct intel_ringbuffer *ring) >> +intel_ringbuffer_free(struct intel_ringbuffer *ringbuf) >> { >> - intel_destroy_ringbuffer_obj(ring); >> - list_del(&ring->link); >> - kfree(ring); >> + if (ringbuf->obj) >> + drm_gem_object_unreference(&ringbuf->obj->base); > > No, let's not add duplicate code. If you worry, > replace to_intel_bo() with I suspect the above doesn't actually duplicate the test, 'cos drm_gem_object_unreference() is inline, and common subexpression elimination should determine that ringbuf->obj and &ringbuf->obj->base are the same thing and then the compiler can collapse the explicit test here with the one inside drm_gem_object_unreference(). > static inline struct drm_i915_gem_object * > to_intel_bo(struct drm_gem_object *gem_obj) > { > BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base)); > return container_of(gem_obj, struct drm_i915_gem_object, base); > } I would quite like to get rid of the "&(...)->base" that intrudes everywhere - we should be able to just (un)reference the object we're working with, without caring that it's a subclass of some other type. So we'd want the opposite function as well, for passing i915 objects to drm functions (as here), maybe call it to_drm_obj()? static inline struct drm_gem_object * to_drm_obj(struct drm_i915_gem_object *i915_obj) { BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base)); return &i915_obj->base; } > and we can begin erradicating the half-baked checks (some places assume, > others do no-op conversions). Hindsight. Or we could make drm_gem_object_unreference() accept a (struct drm_i915_gem_object *) as an alternative to a (struct drm_gem_object *)? >> + >> + list_del(&ringbuf->link); >> + kfree(ringbuf); >> } >> >> static int intel_init_ring_buffer(struct drm_device *dev, >> @@ -2232,6 +2233,13 @@ static int intel_init_ring_buffer(struct drm_device *dev, >> } >> ring->buffer = ringbuf; >> >> + ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); >> + if (ret) { >> + DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", >> + ring->name, ret); >> + goto error; >> + } >> + >> if (I915_NEED_GFX_HWS(dev)) { >> ret = init_status_page(ring); >> if (ret) >> @@ -2243,14 +2251,6 @@ static int intel_init_ring_buffer(struct drm_device *dev, >> goto error; >> } >> >> - ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); >> - if (ret) { >> - DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", >> - ring->name, ret); >> - intel_destroy_ringbuffer_obj(ringbuf); >> - goto error; >> - } >> - >> ret = i915_cmd_parser_init_ring(ring); >> if (ret) >> goto error; >> @@ -2264,19 +2264,16 @@ 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; >> + struct intel_ringbuffer *ringbuf; >> >> if (!intel_ring_initialized(ring)) >> return; >> >> - dev_priv = to_i915(ring->dev); >> - >> - if (ring->buffer) { >> + ringbuf = ring->buffer; >> + if (ringbuf) { >> 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); >> + intel_unpin_ringbuffer_obj(ringbuf); >> + intel_ringbuffer_free(ringbuf); >> ring->buffer = NULL; > > Conversions to a horrible name for no reason. ringbuf was a mistake, > please no more > -Chris 'ringbuf' is a perfectly good name for a (struct intel_ringbuffer *). What we *can't* call it is 'ring', because that name is used here and everywhere else for a (struct intel_engine_cs *). Would you prefer 'rbp' (*r*ing*b*uffer*p*ointer) ? Other suggestions considered, but NOT 'ring' Or are you simply objecting to having a local variable at all? It's generally a good idea to take a local copy of any structure member that's going to be referenced more than once, because it lets the compiler know that this function owns the value and it doesn't have to be fetched from memory more than once, even after some function has been called that could in theory have modified the structure member. .Dave.
On Mon, Feb 01, 2016 at 09:38:02AM +0000, Dave Gordon wrote: > On 30/01/16 10:50, Chris Wilson wrote: > >On Fri, Jan 29, 2016 at 07:19:26PM +0000, Dave Gordon wrote: > >>1. Fix intel_cleanup_ring_buffer() to handle the error cleanup > >> case where the ringbuffer has been allocated but map-and-pin > >> failed. Unpin it iff it's previously been mapped-and-pinned. > >> > >>2. Fix the error path in intel_init_ring_buffer(), which already > >> called intel_destroy_ringbuffer_obj(), but failed to free the > >> actual ringbuffer structure. Calling intel_ringbuffer_free() > >> instead does both in one go. > >> > >>3. With the above change, intel_destroy_ringbuffer_obj() is only > >> called in one place (intel_ringbuffer_free()), so flatten it > >> into that function. > >> > >>4. move low-level register accesses from intel_cleanup_ring_buffer() > >> (which calls intel_stop_ring_buffer(ring) which calls stop_ring()) > >> down into stop_ring() itself), which is already doing low-level > >> register accesses. Then, intel_cleanup_ring_buffer() no longer > >> needs 'dev_priv'. > >> > >>v3: > >> Make intel_unpin_ringbuffer_obj() return early if ringbuffer is > >> not mapped, simplifying matters for the caller. [Chris Wilson, > >> Daniel Vetter] > >> Don't bother to clear a pointer in an object about to be freed. > >> [Chris Wilson] > >> > >>Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > >>Cc: Chris Wilson <chris@chris-wilson.co.uk> > >>--- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 49 ++++++++++++++++----------------- > >> 1 file changed, 23 insertions(+), 26 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>index 6f5b511..db02893 100644 > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>@@ -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); > > > >This warn was guarding the release of the backing pages, to verify that > >we had stopped the rings first. > > It's still effectively at the same point in the execution sequence, > just not intruding into a function that has no business peeking at > registers. It's now at the end of the called subfunctions, rather > than just after the call at the top-level, where it was a layering > violation. Nope, semantically it is an important difference and the assertion that the hardware is not reading from the object as we return the pages back to the system is very, very important documentation. > Or we could put it in a separate function, and say > WARN_ON(!ring_is_idle(ring)); > > >>+ > >> return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0; > >> } > >> > >>@@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct intel_engine_cs *ring) > >> > >> void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > >> { > >>+ if (!ringbuf->virtual_start) > >>+ return; > >>+ > >> if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) > >> vunmap(ringbuf->virtual_start); > >> else > >>@@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, > >> return 0; > >> } > >> > >>-static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf) > >>-{ > >>- drm_gem_object_unreference(&ringbuf->obj->base); > >>- ringbuf->obj = NULL; > >>-} > >>- > >> static int intel_alloc_ringbuffer_obj(struct drm_device *dev, > >> struct intel_ringbuffer *ringbuf) > >> { > >>@@ -2200,11 +2199,13 @@ struct intel_ringbuffer * > >> } > >> > >> void > >>-intel_ringbuffer_free(struct intel_ringbuffer *ring) > >>+intel_ringbuffer_free(struct intel_ringbuffer *ringbuf) > >> { > >>- intel_destroy_ringbuffer_obj(ring); > >>- list_del(&ring->link); > >>- kfree(ring); > >>+ if (ringbuf->obj) > >>+ drm_gem_object_unreference(&ringbuf->obj->base); > > > >No, let's not add duplicate code. If you worry, > >replace to_intel_bo() with > > I suspect the above doesn't actually duplicate the test, 'cos > drm_gem_object_unreference() is inline, and common subexpression > elimination should determine that ringbuf->obj and > &ringbuf->obj->base are the same thing and then the compiler can > collapse the explicit test here with the one inside > drm_gem_object_unreference(). So why did you write it? Since it clearly duplicates the existing code, the only reason I could think of was if you did not trust &ringbuf->obj == &ringbuf->obj->base. > >static inline struct drm_i915_gem_object * > >to_intel_bo(struct drm_gem_object *gem_obj) > >{ > > BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base)); > > return container_of(gem_obj, struct drm_i915_gem_object, base); > >} > > I would quite like to get rid of the "&(...)->base" that intrudes > everywhere - we should be able to just (un)reference the object > we're working with, without caring that it's a subclass of some > other type. So we'd want the opposite function as well, for passing > i915 objects to drm functions (as here), maybe call it to_drm_obj()? See above. -Chris
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 6f5b511..db02893 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -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; } @@ -2055,6 +2057,9 @@ static int init_phys_status_page(struct intel_engine_cs *ring) void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) { + if (!ringbuf->virtual_start) + return; + if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) vunmap(ringbuf->virtual_start); else @@ -2132,12 +2137,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, return 0; } -static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf) -{ - drm_gem_object_unreference(&ringbuf->obj->base); - ringbuf->obj = NULL; -} - static int intel_alloc_ringbuffer_obj(struct drm_device *dev, struct intel_ringbuffer *ringbuf) { @@ -2200,11 +2199,13 @@ struct intel_ringbuffer * } void -intel_ringbuffer_free(struct intel_ringbuffer *ring) +intel_ringbuffer_free(struct intel_ringbuffer *ringbuf) { - intel_destroy_ringbuffer_obj(ring); - list_del(&ring->link); - kfree(ring); + if (ringbuf->obj) + drm_gem_object_unreference(&ringbuf->obj->base); + + list_del(&ringbuf->link); + kfree(ringbuf); } static int intel_init_ring_buffer(struct drm_device *dev, @@ -2232,6 +2233,13 @@ static int intel_init_ring_buffer(struct drm_device *dev, } ring->buffer = ringbuf; + ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); + if (ret) { + DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", + ring->name, ret); + goto error; + } + if (I915_NEED_GFX_HWS(dev)) { ret = init_status_page(ring); if (ret) @@ -2243,14 +2251,6 @@ static int intel_init_ring_buffer(struct drm_device *dev, goto error; } - ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); - if (ret) { - DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", - ring->name, ret); - intel_destroy_ringbuffer_obj(ringbuf); - goto error; - } - ret = i915_cmd_parser_init_ring(ring); if (ret) goto error; @@ -2264,19 +2264,16 @@ 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; + struct intel_ringbuffer *ringbuf; if (!intel_ring_initialized(ring)) return; - dev_priv = to_i915(ring->dev); - - if (ring->buffer) { + ringbuf = ring->buffer; + if (ringbuf) { 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); + intel_unpin_ringbuffer_obj(ringbuf); + intel_ringbuffer_free(ringbuf); ring->buffer = NULL; }
1. Fix intel_cleanup_ring_buffer() to handle the error cleanup case where the ringbuffer has been allocated but map-and-pin failed. Unpin it iff it's previously been mapped-and-pinned. 2. Fix the error path in intel_init_ring_buffer(), which already called intel_destroy_ringbuffer_obj(), but failed to free the actual ringbuffer structure. Calling intel_ringbuffer_free() instead does both in one go. 3. With the above change, intel_destroy_ringbuffer_obj() is only called in one place (intel_ringbuffer_free()), so flatten it into that function. 4. move low-level register accesses from intel_cleanup_ring_buffer() (which calls intel_stop_ring_buffer(ring) which calls stop_ring()) down into stop_ring() itself), which is already doing low-level register accesses. Then, intel_cleanup_ring_buffer() no longer needs 'dev_priv'. v3: Make intel_unpin_ringbuffer_obj() return early if ringbuffer is not mapped, simplifying matters for the caller. [Chris Wilson, Daniel Vetter] Don't bother to clear a pointer in an object about to be freed. [Chris Wilson] Signed-off-by: Dave Gordon <david.s.gordon@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 49 ++++++++++++++++----------------- 1 file changed, 23 insertions(+), 26 deletions(-)