[Intel-gfx] [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'
John Harrison
John.C.Harrison at Intel.com
Fri Mar 13 06:32:38 PDT 2015
On 10/03/2015 10:18, Daniel Vetter wrote:
> On Mon, Mar 09, 2015 at 11:51:26PM +0000, Tomas Elf wrote:
>> On 19/02/2015 17:18, John.C.Harrison at Intel.com wrote:
>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>
>>> The outstanding_lazy_request is no longer used anywhere in the driver.
>>> Everything that was looking at it now has a request explicitly passed in from on
>>> high. Everything that was relying upon behind the scenes is now explicitly
>>> creating/passing/submitting it's own private request. Thus the OLR can be
>>> removed.
>>>
>> "Everything that was relying upon behind the scenes is now explicitly
>> creating/passing/submitting it's ..."
>> --->
>> "Everything that was relying on it behind the scenes is now explicitly
>> creating/passing/submitting its ..." ?
>>
>>
>>> For: VIZ-5115
>>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_gem.c | 16 +---------------
>>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +---
>>> drivers/gpu/drm/i915/intel_lrc.c | 6 ++----
>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 17 ++---------------
>>> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ----
>>> 5 files changed, 6 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index 60f6671..8e7418b 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -1156,15 +1156,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
>>> int
>>> i915_gem_check_olr(struct drm_i915_gem_request *req)
>>> {
>>> - int ret;
>>> -
>>> WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
>>>
>>> - ret = 0;
>>> - if (req == req->ring->outstanding_lazy_request)
>>> - ret = i915_add_request(req);
>>> -
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static void fake_irq(unsigned long data)
>>> @@ -2424,8 +2418,6 @@ int __i915_add_request(struct drm_i915_gem_request *request,
>>> dev_priv = ring->dev->dev_private;
>>> ringbuf = request->ringbuf;
>>>
>>> - WARN_ON(request != ring->outstanding_lazy_request);
>>> -
>>> request_start = intel_ring_get_tail(ringbuf);
>>> /*
>>> * Emit any outstanding flushes - execbuf can fail to emit the flush
>>> @@ -2486,7 +2478,6 @@ int __i915_add_request(struct drm_i915_gem_request *request,
>>> }
>>>
>>> trace_i915_gem_request_add(request);
>>> - ring->outstanding_lazy_request = NULL;
>>>
>>> i915_queue_hangcheck(ring->dev);
>>>
>>> @@ -2672,9 +2663,6 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>>>
>>> i915_gem_free_request(request);
>>> }
>>> -
>>> - /* This may not have been flushed before the reset, so clean it now */
>>> - i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>>> }
>>>
>>> void i915_gem_restore_fences(struct drm_device *dev)
>>> @@ -3124,8 +3112,6 @@ int i915_gpu_idle(struct drm_device *dev)
>>> }
>>> }
>>>
>>> - WARN_ON(ring->outstanding_lazy_request);
>>> -
>>> ret = intel_ring_idle(ring);
>>> if (ret)
>>> return ret;
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> index 6a703e6..0eae592 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> @@ -1571,10 +1571,8 @@ err:
>>> * must be freed again. If it was submitted then it is being tracked
>>> * on the active request list and no clean up is required here.
>>> */
>>> - if (ret && params->request) {
>>> + if (ret && params->request)
>>> i915_gem_request_unreference(params->request);
>>> - ring->outstanding_lazy_request = NULL;
>>> - }
>>>
>>> mutex_unlock(&dev->struct_mutex);
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 2911cf6..db63ea0 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1032,8 +1032,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring,
>>> if (!req_out)
>>> return -EINVAL;
>>>
>>> - if ((*req_out = ring->outstanding_lazy_request) != NULL)
>>> - return 0;
>>> + *req_out = NULL;
>>>
>>> request = kzalloc(sizeof(*request), GFP_KERNEL);
>>> if (request == NULL)
>>> @@ -1067,7 +1066,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring,
>>> i915_gem_context_reference(request->ctx);
>>> request->ringbuf = ctx->engine[ring->id].ringbuf;
>>>
>>> - *req_out = ring->outstanding_lazy_request = request;
>>> + *req_out = request;
>>> return 0;
>>> }
>>>
>>> @@ -1393,7 +1392,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
>>>
>>> intel_logical_ring_stop(ring);
>>> WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
>>> - i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>>>
>>> if (ring->cleanup)
>>> ring->cleanup(ring);
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index 5eef02e..85daa18 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -2034,7 +2034,6 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
>>>
>>> intel_unpin_ringbuffer_obj(ringbuf);
>>> intel_destroy_ringbuffer_obj(ringbuf);
>>> - i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
>>>
>>> if (ring->cleanup)
>>> ring->cleanup(ring);
>>> @@ -2153,15 +2152,6 @@ static int intel_wrap_ring_buffer(struct intel_engine_cs *ring)
>>> int intel_ring_idle(struct intel_engine_cs *ring)
>>> {
>>> struct drm_i915_gem_request *req;
>>> - int ret;
>>> -
>>> - /* We need to add any requests required to flush the objects and ring */
>>> - WARN_ON(ring->outstanding_lazy_request);
>>> - if (ring->outstanding_lazy_request) {
>>> - ret = i915_add_request(ring->outstanding_lazy_request);
>>> - if (ret)
>>> - return ret;
>>> - }
>>>
>>> /* Wait upon the last request to be completed */
>>> if (list_empty(&ring->request_list))
>>> @@ -2186,8 +2176,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring,
>>> if (!req_out)
>>> return -EINVAL;
>>>
>>> - if ((*req_out = ring->outstanding_lazy_request) != NULL)
>>> - return 0;
>>> + *req_out = NULL;
>>>
>>> request = kzalloc(sizeof(*request), GFP_KERNEL);
>>> if (request == NULL)
>>> @@ -2206,7 +2195,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring,
>>> return ret;
>>> }
>>>
>>> - *req_out = ring->outstanding_lazy_request = request;
>>> + *req_out = request;
>>> return 0;
>>> }
>>>
>>> @@ -2281,8 +2270,6 @@ void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno)
>>> struct drm_device *dev = ring->dev;
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>>
>>> - BUG_ON(ring->outstanding_lazy_request);
>>> -
>>> if (INTEL_INFO(dev)->gen == 6 || INTEL_INFO(dev)->gen == 7) {
>>> I915_WRITE(RING_SYNC_0(ring->mmio_base), 0);
>>> I915_WRITE(RING_SYNC_1(ring->mmio_base), 0);
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> index 3b0261f..d2c6427 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>> @@ -258,10 +258,6 @@ struct intel_engine_cs {
>>> */
>>> struct list_head request_list;
>>>
>>> - /**
>>> - * Do we have some not yet emitted requests outstanding?
>>> - */
>>> - struct drm_i915_gem_request *outstanding_lazy_request;
>>> bool gpu_caches_dirty;
>>> bool fbc_dirty;
>>>
>>>
>> Since we're removing the i915_add_request from i915_check_olr and thereby
>> removing the OLR emission the following comment at
>> i915_gem_object_flush_active is no longer valid:
>>
>> /**
>> * Ensures that an object will eventually get non-busy by flushing
>> any required
>> * write domains, emitting any outstanding lazy request and retiring
>> and
>> * completed requests.
>> */
>> static int
>> i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
>> {
>> struct intel_engine_cs *ring;
>> int ret;
>>
>> if (obj->active) {
>> ring =
>> i915_gem_request_get_ring(obj->last_read_req);
>> ret = i915_gem_check_olr(obj->last_read_req);
> Nice catch! On top of that the int return value is no longer needed, so a
> follow-up patch should simplify it to void.
> -Daniel
The comment also talks about flushing write domains but there is no
obvious flush call. All it does is the check_olr (which is now a no-op)
and the retire (which won't do anything unless the request has already
completed). So is there any need for this function at all anymore? Or
should it just be removed and replaced with a call to retire instead?
More information about the Intel-gfx
mailing list