Quoting Tvrtko Ursulin (2018-05-18 09:06:03) > > 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. :) Ba! Compared to the previous best state 2 years ago, we're an order magnitude better. (Unbelievable!) I think it's simply a lack of a reference point, we're about 100% faster in some microbenchmark stress igts, about 10% faster in regular stress igts, and will not suffer some of the 200ms timeout -> wedged! Any latency is bad, but at what point do we worry about the trade-off between our latency and the rest of the system? > 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? No, it's not just an issue from RT. We see examples of the driver being so starved we declare the system is wedged, to much more mundane artifacts of our throughput being diminished due to the latency in submitting work. > Or perhaps you need to add data from gem_latency as well if that shows > some wins purely on the i915 side? The patch before (process CSB from interrupt handler) is justifiable just by the bugs it solves. This patch, I think is justifiable by our perf gains and the impact it has for low latency requirements. Or we can wait until the next patch is mature, that tries to reduce the lock contention by splitting the data structures. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx