#
def25b7b |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Make an alignment check more sensible
What we really want to check is that size of the engines array, i.e. args->size - sizeof(*user) is divisible by the element size, i.e. sizeof(*use
drm/i915/gem: Make an alignment check more sensible
What we really want to check is that size of the engines array, i.e. args->size - sizeof(*user) is divisible by the element size, i.e. sizeof(*user->engines) because that's what's required for computing the array length right below the check. However, we're currently not doing this and instead doing a compile-time check that sizeof(*user) is divisible by sizeof(*user->engines) and avoiding the subtraction. As far as I can tell, the only reason for the more confusing pair of checks is to avoid a single subtraction of a constant.
The other thing the BUILD_BUG_ON might be trying to implicitly check is that offsetof(user->engines) == sizeof(*user) and we don't have any weird padding throwing us off. However, that's not the check it's doing and it's not even a reliable way to do that check.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-21-jason@jlekstrand.net
show more ...
|
#
bc2ceb7a |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Add an i915_gem_vm_lookup helper
This is the VM equivalent of i915_gem_context_lookup. It's only used once in this patch but future patches will need to duplicate this lookup code so it's
drm/i915: Add an i915_gem_vm_lookup helper
This is the VM equivalent of i915_gem_context_lookup. It's only used once in this patch but future patches will need to duplicate this lookup code so it's better to have it in a helper.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-20-jason@jlekstrand.net
show more ...
|
#
263ae12c |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Optionally set SSEU in intel_context_set_gem
For now this is a no-op because everyone passes in a null SSEU but it lets us get some of the error handling and selftest refactoring plumb
drm/i915/gem: Optionally set SSEU in intel_context_set_gem
For now this is a no-op because everyone passes in a null SSEU but it lets us get some of the error handling and selftest refactoring plumbed through.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-19-jason@jlekstrand.net
show more ...
|
#
07a635a8 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Rework error handling in default_engines
Since free_engines works for partially constructed engine sets, we can use the usual goto pattern.
Signed-off-by: Jason Ekstrand <jason@jlekst
drm/i915/gem: Rework error handling in default_engines
Since free_engines works for partially constructed engine sets, we can use the usual goto pattern.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-18-jason@jlekstrand.net
show more ...
|
#
a34857dc |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Add an intermediate proto_context struct (v5)
The current context uAPI allows for two methods of setting context parameters: SET_CONTEXT_PARAM and CONTEXT_CREATE_EXT_SETPARAM. The for
drm/i915/gem: Add an intermediate proto_context struct (v5)
The current context uAPI allows for two methods of setting context parameters: SET_CONTEXT_PARAM and CONTEXT_CREATE_EXT_SETPARAM. The former is allowed to be called at any time while the later happens as part of GEM_CONTEXT_CREATE. Currently, everything settable via one is settable via the other. While some params are fairly simple and setting them on a live context is harmless such the context priority, others are far trickier such as the VM or the set of engines. In order to swap out the VM, for instance, we have to delay until all current in-flight work is complete, swap in the new VM, and then continue. This leads to a plethora of potential race conditions we'd really rather avoid.
Unfortunately, both methods of setting the VM and the engine set are in active use today so we can't simply disallow setting the VM or engine set vial SET_CONTEXT_PARAM. In order to work around this wart, this commit adds a proto-context struct which contains all the context create parameters.
v2 (Daniel Vetter): - Better commit message - Use __set/clear_bit instead of set/clear_bit because there's no race and we don't need the atomics
v3 (Daniel Vetter): - Use manual bitops and BIT() instead of __set_bit
v4 (Daniel Vetter): - Add a changelog to the commit message - Better hyperlinking in docs - Create the default PPGTT in i915_gem_create_context
v5 (Daniel Vetter): - Hand-roll the initialization of UCONTEXT_PERSISTENCE
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-17-jason@jlekstrand.net
show more ...
|
#
aaa5957c |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Add a separate validate_priority helper
With the proto-context stuff added later in this series, we end up having to duplicate set_priority. This lets us avoid duplicating the validat
drm/i915/gem: Add a separate validate_priority helper
With the proto-context stuff added later in this series, we end up having to duplicate set_priority. This lets us avoid duplicating the validation logic.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-15-jason@jlekstrand.net
show more ...
|
#
a4839cb1 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Stop manually RCU banging in reset_stats_ioctl (v2)
As far as I can tell, the only real reason for this is to avoid taking a reference to the i915_gem_context. The cost of those two atomi
drm/i915: Stop manually RCU banging in reset_stats_ioctl (v2)
As far as I can tell, the only real reason for this is to avoid taking a reference to the i915_gem_context. The cost of those two atomics probably pales in comparison to the cost of the ioctl itself so we're really not buying ourselves anything here. We're about to make context lookup a tiny bit more complicated, so let's get rid of the one hand- rolled case.
Some usermode drivers such as our Vulkan driver call GET_RESET_STATS on every execbuf so the perf here could theoretically be an issue. If this ever does become a performance issue for any such userspace drivers, they can use set CONTEXT_PARAM_RECOVERABLE to false and look for -EIO coming from execbuf to check for hangs instead.
v2 (Daniel Vetter): - Add a comment in the commit message about recoverable contexts
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-14-jason@jlekstrand.net
show more ...
|
#
ebb1ca74 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Disallow creating contexts with too many engines
There's no sense in allowing userspace to create more engines than it can possibly access via execbuf.
Signed-off-by: Jason Ekstrand <
drm/i915/gem: Disallow creating contexts with too many engines
There's no sense in allowing userspace to create more engines than it can possibly access via execbuf.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-13-jason@jlekstrand.net
show more ...
|
#
521695c6 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Disallow bonding of virtual engines (v3)
This adds a bunch of complexity which the media driver has never actually used. The media driver does technically bond a balanced engine to an
drm/i915/gem: Disallow bonding of virtual engines (v3)
This adds a bunch of complexity which the media driver has never actually used. The media driver does technically bond a balanced engine to another engine but the balanced engine only has one engine in the sibling set. This doesn't actually result in a virtual engine.
This functionality was originally added to handle cases where we may have more than two video engines and media might want to load-balance their bonded submits by, for instance, submitting to a balanced vcs0-1 as the primary and then vcs2-3 as the secondary. However, no such hardware has shipped thus far and, if we ever want to enable such use-cases in the future, we'll use the up-and-coming parallel submit API which targets GuC submission.
This makes I915_CONTEXT_ENGINES_EXT_BOND a total no-op. We leave the validation code in place in case we ever decide we want to do something interesting with the bonding information.
v2 (Jason Ekstrand): - Don't delete quite as much code.
v3 (Tvrtko Ursulin): - Add some history to the commit message
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-10-jason@jlekstrand.net
show more ...
|
#
c7a71fc8 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Drop getparam support for I915_CONTEXT_PARAM_ENGINES
This has never been used by any userspace except IGT and provides no real functionality beyond parroting back parameters userspace pass
drm/i915: Drop getparam support for I915_CONTEXT_PARAM_ENGINES
This has never been used by any userspace except IGT and provides no real functionality beyond parroting back parameters userspace passed in as part of context creation or via setparam. If the context is in legacy mode (where you use I915_EXEC_RENDER and friends), it returns success with zero data so it's not useful for discovering what engines are in the context. It's also not a replacement for the recently removed I915_CONTEXT_CLONE_ENGINES because it doesn't return any of the balancing or bonding information.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-9-jason@jlekstrand.net
show more ...
|
#
00dae4d3 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)
This API is entirely unnecessary and I'd love to get rid of it. If userspace wants a single timeline across multiple contexts, they can eithe
drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)
This API is entirely unnecessary and I'd love to get rid of it. If userspace wants a single timeline across multiple contexts, they can either use implicit synchronization or a syncobj, both of which existed at the time this feature landed. The justification given at the time was that it would help GL drivers which are inherently single-timeline. However, neither of our GL drivers actually wanted the feature. i965 was already in maintenance mode at the time and iris uses syncobj for everything.
Unfortunately, as much as I'd love to get rid of it, it is used by the media driver so we can't do that. We can, however, do the next-best thing which is to embed a syncobj in the context and do exactly what we'd expect from userspace internally. This isn't an entirely identical implementation because it's no longer atomic if userspace races with itself by calling execbuffer2 twice simultaneously from different threads. It won't crash in that case; it just doesn't guarantee any ordering between those two submits. It also means that sync files exported from different engines on a SINGLE_TIMELINE context will have different fence contexts. This is visible to userspace if it looks at the obj_name field of sync_fence_info.
Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical advantages beyond mere annoyance. One is that intel_timeline is no longer an api-visible object and can remain entirely an implementation detail. This may be advantageous as we make scheduler changes going forward. Second is that, together with deleting the CLONE_CONTEXT API, we should now have a 1:1 mapping between intel_context and intel_timeline which may help us reduce locking.
v2 (Tvrtko Ursulin): - Update the comment on i915_gem_context::syncobj to mention that it's an emulation and the possible race if userspace calls execbuffer2 twice on the same context concurrently. v2 (Jason Ekstrand): - Wrap the checks for eb.gem_context->syncobj in unlikely() - Drop the dma_fence reference - Improved commit message
v3 (Jason Ekstrand): - Move the dma_fence_put() to before the error exit
v4 (Tvrtko Ursulin): - Add a comment about fence contexts to the commit message
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-8-jason@jlekstrand.net
show more ...
|
#
4a766ae4 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Drop the CONTEXT_CLONE API (v2)
This API allows one context to grab bits out of another context upon creation. It can be used as a short-cut for setparam(getparam()) for things like I915_
drm/i915: Drop the CONTEXT_CLONE API (v2)
This API allows one context to grab bits out of another context upon creation. It can be used as a short-cut for setparam(getparam()) for things like I915_CONTEXT_PARAM_VM. However, it's never been used by any real userspace. It's used by a few IGT tests and that's it. Since it doesn't add any real value (most of the stuff you can CLONE you can copy in other ways), drop it.
There is one thing that this API allows you to clone which you cannot clone via getparam/setparam: timelines. However, timelines are an implementation detail of i915 and not really something that needs to be exposed to userspace. Also, sharing timelines between contexts isn't obviously useful and supporting it has the potential to complicate i915 internally. It also doesn't add any functionality that the client can't get in other ways. If a client really wants a shared timeline, they can use a syncobj and set it as an in and out fence on every submit.
v2 (Jason Ekstrand): - More detailed commit message
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-7-jason@jlekstrand.net
show more ...
|
#
8cc256a2 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Return void from context_apply_all
None of the callbacks we use with it return an error code anymore; they all return 0 unconditionally.
Signed-off-by: Jason Ekstrand <jason@jlekstran
drm/i915/gem: Return void from context_apply_all
None of the callbacks we use with it return an error code anymore; they all return 0 unconditionally.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-6-jason@jlekstrand.net
show more ...
|
#
677db6ad |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915/gem: Set the watchdog timeout directly in intel_context_set_gem (v2)
Instead of handling it like a context param, unconditionally set it when intel_contexts are created. For years we've ha
drm/i915/gem: Set the watchdog timeout directly in intel_context_set_gem (v2)
Instead of handling it like a context param, unconditionally set it when intel_contexts are created. For years we've had the idea of a watchdog uAPI floating about. The aim was for media, so that they could set very tight deadlines for their transcodes jobs, so that if you have a corrupt bitstream (especially for decoding) you don't hang your desktop too hard. But it's been stuck in limbo since forever, and this simplifies things a bit in preparation for the proto-context work. If we decide to actually make said uAPI a reality, we can do it through the proto- context easily enough.
This does mean that we move from reading the request_timeout_ms param once per engine when engines are created instead of once at context creation. If someone changes request_timeout_ms between creating a context and setting engines, it will mean that they get the new timeout. If someone races setting request_timeout_ms and context creation, they can theoretically end up with different timeouts. However, since both of these are fairly harmless and require changing kernel params, we don't care.
v2 (Tvrtko Ursulin): - Add a comment about races with request_timeout_ms
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-5-jason@jlekstrand.net
show more ...
|
#
6ff6d61d |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Drop I915_CONTEXT_PARAM_NO_ZEROMAP
The idea behind this param is to support OpenCL drivers with relocations because OpenCL reserves 0x0 for NULL and, if we placed memory there, it would co
drm/i915: Drop I915_CONTEXT_PARAM_NO_ZEROMAP
The idea behind this param is to support OpenCL drivers with relocations because OpenCL reserves 0x0 for NULL and, if we placed memory there, it would confuse CL kernels. It was originally sent out as part of a patch series including libdrm [1] and Beignet [2] support. However, the libdrm and Beignet patches never landed in their respective upstream projects so this API has never been used. It's never been used in Mesa or any other driver, either.
Dropping this API allows us to delete a small bit of code.
[1]: https://lists.freedesktop.org/archives/intel-gfx/2015-May/067030.html [2]: https://lists.freedesktop.org/archives/intel-gfx/2015-May/067031.html
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-4-jason@jlekstrand.net
show more ...
|
#
74e4b909 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Stop storing the ring size in the ring pointer (v3)
Previously, we were storing the ring size in the ring pointer before it was actually allocated. We would then guard setting the ring si
drm/i915: Stop storing the ring size in the ring pointer (v3)
Previously, we were storing the ring size in the ring pointer before it was actually allocated. We would then guard setting the ring size on checking for CONTEXT_ALLOC_BIT. This is error-prone at best and really only saves us a few bytes on something that already burns at least 4K. Instead, this patch adds a new ring_size field and makes everything use that.
v2 (Daniel Vetter): - Replace 512 * SZ_4K with SZ_2M
v2 (Jason Ekstrand): - Rebase on top of page migration code
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-3-jason@jlekstrand.net
show more ...
|
#
fe4751c3 |
| 08-Jul-2021 |
Jason Ekstrand <jason@jlekstrand.net> |
drm/i915: Drop I915_CONTEXT_PARAM_RINGSIZE
This reverts commit 88be76cdafc7 ("drm/i915: Allow userspace to specify ringsize on construction"). This API was originally added for OpenCL but the compu
drm/i915: Drop I915_CONTEXT_PARAM_RINGSIZE
This reverts commit 88be76cdafc7 ("drm/i915: Allow userspace to specify ringsize on construction"). This API was originally added for OpenCL but the compute-runtime PR has sat open for a year without action so we can still pull it out if we want. I argue we should drop it for three reasons:
1. If the compute-runtime PR has sat open for a year, this clearly isn't that important.
2. It's a very leaky API. Ring size is an implementation detail of the current execlist scheduler and really only makes sense there. It can't apply to the older ring-buffer scheduler on pre-execlist hardware because that's shared across all contexts and it won't apply to the GuC scheduler that's in the pipeline.
3. Having userspace set a ring size in bytes is a bad solution to the problem of having too small a ring. There is no way that userspace has the information to know how to properly set the ring size so it's just going to detect the feature and always set it to the maximum of 512K. This is what the compute-runtime PR does. The scheduler in i915, on the other hand, does have the information to make an informed choice. It could detect if the ring size is a problem and grow it itself. Or, if that's too hard, we could just increase the default size from 16K to 32K or even 64K instead of relying on userspace to do it.
Let's drop this API for now and, if someone decides they really care about solving this problem, they can do it properly.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-2-jason@jlekstrand.net
show more ...
|
Revision tags: v5.13, v5.10.46, v5.10.43 |
|
#
40e1956e |
| 05-Jun-2021 |
Lucas De Marchi <lucas.demarchi@intel.com> |
drm/i915/gem: replace IS_GEN and friends with GRAPHICS_VER
This was done by the following semantic patch:
@@ expression i915; @@ - INTEL_GEN(i915) + GRAPHICS_VER(i915)
@@ expression i915; expr
drm/i915/gem: replace IS_GEN and friends with GRAPHICS_VER
This was done by the following semantic patch:
@@ expression i915; @@ - INTEL_GEN(i915) + GRAPHICS_VER(i915)
@@ expression i915; expression E; @@ - INTEL_GEN(i915) >= E + GRAPHICS_VER(i915) >= E
@@ expression dev_priv; expression E; @@ - !IS_GEN(dev_priv, E) + GRAPHICS_VER(dev_priv) != E
@@ expression dev_priv; expression E; @@ - IS_GEN(dev_priv, E) + GRAPHICS_VER(dev_priv) == E
@@ expression dev_priv; expression from, until; @@ - IS_GEN_RANGE(dev_priv, from, until) + IS_GRAPHICS_VER(dev_priv, from, until)
@def@ expression E; identifier id =~ "^gen$"; @@ - id = GRAPHICS_VER(E) + ver = GRAPHICS_VER(E)
@@ identifier def.id; @@ - id + ver
It also takes care of renaming the variable we assign to GRAPHICS_VER() so to use "ver" rather than "gen".
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210605155356.4183026-4-lucas.demarchi@intel.com
show more ...
|
Revision tags: v5.10.42, v5.10.41, v5.10.40, v5.10.39, v5.4.119, v5.10.36, v5.10.35 |
|
#
c3b14760 |
| 04-May-2021 |
Matthew Auld <matthew.auld@intel.com> |
drm/i915: drop the __i915_active_call pointer packing
We use some of the lower bits of the retire function pointer for potential flags, which is quite thorny, since the caller needs to remember to g
drm/i915: drop the __i915_active_call pointer packing
We use some of the lower bits of the retire function pointer for potential flags, which is quite thorny, since the caller needs to remember to give the function the correct alignment with __i915_active_call, otherwise we might incorrectly unpack the pointer and jump to some garbage address later. Instead of all this let's just pass the flags along as a separate parameter.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Suggested-by: Daniel Vetter <daniel@ffwll.ch> References: ca419f407b43 ("drm/i915: Fix crash in auto_retire") References: d8e44e4dd221 ("drm/i915/overlay: Fix active retire callback alignment") References: fd5f262db118 ("drm/i915/selftests: Fix active retire callback alignment") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Brost <matthew.brost@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210504164136.96456-1-matthew.auld@intel.com
show more ...
|
Revision tags: v5.10.34, v5.4.116, v5.10.33, v5.12, v5.10.32, v5.10.31, v5.10.30, v5.10.27, v5.10.26 |
|
#
54d4e9f5 |
| 24-Mar-2021 |
Tvrtko Ursulin <tvrtko.ursulin@intel.com> |
drm/i915: Allow configuring default request expiry via modparam
Module parameter is added (request_timeout_ms) to allow configuring the default request/fence expiry.
Default value is inherited from
drm/i915: Allow configuring default request expiry via modparam
Module parameter is added (request_timeout_ms) to allow configuring the default request/fence expiry.
Default value is inherited from CONFIG_DRM_I915_REQUEST_TIMEOUT.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210324121335.2307063-8-tvrtko.ursulin@linux.intel.com
show more ...
|
#
e8dbb566 |
| 24-Mar-2021 |
Tvrtko Ursulin <tvrtko.ursulin@intel.com> |
drm/i915: Fail too long user submissions by default
A new Kconfig option CONFIG_DRM_I915_REQUEST_TIMEOUT is added, defaulting to 20s, and this timeout is applied to all users contexts using the prev
drm/i915: Fail too long user submissions by default
A new Kconfig option CONFIG_DRM_I915_REQUEST_TIMEOUT is added, defaulting to 20s, and this timeout is applied to all users contexts using the previously added watchdog facility.
Result of this is that any user submission will simply fail after this timeout, either causing a reset (for non-preemptable), or incomplete results.
This can have an effect that workloads which used to work fine will suddenly start failing. Even workloads comprised of short batches but in long dependency chains can be terminated.
And because of lack of agreement on usefulness and safety of fence error propagation this partial execution can be invisible to userspace even if it is "listening" to returned fence status.
Another interaction is with hangcheck where care needs to be taken timeout is not set lower or close to three times the heartbeat interval. Otherwise a hang in any application can cause complete termination of all submissions from unrelated clients. Any users modifying the per engine heartbeat intervals therefore need to be aware of this potential denial of service to avoid inadvertently enabling it.
Given all this I am personally not convinced the scheme is a good idea. Intuitively it feels object importers would be better positioned to enforce the time they are willing to wait for something to complete.
v2: * Improved commit message and Kconfig text. * Pull in some helper code from patch which got dropped.
v3: * Bump timeout to 20s to see if it helps Tigerlake.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210324121335.2307063-7-tvrtko.ursulin@linux.intel.com
show more ...
|
#
7dbc19da |
| 24-Mar-2021 |
Tvrtko Ursulin <tvrtko.ursulin@intel.com> |
drm/i915: Extract active lookup engine to a helper
Move active engine lookup to exported i915_request_active_engine.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Matthew Au
drm/i915: Extract active lookup engine to a helper
Move active engine lookup to exported i915_request_active_engine.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> [danvet: Slight rebase, engine->sched.lock is still called engine->active.lock.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210324121335.2307063-2-tvrtko.ursulin@linux.intel.com
show more ...
|
Revision tags: v5.10.25, v5.10.24, v5.10.23, v5.10.22, v5.10.21, v5.10.20, v5.10.19, v5.4.101, v5.10.18, v5.10.17, v5.11, v5.10.16, v5.10.15, v5.10.14 |
|
#
eb5c10cb |
| 20-Jan-2021 |
Chris Wilson <chris@chris-wilson.co.uk> |
drm/i915: Remove I915_USER_PRIORITY_SHIFT
As we do not have any internal priority levels, the priority can be set directed from the user values.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.u
drm/i915: Remove I915_USER_PRIORITY_SHIFT
As we do not have any internal priority levels, the priority can be set directed from the user values.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210120121439.17600-2-chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
show more ...
|
#
2d60d3e0 |
| 14-Jan-2021 |
Chris Wilson <chris@chris-wilson.co.uk> |
drm/i915/gem: Reduce ctx->engines_mutex for get_engines()
Take a snapshot of the ctx->engines, so we can avoid taking the ctx->engines_mutex for a mere read in get_engines().
Signed-off-by: Chris W
drm/i915/gem: Reduce ctx->engines_mutex for get_engines()
Take a snapshot of the ctx->engines, so we can avoid taking the ctx->engines_mutex for a mere read in get_engines().
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210114135612.13210-4-chris@chris-wilson.co.uk
show more ...
|
#
1a72e741 |
| 14-Jan-2021 |
Chris Wilson <chris@chris-wilson.co.uk> |
drm/i915/gem: Reduce ctx->engine_mutex for reading the clone source
When cloning the engines from the source context, we need to ensure that the engines are not freed as we copy them, and that the f
drm/i915/gem: Reduce ctx->engine_mutex for reading the clone source
When cloning the engines from the source context, we need to ensure that the engines are not freed as we copy them, and that the flags we clone from the source correspond with the engines we copy across. To do this we need only take a reference to the src->engines, rather than hold the src->engine_mutex, so long as we verify that nothing changed under the read.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210114135612.13210-3-chris@chris-wilson.co.uk
show more ...
|