[Intel-gfx] [RFC 16/25] drm/i915: Convert most 'i915_seqno_passed' calls into 'i915_gem_request_completed'
John Harrison
John.C.Harrison at Intel.com
Tue Oct 28 15:02:16 CET 2014
On 19/10/2014 15:04, Daniel Vetter wrote:
> On Fri, Oct 10, 2014 at 12:39:49PM +0100, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> For: VIZ-4377
>> Signed-off-by: John.C.Harrison at Intel.com
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 3 +--
>> drivers/gpu/drm/i915/i915_drv.h | 18 ++++++++++++++++++
>> drivers/gpu/drm/i915/i915_gem.c | 12 ++++--------
>> drivers/gpu/drm/i915/intel_display.c | 11 +++--------
>> 4 files changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 52ddd19..e764af9 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -550,8 +550,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
>> i915_gem_request_get_seqno(work->flip_queued_req),
>> dev_priv->next_seqno,
>> work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
>> - i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
>> - i915_gem_request_get_seqno(work->flip_queued_req)));
>> + i915_gem_request_completed(work->flip_queued_req, true));
>> } else
>> seq_printf(m, "Flip not associated with any ring\n");
>> seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 6fe3be0..650f712 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1970,6 +1970,12 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req)
>> kref_put(&req->ref, i915_gem_request_free);
>> }
>>
>> +/*
>> + * NB: i915_gem_request_completed should be here but currently needs the
>> + * definition of i915_seqno_passed() which is below. It will be moved in
>> + * a later patch when the call to i915_seqno_passed() is obsoleted...
>> + */
>> +
>> struct drm_i915_file_private {
>> struct drm_i915_private *dev_priv;
>> struct drm_file *file;
>> @@ -3019,4 +3025,16 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
>> }
>> }
>>
>> +static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
>> + bool lazy_coherency)
>> +{
>> + u32 seqno;
>> +
>> + BUG_ON(req == NULL);
>> +
>> + seqno = req->ring->get_seqno(req->ring, lazy_coherency);
>> +
>> + return i915_seqno_passed(seqno, req->seqno);
>> +}
> Tbh this thing looks a bit big for a static inline. Have you
> double-checked that inlining this doesn't increase binary size? If it
> does then we need performance data justifying the cost. Same probably
> applies to some of the other static inline functions you've added.
> -Daniel
It is a replacement for i915_seqno_passed() which is a static inline so
it seemed sensible that this should be too. Note that the ultimate aim
is for this to be a simple 'return req->completed'. At which point it
should certainly be worthy of inlining.
But no I haven't done any actual analysis of the benefit/cost yet. As
has been mentioned, this RFC was a quick knock up to get something to
discuss as soon as possible.
>> +
>> #endif
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index e1a58f1..b602e8c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2253,8 +2253,7 @@ i915_gem_object_retire(struct drm_i915_gem_object *obj)
>> if (ring == NULL)
>> return;
>>
>> - if (i915_seqno_passed(ring->get_seqno(ring, true),
>> - i915_gem_request_get_seqno(obj->last_read_req)))
>> + if (i915_gem_request_completed(obj->last_read_req, true))
>> i915_gem_object_move_to_inactive(obj);
>> }
>>
>> @@ -2521,12 +2520,9 @@ struct drm_i915_gem_request *
>> i915_gem_find_active_request(struct intel_engine_cs *ring)
>> {
>> struct drm_i915_gem_request *request;
>> - u32 completed_seqno;
>> -
>> - completed_seqno = ring->get_seqno(ring, false);
>>
>> list_for_each_entry(request, &ring->request_list, list) {
>> - if (i915_seqno_passed(completed_seqno, request->seqno))
>> + if (i915_gem_request_completed(request, false))
>> continue;
>>
>> return request;
>> @@ -2670,7 +2666,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
>> struct drm_i915_gem_object,
>> ring_list);
>>
>> - if (!i915_seqno_passed(seqno, i915_gem_request_get_seqno(obj->last_read_req)))
>> + if (!i915_gem_request_completed(obj->last_read_req, true))
>> break;
>>
>> i915_gem_object_move_to_inactive(obj);
>> @@ -2685,7 +2681,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
>> struct drm_i915_gem_request,
>> list);
>>
>> - if (!i915_seqno_passed(seqno, request->seqno))
>> + if (!i915_gem_request_completed(request, true))
>> break;
>>
>> trace_i915_gem_request_retire(ring, request->seqno);
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 26fdd96..9ca8f94 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9771,8 +9771,7 @@ static int intel_postpone_flip(struct drm_i915_gem_object *obj)
>>
>> ring = obj->ring;
>>
>> - if (i915_seqno_passed(ring->get_seqno(ring, true),
>> - i915_gem_request_get_seqno(obj->last_write_req)))
>> + if (i915_gem_request_completed(obj->last_write_req, true))
>> return 0;
>>
>> ret = i915_gem_check_olr(obj->last_write_req);
>> @@ -9790,9 +9789,6 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
>> struct drm_i915_private *dev_priv = to_i915(ring->dev);
>> struct intel_crtc *intel_crtc;
>> unsigned long irq_flags;
>> - u32 seqno;
>> -
>> - seqno = ring->get_seqno(ring, false);
>>
>> spin_lock_irqsave(&dev_priv->mmio_flip_lock, irq_flags);
>> for_each_intel_crtc(ring->dev, intel_crtc) {
>> @@ -9805,7 +9801,7 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
>> if (ring->id != mmio_flip->ring_id)
>> continue;
>>
>> - if (i915_seqno_passed(seqno, i915_gem_request_get_seqno(mmio_flip->req))) {
>> + if (i915_gem_request_completed(mmio_flip->req, false)) {
>> intel_do_mmio_flip(intel_crtc);
>> i915_gem_request_unreference(mmio_flip->req);
>> mmio_flip->req = NULL;
>> @@ -9878,8 +9874,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
>>
>> if (work->flip_ready_vblank == 0) {
>> if (work->flip_queued_ring &&
>> - !i915_seqno_passed(work->flip_queued_ring->get_seqno(work->flip_queued_ring, true),
>> - i915_gem_request_get_seqno(work->flip_queued_req)))
>> + !i915_gem_request_completed(work->flip_queued_req, true))
>> return false;
>>
>> work->flip_ready_vblank = drm_vblank_count(dev, intel_crtc->pipe);
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list