All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 18/19] drm/i915/execlists: Direct submission (avoid tasklet/ksoftirqd)
Date: Fri, 18 May 2018 09:06:03 +0100	[thread overview]
Message-ID: <c0be9972-df1f-52d1-32d2-46ef68173a65@linux.intel.com> (raw)
In-Reply-To: <152657685476.26696.8069009167621824484@mail.alporthouse.com>


On 17/05/2018 18:07, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-05-17 14:13:00)
>>
>> On 17/05/2018 08:40, Chris Wilson wrote:
>>> Back in commit 27af5eea54d1 ("drm/i915: Move execlists irq handler to a
>>> bottom half"), we came to the conclusion that running our CSB processing
>>> and ELSP submission from inside the irq handler was a bad idea. A really
>>> bad idea as we could impose nearly 1s latency on other users of the
>>> system, on average! Deferring our work to a tasklet allowed us to do the
>>> processing with irqs enabled, reducing the impact to an average of about
>>> 50us.
>>>
>>> We have since eradicated the use of forcewaked mmio from inside the CSB
>>> processing and ELSP submission, bringing the impact down to around 5us
>>> (on Kabylake); an order of magnitude better than our measurements 2
>>> years ago on Broadwell and only about 2x worse on average than the
>>> gem_syslatency on an unladen system.
>>>
>>> Comparing the impact on the maximum latency observed over a 120s interval,
>>> repeated several times (using gem_syslatency, similar to RT's cyclictest)
>>> while the system is fully laden with i915 nops, we see that direct
>>> submission definitely worsens the response but not to the same outlandish
>>> degree as before.
>>>
>>> x Unladen baseline
>>> + Using tasklet
>>> * Direct submission
>>>
>>> +------------------------------------------------------------------------+
>>> |xx x          ++    +++ +                           *  * *   ** *** *  *|
>>> ||A|              |__AM__|                               |_____A_M___|   |
>>> +------------------------------------------------------------------------+
>>
>> What are these headers? This one and below, I cannot decipher them at all.
> 
> Ministat histogram. The headers being the label for the charts; it's a
> bit flat so hard to tell it's a histogram.
> 
>>>       N           Min           Max        Median           Avg        Stddev
>>> x  10             5            18            10           9.3     3.6530049
>>> +  10            72           120           108         102.9     15.758243
>>> *  10           255           348           316         305.7      28.74814
>>
>> In micro-seconds? so tasklet is 108us median? Direct submission 316us
>> median?
> 
> Yup, more runs required so you have prettier graphs and units.

Biggest problem for me is that in these tests it is only showing as 
significantly worse than the current tasklet. So it is kind of difficult 
the sell the series. :)

If it only solves issues when submitting from a RT task then it is not 
so interesting. RT tasks are rare and which deal with 3d/media/ocl 
probably even rarer. Or you know of any?

Or perhaps you need to add data from gem_latency as well if that shows 
some wins purely on the i915 side?

Regards,

Tvrtko

>>> And with a background load
>>
>> This is IO background load?
> 
> Yes, background writeout.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-05-18  8:06 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17  7:40 [PATCH 01/19] drm/i915: Move request->ctx aside Chris Wilson
2018-05-17  7:40 ` [PATCH 02/19] drm/i915: Move fiddling with engine->last_retired_context Chris Wilson
2018-05-17  7:40 ` [PATCH 03/19] drm/i915: Store a pointer to intel_context in i915_request Chris Wilson
2018-05-17  7:40 ` [PATCH 04/19] drm/i915: Pull the context->pin_count dec into the common intel_context_unpin Chris Wilson
2018-05-17 10:20   ` Tvrtko Ursulin
2018-05-17 10:35     ` Chris Wilson
2018-05-17  7:40 ` [PATCH 05/19] drm/i915: Be irqsafe inside reset Chris Wilson
2018-05-17 10:27   ` Tvrtko Ursulin
2018-05-17 10:46     ` Chris Wilson
2018-05-17  7:40 ` [PATCH 06/19] drm/i915: Make intel_engine_dump irqsafe Chris Wilson
2018-05-17 10:28   ` Tvrtko Ursulin
2018-05-17  7:40 ` [PATCH 07/19] drm/i915/execlists: Handle copying default context state for atomic reset Chris Wilson
2018-05-17 10:37   ` Tvrtko Ursulin
2018-05-17  7:40 ` [PATCH 08/19] drm/i915: Allow init_breadcrumbs to be used from irq context Chris Wilson
2018-05-17 10:40   ` Tvrtko Ursulin
2018-05-17  7:40 ` [PATCH 09/19] drm/i915/execlists: HWACK checking superseded checking port[0].count Chris Wilson
2018-05-17 10:55   ` Tvrtko Ursulin
2018-05-17 17:03     ` Chris Wilson
2018-05-17  7:40 ` [PATCH 10/19] drm/i915: Remove USES_GUC_SUBMISSION() pointer chasing from gen8_cs_irq_handler Chris Wilson
2018-05-17 10:58   ` Tvrtko Ursulin
2018-05-17 11:24     ` Chris Wilson
2018-05-17 13:13       ` Tvrtko Ursulin
2018-05-17  7:40 ` [PATCH 11/19] drm/i915/execlists: Double check rpm wakeref Chris Wilson
2018-05-17 11:04   ` Tvrtko Ursulin
2018-05-17  7:40 ` [PATCH 12/19] drm/i915: After reset on sanitization, reset the engine backends Chris Wilson
2018-05-17  7:40 ` [PATCH 13/19] drm/i915/execlists: Reset the CSB head tracking on reset/sanitization Chris Wilson
2018-05-17  7:40 ` [PATCH 14/19] drm/i915/execlists: Pull submit after dequeue under timeline lock Chris Wilson
2018-05-17  7:40 ` [PATCH 15/19] drm/i915/execlists: Process one CSB interrupt at a time Chris Wilson
2018-05-17  7:40 ` [PATCH 16/19] drm/i915/execlists: Unify CSB access pointers Chris Wilson
2018-05-17  7:40 ` [PATCH 17/19] drm/i915/execlists: Process the CSB directly from inside the irq handler Chris Wilson
2018-05-17  7:40 ` [PATCH 18/19] drm/i915/execlists: Direct submission (avoid tasklet/ksoftirqd) Chris Wilson
2018-05-17 13:13   ` Tvrtko Ursulin
2018-05-17 17:07     ` Chris Wilson
2018-05-18  8:06       ` Tvrtko Ursulin [this message]
2018-05-18  8:18         ` Chris Wilson
2018-05-18 19:36         ` Chris Wilson
2018-05-18 21:21     ` Chris Wilson
2018-05-17  7:40 ` [PATCH 19/19] drm/i915: Combine gt irq ack/handlers Chris Wilson
2018-05-17  8:01 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/19] drm/i915: Move request->ctx aside Patchwork
2018-05-17  8:06 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-05-17  8:16 ` ✓ Fi.CI.BAT: success " Patchwork
2018-05-17 11:05 ` ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0be9972-df1f-52d1-32d2-46ef68173a65@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.