- 04 May, 2019 1 commit
-
-
Chris Wilson authored
Asking the GPU to busywait on a memory address, perhaps not unexpectedly in hindsight for a shared system, leads to bus contention that affects CPU programs trying to concurrently access memory. This can manifest as a drop in transcode throughput on highly over-saturated workloads. The only clue offered by perf, is that the bus-cycles (perf stat -e bus-cycles) jumped by 50% when enabling semaphores. This corresponds with extra CPU active cycles being attributed to intel_idle's mwait. This patch introduces a heuristic to try and detect when more than one client is submitting to the GPU pushing it into an oversaturated state. As we already keep track of when the semaphores are signaled, we can inspect their state on submitting the busywait batch and if we planned to use a semaphore but were too late, conclude that the GPU is overloaded and not try to use semaphores in future requests. In practice, this means we optimistically try to use semaphores for the first frame of a transcode job split over multiple engines, and fail if there are multiple clients active and continue not to use semaphores for the subsequent frames in the sequence. Periodically, we try to optimistically switch semaphores back on whenever the client waits to catch up with the transcode results. With 1 client, on Broxton J3455, with the relative fps normalized by %cpu: x no semaphores + drm-tip * patched +------------------------------------------------------------------------+ | * | | *+ | | **+ | | **+ x | | x * +**+ x | | x x * * +***x xx | | x x * * *+***x *x | | x x* + * * *****x *x x | | + x xx+x* + *** * ********* x * | | + x xx+x* * *** +** ********* xx * | | * + ++++* + x*x****+*+* ***+*************+x* * | |*+ +** *+ + +* + *++****** *xxx**********x***+*****************+*++ *| | |__________A_____M_____| | | |_______________A____M_________| | | |____________A___M________| | +------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 120 2.60475 3.50941 3.31123 3.2143953 0.21117399 + 120 2.3826 3.57077 3.25101 3.1414161 0.28146407 Difference at 95.0% confidence -0.0729792 +/- 0.0629585 -2.27039% +/- 1.95864% (Student's t, pooled s = 0.248814) * 120 2.35536 3.66713 3.2849 3.2059917 0.24618565 No difference proven at 95.0% confidence With 10 clients over-saturating the pipeline: x no semaphores + drm-tip * patched +------------------------------------------------------------------------+ | ++ ** | | ++ ** | | ++ ** | | ++ ** | | ++ xx *** | | ++ xx *** | | ++ xxx*** | | ++ xxx*** | | +++ xxx*** | | +++ xx**** | | +++ xx**** | | +++ xx**** | | +++ xx**** | | ++++ xx**** | | +++++ xx**** | | +++++ x x****** | | ++++++ xxx******* | | ++++++ xxx******* | | ++++++ xxx******* | | ++++++ xx******** | | ++++++ xxxx******** | | ++++++ xxxx******** | | ++++++++ xxxxx********* | |+ + + + ++++++++ xxx*xx**********x* *| | |__A__| | | |__AM__| | | |__A_| | +------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 120 2.47855 2.8972 2.72376 2.7193402 0.074604933 + 120 1.17367 1.77459 1.71977 1.6966782 0.085850697 Difference at 95.0% confidence -1.02266 +/- 0.0203502 -37.607% +/- 0.748352% (Student's t, pooled s = 0.0804246) * 120 2.57868 3.00821 2.80142 2.7923878 0.058646477 Difference at 95.0% confidence 0.0730476 +/- 0.0169791 2.68622% +/- 0.624383% (Student's t, pooled s = 0.0671018) Indicating that we've recovered the regression from enabling semaphores on this saturated setup, with a hint towards an overall improvement. Very similar, but of smaller magnitude, results are observed on both Skylake(gt2) and Kabylake(gt4). This may be due to the reduced impact of bus-cycles, where we see a 50% hit on Broxton, it is only 10% on the big core, in this particular test. One observation to make here is that for a greedy client trying to maximise its own throughput, using semaphores is the right choice. It is only the holistic system-wide view that semaphores of one client impacts another and reduces the overall throughput where we would choose to disable semaphores. The most noticeable negactive impact this has is on the no-op microbenchmarks, which are also very notable for having no cpu bus load. In particular, this increases the runtime and energy consumption of gem_exec_whisper. Signed-off-by:Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com> Cc: Dmitry Ermilov <dmitry.ermilov@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190504070707.30902-1-chris@chris-wilson.co.uk
-
- 03 May, 2019 22 commits
-
-
Ville Syrjälä authored
We have a lot of '(u64)foo * bar' everywhere. Replace with mul_u32_u32() to avoid gcc failing to use a regular 32x32->64 multiply for this. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190408152702.4153-1-ville.syrjala@linux.intel.comReviewed-by:
-
Ville Syrjälä authored
Turns out the cursor is compatible with the pipe "HDR mode". It's only the actual SDR planes that get entirely bypassed during blending. So let's ignore the cursor when checking if we have any planes active that aren't HDR compatible. This fixes the regressions in the kms_cursor_crc and kms_plane_cursor tests. Cc: Uma Shankar <uma.shankar@intel.com> Cc: Shashank Sharma <shashank.sharma@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110579 Fixes: 09b25812 ("drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used") Signed-off-by:
Uma Shankar <uma.shankar@intel.com>
-
Ville Syrjälä authored
I fumbled the PIPEMISC write into the wrong place. It only gets called for fastsets, but since value needs to be updated based on the set of active planes it needs to be done for all plane updates. Move it to the correct spot. The symptoms include SDR planes never showing up if a previous modeset/fastset left the pipe in HDR mode. This was immediately obvious when running the kms_plane pixel format tests. Unfortunately the test didn't realize it was scanning out pure black all the time and declared success anyway. Cc: Uma Shankar <uma.shankar@intel.com> Cc: Shashank Sharma <shashank.sharma@intel.com> Fixes: 09b25812 ("drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used") Signed-off-by:
-
Chris Wilson authored
Currently we submit the semaphore busywait as soon as the signaler is submitted to HW. However, we may submit the signaler as the tail of a batch of requests, and even not as the first context in the HW list, i.e. the busywait may start spinning far in advance of the signaler even starting. If we wait until the request before the signaler is completed before submitting the busywait, we prevent the busywait from starting too early, if the signaler is not first in submission port. To handle the case where the signaler is at the start of the second (or later) submission port, we will need to delay the execution callback until we know the context is promoted to port0. A challenge for later. Fixes: e8861964 ("drm/i915: Use HW semaphores for inter-engine synchroni sation on gen8+") Signed-off-by:
-
Chris Wilson authored
Given sufficient preemption, we may see a busy system that doesn't advance seqno while performing work across multiple contexts, and given sufficient pathology not even notice a change in ACTHD. What does change between the preempting contexts is their RING, so take note of that and treat a change in the ring address as being an indication of forward progress. Signed-off-by:
-
Chris Wilson authored
Drop the check in GEM parking that the engines were already parked. The intention here was that before we dropped the GT wakeref, we were sure that no more interrupts could be raised -- however, we have already dropped the wakeref by this point and the warning is no longer valid. Signed-off-by:
-
Chris Wilson authored
Tidy up the cleanup sequence by always ensure that the tasklet is flushed on parking (before we cleanup). The parking provides a convenient point to ensure that the backend is truly idle. v2: Do the full check for idleness before parking, to be sure we flush any residual interrupt. Signed-off-by:
-
Chris Wilson authored
We are not allowed to rpm_get() inside the runtime-suspend callback, so split the intel_uc_suspend() into the core that assumes the caller holds the wakeref (intel_uc_runtime_suspend), and one that acquires the wakeref as necessary (intel_uc_suspend). Reported-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Fixes: 79ffac85 ("drm/i915: Invert the GEM wakeref hierarchy") Signed-off-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. While at it, rename intel_i2c.c to intel_gmbus.c and the functions to intel_gmbus_*. No functional changes. Reviewed-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/5834b8fbbfd4ac2e3d0159e69c87f6926066f537.1556809195.git.jani.nikula@intel.com
-
Jani Nikula authored
Reduce clutter from i915_drv.h and intel_drv.h. Reviewed-by:
-
Jani Nikula authored
And ensure it stays that way. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from i915_drv.h. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from i915_drv.h. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from intel_drv.h. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from intel_drv.h with the minimal change. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from intel_drv.h with the minimal change. Reviewed-by:
-
Jani Nikula authored
Reduce clutter from intel_drv.h with the minimal change. Reviewed-by:
-
Jani Nikula authored
Work on the principle that files should prefer not to expose platform specific functions. v2, v3: Rebase Cc: Imre Deak <imre.deak@intel.com> Reviewed-by:
-
- 02 May, 2019 8 commits
-
-
Imre Deak authored
Looks like VBT contains again the wrong information about a port's TypeC legacy vs. DP-alt/TBT-alt type. There is no further issues after we notice this and fix it up, so tune down the WARN to be a a DRM_ERROR. This also avoids CI tainting the kernel and stopping the test run. v2: - Update also code coment accordingly. (Jani) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110578 Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
-
Chris Wilson authored
Show the fence flags view of request completion in addition to the normal hwsp check and whether signaling is enabled. Signed-off-by:
-
Imre Deak authored
This step of the BSpec combo PHY port enabling is missing, so add it now. v2: - Rebased on the new fixed v2 version of the helper. v3: - Use intel_ instead of icl_ prefix. (Jani) Reported-by:
-
Imre Deak authored
Factor out the combo PHY lane power configuration code to a separate helper; it will be also needed by the next patch adding the same configuration for DDI ports. Add support for DDI ports and lane reversal as preparation for the next patch. The PWR_DOWN_LN_1 value is unspecified in the BSpec register description so remove it. v2: - Fix up the wrong assumption that the encodings are the same for DDI and DSI ports. (Jani) v3: - Use intel_ instead of icl_ prefix. (Jani) - Add required headers to intel_combo_phy.h after the upstream header refactoring. Cc: Jani Nikula <jani.nikula@intel.com> Cc: Madhav Chauhan <madhav.chauhan@intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
s/pipe/transcoder/ when dealing with hsw+ audio registers. This won't actually make any real difference since there is no audio on the EDP transcoder. But this should avoid a bit of confusion when cross checking against the spec. Signed-off-by:
-
Ville Syrjälä authored
We've already committed to enabling audio when intel_audio_codec_enable() is called. We can't back out even if the ELD has turned sour in the meantime. So just spew some debug log and plow ahead. Otherwise the state checker gets unhappy when audio isn't enabled when it is expected to be. I suppose we really ought to precompute the ELD as well, but let's just toss in a FIXME for the future. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103841Signed-off-by:
-
Jani Nikula authored
Debug logging should not be dependent on alpha support flag. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Stanislav Lisovskiy authored
Currently due to regression CI machine displays show corrupt picture. Problem is when CDCLK is as low as 79200, picture gets unstable, while DSI and DE pll values were confirmed to be correct. Limiting to 158400 as agreed with Ville. We could not come up with any better solution yet, as PLL divider values both for MIPI(DSI PLL) and CDCLK(DE PLL) are correct, however seems that due to some boundary conditions, when clocking is too low we get wrong timings for DSI display. Similar workaround exists for VLV though, so just took similar condition into use. At least that way GLK platform will start to be usable again, with current drm-tip. v2: Fixed commit subject as suggested. v3: Added generic bugs(crc failures, screen not init for GLK DSI which might be affected). v4: Added references tag for bugs affected. Signed-off-by:
Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Acked-by:
-
- 01 May, 2019 2 commits
-
-
Chris Wilson authored
The workqueue code complains viciously if we try to queue more work onto the queue while attampting to drain it. As we asynchronously free objects and defer their enqueuing with RCU, it is quite tricky to quiesce the system before attempting to drain the workqueue. Yet drain we must to ensure that the worker is idle before unloading the module. Give the freed object drain 3 whole passes with multiple rcu_barrier() to give the defer freeing of several levels each protected by RCU and needing a grace period before its parent can be freed, ultimately resulting in a GEM object being freed after another RCU period. A consequence is that it will make module unload even slower. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110550Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190501135753.8711-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Make the engine responsible for cleaning itself up! This removes the i915->gt.cleanup vfunc that has been annoying the casual reader and myself for the last several years, and helps keep a future patch to add more cleanup tidy. v2: Assert that engine->destroy is set after the backend starts allocating its own state. Signed-off-by:
-
- 30 Apr, 2019 7 commits
-
-
Ville Syrjälä authored
The pipe has a special HDR mode with higher precision when only HDR planes are active. Let's use it. Curiously this fixes the kms_color gamma/degamma tests when using a HDR plane, which is always the case unless one hacks the test to use an SDR plane. If one does hack the test to use an SDR plane it does pass already. I have no actual explanation how the output after the gamma LUT can be different between the two modes. The way the tests are written should mean that the output should be identical between the solid color vs. the gradient. But clearly that somehow doesn't hold true for the HDR planes in non-HDR pipe mode. Anyways, as long as we stick to one type of plane the test should produce sensible results now. v2: s/HDR_MODE/HDR_MODE_PRECISION/ (Shashank) Signed-off-by:
Shashank Sharma <shashank.sharma@intel.com>
-
Ville Syrjälä authored
Move the platform checks out from haswell_set_pipemisc() and rename it to bdw_set_pipemisc() to make it clear when to call it. Signed-off-by:
-
Chris Wilson authored
When the system is idling, contention for struct_mutex should be low and so we will be more efficient to wait for a contended mutex than reschedule. Signed-off-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time i915_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
Commit 696173b0 ("drm/i915: extract intel_pm.h from intel_drv.h") missed the declarations in i915_drv.h. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time intel_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. Reviewed-by:
-
Jani Nikula authored
It used to be handy that we only had a couple of headers, but over time intel_drv.h has become unwieldy. Extract declarations to a separate header file corresponding to the implementation module, clarifying the modularity of the driver. Ensure the new header is self-contained, and do so with minimal further includes, using forward declarations as needed. Include the new header only where needed, and sort the modified include directives while at it and as needed. No functional changes. v2: fix sparse warnings on undeclared global functions Reviewed-by:
-
