[Intel-gfx] ✗ Fi.CI.BUILD: failure for linux-next: build failure after merge of the drm tree
== Series Details == Series: linux-next: build failure after merge of the drm tree URL : https://patchwork.freedesktop.org/series/77056/ State : failure == Summary == Applying: linux-next: build failure after merge of the drm tree error: sha1 information is lacking or useless (drivers/gpu/drm/i915/gt/intel_lrc.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 linux-next: build failure after merge of the drm tree When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
> -Original Message- > From: Jani Nikula > Sent: 08 May 2020 12:19 > To: Laxminarayan Bharadiya, Pankaj > ; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Joonas Lahtinen > ; Vivi, Rodrigo ; > David Airlie ; Ville Syrjälä > ; Chris > Wilson ; Deak, Imre ; > Maarten Lankhorst ; Laxminarayan > Bharadiya, Pankaj > Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over > WARN* > > On Mon, 04 May 2020, Pankaj Bharadiya > wrote: > > struct drm_device specific drm_WARN* macros include device information > > in the backtrace, so we know what device the warnings originate from. > > > > Prefer drm_WARN* over WARN* calls. > > > > changes since v1: > > - Added dev_priv local variable and used it in drm_WARN* calls (Jani) > > In the earlier patches you're adding i915 local variable, here it's dev_priv. > We're > gradually transitioning from dev_priv to i915, so I'm not thrilled about > adding > new dev_priv. dev_priv name is being used throughout the file. So to be consistent with rest of the code, I used dev_priv variable in this specific file. Shall I rename it to i915? I used i915 or dev_priv variable name based on what variable name being already used for struct drm_i915_private pointer in a given file. Thanks, Pankaj > > BR, > Jani. > > > > > > > Signed-off-by: Pankaj Bharadiya > > > > --- > > drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++--- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c > > b/drivers/gpu/drm/i915/display/intel_sdvo.c > > index bc6c26818e15..773523dcd107 100644 > > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c > > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd) static > > void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd, > >const void *args, int args_len) { > > + struct drm_i915_private *dev_priv = > > +to_i915(intel_sdvo->base.base.dev); > > const char *cmd_name; > > int i, pos = 0; > > char buffer[64]; > > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo > *intel_sdvo, u8 cmd, > > else > > BUF_PRINT("(%02X)", cmd); > > > > - WARN_ON(pos >= sizeof(buffer) - 1); > > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1); > > #undef BUF_PRINT > > > > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, > > buffer); @@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct > > intel_sdvo *intel_sdvo, u8 cmd, static bool intel_sdvo_read_response(struct > intel_sdvo *intel_sdvo, > > void *response, int response_len) { > > + struct drm_i915_private *dev_priv = > > +to_i915(intel_sdvo->base.base.dev); > > const char *cmd_status; > > u8 retry = 15; /* 5 quick checks, followed by 10 long checks */ > > u8 status; > > @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct > intel_sdvo *intel_sdvo, > > BUF_PRINT(" %02X", ((u8 *)response)[i]); > > } > > > > - WARN_ON(pos >= sizeof(buffer) - 1); > > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1); > > #undef BUF_PRINT > > > > DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer); > @@ > > -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct > intel_sdvo *intel_sdvo, > > struct intel_crtc_state > > *crtc_state, > > struct drm_connector_state > *conn_state) { > > + struct drm_i915_private *dev_priv = > > +to_i915(intel_sdvo->base.base.dev); > > struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi; > > const struct drm_display_mode *adjusted_mode = > > &crtc_state->hw.adjusted_mode; > > @@ -1106,7 +1109,7 @@ static bool > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, > > > HDMI_QUANTIZATION_RANGE_FULL); > > > > ret = hdmi_avi_infoframe_check(frame); > > - if (WARN_ON(ret)) > > + if (drm_WARN_ON(&dev_priv->drm, ret)) > > return false; > > > > return true; > > @@ -1115,6 +1118,7 @@ static bool > > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, static bool > intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, > > const struct intel_crtc_state > *crtc_state) { > > + struct drm_i915_private *dev_priv = > > +to_i915(intel_sdvo->base.base.dev); > > u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)]; > > const union hdmi_infoframe *frame = &crtc_state->infoframes.avi; > > ssize_t len; > > @@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct > intel_sdvo *intel_sdvo, > > intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0) > > return true; > > > > - if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI)) > > + if (d
[Intel-gfx] [PATCH 06/11] drm/syncobj: Allow use of dma-fence-proxy
Allow the callers to supply a dma-fence-proxy for asynchronous waiting on future fences. Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_syncobj.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 42d46414f767..e141db0e1eb6 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -184,6 +184,7 @@ */ #include +#include #include #include #include @@ -324,14 +325,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *old_fence; struct syncobj_wait_entry *cur, *tmp; - if (fence) - dma_fence_get(fence); - spin_lock(&syncobj->lock); - old_fence = rcu_dereference_protected(syncobj->fence, - lockdep_is_held(&syncobj->lock)); - rcu_assign_pointer(syncobj->fence, fence); + old_fence = dma_fence_replace_proxy(&syncobj->fence, fence); if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/11] dma-buf: Proxy fence, an unsignaled fence placeholder
Often we need to create a fence for a future event that has not yet been associated with a fence. We can store a proxy fence, a placeholder, in the timeline and replace it later when the real fence is known. Any listeners that attach to the proxy fence will automatically be signaled when the real fence completes, and any future listeners will instead be attach directly to the real fence avoiding any indirection overhead. Signed-off-by: Chris Wilson Cc: Lionel Landwerlin --- drivers/dma-buf/Makefile | 13 +- drivers/dma-buf/dma-fence-private.h | 20 + drivers/dma-buf/dma-fence-proxy.c| 248 ++ drivers/dma-buf/dma-fence.c | 4 +- drivers/dma-buf/selftests.h | 1 + drivers/dma-buf/st-dma-fence-proxy.c | 699 +++ include/linux/dma-fence-proxy.h | 34 ++ 7 files changed, 1015 insertions(+), 4 deletions(-) create mode 100644 drivers/dma-buf/dma-fence-private.h create mode 100644 drivers/dma-buf/dma-fence-proxy.c create mode 100644 drivers/dma-buf/st-dma-fence-proxy.c create mode 100644 include/linux/dma-fence-proxy.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 995e05f609ff..afaf6dadd9a3 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,6 +1,12 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ -dma-resv.o seqno-fence.o +obj-y := \ + dma-buf.o \ + dma-fence.o \ + dma-fence-array.o \ + dma-fence-chain.o \ + dma-fence-proxy.o \ + dma-resv.o \ + seqno-fence.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE)+= sync_file.o @@ -10,6 +16,7 @@ obj-$(CONFIG_UDMABUF) += udmabuf.o dmabuf_selftests-y := \ selftest.o \ st-dma-fence.o \ - st-dma-fence-chain.o + st-dma-fence-chain.o \ + st-dma-fence-proxy.o obj-$(CONFIG_DMABUF_SELFTESTS) += dmabuf_selftests.o diff --git a/drivers/dma-buf/dma-fence-private.h b/drivers/dma-buf/dma-fence-private.h new file mode 100644 index ..6924d28af0fa --- /dev/null +++ b/drivers/dma-buf/dma-fence-private.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Fence mechanism for dma-buf and to allow for asynchronous dma access + * + * Copyright (C) 2012 Canonical Ltd + * Copyright (C) 2012 Texas Instruments + * + * Authors: + * Rob Clark + * Maarten Lankhorst + */ + +#ifndef DMA_FENCE_PRIVATE_H +#define DMA_FENCE_PRIAVTE_H + +struct dma_fence; + +bool __dma_fence_enable_signaling(struct dma_fence *fence); + +#endif /* DMA_FENCE_PRIAVTE_H */ diff --git a/drivers/dma-buf/dma-fence-proxy.c b/drivers/dma-buf/dma-fence-proxy.c new file mode 100644 index ..f0cd89b966e0 --- /dev/null +++ b/drivers/dma-buf/dma-fence-proxy.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * dma-fence-proxy: placeholder unsignaled fence + * + * Copyright (C) 2017-2019 Intel Corporation + */ + +#include +#include +#include +#include +#include + +#include "dma-fence-private.h" + +struct dma_fence_proxy { + struct dma_fence base; + + struct dma_fence *real; + struct dma_fence_cb cb; + struct irq_work work; + + wait_queue_head_t wq; +}; + +#ifdef CONFIG_DEBUG_LOCK_ALLOC +#define same_lockclass(A, B) (A)->dep_map.key == (B)->dep_map.key +#else +#define same_lockclass(A, B) 0 +#endif + +static const char *proxy_get_driver_name(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + + return real ? real->ops->get_driver_name(real) : "proxy"; +} + +static const char *proxy_get_timeline_name(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + + return real ? real->ops->get_timeline_name(real) : "unset"; +} + +static void proxy_irq_work(struct irq_work *work) +{ + struct dma_fence_proxy *p = container_of(work, typeof(*p), work); + + dma_fence_signal(&p->base); + dma_fence_put(&p->base); +} + +static void proxy_callback(struct dma_fence *real, struct dma_fence_cb *cb) +{ + struct dma_fence_proxy *p = container_of(cb, typeof(*p), cb); + + if (real->error) + dma_fence_set_error(&p->base, real->error); + + /* Lower the height of the proxy chain -> single stack frame */ + irq_work_queue(&p->work); +} + +static bool proxy_enable_signaling(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + bool ret = true; + + if (real) { + spin_lock_nested(real->lock, +same_lockclass(&p->wq.lock, real->lock)); + ret = __dma_fence_enable_si
[Intel-gfx] [PATCH 07/11] drm/i915/gem: Teach execbuf how to wait on future syncobj
If a syncobj has not yet been assigned, treat it as a future fence and install and wait upon a dma-fence-proxy. The proxy will be replace by the real fence later, and that fence will be responsible for signaling our waiter. Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 21 ++- drivers/gpu/drm/i915/gt/intel_lrc.c | 3 + drivers/gpu/drm/i915/i915_request.c | 135 ++ drivers/gpu/drm/i915/i915_scheduler.c | 41 ++ drivers/gpu/drm/i915/i915_scheduler.h | 3 + 5 files changed, 201 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index d54a4933cc05..199131db200f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -2524,8 +2525,24 @@ await_fence_array(struct i915_execbuffer *eb, continue; fence = drm_syncobj_fence_get(syncobj); - if (!fence) - return -EINVAL; + if (!fence) { + struct dma_fence *old; + + fence = dma_fence_create_proxy(); + if (!fence) + return -ENOMEM; + + spin_lock(&syncobj->lock); + old = rcu_dereference_protected(syncobj->fence, true); + if (unlikely(old)) { + dma_fence_put(fence); + fence = dma_fence_get(old); + } else { + rcu_assign_pointer(syncobj->fence, + dma_fence_get(fence)); + } + spin_unlock(&syncobj->lock); + } err = i915_request_await_dma_fence(eb->request, fence); dma_fence_put(fence); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 400b9b5a6882..4792fa26a6c3 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3492,6 +3492,9 @@ static int gen8_emit_init_breadcrumb(struct i915_request *rq) { u32 *cs; + /* Seal the semaphore section -- we are ready to begin */ + rq->sched.semaphores |= ALL_ENGINES; + if (!i915_request_timeline(rq)->has_initial_breadcrumb) return 0; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index f0f9393e2ade..4f1d8cc2d5af 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -378,6 +379,7 @@ static bool fatal_error(int error) case 0: /* not an error! */ case -EAGAIN: /* innocent victim of a GT reset (__i915_request_reset) */ case -ETIMEDOUT: /* waiting for Godot (timer_i915_sw_fence_wake) */ + case -EDEADLK: /* cyclic fence lockup (await_proxy) */ return false; default: return true; @@ -1101,6 +1103,137 @@ i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) I915_FENCE_GFP); } +struct await_proxy { + struct wait_queue_entry base; + struct i915_request *request; + struct dma_fence *fence; + struct timer_list timer; + struct work_struct work; + int (*attach)(struct await_proxy *ap); + void *data; +}; + +static void await_proxy_work(struct work_struct *work) +{ + struct await_proxy *ap = container_of(work, typeof(*ap), work); + struct i915_request *rq = ap->request; + + del_timer_sync(&ap->timer); + + if (ap->fence) { + int err = 0; + + /* +* If the fence is external, we impose a 10s timeout. +* However, if the fence is internal, we skip a timeout in +* the belief that all fences are in-order (DAG, no cycles) +* and we can enforce forward progress by reset the GPU if +* necessary. A future fence, provided userspace, can trivially +* generate a cycle in the dependency graph, and so cause +* that entire cycle to become deadlocked and for no forward +* progress to either be made, and the driver being kept +* eternally awake. +*/ + if (dma_fence_is_i915(ap->fence) && + !i915_sched_node_verify_dag(&rq->sched, + &to_request(ap->fence)->sched)) + err = -EDEADLK; + + if (!err) { +
[Intel-gfx] [PATCH 03/11] drm/i915: Prevent using semaphores to chain up to external fences
The downside of using semaphores is that we lose metadata passing along the signaling chain. This is particularly nasty when we need to pass along a fatal error such as EFAULT or EDEADLK. For fatal errors we want to scrub the request before it is executed, which means that we cannot preload the request onto HW and have it wait upon a semaphore. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 26 + drivers/gpu/drm/i915/i915_scheduler_types.h | 1 + 2 files changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 94189c7d43cd..f0f9393e2ade 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1002,6 +1002,15 @@ emit_semaphore_wait(struct i915_request *to, if (!rcu_access_pointer(from->hwsp_cacheline)) goto await_fence; + /* +* If this or its dependents are waiting on an external fence +* that may fail catastrophically, then we want to avoid using +* sempahores as they bypass the fence signaling metadata, and we +* lose the fence->error propagation. +*/ + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) + goto await_fence; + /* Just emit the first semaphore we see as request space is limited. */ if (already_busywaiting(to) & mask) goto await_fence; @@ -1064,12 +1073,29 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return ret; } + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) + to->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; + return 0; } +static void mark_external(struct i915_request *rq) +{ + /* +* The downside of using semaphores is that we lose metadata passing +* along the signaling chain. This is particularly nasty when we +* need to pass along a fatal error such as EFAULT or EDEADLK. For +* fatal errors we want to scrub the request before it is executed, +* which means that we cannot preload the request onto HW and have +* it wait upon a semaphore. +*/ + rq->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; +} + static int i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) { + mark_external(rq); return i915_sw_fence_await_dma_fence(&rq->submit, fence, fence->context ? I915_FENCE_TIMEOUT : 0, I915_FENCE_GFP); diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 7186875088a0..6ab2c5289bed 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -66,6 +66,7 @@ struct i915_sched_node { struct i915_sched_attr attr; unsigned int flags; #define I915_SCHED_HAS_SEMAPHORE_CHAIN BIT(0) +#define I915_SCHED_HAS_EXTERNAL_CHAIN BIT(1) intel_engine_mask_t semaphores; }; -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/11] drm/i915/gt: Declare when we enabled timeslicing
Let userspace know if they can trust timeslicing by including it as part of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING v2: Only declare timeslicing if we can safely preempt userspace. Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing") Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3802 Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson Cc: Kenneth Graunke Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/gt/intel_engine_user.c | 1 + include/uapi/drm/i915_drm.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 848decee9066..8415511f1465 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -98,6 +98,7 @@ static void set_scheduler_caps(struct drm_i915_private *i915) MAP(HAS_PREEMPTION, PREEMPTION), MAP(HAS_SEMAPHORES, SEMAPHORES), MAP(SUPPORTS_STATS, ENGINE_BUSY_STATS), + MAP(HAS_TIMESLICES, TIMESLICING), #undef MAP }; struct intel_engine_cs *engine; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 704dd0e3bc1d..1ee227b5131a 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -523,6 +523,7 @@ typedef struct drm_i915_irq_wait { #define I915_SCHEDULER_CAP_PREEMPTION(1ul << 2) #define I915_SCHEDULER_CAP_SEMAPHORES(1ul << 3) #define I915_SCHEDULER_CAP_ENGINE_BUSY_STATS (1ul << 4) +#define I915_SCHEDULER_CAP_TIMESLICING (1ul << 5) #define I915_PARAM_HUC_STATUS 42 -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/11] drm/i915: Ignore submit-fences on the same timeline
While we ordinarily do not skip submit-fences due to the accompanying hook that we want to callback on execution, a submit-fence on the same timeline is meaningless. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_request.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 589739bfee25..be2ce9065a29 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1242,6 +1242,9 @@ i915_request_await_execution(struct i915_request *rq, continue; } + if (fence->context == rq->fence.context) + continue; + /* * We don't squash repeated fence dependencies here as we * want to run our callback in all cases. -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/11] drm/i915: Move saturated workload detection back to the context
When we introduced the saturated workload detection to tell us to back off from semaphore usage [semaphores have a noticeable impact on contended bus cycles with the CPU for some heavy workloads], we first introduced it as a per-context tracker. This allows individual contexts to try and optimise their own usage, but we found that with the local tracking and the no-semaphore boosting, the first context to disable semaphores got a massive priority boost and so would starve the rest and all new contexts (as they started with semaphores enabled and lower priority). Hence we moved the saturated workload detection to the engine, and a consequence had to disable semaphores on virtual engines. Now that we do not have semaphore priority boosting, we can move the tracking back to the context and virtual engines can now utilise the faster inter-engine synchronisation. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_context.c | 1 + drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 -- drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 -- drivers/gpu/drm/i915/gt/intel_lrc.c | 15 --- drivers/gpu/drm/i915/i915_request.c | 4 ++-- 6 files changed, 5 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index e4aece20bc80..762a251d553b 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -268,6 +268,7 @@ static int __intel_context_active(struct i915_active *active) if (err) goto err_timeline; + ce->saturated = 0; return 0; err_timeline: diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 4954b0df4864..aed26d93c2ca 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -78,6 +78,8 @@ struct intel_context { } lrc; u32 tag; /* cookie passed to HW to track this context on submission */ + intel_engine_mask_t saturated; /* submitting semaphores too late? */ + /* Time on GPU as tracked by the hw. */ struct { struct ewma_runtime avg; diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index d0a1078ef632..6d7fdba5adef 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -229,8 +229,6 @@ static int __engine_park(struct intel_wakeref *wf) struct intel_engine_cs *engine = container_of(wf, typeof(*engine), wakeref); - engine->saturated = 0; - /* * If one and only one request is completed between pm events, * we know that we are inside the kernel context and it is diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index c113b7805e65..2b1232a233bc 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -332,8 +332,6 @@ struct intel_engine_cs { struct intel_context *kernel_context; /* pinned */ - intel_engine_mask_t saturated; /* submitting semaphores too late? */ - struct { struct delayed_work work; struct i915_request *systole; diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index fd0cb419fc11..e9336547212b 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -5582,21 +5582,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, ve->base.instance = I915_ENGINE_CLASS_INVALID_VIRTUAL; ve->base.uabi_instance = I915_ENGINE_CLASS_INVALID_VIRTUAL; - /* -* The decision on whether to submit a request using semaphores -* depends on the saturated state of the engine. We only compute -* this during HW submission of the request, and we need for this -* state to be globally applied to all requests being submitted -* to this engine. Virtual engines encompass more than one physical -* engine and so we cannot accurately tell in advance if one of those -* engines is already saturated and so cannot afford to use a semaphore -* and be pessimized in priority for doing so -- if we are the only -* context using semaphores after all other clients have stopped, we -* will be starved on the saturated system. Such a global switch for -* semaphores is less than ideal, but alas is the current compromise. -*/ - ve->base.saturated = ALL_ENGINES; - snprintf(ve->base.name, sizeof(ve->base.name), "virtual"); intel_engine_init_active(&ve->base, ENGINE_VIRTUAL); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 87ae6
[Intel-gfx] [PATCH 08/11] drm/i915/gem: Allow combining submit-fences with syncobj
We allow exported sync_file fences to be used as submit fences, but they are not the only source of user fences. We also accept an array of syncobj, and as with sync_file these are dma_fences underneath and so feature the same set of controls. The submit-fence allows for a request to be scheduled at the same time as the signaler, rather than as normal after. Userspace can combine submit-fence with its own semaphores for intra-batch scheduling. Not exposing submit-fences to syncobj was at the time just a matter of pragmatic expediency. Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Lionel Landwerlin Reviewed-by: Tvrtko Ursulin --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 14 +++ drivers/gpu/drm/i915/i915_request.c | 24 +++ include/uapi/drm/i915_drm.h | 7 +++--- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 199131db200f..6368f0070157 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2432,7 +2432,7 @@ static void __free_fence_array(struct drm_syncobj **fences, unsigned int n) { while (n--) - drm_syncobj_put(ptr_mask_bits(fences[n], 2)); + drm_syncobj_put(ptr_mask_bits(fences[n], 3)); kvfree(fences); } @@ -2489,7 +2489,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) & ~__I915_EXEC_FENCE_UNKNOWN_FLAGS); - fences[n] = ptr_pack_bits(syncobj, fence.flags, 2); + fences[n] = ptr_pack_bits(syncobj, fence.flags, 3); } return fences; @@ -2520,7 +2520,7 @@ await_fence_array(struct i915_execbuffer *eb, struct dma_fence *fence; unsigned int flags; - syncobj = ptr_unpack_bits(fences[n], &flags, 2); + syncobj = ptr_unpack_bits(fences[n], &flags, 3); if (!(flags & I915_EXEC_FENCE_WAIT)) continue; @@ -2544,7 +2544,11 @@ await_fence_array(struct i915_execbuffer *eb, spin_unlock(&syncobj->lock); } - err = i915_request_await_dma_fence(eb->request, fence); + if (flags & I915_EXEC_FENCE_WAIT_SUBMIT) + err = i915_request_await_execution(eb->request, fence, + eb->engine->bond_execute); + else + err = i915_request_await_dma_fence(eb->request, fence); dma_fence_put(fence); if (err < 0) return err; @@ -2565,7 +2569,7 @@ signal_fence_array(struct i915_execbuffer *eb, struct drm_syncobj *syncobj; unsigned int flags; - syncobj = ptr_unpack_bits(fences[n], &flags, 2); + syncobj = ptr_unpack_bits(fences[n], &flags, 3); if (!(flags & I915_EXEC_FENCE_SIGNAL)) continue; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 4f1d8cc2d5af..6afd77e6f141 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1382,6 +1382,26 @@ __i915_request_await_execution(struct i915_request *to, &from->fence); } +static int execution_proxy(struct await_proxy *ap) +{ + return i915_request_await_execution(ap->request, ap->fence, ap->data); +} + +static int +i915_request_await_proxy_execution(struct i915_request *rq, + struct dma_fence *fence, + void (*hook)(struct i915_request *rq, + struct dma_fence *signal)) +{ + /* +* We have to wait until the real request is known in order to +* be able to hook into its execution, as opposed to waiting for +* its completion. +*/ + return __i915_request_await_proxy(rq, fence, I915_FENCE_TIMEOUT, + execution_proxy, hook); +} + int i915_request_await_execution(struct i915_request *rq, struct dma_fence *fence, @@ -1421,6 +1441,10 @@ i915_request_await_execution(struct i915_request *rq, ret = __i915_request_await_execution(rq, to_request(fence), hook); + else if (dma_fence_is_proxy(fence)) + ret = i915_request_await_proxy_execution(rq, +
[Intel-gfx] [PATCH 04/11] drm/i915: Tidy awaiting on dma-fences
Just tidy up the return handling for completed dma-fences. While it may return errors for invalid fence, we already know that we have a good fence and the only error will be an already signaled fence. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_sw_fence.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 7daf81f55c90..295b9829e2da 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, cb->fence = fence; i915_sw_fence_await(fence); - ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake); - if (ret == 0) { - ret = 1; - } else { + ret = 1; + if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) { + /* fence already signaled */ __dma_i915_sw_fence_wake(dma, &cb->base); - if (ret == -ENOENT) /* fence already signaled */ - ret = 0; + ret = 0; } return ret; -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/11] drm/i915: Pull waiting on an external dma-fence into its routine
As a means for a small code consolidation, but primarily to start thinking more carefully about internal-vs-external linkage, pull the pair of i915_sw_fence_await_dma_fence() calls into a common routine. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index be2ce9065a29..94189c7d43cd 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return 0; } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + return i915_sw_fence_await_dma_fence(&rq->submit, fence, +fence->context ? I915_FENCE_TIMEOUT : 0, +I915_FENCE_GFP); +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) if (dma_fence_is_i915(fence)) ret = i915_request_await_request(rq, to_request(fence)); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - fence->context ? I915_FENCE_TIMEOUT : 0, - I915_FENCE_GFP); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; @@ -1255,9 +1261,7 @@ i915_request_await_execution(struct i915_request *rq, to_request(fence), hook); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - I915_FENCE_TIMEOUT, - GFP_KERNEL); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; } while (--nchild); -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/11] drm/i915: Drop no-semaphore boosting
Now that we have fast timeslicing on semaphores, we no longer need to prioritise none-semaphore work as we will yield any work blocked on a sempahore to the next in the queue. Previously with no timeslicing, blocking on the semaphore caused extremely bad scheduling with multiple clients utilising multiple rings. Now, there is no impact and we can remove the complication. Signed-off-by: Chris Wilson --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 15 drivers/gpu/drm/i915/gt/intel_lrc.c | 9 - drivers/gpu/drm/i915/i915_priolist_types.h| 4 +-- drivers/gpu/drm/i915/i915_request.c | 36 +++ drivers/gpu/drm/i915/i915_scheduler.c | 7 +--- drivers/gpu/drm/i915/i915_scheduler_types.h | 3 +- 6 files changed, 7 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 6368f0070157..f7261cf11eed 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2603,21 +2603,6 @@ static void eb_request_add(struct i915_execbuffer *eb) /* Check that the context wasn't destroyed before submission */ if (likely(!intel_context_is_closed(eb->context))) { attr = eb->gem_context->sched; - - /* -* Boost actual workloads past semaphores! -* -* With semaphores we spin on one engine waiting for another, -* simply to reduce the latency of starting our work when -* the signaler completes. However, if there is any other -* work that we could be doing on this engine instead, that -* is better utilisation and will reduce the overall duration -* of the current work. To avoid PI boosting a semaphore -* far in the distance past over useful work, we keep a history -* of any semaphore use along our dependency chain. -*/ - if (!(rq->sched.flags & I915_SCHED_HAS_SEMAPHORE_CHAIN)) - attr.priority |= I915_PRIORITY_NOSEMAPHORE; } else { /* Serialise with context_close via the add_to_timeline */ i915_request_set_error_once(rq, -ENOENT); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 4792fa26a6c3..fd0cb419fc11 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -429,15 +429,6 @@ static int effective_prio(const struct i915_request *rq) if (i915_request_has_nopreempt(rq)) prio = I915_PRIORITY_UNPREEMPTABLE; - /* -* On unwinding the active request, we give it a priority bump -* if it has completed waiting on any semaphore. If we know that -* the request has already started, we can prevent an unwanted -* preempt-to-idle cycle by taking that into account now. -*/ - if (__i915_request_has_started(rq)) - prio |= I915_PRIORITY_NOSEMAPHORE; - return prio; } diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h b/drivers/gpu/drm/i915/i915_priolist_types.h index e18723d8df86..5003a71113cb 100644 --- a/drivers/gpu/drm/i915/i915_priolist_types.h +++ b/drivers/gpu/drm/i915/i915_priolist_types.h @@ -24,14 +24,12 @@ enum { I915_PRIORITY_DISPLAY, }; -#define I915_USER_PRIORITY_SHIFT 1 +#define I915_USER_PRIORITY_SHIFT 0 #define I915_USER_PRIORITY(x) ((x) << I915_USER_PRIORITY_SHIFT) #define I915_PRIORITY_COUNT BIT(I915_USER_PRIORITY_SHIFT) #define I915_PRIORITY_MASK (I915_PRIORITY_COUNT - 1) -#define I915_PRIORITY_NOSEMAPHORE ((u8)BIT(0)) - /* Smallest priority value that cannot be bumped. */ #define I915_PRIORITY_INVALID (INT_MIN | (u8)I915_PRIORITY_MASK) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 6afd77e6f141..87ae67fc5022 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -368,8 +368,6 @@ __await_execution(struct i915_request *rq, } spin_unlock_irq(&signal->lock); - /* Copy across semaphore status as we need the same behaviour */ - rq->sched.flags |= signal->sched.flags; return 0; } @@ -538,10 +536,8 @@ void __i915_request_unsubmit(struct i915_request *request) spin_unlock(&request->lock); /* We've already spun, don't charge on resubmitting. */ - if (request->sched.semaphores && i915_request_started(request)) { - request->sched.attr.priority |= I915_PRIORITY_NOSEMAPHORE; + if (request->sched.semaphores && i915_request_started(request)) request->sched.semaphores = 0; - } /* * We don't need to wake_up any waiters on request->execute, they @@ -599,15 +595,6 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_no
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77060/ State : warning == Summary == $ dim checkpatch origin/drm-tip 9fb07acb437e drm/i915: Ignore submit-fences on the same timeline 158d8bd69a3b drm/i915: Pull waiting on an external dma-fence into its routine ee0daf3babfc drm/i915: Prevent using semaphores to chain up to external fences 526e31404a68 drm/i915: Tidy awaiting on dma-fences 98775f5d4e71 dma-buf: Proxy fence, an unsignaled fence placeholder -:45: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #45: new file mode 100644 -:380: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment #380: FILE: drivers/dma-buf/st-dma-fence-proxy.c:20: + spinlock_t lock; -:540: WARNING:MEMORY_BARRIER: memory barrier without comment #540: FILE: drivers/dma-buf/st-dma-fence-proxy.c:180: + smp_store_mb(container_of(cb, struct simple_cb, cb)->seen, true); total: 0 errors, 2 warnings, 1 checks, 1043 lines checked d61994aed2e1 drm/syncobj: Allow use of dma-fence-proxy 511354e09395 drm/i915/gem: Teach execbuf how to wait on future syncobj 6e1e40d0faf8 drm/i915/gem: Allow combining submit-fences with syncobj c0b499a14f32 drm/i915/gt: Declare when we enabled timeslicing 0431602bf7c8 drm/i915: Drop no-semaphore boosting 24152ce78a17 drm/i915: Move saturated workload detection back to the context ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Take the atomic toys away from X
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On Thu, 2019-09-05 at 20:53 +0200, Daniel Vetter wrote: > The -modesetting ddx has a totally broken idea of how atomic works: > - doesn't disable old connectors, assuming they get auto-disable like > with the legacy setcrtc > - assumes ASYNC_FLIP is wired through for the atomic ioctl > - not a single call to TEST_ONLY > > Iow the implementation is a 1:1 translation of legacy ioctls to > atomic, which is a) broken b) pointless. > > We already have bugs in both i915 and amdgpu-DC where this prevents us > from enabling neat features. > > If anyone ever cares about atomic in X we can easily add a new atomic > level (req->value == 2) for X to get back the shiny toys. > > Since these broken versions of -modesetting have been shipping, > there's really no other way to get out of this bind. Hi Daniel and Greg (especially). It seems that this patch was never applied to stable, maybe it fell through the cracks? It doesn't apply as-is in 4.19 branch but a small change in the context makes it apply. I'm experiencing issues with lightdm and vt-switch in Debian Buster (which has a 4.19 kernel) so I'd appreciate if the patch was included in at least that release. Regards, - -- Yves-Alexis -BEGIN PGP SIGNATURE- iQEzBAEBCAAdFiEE8vi34Qgfo83x35gF3rYcyPpXRFsFAl61ITAACgkQ3rYcyPpX RFvlaAf9HZ0DTX1fAkNeNFoAgn4pFztnFq0fAwGj5iVIL4q6upE1wE3E8cDgUHeT maQQvL3YHFXjgzgDHYNIuUMipFE1Djymoy+EB4ZoOftqsJ4CPy4pCMUAh57u7BrV T+eBtj4n0wY0SgvoPism3QdbxY7CLLgCMJKLNrCPlkDCdJyGsZX9RIgfqvbkGM36 ftwBKcyy1iW5cAv10ehiXi/1zszA8bx2gULim3abcSjjz12ckNvBPy/BDvfFx19V 8cGgG3qD9PLmxRl80H1/mX30Ddw8Md5Fu7I/ndh3EGXLu8p8zod0rQVCQjAEW4X4 ew4tajDD2l9vWzN0sZIlyjq9fNgXBw== =lPBO -END PGP SIGNATURE- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77060/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8450 -> Patchwork_17608 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_17608 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_17608, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_17608: ### IGT changes ### Possible regressions * igt@i915_selftest@live@gt_contexts: - fi-kbl-guc: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-kbl-guc/igt@i915_selftest@live@gt_contexts.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-kbl-guc/igt@i915_selftest@live@gt_contexts.html - fi-bsw-kefka: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bsw-kefka/igt@i915_selftest@live@gt_contexts.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-bsw-kefka/igt@i915_selftest@live@gt_contexts.html - fi-bdw-5557u: [PASS][5] -> [INCOMPLETE][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bdw-5557u/igt@i915_selftest@live@gt_contexts.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-bdw-5557u/igt@i915_selftest@live@gt_contexts.html - fi-kbl-8809g: [PASS][7] -> [INCOMPLETE][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-kbl-8809g/igt@i915_selftest@live@gt_contexts.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-kbl-8809g/igt@i915_selftest@live@gt_contexts.html - fi-skl-guc: [PASS][9] -> [INCOMPLETE][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-skl-guc/igt@i915_selftest@live@gt_contexts.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-skl-guc/igt@i915_selftest@live@gt_contexts.html - fi-kbl-r: [PASS][11] -> [INCOMPLETE][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-kbl-r/igt@i915_selftest@live@gt_contexts.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-kbl-r/igt@i915_selftest@live@gt_contexts.html - fi-cfl-8109u: [PASS][13] -> [INCOMPLETE][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-cfl-8109u/igt@i915_selftest@live@gt_contexts.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-cfl-8109u/igt@i915_selftest@live@gt_contexts.html - fi-apl-guc: [PASS][15] -> [INCOMPLETE][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-apl-guc/igt@i915_selftest@live@gt_contexts.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-apl-guc/igt@i915_selftest@live@gt_contexts.html - fi-bxt-dsi: [PASS][17] -> [INCOMPLETE][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bxt-dsi/igt@i915_selftest@live@gt_contexts.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-bxt-dsi/igt@i915_selftest@live@gt_contexts.html - fi-icl-y: [PASS][19] -> [INCOMPLETE][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-icl-y/igt@i915_selftest@live@gt_contexts.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-icl-y/igt@i915_selftest@live@gt_contexts.html - fi-bsw-nick:[PASS][21] -> [INCOMPLETE][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bsw-nick/igt@i915_selftest@live@gt_contexts.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-bsw-nick/igt@i915_selftest@live@gt_contexts.html - fi-whl-u: [PASS][23] -> [INCOMPLETE][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-whl-u/igt@i915_selftest@live@gt_contexts.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-whl-u/igt@i915_selftest@live@gt_contexts.html - fi-skl-6600u: [PASS][25] -> [INCOMPLETE][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-skl-6600u/igt@i915_selftest@live@gt_contexts.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-skl-6600u/igt@i915_selftest@live@gt_contexts.html - fi-icl-u2: [PASS][27] -> [INCOMPLETE][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-icl-u2/igt@i915_selftest@live@gt_contexts.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/fi-icl-u2/igt@i915_selftest@live@gt_contex
[Intel-gfx] ✗ Fi.CI.BUILD: warning for series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [01/11] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77060/ State : warning == Summary == CALLscripts/checksyscalls.sh CALLscripts/atomic/check-atomics.sh CHK include/generated/compile.h CC [M] drivers/gpu/drm/i915/i915_scheduler.o drivers/gpu/drm/i915/i915_scheduler.c: In function ‘assert_priolists’: drivers/gpu/drm/i915/i915_scheduler.c:54:52: error: integer overflow in expression [-Werror=overflow] last_prio = (INT_MAX >> I915_USER_PRIORITY_SHIFT) + 1l; ^ cc1: all warnings being treated as errors scripts/Makefile.build:266: recipe for target 'drivers/gpu/drm/i915/i915_scheduler.o' failed make[4]: *** [drivers/gpu/drm/i915/i915_scheduler.o] Error 1 scripts/Makefile.build:488: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:488: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:488: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:1722: recipe for target 'drivers' failed make: *** [drivers] Error 2 == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17608/build_32bit.log ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/9] drm/i915: Prevent using semaphores to chain up to external fences
The downside of using semaphores is that we lose metadata passing along the signaling chain. This is particularly nasty when we need to pass along a fatal error such as EFAULT or EDEADLK. For fatal errors we want to scrub the request before it is executed, which means that we cannot preload the request onto HW and have it wait upon a semaphore. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 26 + drivers/gpu/drm/i915/i915_scheduler_types.h | 1 + 2 files changed, 27 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 94189c7d43cd..f0f9393e2ade 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1002,6 +1002,15 @@ emit_semaphore_wait(struct i915_request *to, if (!rcu_access_pointer(from->hwsp_cacheline)) goto await_fence; + /* +* If this or its dependents are waiting on an external fence +* that may fail catastrophically, then we want to avoid using +* sempahores as they bypass the fence signaling metadata, and we +* lose the fence->error propagation. +*/ + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) + goto await_fence; + /* Just emit the first semaphore we see as request space is limited. */ if (already_busywaiting(to) & mask) goto await_fence; @@ -1064,12 +1073,29 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return ret; } + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) + to->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; + return 0; } +static void mark_external(struct i915_request *rq) +{ + /* +* The downside of using semaphores is that we lose metadata passing +* along the signaling chain. This is particularly nasty when we +* need to pass along a fatal error such as EFAULT or EDEADLK. For +* fatal errors we want to scrub the request before it is executed, +* which means that we cannot preload the request onto HW and have +* it wait upon a semaphore. +*/ + rq->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; +} + static int i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) { + mark_external(rq); return i915_sw_fence_await_dma_fence(&rq->submit, fence, fence->context ? I915_FENCE_TIMEOUT : 0, I915_FENCE_GFP); diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 7186875088a0..6ab2c5289bed 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -66,6 +66,7 @@ struct i915_sched_node { struct i915_sched_attr attr; unsigned int flags; #define I915_SCHED_HAS_SEMAPHORE_CHAIN BIT(0) +#define I915_SCHED_HAS_EXTERNAL_CHAIN BIT(1) intel_engine_mask_t semaphores; }; -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915/gt: Declare when we enabled timeslicing
Let userspace know if they can trust timeslicing by including it as part of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING v2: Only declare timeslicing if we can safely preempt userspace. Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing") Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3802 Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson Cc: Kenneth Graunke Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/gt/intel_engine_user.c | 1 + include/uapi/drm/i915_drm.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 848decee9066..8415511f1465 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -98,6 +98,7 @@ static void set_scheduler_caps(struct drm_i915_private *i915) MAP(HAS_PREEMPTION, PREEMPTION), MAP(HAS_SEMAPHORES, SEMAPHORES), MAP(SUPPORTS_STATS, ENGINE_BUSY_STATS), + MAP(HAS_TIMESLICES, TIMESLICING), #undef MAP }; struct intel_engine_cs *engine; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 704dd0e3bc1d..1ee227b5131a 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -523,6 +523,7 @@ typedef struct drm_i915_irq_wait { #define I915_SCHEDULER_CAP_PREEMPTION(1ul << 2) #define I915_SCHEDULER_CAP_SEMAPHORES(1ul << 3) #define I915_SCHEDULER_CAP_ENGINE_BUSY_STATS (1ul << 4) +#define I915_SCHEDULER_CAP_TIMESLICING (1ul << 5) #define I915_PARAM_HUC_STATUS 42 -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915/gem: Teach execbuf how to wait on future syncobj
If a syncobj has not yet been assigned, treat it as a future fence and install and wait upon a dma-fence-proxy. The proxy will be replace by the real fence later, and that fence will be responsible for signaling our waiter. Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 21 ++- drivers/gpu/drm/i915/gt/intel_lrc.c | 3 + drivers/gpu/drm/i915/i915_request.c | 135 ++ drivers/gpu/drm/i915/i915_scheduler.c | 41 ++ drivers/gpu/drm/i915/i915_scheduler.h | 3 + 5 files changed, 201 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index d54a4933cc05..199131db200f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -2524,8 +2525,24 @@ await_fence_array(struct i915_execbuffer *eb, continue; fence = drm_syncobj_fence_get(syncobj); - if (!fence) - return -EINVAL; + if (!fence) { + struct dma_fence *old; + + fence = dma_fence_create_proxy(); + if (!fence) + return -ENOMEM; + + spin_lock(&syncobj->lock); + old = rcu_dereference_protected(syncobj->fence, true); + if (unlikely(old)) { + dma_fence_put(fence); + fence = dma_fence_get(old); + } else { + rcu_assign_pointer(syncobj->fence, + dma_fence_get(fence)); + } + spin_unlock(&syncobj->lock); + } err = i915_request_await_dma_fence(eb->request, fence); dma_fence_put(fence); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 400b9b5a6882..4792fa26a6c3 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3492,6 +3492,9 @@ static int gen8_emit_init_breadcrumb(struct i915_request *rq) { u32 *cs; + /* Seal the semaphore section -- we are ready to begin */ + rq->sched.semaphores |= ALL_ENGINES; + if (!i915_request_timeline(rq)->has_initial_breadcrumb) return 0; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index f0f9393e2ade..4f1d8cc2d5af 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -378,6 +379,7 @@ static bool fatal_error(int error) case 0: /* not an error! */ case -EAGAIN: /* innocent victim of a GT reset (__i915_request_reset) */ case -ETIMEDOUT: /* waiting for Godot (timer_i915_sw_fence_wake) */ + case -EDEADLK: /* cyclic fence lockup (await_proxy) */ return false; default: return true; @@ -1101,6 +1103,137 @@ i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) I915_FENCE_GFP); } +struct await_proxy { + struct wait_queue_entry base; + struct i915_request *request; + struct dma_fence *fence; + struct timer_list timer; + struct work_struct work; + int (*attach)(struct await_proxy *ap); + void *data; +}; + +static void await_proxy_work(struct work_struct *work) +{ + struct await_proxy *ap = container_of(work, typeof(*ap), work); + struct i915_request *rq = ap->request; + + del_timer_sync(&ap->timer); + + if (ap->fence) { + int err = 0; + + /* +* If the fence is external, we impose a 10s timeout. +* However, if the fence is internal, we skip a timeout in +* the belief that all fences are in-order (DAG, no cycles) +* and we can enforce forward progress by reset the GPU if +* necessary. A future fence, provided userspace, can trivially +* generate a cycle in the dependency graph, and so cause +* that entire cycle to become deadlocked and for no forward +* progress to either be made, and the driver being kept +* eternally awake. +*/ + if (dma_fence_is_i915(ap->fence) && + !i915_sched_node_verify_dag(&rq->sched, + &to_request(ap->fence)->sched)) + err = -EDEADLK; + + if (!err) { +
[Intel-gfx] [PATCH 6/9] drm/syncobj: Allow use of dma-fence-proxy
Allow the callers to supply a dma-fence-proxy for asynchronous waiting on future fences. Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_syncobj.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 42d46414f767..e141db0e1eb6 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -184,6 +184,7 @@ */ #include +#include #include #include #include @@ -324,14 +325,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *old_fence; struct syncobj_wait_entry *cur, *tmp; - if (fence) - dma_fence_get(fence); - spin_lock(&syncobj->lock); - old_fence = rcu_dereference_protected(syncobj->fence, - lockdep_is_held(&syncobj->lock)); - rcu_assign_pointer(syncobj->fence, fence); + old_fence = dma_fence_replace_proxy(&syncobj->fence, fence); if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915: Pull waiting on an external dma-fence into its routine
As a means for a small code consolidation, but primarily to start thinking more carefully about internal-vs-external linkage, pull the pair of i915_sw_fence_await_dma_fence() calls into a common routine. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_request.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index be2ce9065a29..94189c7d43cd 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return 0; } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + return i915_sw_fence_await_dma_fence(&rq->submit, fence, +fence->context ? I915_FENCE_TIMEOUT : 0, +I915_FENCE_GFP); +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) if (dma_fence_is_i915(fence)) ret = i915_request_await_request(rq, to_request(fence)); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - fence->context ? I915_FENCE_TIMEOUT : 0, - I915_FENCE_GFP); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; @@ -1255,9 +1261,7 @@ i915_request_await_execution(struct i915_request *rq, to_request(fence), hook); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - I915_FENCE_TIMEOUT, - GFP_KERNEL); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; } while (--nchild); -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915: Tidy awaiting on dma-fences
Just tidy up the return handling for completed dma-fences. While it may return errors for invalid fence, we already know that we have a good fence and the only error will be an already signaled fence. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_sw_fence.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 7daf81f55c90..295b9829e2da 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, cb->fence = fence; i915_sw_fence_await(fence); - ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake); - if (ret == 0) { - ret = 1; - } else { + ret = 1; + if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) { + /* fence already signaled */ __dma_i915_sw_fence_wake(dma, &cb->base); - if (ret == -ENOENT) /* fence already signaled */ - ret = 0; + ret = 0; } return ret; -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915/gem: Allow combining submit-fences with syncobj
We allow exported sync_file fences to be used as submit fences, but they are not the only source of user fences. We also accept an array of syncobj, and as with sync_file these are dma_fences underneath and so feature the same set of controls. The submit-fence allows for a request to be scheduled at the same time as the signaler, rather than as normal after. Userspace can combine submit-fence with its own semaphores for intra-batch scheduling. Not exposing submit-fences to syncobj was at the time just a matter of pragmatic expediency. Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4854 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Lionel Landwerlin Reviewed-by: Tvrtko Ursulin --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 14 +++ drivers/gpu/drm/i915/i915_request.c | 24 +++ include/uapi/drm/i915_drm.h | 7 +++--- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 199131db200f..6368f0070157 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2432,7 +2432,7 @@ static void __free_fence_array(struct drm_syncobj **fences, unsigned int n) { while (n--) - drm_syncobj_put(ptr_mask_bits(fences[n], 2)); + drm_syncobj_put(ptr_mask_bits(fences[n], 3)); kvfree(fences); } @@ -2489,7 +2489,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) & ~__I915_EXEC_FENCE_UNKNOWN_FLAGS); - fences[n] = ptr_pack_bits(syncobj, fence.flags, 2); + fences[n] = ptr_pack_bits(syncobj, fence.flags, 3); } return fences; @@ -2520,7 +2520,7 @@ await_fence_array(struct i915_execbuffer *eb, struct dma_fence *fence; unsigned int flags; - syncobj = ptr_unpack_bits(fences[n], &flags, 2); + syncobj = ptr_unpack_bits(fences[n], &flags, 3); if (!(flags & I915_EXEC_FENCE_WAIT)) continue; @@ -2544,7 +2544,11 @@ await_fence_array(struct i915_execbuffer *eb, spin_unlock(&syncobj->lock); } - err = i915_request_await_dma_fence(eb->request, fence); + if (flags & I915_EXEC_FENCE_WAIT_SUBMIT) + err = i915_request_await_execution(eb->request, fence, + eb->engine->bond_execute); + else + err = i915_request_await_dma_fence(eb->request, fence); dma_fence_put(fence); if (err < 0) return err; @@ -2565,7 +2569,7 @@ signal_fence_array(struct i915_execbuffer *eb, struct drm_syncobj *syncobj; unsigned int flags; - syncobj = ptr_unpack_bits(fences[n], &flags, 2); + syncobj = ptr_unpack_bits(fences[n], &flags, 3); if (!(flags & I915_EXEC_FENCE_SIGNAL)) continue; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 4f1d8cc2d5af..6afd77e6f141 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1382,6 +1382,26 @@ __i915_request_await_execution(struct i915_request *to, &from->fence); } +static int execution_proxy(struct await_proxy *ap) +{ + return i915_request_await_execution(ap->request, ap->fence, ap->data); +} + +static int +i915_request_await_proxy_execution(struct i915_request *rq, + struct dma_fence *fence, + void (*hook)(struct i915_request *rq, + struct dma_fence *signal)) +{ + /* +* We have to wait until the real request is known in order to +* be able to hook into its execution, as opposed to waiting for +* its completion. +*/ + return __i915_request_await_proxy(rq, fence, I915_FENCE_TIMEOUT, + execution_proxy, hook); +} + int i915_request_await_execution(struct i915_request *rq, struct dma_fence *fence, @@ -1421,6 +1441,10 @@ i915_request_await_execution(struct i915_request *rq, ret = __i915_request_await_execution(rq, to_request(fence), hook); + else if (dma_fence_is_proxy(fence)) + ret = i915_request_await_proxy_execution(rq, +
[Intel-gfx] [PATCH 5/9] dma-buf: Proxy fence, an unsignaled fence placeholder
Often we need to create a fence for a future event that has not yet been associated with a fence. We can store a proxy fence, a placeholder, in the timeline and replace it later when the real fence is known. Any listeners that attach to the proxy fence will automatically be signaled when the real fence completes, and any future listeners will instead be attach directly to the real fence avoiding any indirection overhead. Signed-off-by: Chris Wilson Cc: Lionel Landwerlin --- drivers/dma-buf/Makefile | 13 +- drivers/dma-buf/dma-fence-private.h | 20 + drivers/dma-buf/dma-fence-proxy.c| 248 ++ drivers/dma-buf/dma-fence.c | 4 +- drivers/dma-buf/selftests.h | 1 + drivers/dma-buf/st-dma-fence-proxy.c | 699 +++ include/linux/dma-fence-proxy.h | 34 ++ 7 files changed, 1015 insertions(+), 4 deletions(-) create mode 100644 drivers/dma-buf/dma-fence-private.h create mode 100644 drivers/dma-buf/dma-fence-proxy.c create mode 100644 drivers/dma-buf/st-dma-fence-proxy.c create mode 100644 include/linux/dma-fence-proxy.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 995e05f609ff..afaf6dadd9a3 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,6 +1,12 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ -dma-resv.o seqno-fence.o +obj-y := \ + dma-buf.o \ + dma-fence.o \ + dma-fence-array.o \ + dma-fence-chain.o \ + dma-fence-proxy.o \ + dma-resv.o \ + seqno-fence.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE)+= sync_file.o @@ -10,6 +16,7 @@ obj-$(CONFIG_UDMABUF) += udmabuf.o dmabuf_selftests-y := \ selftest.o \ st-dma-fence.o \ - st-dma-fence-chain.o + st-dma-fence-chain.o \ + st-dma-fence-proxy.o obj-$(CONFIG_DMABUF_SELFTESTS) += dmabuf_selftests.o diff --git a/drivers/dma-buf/dma-fence-private.h b/drivers/dma-buf/dma-fence-private.h new file mode 100644 index ..6924d28af0fa --- /dev/null +++ b/drivers/dma-buf/dma-fence-private.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Fence mechanism for dma-buf and to allow for asynchronous dma access + * + * Copyright (C) 2012 Canonical Ltd + * Copyright (C) 2012 Texas Instruments + * + * Authors: + * Rob Clark + * Maarten Lankhorst + */ + +#ifndef DMA_FENCE_PRIVATE_H +#define DMA_FENCE_PRIAVTE_H + +struct dma_fence; + +bool __dma_fence_enable_signaling(struct dma_fence *fence); + +#endif /* DMA_FENCE_PRIAVTE_H */ diff --git a/drivers/dma-buf/dma-fence-proxy.c b/drivers/dma-buf/dma-fence-proxy.c new file mode 100644 index ..f0cd89b966e0 --- /dev/null +++ b/drivers/dma-buf/dma-fence-proxy.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * dma-fence-proxy: placeholder unsignaled fence + * + * Copyright (C) 2017-2019 Intel Corporation + */ + +#include +#include +#include +#include +#include + +#include "dma-fence-private.h" + +struct dma_fence_proxy { + struct dma_fence base; + + struct dma_fence *real; + struct dma_fence_cb cb; + struct irq_work work; + + wait_queue_head_t wq; +}; + +#ifdef CONFIG_DEBUG_LOCK_ALLOC +#define same_lockclass(A, B) (A)->dep_map.key == (B)->dep_map.key +#else +#define same_lockclass(A, B) 0 +#endif + +static const char *proxy_get_driver_name(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + + return real ? real->ops->get_driver_name(real) : "proxy"; +} + +static const char *proxy_get_timeline_name(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + + return real ? real->ops->get_timeline_name(real) : "unset"; +} + +static void proxy_irq_work(struct irq_work *work) +{ + struct dma_fence_proxy *p = container_of(work, typeof(*p), work); + + dma_fence_signal(&p->base); + dma_fence_put(&p->base); +} + +static void proxy_callback(struct dma_fence *real, struct dma_fence_cb *cb) +{ + struct dma_fence_proxy *p = container_of(cb, typeof(*p), cb); + + if (real->error) + dma_fence_set_error(&p->base, real->error); + + /* Lower the height of the proxy chain -> single stack frame */ + irq_work_queue(&p->work); +} + +static bool proxy_enable_signaling(struct dma_fence *fence) +{ + struct dma_fence_proxy *p = container_of(fence, typeof(*p), base); + struct dma_fence *real = READ_ONCE(p->real); + bool ret = true; + + if (real) { + spin_lock_nested(real->lock, +same_lockclass(&p->wq.lock, real->lock)); + ret = __dma_fence_enable_si
[Intel-gfx] [PATCH 1/9] drm/i915: Ignore submit-fences on the same timeline
While we ordinarily do not skip submit-fences due to the accompanying hook that we want to callback on execution, a submit-fence on the same timeline is meaningless. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_request.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 589739bfee25..be2ce9065a29 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1242,6 +1242,9 @@ i915_request_await_execution(struct i915_request *rq, continue; } + if (fence->context == rq->fence.context) + continue; + /* * We don't squash repeated fence dependencies here as we * want to run our callback in all cases. -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 14/22] drm/i915/rkl: provide port/phy mapping for vbt
On Thu, May 07, 2020 at 11:05:33AM -0700, Matt Roper wrote: > On Thu, May 07, 2020 at 03:04:30PM +0300, Ville Syrjälä wrote: > > On Mon, May 04, 2020 at 03:52:19PM -0700, Matt Roper wrote: > > > From: Lucas De Marchi > > > > > > RKL uses the DDI A, DDI B, DDI USBC1, DDI USBC2 from the DE point of > > > view, so all DDI/pipe/transcoder register use these indexes to refer to > > > them. Combo phy and IO functions follow another namespace that we keep > > > as "enum phy". The VBT in theory would use the DE point of view, but > > > that does not happen in practice. > > > > > > Provide a table to convert the child devices to the "correct" port > > > numbering we use. Now this is the output we get while reading the VBT: > > > > > > DDIA: > > > [drm:intel_bios_port_aux_ch [i915]] using AUX A for port A (VBT) > > > [drm:intel_dp_init_connector [i915]] Adding DP connector on > > > [ENCODER:275:DDI A] > > > [drm:intel_hdmi_init_connector [i915]] Adding HDMI connector on > > > [ENCODER:275:DDI A] > > > [drm:intel_hdmi_init_connector [i915]] Using DDC pin 0x1 for port A (VBT) > > > > > > DDIB: > > > [drm:intel_bios_port_aux_ch [i915]] using AUX B for port B (platform > > > default) > > > [drm:intel_hdmi_init_connector [i915]] Adding HDMI connector on > > > [ENCODER:291:DDI B] > > > [drm:intel_hdmi_init_connector [i915]] Using DDC pin 0x2 for port B (VBT) > > > > > > DDI USBC1: > > > [drm:intel_bios_port_aux_ch [i915]] using AUX D for port D (VBT) > > > [drm:intel_dp_init_connector [i915]] Adding DP connector on > > > [ENCODER:295:DDI D] > > > [drm:intel_hdmi_init_connector [i915]] Adding HDMI connector on > > > [ENCODER:295:DDI D] > > > [drm:intel_hdmi_init_connector [i915]] Using DDC pin 0x3 for port D (VBT) > > > > > > DDI USBC2: > > > [drm:intel_bios_port_aux_ch [i915]] using AUX E for port E (VBT) > > > [drm:intel_dp_init_connector [i915]] Adding DP connector on > > > [ENCODER:306:DDI E] > > > [drm:intel_hdmi_init_connector [i915]] Adding HDMI connector on > > > [ENCODER:306:DDI E] > > > [drm:intel_hdmi_init_connector [i915]] Using DDC pin 0x9 for port E (VBT) > > > > > > Cc: Clinton Taylor > > > Cc: Aditya Swarup > > > Signed-off-by: Lucas De Marchi > > > Signed-off-by: Matt Roper > > > --- > > > drivers/gpu/drm/i915/display/intel_bios.c | 72 --- > > > 1 file changed, 51 insertions(+), 21 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > > > b/drivers/gpu/drm/i915/display/intel_bios.c > > > index 839124647202..4f1a72a90b8f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_bios.c > > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > > > @@ -1619,30 +1619,18 @@ static u8 map_ddc_pin(struct drm_i915_private > > > *dev_priv, u8 vbt_pin) > > > return 0; > > > } > > > > > > -static enum port dvo_port_to_port(u8 dvo_port) > > > +static enum port __dvo_port_to_port(int n_ports, int n_dvo, > > > + const int port_mapping[][3], u8 dvo_port) > > > { > > > - /* > > > - * Each DDI port can have more than one value on the "DVO Port" field, > > > - * so look for all the possible values for each port. > > > - */ > > > - static const int dvo_ports[][3] = { > > > - [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, > > > - [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, > > > - [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, -1}, > > > - [PORT_D] = { DVO_PORT_HDMID, DVO_PORT_DPD, -1}, > > > - [PORT_E] = { DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, > > > - [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1}, > > > - [PORT_G] = { DVO_PORT_HDMIG, DVO_PORT_DPG, -1}, > > > - }; > > > enum port port; > > > int i; > > > > > > - for (port = PORT_A; port < ARRAY_SIZE(dvo_ports); port++) { > > > - for (i = 0; i < ARRAY_SIZE(dvo_ports[port]); i++) { > > > - if (dvo_ports[port][i] == -1) > > > + for (port = PORT_A; port < n_ports; port++) { > > > + for (i = 0; i < n_dvo; i++) { > > > + if (port_mapping[port][i] == -1) > > > break; > > > > > > - if (dvo_port == dvo_ports[port][i]) > > > + if (dvo_port == port_mapping[port][i]) > > > return port; > > > } > > > } > > > @@ -1650,6 +1638,48 @@ static enum port dvo_port_to_port(u8 dvo_port) > > > return PORT_NONE; > > > } > > > > > > +static enum port dvo_port_to_port(struct drm_i915_private *dev_priv, > > > + u8 dvo_port) > > > +{ > > > + /* > > > + * Each DDI port can have more than one value on the "DVO Port" field, > > > + * so look for all the possible values for each port. > > > + */ > > > + static const int port_mapping[][3] = { > > > + [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, -1 }, > > > + [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, -1 }, > > > + [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, -1 }, > > > + [PORT_D] = {
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Ignore submit-fences on the same timeline
Chris Wilson writes: > While we ordinarily do not skip submit-fences due to the accompanying > hook that we want to callback on execution, a submit-fence on the same > timeline is meaningless. > > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/i915_request.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_request.c > b/drivers/gpu/drm/i915/i915_request.c > index 589739bfee25..be2ce9065a29 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1242,6 +1242,9 @@ i915_request_await_execution(struct i915_request *rq, > continue; > } > > + if (fence->context == rq->fence.context) > + continue; > + > /* >* We don't squash repeated fence dependencies here as we >* want to run our callback in all cases. The comment in here makes me nervous. Is this skipping on same context other than squashing? -Mika > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Take the atomic toys away from X
On Fri, May 08, 2020 at 11:06:56AM +0200, Yves-Alexis Perez wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > On Thu, 2019-09-05 at 20:53 +0200, Daniel Vetter wrote: > > The -modesetting ddx has a totally broken idea of how atomic works: > > - doesn't disable old connectors, assuming they get auto-disable like > > with the legacy setcrtc > > - assumes ASYNC_FLIP is wired through for the atomic ioctl > > - not a single call to TEST_ONLY > > > > Iow the implementation is a 1:1 translation of legacy ioctls to > > atomic, which is a) broken b) pointless. > > > > We already have bugs in both i915 and amdgpu-DC where this prevents us > > from enabling neat features. > > > > If anyone ever cares about atomic in X we can easily add a new atomic > > level (req->value == 2) for X to get back the shiny toys. > > > > Since these broken versions of -modesetting have been shipping, > > there's really no other way to get out of this bind. > > Hi Daniel and Greg (especially). It seems that this patch was never applied to > stable, maybe it fell through the cracks? What patch is "this patch"? > It doesn't apply as-is in 4.19 branch but a small change in the context makes > it apply. I'm experiencing issues with lightdm and vt-switch in Debian Buster > (which has a 4.19 kernel) so I'd appreciate if the patch was included in at > least that release. What is the git commit id of the patch in Linus's tree? If you have a working backport, that makes it much easier than hoping I can fix it up... thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Ignore submit-fences on the same timeline
Quoting Mika Kuoppala (2020-05-08 10:57:37) > Chris Wilson writes: > > > While we ordinarily do not skip submit-fences due to the accompanying > > hook that we want to callback on execution, a submit-fence on the same > > timeline is meaningless. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > drivers/gpu/drm/i915/i915_request.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index 589739bfee25..be2ce9065a29 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1242,6 +1242,9 @@ i915_request_await_execution(struct i915_request *rq, > > continue; > > } > > > > + if (fence->context == rq->fence.context) > > + continue; > > + > > /* > >* We don't squash repeated fence dependencies here as we > >* want to run our callback in all cases. > > The comment in here makes me nervous. Is this skipping on same context > other than squashing? The hooks we have only apply between timelines, so skipping isn't an issue. Suppressing the wait ensures that syncobj-future-submit-past: I915_EXEC_FENCE_WAIT | I915_EXEC_FENCE_WAIT_SUBMIT | I915_EXEC_FENCE_SIGNAL is a no-op. That is if you declare that request should wait for itself to be submitted before it is submitted, we correctly conclude that is degenerate and a no-op. We can generalise that to realise that waiting for any fence along the same timeline to be submitted before we are submitted is guaranteed by the timeline itself, and so all are no-ops. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77072/ State : warning == Summary == $ dim checkpatch origin/drm-tip 3db130b5548c drm/i915: Ignore submit-fences on the same timeline d17611e9e0a3 drm/i915: Pull waiting on an external dma-fence into its routine fccd141053d3 drm/i915: Prevent using semaphores to chain up to external fences fe5b438ff7a9 drm/i915: Tidy awaiting on dma-fences 4e705a4d23cc dma-buf: Proxy fence, an unsignaled fence placeholder -:45: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #45: new file mode 100644 -:380: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment #380: FILE: drivers/dma-buf/st-dma-fence-proxy.c:20: + spinlock_t lock; -:540: WARNING:MEMORY_BARRIER: memory barrier without comment #540: FILE: drivers/dma-buf/st-dma-fence-proxy.c:180: + smp_store_mb(container_of(cb, struct simple_cb, cb)->seen, true); total: 0 errors, 2 warnings, 1 checks, 1043 lines checked d5504a1bf6f5 drm/syncobj: Allow use of dma-fence-proxy 61b47826215b drm/i915/gem: Teach execbuf how to wait on future syncobj f44784ab7169 drm/i915/gem: Allow combining submit-fences with syncobj 96dc49eecf66 drm/i915/gt: Declare when we enabled timeslicing ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Pull waiting on an external dma-fence into its routine
Chris Wilson writes: > As a means for a small code consolidation, but primarily to start > thinking more carefully about internal-vs-external linkage, pull the > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_request.c | 16 ++-- > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c > b/drivers/gpu/drm/i915/i915_request.c > index be2ce9065a29..94189c7d43cd 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, > struct i915_request *from) > return 0; > } > > +static int > +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) > +{ > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > + fence->context ? > I915_FENCE_TIMEOUT : 0, > + I915_FENCE_GFP); > +} > + > int > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence > *fence) > { > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, > struct dma_fence *fence) > if (dma_fence_is_i915(fence)) > ret = i915_request_await_request(rq, to_request(fence)); > else > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - fence->context ? > I915_FENCE_TIMEOUT : 0, > - I915_FENCE_GFP); > + ret = i915_request_await_external(rq, fence); For us (rq, rq), for external (rq, fence). It looks neat for a reader. But then, how can external fence have a context? -Mika > if (ret < 0) > return ret; > > @@ -1255,9 +1261,7 @@ i915_request_await_execution(struct i915_request *rq, >to_request(fence), >hook); > else > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - I915_FENCE_TIMEOUT, > - GFP_KERNEL); > + ret = i915_request_await_external(rq, fence); > if (ret < 0) > return ret; > } while (--nchild); > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Pull waiting on an external dma-fence into its routine
Quoting Mika Kuoppala (2020-05-08 11:19:25) > Chris Wilson writes: > > > As a means for a small code consolidation, but primarily to start > > thinking more carefully about internal-vs-external linkage, pull the > > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_request.c | 16 ++-- > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index be2ce9065a29..94189c7d43cd 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, > > struct i915_request *from) > > return 0; > > } > > > > +static int > > +i915_request_await_external(struct i915_request *rq, struct dma_fence > > *fence) > > +{ > > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > > + fence->context ? > > I915_FENCE_TIMEOUT : 0, > > + I915_FENCE_GFP); > > +} > > + > > int > > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence > > *fence) > > { > > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, > > struct dma_fence *fence) > > if (dma_fence_is_i915(fence)) > > ret = i915_request_await_request(rq, > > to_request(fence)); > > else > > - ret = i915_sw_fence_await_dma_fence(&rq->submit, > > fence, > > - fence->context ? > > I915_FENCE_TIMEOUT : 0, > > - I915_FENCE_GFP); > > + ret = i915_request_await_external(rq, fence); > > For us (rq, rq), for external (rq, fence). > > It looks neat for a reader. But then, how can external fence have > a context? The 'context' here is the dma_fence timeline context. All dma_fences have a context:seqno tuple which denotes their timeline and position along the timeline. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Pull waiting on an external dma-fence into its routine
Quoting Mika Kuoppala (2020-05-08 11:19:25) > Chris Wilson writes: > > > As a means for a small code consolidation, but primarily to start > > thinking more carefully about internal-vs-external linkage, pull the > > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_request.c | 16 ++-- > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index be2ce9065a29..94189c7d43cd 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, > > struct i915_request *from) > > return 0; > > } > > > > +static int > > +i915_request_await_external(struct i915_request *rq, struct dma_fence > > *fence) > > +{ > > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > > + fence->context ? > > I915_FENCE_TIMEOUT : 0, > > + I915_FENCE_GFP); > > +} > > + > > int > > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence > > *fence) > > { > > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, > > struct dma_fence *fence) > > if (dma_fence_is_i915(fence)) > > ret = i915_request_await_request(rq, > > to_request(fence)); > > else > > - ret = i915_sw_fence_await_dma_fence(&rq->submit, > > fence, > > - fence->context ? > > I915_FENCE_TIMEOUT : 0, > > - I915_FENCE_GFP); > > + ret = i915_request_await_external(rq, fence); > > For us (rq, rq), for external (rq, fence). > > It looks neat for a reader. But then, how can external fence have > a context? How about s/fence/dma/? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77072/ State : success == Summary == CI Bug Log - changes from CI_DRM_8450 -> Patchwork_17609 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/index.html New tests - New tests have been introduced between CI_DRM_8450 and Patchwork_17609: ### New IGT tests (1) ### * igt@dmabuf@all@dma_fence_proxy: - Statuses : 39 pass(s) - Exec time: [0.03, 0.11] s Known issues Here are the changes found in Patchwork_17609 that come from known issues: ### IGT changes ### Possible fixes * igt@i915_selftest@live@mman: - fi-bwr-2160:[INCOMPLETE][1] ([i915#489]) -> [PASS][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bwr-2160/igt@i915_selftest@l...@mman.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/fi-bwr-2160/igt@i915_selftest@l...@mman.html * {igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1}: - fi-bsw-kefka: [FAIL][3] ([i915#34]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-bsw-kefka/igt@kms_flip@basic-flip-vs-wf_vbl...@a-edp1.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/fi-bsw-kefka/igt@kms_flip@basic-flip-vs-wf_vbl...@a-edp1.html Warnings * igt@i915_pm_rpm@module-reload: - fi-kbl-x1275: [FAIL][5] ([i915#62] / [i915#95]) -> [SKIP][6] ([fdo#109271]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/fi-kbl-x1275/igt@i915_pm_...@module-reload.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34 [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (49 -> 43) -- Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * CI: CI-20190529 -> None * Linux: CI_DRM_8450 -> Patchwork_17609 CI-20190529: 20190529 CI_DRM_8450: 4c642c074acca27a763a00be28132b6f40bc361c @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5640: 5efb4a1c9cc944eff129cae7794951ae617bca17 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_17609: 96dc49eecf667ccb1418bcfac0bc189412868acb @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 96dc49eecf66 drm/i915/gt: Declare when we enabled timeslicing f44784ab7169 drm/i915/gem: Allow combining submit-fences with syncobj 61b47826215b drm/i915/gem: Teach execbuf how to wait on future syncobj d5504a1bf6f5 drm/syncobj: Allow use of dma-fence-proxy 4e705a4d23cc dma-buf: Proxy fence, an unsignaled fence placeholder fe5b438ff7a9 drm/i915: Tidy awaiting on dma-fences fccd141053d3 drm/i915: Prevent using semaphores to chain up to external fences d17611e9e0a3 drm/i915: Pull waiting on an external dma-fence into its routine 3db130b5548c drm/i915: Ignore submit-fences on the same timeline == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gt: Improve precision on defer_request assert
The kernel_context does not use initial-breadcrumbs, so when we ask if its requests have started we do so by comparing against the completion seqno of the previous request. This is very imprecise, not precise enough for the defer_request assertion. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1847 Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_lrc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 400b9b5a6882..28b477f531a5 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1886,7 +1886,8 @@ static void defer_request(struct i915_request *rq, struct list_head * const pl) continue; /* No waiter should start before its signaler */ - GEM_BUG_ON(i915_request_started(w) && + GEM_BUG_ON(w->context->timeline->has_initial_breadcrumb && + i915_request_started(w) && !i915_request_completed(rq)); GEM_BUG_ON(i915_request_is_active(w)); -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Pull waiting on an external dma-fence into its routine
Chris Wilson writes: > Quoting Mika Kuoppala (2020-05-08 11:19:25) >> Chris Wilson writes: >> >> > As a means for a small code consolidation, but primarily to start >> > thinking more carefully about internal-vs-external linkage, pull the >> > pair of i915_sw_fence_await_dma_fence() calls into a common routine. >> > >> > Signed-off-by: Chris Wilson >> > --- >> > drivers/gpu/drm/i915/i915_request.c | 16 ++-- >> > 1 file changed, 10 insertions(+), 6 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c >> > b/drivers/gpu/drm/i915/i915_request.c >> > index be2ce9065a29..94189c7d43cd 100644 >> > --- a/drivers/gpu/drm/i915/i915_request.c >> > +++ b/drivers/gpu/drm/i915/i915_request.c >> > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, >> > struct i915_request *from) >> > return 0; >> > } >> > >> > +static int >> > +i915_request_await_external(struct i915_request *rq, struct dma_fence >> > *fence) >> > +{ >> > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, >> > + fence->context ? >> > I915_FENCE_TIMEOUT : 0, >> > + I915_FENCE_GFP); >> > +} >> > + >> > int >> > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence >> > *fence) >> > { >> > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request >> > *rq, struct dma_fence *fence) >> > if (dma_fence_is_i915(fence)) >> > ret = i915_request_await_request(rq, >> > to_request(fence)); >> > else >> > - ret = i915_sw_fence_await_dma_fence(&rq->submit, >> > fence, >> > - fence->context ? >> > I915_FENCE_TIMEOUT : 0, >> > - I915_FENCE_GFP); >> > + ret = i915_request_await_external(rq, fence); >> >> For us (rq, rq), for external (rq, fence). >> >> It looks neat for a reader. But then, how can external fence have >> a context? > > How about s/fence/dma/? It is fine like it is. I was just so confused that we piggyback our context along the fence. But yeah this is the fence->context and zero is a special. (thanks for explaining in irc) Reviewed-by: Mika Kuoppala > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] dma-buf: Use atomic_fetch_add() for the context id
Now that atomic64_fetch_add() exists we can use it to return the base context id, rather than the atomic64_add_return(N) - N concoction. Suggested-by: Mika Kuoppala Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/dma-buf/dma-fence.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 052a41e2451c..90edf2b281b0 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -106,7 +106,7 @@ EXPORT_SYMBOL(dma_fence_get_stub); u64 dma_fence_context_alloc(unsigned num) { WARN_ON(!num); - return atomic64_add_return(num, &dma_fence_context_counter) - num; + return atomic64_fetch_add(num, &dma_fence_context_counter); } EXPORT_SYMBOL(dma_fence_context_alloc); -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Ignore submit-fences on the same timeline
Chris Wilson writes: > Quoting Mika Kuoppala (2020-05-08 10:57:37) >> Chris Wilson writes: >> >> > While we ordinarily do not skip submit-fences due to the accompanying >> > hook that we want to callback on execution, a submit-fence on the same >> > timeline is meaningless. >> > >> > Signed-off-by: Chris Wilson >> > Cc: Tvrtko Ursulin >> > --- >> > drivers/gpu/drm/i915/i915_request.c | 3 +++ >> > 1 file changed, 3 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c >> > b/drivers/gpu/drm/i915/i915_request.c >> > index 589739bfee25..be2ce9065a29 100644 >> > --- a/drivers/gpu/drm/i915/i915_request.c >> > +++ b/drivers/gpu/drm/i915/i915_request.c >> > @@ -1242,6 +1242,9 @@ i915_request_await_execution(struct i915_request *rq, >> > continue; >> > } >> > >> > + if (fence->context == rq->fence.context) >> > + continue; >> > + >> > /* >> >* We don't squash repeated fence dependencies here as we >> >* want to run our callback in all cases. >> >> The comment in here makes me nervous. Is this skipping on same context >> other than squashing? > > The hooks we have only apply between timelines, so skipping isn't an > issue. Suppressing the wait ensures that > > syncobj-future-submit-past: > I915_EXEC_FENCE_WAIT | > I915_EXEC_FENCE_WAIT_SUBMIT | > I915_EXEC_FENCE_SIGNAL > > is a no-op. That is if you declare that request should wait for itself > to be submitted before it is submitted, we correctly conclude that is > degenerate and a no-op. We can generalise that to realise that waiting for > any fence along the same timeline to be submitted before we are > submitted is guaranteed by the timeline itself, and so all are no-ops. The last sentence nails it. Reviewed-by: Mika Kuoppala > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915/dsb: Pre allocate and late cleanup of cmd buffer
Op 06-05-2020 om 15:11 schreef Animesh Manna: > Pre-allocate command buffer in atomic_commit using intel_dsb_prepare > function which also includes pinning and map in cpu domain. > > No change is dsb write/commit functions. > > Now dsb get/put function is refactored and currently used only for > reference counting. Below dsb api added to do respective job > mentioned below. > > intel_dsb_prepare - Allocate, pin and map the buffer. > intel_dsb_cleanup - Unpin and release the gem object. > > RFC: Initial patch for design review. > v2: included _init() part in _prepare(). [Daniel, Ville] > v3: dsb_cleanup called after cleanup_planes. [Daniel] > v4: dsb structure is moved to intel_crtc_state from intel_crtc. [Maarten] > v5: dsb get/put/ref-count mechanism removed. [Maarten] > > Cc: Maarten Lankhorst > Cc: Ville Syrjälä > Cc: Jani Nikula > Cc: Daniel Vetter > Acked-by: Daniel Vetter > Signed-off-by: Animesh Manna > --- > drivers/gpu/drm/i915/display/intel_color.c| 27 ++- > drivers/gpu/drm/i915/display/intel_display.c | 60 - > .../drm/i915/display/intel_display_types.h| 6 +- > drivers/gpu/drm/i915/display/intel_dsb.c | 207 +- > drivers/gpu/drm/i915/display/intel_dsb.h | 8 +- > 5 files changed, 178 insertions(+), 130 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c > b/drivers/gpu/drm/i915/display/intel_color.c > index 98ece9cd7cdd..dba820136106 100644 > --- a/drivers/gpu/drm/i915/display/intel_color.c > +++ b/drivers/gpu/drm/i915/display/intel_color.c > @@ -717,7 +717,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc, > static void ivb_load_lut_ext_max(struct intel_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - struct intel_dsb *dsb = intel_dsb_get(crtc); > + struct intel_crtc_state *crtc_state = > to_intel_crtc_state(crtc->base.state); > + struct intel_dsb *dsb = (struct intel_dsb *)&crtc_state->dsb; > enum pipe pipe = crtc->pipe; > > /* Program the max register to clamp values > 1.0. */ Please pass the crtc_state, as crtc->state should not be used directly. > @@ -738,8 +739,6 @@ static void ivb_load_lut_ext_max(struct intel_crtc *crtc) > intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 2), > 1 << 16); > } > - > - intel_dsb_put(dsb); > } > > static void ivb_load_luts(const struct intel_crtc_state *crtc_state) > @@ -900,14 +899,13 @@ icl_load_gcmax(const struct intel_crtc_state > *crtc_state, > const struct drm_color_lut *color) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - struct intel_dsb *dsb = intel_dsb_get(crtc); > + struct intel_dsb *dsb = (struct intel_dsb *)&crtc_state->dsb; Why the cast? Unconstify crtc_state as it's obviously not const any more. > enum pipe pipe = crtc->pipe; > > /* FIXME LUT entries are 16 bit only, so we can prog 0x max */ > intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 0), color->red); > intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 1), color->green); > intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 2), color->blue); > - intel_dsb_put(dsb); > } > > static void > @@ -916,7 +914,7 @@ icl_program_gamma_superfine_segment(const struct > intel_crtc_state *crtc_state) > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > const struct drm_property_blob *blob = crtc_state->hw.gamma_lut; > const struct drm_color_lut *lut = blob->data; > - struct intel_dsb *dsb = intel_dsb_get(crtc); > + struct intel_dsb *dsb = (struct intel_dsb *)&crtc_state->dsb; Same. > enum pipe pipe = crtc->pipe; > int i; > > @@ -938,8 +936,6 @@ icl_program_gamma_superfine_segment(const struct > intel_crtc_state *crtc_state) > intel_dsb_indexed_reg_write(dsb, PREC_PAL_MULTI_SEG_DATA(pipe), > ilk_lut_12p4_udw(entry)); > } > - > - intel_dsb_put(dsb); > } > > static void > @@ -949,7 +945,7 @@ icl_program_gamma_multi_segment(const struct > intel_crtc_state *crtc_state) > const struct drm_property_blob *blob = crtc_state->hw.gamma_lut; > const struct drm_color_lut *lut = blob->data; > const struct drm_color_lut *entry; > - struct intel_dsb *dsb = intel_dsb_get(crtc); > + struct intel_dsb *dsb = (struct intel_dsb *)&crtc_state->dsb; > enum pipe pipe = crtc->pipe; > int i; > > @@ -996,14 +992,22 @@ icl_program_gamma_multi_segment(const struct > intel_crtc_state *crtc_state) > entry = &lut[256 * 8 * 128]; > icl_load_gcmax(crtc_state, entry); > ivb_load_lut_ext_max(crtc); > - intel_dsb_put(dsb); > } > > static void icl_load_luts(const struct intel_crtc_state *crtc_state) > { > const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut; > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Improve precision on defer_request assert
== Series Details == Series: drm/i915/gt: Improve precision on defer_request assert URL : https://patchwork.freedesktop.org/series/77074/ State : success == Summary == CI Bug Log - changes from CI_DRM_8451 -> Patchwork_17610 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/index.html Changes --- No changes found Participating hosts (48 -> 42) -- Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * CI: CI-20190529 -> None * Linux: CI_DRM_8451 -> Patchwork_17610 CI-20190529: 20190529 CI_DRM_8451: 901f1db9311638ff4fa9070be8248f1edd199aee @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5641: 4f6c17f0dbbdf2c7b4e647bb909e6d31dfce9827 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_17610: 86eb8a99077fe01789dcf50d78d4ffee6db73d83 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 86eb8a99077f drm/i915/gt: Improve precision on defer_request assert == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Take the atomic toys away from X
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On Fri, 2020-05-08 at 11:54 +0200, Greg KH wrote: > > Hi Daniel and Greg (especially). It seems that this patch was never > > applied to > > stable, maybe it fell through the cracks? > > What patch is "this patch"? Sorry, the patch was in the mail I was replying to: commit 26b1d3b527e7bf3e24b814d617866ac5199ce68d Author: Daniel Vetter Date: Thu Sep 5 20:53:18 2019 +0200 drm/atomic: Take the atomic toys away from X > > > It doesn't apply as-is in 4.19 branch but a small change in the context > > makes > > it apply. I'm experiencing issues with lightdm and vt-switch in Debian > > Buster > > (which has a 4.19 kernel) so I'd appreciate if the patch was included in > > at > > least that release. > > What is the git commit id of the patch in Linus's tree? If you have a > working backport, that makes it much easier than hoping I can fix it > up... The commit id is in Linus tree is 26b1d3b527e7bf3e24b814d617866ac5199ce68d. To apply properly 69fdf4206a8ba91a277b3d50a3a05b71247635b2 would need to be cherry-picked as well but it wasn't marked for stable so I didn't bother and only fixed the context. Here's the backport to 4.19, compile and runtime tested. It does fix the issue for me (like it did on mainline). So I guess Tested-By: Yves-Alexis Perez commit 8a99914f7b539542622dc571c82d6cd203bddf64 Author: Daniel Vetter Date: Thu Sep 5 20:53:18 2019 +0200 drm/atomic: Take the atomic toys away from X The -modesetting ddx has a totally broken idea of how atomic works: - doesn't disable old connectors, assuming they get auto-disable like with the legacy setcrtc - assumes ASYNC_FLIP is wired through for the atomic ioctl - not a single call to TEST_ONLY Iow the implementation is a 1:1 translation of legacy ioctls to atomic, which is a) broken b) pointless. We already have bugs in both i915 and amdgpu-DC where this prevents us from enabling neat features. If anyone ever cares about atomic in X we can easily add a new atomic level (req->value == 2) for X to get back the shiny toys. Since these broken versions of -modesetting have been shipping, there's really no other way to get out of this bind. v2: - add an informational dmesg output (Rob, Ajax) - reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia) - allow req->value > 2 so that X can do another attempt at atomic in the future v3: Go with paranoid, insist that the X should be first (suggested by Rob) Cc: Ilia Mirkin References: https://gitlab.freedesktop.org/xorg/xserver/issues/629 References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180 References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure untiled displays from master") Cc: Maarten Lankhorst Reviewed-by: Maarten Lankhorst (v1) Reviewed-by: Nicholas Kazlauskas (v1) Cc: Michel Dänzer Cc: Alex Deucher Cc: Adam Jackson Acked-by: Adam Jackson Cc: Sean Paul Cc: David Airlie Cc: Rob Clark Acked-by: Rob Clark Cc: sta...@vger.kernel.org Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190905185318.31363-1-daniel.vet...@ffwll.ch diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index ba129b64b61f..b92682f037b2 100644 - --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -321,7 +321,12 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) case DRM_CLIENT_CAP_ATOMIC: if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) return -EINVAL; - - if (req->value > 1) + /* The modesetting DDX has a totally broken idea of atomic. */ + if (current->comm[0] == 'X' && req->value == 1) { + pr_info("broken atomic modeset userspace detected, disabling atomic\n"); + return -EOPNOTSUPP; + } + if (req->value > 2) return -EINVAL; file_priv->atomic = req->value; file_priv->universal_planes = req->value; - -- Yves-Alexis -BEGIN PGP SIGNATURE- iQEzBAEBCAAdFiEE8vi34Qgfo83x35gF3rYcyPpXRFsFAl61SZUACgkQ3rYcyPpX RFvPfwgAzMyFqiV592RBKu4tx5Ivqa4EC/1OdR8DojyQlw4AP0bYc+4O67xYbvt5 r4JXuGbSu+jW/29V+2t8ZlLi4S7bAvAo2DEhuBdGVzdmD4gM9EC+69oqVeZWWZTm VUldLrRO8BoPxv14lX/K/kU/o5Pv8ivRoEKs385JU2p1AxNGJI2UUmIXKGtk7Cu/ Fu/flH627RHjTRk2QYsemqHqSAONaHYuSiYm783hz4wYiPOZQdGvS+ifHwMAhUqh scaCxv+pBOxaLuZAfUXFjDX+qAcuJCxaP9bT4soweIpYqjzcAdBSmny0+OLOnie/ HcBzKwpgitKR/cVadZgb0US1oHeo2A== =l8z1 -END PGP SIGNATURE- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for dma-buf: Use atomic_fetch_add() for the context id
== Series Details == Series: dma-buf: Use atomic_fetch_add() for the context id URL : https://patchwork.freedesktop.org/series/77076/ State : success == Summary == CI Bug Log - changes from CI_DRM_8451 -> Patchwork_17611 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/index.html Changes --- No changes found Participating hosts (48 -> 43) -- Additional (1): fi-kbl-7560u Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * CI: CI-20190529 -> None * Linux: CI_DRM_8451 -> Patchwork_17611 CI-20190529: 20190529 CI_DRM_8451: 901f1db9311638ff4fa9070be8248f1edd199aee @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5641: 4f6c17f0dbbdf2c7b4e647bb909e6d31dfce9827 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_17611: 1603bf6c1212dd003776b038e82a5e20b604b01f @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 1603bf6c1212 dma-buf: Use atomic_fetch_add() for the context id == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Take the atomic toys away from X
On Fri, May 08, 2020 at 01:59:17PM +0200, Yves-Alexis Perez wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > On Fri, 2020-05-08 at 11:54 +0200, Greg KH wrote: > > > Hi Daniel and Greg (especially). It seems that this patch was never > > > applied to > > > stable, maybe it fell through the cracks? > > > > What patch is "this patch"? > > Sorry, the patch was in the mail I was replying to: > > commit 26b1d3b527e7bf3e24b814d617866ac5199ce68d > Author: Daniel Vetter > Date: Thu Sep 5 20:53:18 2019 +0200 > > drm/atomic: Take the atomic toys away from X > > > > > > It doesn't apply as-is in 4.19 branch but a small change in the context > > > makes > > > it apply. I'm experiencing issues with lightdm and vt-switch in Debian > > > Buster > > > (which has a 4.19 kernel) so I'd appreciate if the patch was included in > > > at > > > least that release. > > > > What is the git commit id of the patch in Linus's tree? If you have a > > working backport, that makes it much easier than hoping I can fix it > > up... > > The commit id is in Linus tree is 26b1d3b527e7bf3e24b814d617866ac5199ce68d. To > apply properly 69fdf4206a8ba91a277b3d50a3a05b71247635b2 would need to be > cherry-picked as well but it wasn't marked for stable so I didn't bother and > only fixed the context. Here's the backport to 4.19, compile and runtime > tested. It does fix the issue for me (like it did on mainline). > > So I guess > Tested-By: Yves-Alexis Perez > > commit 8a99914f7b539542622dc571c82d6cd203bddf64 > Author: Daniel Vetter > Date: Thu Sep 5 20:53:18 2019 +0200 > > drm/atomic: Take the atomic toys away from X > > The -modesetting ddx has a totally broken idea of how atomic works: > - doesn't disable old connectors, assuming they get auto-disable like > with the legacy setcrtc > - assumes ASYNC_FLIP is wired through for the atomic ioctl > - not a single call to TEST_ONLY > > Iow the implementation is a 1:1 translation of legacy ioctls to > atomic, which is a) broken b) pointless. > > We already have bugs in both i915 and amdgpu-DC where this prevents us > from enabling neat features. > > If anyone ever cares about atomic in X we can easily add a new atomic > level (req->value == 2) for X to get back the shiny toys. > > Since these broken versions of -modesetting have been shipping, > there's really no other way to get out of this bind. > > v2: > - add an informational dmesg output (Rob, Ajax) > - reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia) > - allow req->value > 2 so that X can do another attempt at atomic in > the future > > v3: Go with paranoid, insist that the X should be first (suggested by > Rob) > > Cc: Ilia Mirkin > References: https://gitlab.freedesktop.org/xorg/xserver/issues/629 > References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180 > References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure > untiled displays from master") > Cc: Maarten Lankhorst > Reviewed-by: Maarten Lankhorst (v1) > Reviewed-by: Nicholas Kazlauskas (v1) > Cc: Michel Dänzer > Cc: Alex Deucher > Cc: Adam Jackson > Acked-by: Adam Jackson > Cc: Sean Paul > Cc: David Airlie > Cc: Rob Clark > Acked-by: Rob Clark > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter > Link: > https://patchwork.freedesktop.org/patch/msgid/20190905185318.31363-1-daniel.vet...@ffwll.ch > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index ba129b64b61f..b92682f037b2 100644 > - --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -321,7 +321,12 @@ drm_setclientcap(struct drm_device *dev, void *data, > struct drm_file *file_priv) > case DRM_CLIENT_CAP_ATOMIC: > if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > return -EINVAL; > - - if (req->value > 1) > + /* The modesetting DDX has a totally broken idea of atomic. */ > + if (current->comm[0] == 'X' && req->value == 1) { > + pr_info("broken atomic modeset userspace detected, > disabling atomic\n"); > + return -EOPNOTSUPP; > + } > + if (req->value > 2) > return -EINVAL; > file_priv->atomic = req->value; > file_priv->universal_planes = req->value; > This is line-wrapped and can not be applied :( Ugh, let me see if I can do this by hand... greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline
== Series Details == Series: series starting with [1/9] drm/i915: Ignore submit-fences on the same timeline URL : https://patchwork.freedesktop.org/series/77072/ State : success == Summary == CI Bug Log - changes from CI_DRM_8450_full -> Patchwork_17609_full Summary --- **SUCCESS** No regressions found. Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_17609_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@gem_exec_fence@syncobj-invalid-wait}: - shard-snb: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-snb6/igt@gem_exec_fe...@syncobj-invalid-wait.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-snb5/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-tglb: [PASS][3] -> [FAIL][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-tglb2/igt@gem_exec_fe...@syncobj-invalid-wait.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-tglb8/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-skl: [PASS][5] -> [FAIL][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-skl10/igt@gem_exec_fe...@syncobj-invalid-wait.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-skl2/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-glk: [PASS][7] -> [FAIL][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-glk6/igt@gem_exec_fe...@syncobj-invalid-wait.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-glk5/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-kbl: [PASS][9] -> [FAIL][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-kbl2/igt@gem_exec_fe...@syncobj-invalid-wait.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-kbl3/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-iclb: [PASS][11] -> [FAIL][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-iclb4/igt@gem_exec_fe...@syncobj-invalid-wait.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-iclb6/igt@gem_exec_fe...@syncobj-invalid-wait.html - shard-hsw: [PASS][13] -> [FAIL][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-hsw5/igt@gem_exec_fe...@syncobj-invalid-wait.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-hsw6/igt@gem_exec_fe...@syncobj-invalid-wait.html New tests - New tests have been introduced between CI_DRM_8450_full and Patchwork_17609_full: ### New IGT tests (1) ### * igt@dmabuf@all@dma_fence_proxy: - Statuses : 8 pass(s) - Exec time: [0.03, 0.11] s Known issues Here are the changes found in Patchwork_17609_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_params@invalid-bsd-ring: - shard-iclb: [PASS][15] -> [SKIP][16] ([fdo#109276]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-iclb2/igt@gem_exec_par...@invalid-bsd-ring.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-iclb3/igt@gem_exec_par...@invalid-bsd-ring.html * igt@kms_cursor_crc@pipe-a-cursor-suspend: - shard-kbl: [PASS][17] -> [DMESG-WARN][18] ([i915#180]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-kbl3/igt@kms_cursor_...@pipe-a-cursor-suspend.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-kbl1/igt@kms_cursor_...@pipe-a-cursor-suspend.html * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes: - shard-skl: [PASS][19] -> [INCOMPLETE][20] ([i915#648] / [i915#69]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-skl3/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-a-planes.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-skl10/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-a-planes.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][21] -> [FAIL][22] ([fdo#108145] / [i915#265]) +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-skl10/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-skl2/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html * igt@kms_psr@psr2_no_drrs: - shard-iclb: [PASS][23] -> [SKIP][24] ([fdo#109441]) +1 similar issue [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8450/shard-iclb2/igt@kms_psr@psr2_no_drrs.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17609/shard-iclb3
Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
>-Original Message- >From: Chris Wilson >Sent: Thursday, May 7, 2020 9:57 AM >To: Ruhl, Michael J ; intel- >g...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH 12/15] drm/i915: Replace the hardcoded >I915_FENCE_TIMEOUT > >Quoting Ruhl, Michael J (2020-05-07 14:53:00) >> >> >> >-Original Message- >> >From: Intel-gfx On Behalf Of >Chris >> >Wilson >> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >index 3a146aa2593b..d3a86a4d5c04 100644 >> >--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c >> >@@ -288,8 +288,7 @@ int i915_gem_schedule_fill_pages_blt(struct >> >drm_i915_gem_object *obj, >> > >> > i915_gem_object_lock(obj); >> > err = i915_sw_fence_await_reservation(&work->wait, >> >-obj->base.resv, NULL, >> >-true, I915_FENCE_TIMEOUT, >> >+obj->base.resv, NULL, true, 0, >> >> Did you miss this one, or is the '0' on purpose? > >It should be 0, as it should be only handling internal fences which may >take as long as required and should not be timed out. > >Still this is a placeholder function and should not be taken too >seriously. Assuming that the "placeholder function" is the _fill_pages_blt()... Acked-by: Michael J. Ruhl Mike >-Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 53/59] drm/arc: Move to drm/tiny
Hi Daniel, > > Looking at this patch series, feels a bit like hand-rolling of bridge > > code, badly. We should get away from that. > > > > Once you have that I think the end result is tiny enough that it can > > stay, bridges intergrate quite well into simple display pipe drivers. > > > > > BTW should I pull that series in my tree and send you a pull-request > > > or that kind of change needs to go through another tree? > > > > > > Also I'd like to test the change we discuss here to make sure stuff > > > still works. Once we do that I'll send an update. Any hint on > > > when that change needs to be acked/nacked? > > > > Simplest is if this can all land through drm-misc, is arc not > > maintained in there? And there's plenty of time for testing, I'm just > > slowly crawling through the tree to get everything polished and > > cleaned up in this area. > > Any updates on testing this pile here? First patch landed now, and I've > started to push driver patches. So would be good to get this sorted out > too. Sorry we're in the middle of 2 long weekends so missed this one. I guess we'll be able to test it in a week or two from now. Is that OK? -Alexey ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 0/2] tests/gem_exec_nop: Remove submission batching
Execbuf requests are now submitted by subtests in batches of 1024 repetitions. That may be too many under some circumstances (e.g., intensive logging output) and subtests may take far more time than expected. Kill obsolete showcase for an old GuC failure, then remove submission batching from subtests which don't require submicrosecond precision. Janusz Krzysztofik (2): tests/gem_exec_nop: Kill obsolete pass/fail metric tests/gem_exec_nop: Remove submission batching tests/i915/gem_exec_nop.c | 137 -- 1 file changed, 59 insertions(+), 78 deletions(-) -- 2.21.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] tests/gem_exec_nop: Remove submission batching
Execbuf requests are now submitted by subtests in batches of 1024 repetitions. That may be too many under some circumstances (e.g., intensive logging output) and subtests may take far more time than expected. The reason standing behind that batching was unacceptable microsecond imprecision of gettime when gem_exec_nop was a benchmark rather than a test and time measurement was looking for a precision of ~100 ns. Since that measurement is now mostly informative and not a pass/fail metric, we can be more tolerant and accept overhead of gettime after each submission. Remove the batching from the body of subtests which don't require submicrosecond precision and measure time after each execbuf request submission (or a group of one submission per engine). Since there is one subtest - "headless" - which still requires more precise time measurement, don't remove the batching from nop_on_ring() helper but let its users request non-batched submission mode instead. To make this even more flexible, change semantics of the helper argument used so far for returning the count of submissions completed within the requested time frame and use it also for passing desired batch size (number of iterations), then update its users to initialize that argument according to their individual requirements. Suggested-by: Chris Wilson Signed-off-by: Janusz Krzysztofik --- tests/i915/gem_exec_nop.c | 120 +++--- 1 file changed, 59 insertions(+), 61 deletions(-) diff --git a/tests/i915/gem_exec_nop.c b/tests/i915/gem_exec_nop.c index c17d672c3..10639765b 100644 --- a/tests/i915/gem_exec_nop.c +++ b/tests/i915/gem_exec_nop.c @@ -71,12 +71,14 @@ static double elapsed(const struct timespec *start, const struct timespec *end) static double nop_on_ring(int fd, uint32_t handle, const struct intel_execution_engine2 *e, int timeout, - unsigned long *out) + unsigned long *count) { struct drm_i915_gem_execbuffer2 execbuf; struct drm_i915_gem_exec_object2 obj; struct timespec start, now; - unsigned long count; + unsigned long total; + + igt_assert(*count); memset(&obj, 0, sizeof(obj)); obj.handle = handle; @@ -93,18 +95,18 @@ static double nop_on_ring(int fd, uint32_t handle, } intel_detect_and_clear_missed_interrupts(fd); - count = 0; + total = 0; clock_gettime(CLOCK_MONOTONIC, &start); do { - for (int loop = 0; loop < 1024; loop++) + for (int loop = 0; loop < *count; loop++) gem_execbuf(fd, &execbuf); - count += 1024; + total += *count; clock_gettime(CLOCK_MONOTONIC, &now); } while (elapsed(&start, &now) < timeout); igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0); - *out = count; + *count = total; return elapsed(&start, &now); } @@ -353,7 +355,7 @@ static void single(int fd, uint32_t handle, const struct intel_execution_engine2 *e) { double time; - unsigned long count; + unsigned long count = 1; time = nop_on_ring(fd, handle, e, 20, &count); igt_info("%s: %'lu cycles: %.3fus\n", @@ -374,7 +376,7 @@ stable_nop_on_ring(int fd, uint32_t handle, s.is_float = true; while (reps--) { - unsigned long count; + unsigned long count = 1024; double time; time = nop_on_ring(fd, handle, e, timeout, &count); @@ -451,6 +453,7 @@ static void parallel(int fd, uint32_t handle, int timeout) engines[nengine] = e->flags; names[nengine++] = strdup(e->name); + count = 1; time = nop_on_ring(fd, handle, e, 1, &count) / count; sum += time; igt_debug("%s: %.3fus\n", e->name, 1e6*time); @@ -481,9 +484,8 @@ static void parallel(int fd, uint32_t handle, int timeout) count = 0; clock_gettime(CLOCK_MONOTONIC, &start); do { - for (int loop = 0; loop < 1024; loop++) - gem_execbuf(fd, &execbuf); - count += 1024; + gem_execbuf(fd, &execbuf); + count++; clock_gettime(CLOCK_MONOTONIC, &now); } while (elapsed(&start, &now) < timeout); time = elapsed(&start, &now) / count; @@ -513,6 +515,7 @@ static void independent(int fd, uint32_t handle, int timeout) engines[nengine] = e->flags; names[nengine++] = strdup(e->name); + count = 1; time = nop_on_ring(fd, handle, e, 1, &count) / count; sum += time; igt_debug("%s: %.3fus\n", e->name, 1e6*time); @@ -633,6 +636,7 @@ static void
[Intel-gfx] [PATCH i-g-t 1/2] tests/gem_exec_nop: Kill obsolete pass/fail metric
Commit 870c774b866c ("igt/gem_exec_nop: Add expectancy of independent execution between engines") extended a "basic" subtest (now "basic-series") with a pass/fail metric based on comparison of parallel execution time to be less than an average * 2. Since then, that limit has been raised quite a few times: - by commit 41a26b5152a5 ("igt/gem_exec_nop: Relax parallel assertion for short rings") to maximum + minimum, - by commit 7bd4f918c461 ("igt/gem_exec_nop: Explain the parallel execution assertion") to maximum + minimum * 10/9, - by commit a0eebbddecaa ("igt/gem_exec_nop: Relax assertion for parallel execution") to sum * 2. With the criteria relaxed up to that extent, the purpose of that check has been limited to a showcase for an old GuC failure. Since that is now obsolete, kill that assert. Suggested-by: Chris Wilson Signed-off-by: Janusz Krzysztofik --- tests/i915/gem_exec_nop.c | 17 - 1 file changed, 17 deletions(-) diff --git a/tests/i915/gem_exec_nop.c b/tests/i915/gem_exec_nop.c index 357449c5b..c17d672c3 100644 --- a/tests/i915/gem_exec_nop.c +++ b/tests/i915/gem_exec_nop.c @@ -682,23 +682,6 @@ static void series(int fd, uint32_t handle, int timeout) time = elapsed(&start, &now) / count; igt_info("All (%d engines): %'lu cycles, average %.3fus per cycle [expected %.3fus]\n", nengine, count, 1e6*time, 1e6*((max-min)/nengine+min)); - - /* The rate limiting step should be how fast the slowest engine can -* execute its queue of requests, as when we wait upon a full ring all -* dispatch is frozen. So in general we cannot go faster than the -* slowest engine (but as all engines are in lockstep, they should all -* be executing in parallel and so the average should be max/nengines), -* but we should equally not go any slower. -* -* However, that depends upon being able to submit fast enough, and -* that in turns depends upon debugging turned off and no bottlenecks -* within the driver. We cannot assert that we hit ideal conditions -* across all engines, so we only look for an outrageous error -* condition. -*/ - igt_assert_f(time < 2*sum, -"Average time (%.3fus) exceeds expectation for parallel execution (min %.3fus, max %.3fus; limit set at %.3fus)\n", -1e6*time, 1e6*min, 1e6*max, 1e6*sum*2); } static void xchg(void *array, unsigned i, unsigned j) -- 2.21.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: Improve precision on defer_request assert
== Series Details == Series: drm/i915/gt: Improve precision on defer_request assert URL : https://patchwork.freedesktop.org/series/77074/ State : success == Summary == CI Bug Log - changes from CI_DRM_8451_full -> Patchwork_17610_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_17610_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_tiled_swapping@non-threaded: - shard-glk: [PASS][1] -> [INCOMPLETE][2] ([i915#1521] / [i915#58] / [k.org#198133]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-glk8/igt@gem_tiled_swapp...@non-threaded.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-glk9/igt@gem_tiled_swapp...@non-threaded.html * igt@i915_pm_backlight@fade_with_suspend: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([i915#69]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl1/igt@i915_pm_backlight@fade_with_suspend.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-skl7/igt@i915_pm_backlight@fade_with_suspend.html * igt@i915_pm_rpm@system-suspend-modeset: - shard-skl: [PASS][5] -> [INCOMPLETE][6] ([i915#151] / [i915#69]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl6/igt@i915_pm_...@system-suspend-modeset.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-skl3/igt@i915_pm_...@system-suspend-modeset.html * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen: - shard-kbl: [PASS][7] -> [FAIL][8] ([i915#54] / [i915#93] / [i915#95]) +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-kbl7/igt@kms_cursor_...@pipe-a-cursor-64x64-onscreen.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-kbl4/igt@kms_cursor_...@pipe-a-cursor-64x64-onscreen.html * igt@kms_cursor_legacy@pipe-b-torture-bo: - shard-tglb: [PASS][9] -> [DMESG-WARN][10] ([i915#128]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-tglb6/igt@kms_cursor_leg...@pipe-b-torture-bo.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-tglb1/igt@kms_cursor_leg...@pipe-b-torture-bo.html * igt@kms_flip_tiling@flip-changes-tiling: - shard-kbl: [PASS][11] -> [FAIL][12] ([i915#699] / [i915#93] / [i915#95]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-kbl7/igt@kms_flip_til...@flip-changes-tiling.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-kbl6/igt@kms_flip_til...@flip-changes-tiling.html * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes: - shard-apl: [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-apl1/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-b-planes.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-apl4/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-b-planes.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145] / [i915#265]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl5/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-skl8/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html * igt@kms_psr@psr2_cursor_blt: - shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-iclb8/igt@kms_psr@psr2_cursor_blt.html * igt@kms_psr@suspend: - shard-iclb: [PASS][19] -> [INCOMPLETE][20] ([i915#1185]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb7/igt@kms_...@suspend.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-iclb3/igt@kms_...@suspend.html Possible fixes * igt@gem_exec_params@invalid-bsd-ring: - shard-iclb: [SKIP][21] ([fdo#109276]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb5/igt@gem_exec_par...@invalid-bsd-ring.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-iclb2/igt@gem_exec_par...@invalid-bsd-ring.html * igt@gen9_exec_parse@allowed-all: - shard-apl: [DMESG-WARN][23] ([i915#716]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-apl6/igt@gen9_exec_pa...@allowed-all.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17610/shard-apl7/igt@gen9_exec_pa...@allowed-all.html * igt@kms_cursor_crc@pipe-b-cursor-suspend: - shard-kbl:
[Intel-gfx] ✓ Fi.CI.IGT: success for dma-buf: Use atomic_fetch_add() for the context id
== Series Details == Series: dma-buf: Use atomic_fetch_add() for the context id URL : https://patchwork.freedesktop.org/series/77076/ State : success == Summary == CI Bug Log - changes from CI_DRM_8451_full -> Patchwork_17611_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_17611_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_flush@basic-uc-prw-default: - shard-hsw: [PASS][1] -> [INCOMPLETE][2] ([i915#61]) +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-hsw4/igt@gem_exec_fl...@basic-uc-prw-default.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-hsw5/igt@gem_exec_fl...@basic-uc-prw-default.html * igt@i915_pm_dc@dc6-psr: - shard-iclb: [PASS][3] -> [FAIL][4] ([i915#454]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb4/igt@i915_pm...@dc6-psr.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-iclb8/igt@i915_pm...@dc6-psr.html * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen: - shard-kbl: [PASS][5] -> [FAIL][6] ([i915#54] / [i915#93] / [i915#95]) +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-kbl7/igt@kms_cursor_...@pipe-a-cursor-64x64-onscreen.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-kbl7/igt@kms_cursor_...@pipe-a-cursor-64x64-onscreen.html * igt@kms_flip_tiling@flip-changes-tiling: - shard-kbl: [PASS][7] -> [FAIL][8] ([i915#699] / [i915#93] / [i915#95]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-kbl7/igt@kms_flip_til...@flip-changes-tiling.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-kbl3/igt@kms_flip_til...@flip-changes-tiling.html * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes: - shard-apl: [PASS][9] -> [DMESG-WARN][10] ([i915#180]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-apl1/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-b-planes.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-apl4/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-b-planes.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][11] -> [FAIL][12] ([fdo#108145] / [i915#265]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl5/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-skl2/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html * igt@kms_psr@psr2_cursor_blt: - shard-iclb: [PASS][13] -> [SKIP][14] ([fdo#109441]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-iclb8/igt@kms_psr@psr2_cursor_blt.html * igt@perf@stress-open-close: - shard-iclb: [PASS][15] -> [INCOMPLETE][16] ([i915#1356]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb8/igt@p...@stress-open-close.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-iclb5/igt@p...@stress-open-close.html Possible fixes * igt@gem_exec_params@invalid-bsd-ring: - shard-iclb: [SKIP][17] ([fdo#109276]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-iclb5/igt@gem_exec_par...@invalid-bsd-ring.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-iclb4/igt@gem_exec_par...@invalid-bsd-ring.html * igt@gen9_exec_parse@allowed-all: - shard-apl: [DMESG-WARN][19] ([i915#716]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-apl6/igt@gen9_exec_pa...@allowed-all.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-apl6/igt@gen9_exec_pa...@allowed-all.html * igt@i915_suspend@fence-restore-untiled: - shard-skl: [INCOMPLETE][21] ([i915#69]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl1/igt@i915_susp...@fence-restore-untiled.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-skl3/igt@i915_susp...@fence-restore-untiled.html * igt@kms_cursor_crc@pipe-b-cursor-128x42-offscreen: - shard-skl: [FAIL][23] ([i915#54]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8451/shard-skl7/igt@kms_cursor_...@pipe-b-cursor-128x42-offscreen.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17611/shard-skl10/igt@kms_cursor_...@pipe-b-cursor-128x42-offscreen.html * igt@kms_cursor_crc@pipe-b-cursor-suspend: - shard-kbl: [DMESG-WARN][25] ([i915#180])
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Prevent using semaphores to chain up to external fences
Chris Wilson writes: > The downside of using semaphores is that we lose metadata passing > along the signaling chain. This is particularly nasty when we > need to pass along a fatal error such as EFAULT or EDEADLK. For > fatal errors we want to scrub the request before it is executed, > which means that we cannot preload the request onto HW and have > it wait upon a semaphore. b is waiting on a, a fails and we want to release b with error? > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_request.c | 26 + > drivers/gpu/drm/i915/i915_scheduler_types.h | 1 + > 2 files changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_request.c > b/drivers/gpu/drm/i915/i915_request.c > index 94189c7d43cd..f0f9393e2ade 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1002,6 +1002,15 @@ emit_semaphore_wait(struct i915_request *to, > if (!rcu_access_pointer(from->hwsp_cacheline)) > goto await_fence; > > + /* > + * If this or its dependents are waiting on an external fence > + * that may fail catastrophically, then we want to avoid using > + * sempahores as they bypass the fence signaling metadata, and we semaphore -Mika > + * lose the fence->error propagation. > + */ > + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) > + goto await_fence; > + > /* Just emit the first semaphore we see as request space is limited. */ > if (already_busywaiting(to) & mask) > goto await_fence; > @@ -1064,12 +1073,29 @@ i915_request_await_request(struct i915_request *to, > struct i915_request *from) > return ret; > } > > + if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) > + to->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; > + > return 0; > } > > +static void mark_external(struct i915_request *rq) > +{ > + /* > + * The downside of using semaphores is that we lose metadata passing > + * along the signaling chain. This is particularly nasty when we > + * need to pass along a fatal error such as EFAULT or EDEADLK. For > + * fatal errors we want to scrub the request before it is executed, > + * which means that we cannot preload the request onto HW and have > + * it wait upon a semaphore. > + */ > + rq->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN; > +} > + > static int > i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) > { > + mark_external(rq); > return i915_sw_fence_await_dma_fence(&rq->submit, fence, >fence->context ? > I915_FENCE_TIMEOUT : 0, >I915_FENCE_GFP); > diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h > b/drivers/gpu/drm/i915/i915_scheduler_types.h > index 7186875088a0..6ab2c5289bed 100644 > --- a/drivers/gpu/drm/i915/i915_scheduler_types.h > +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h > @@ -66,6 +66,7 @@ struct i915_sched_node { > struct i915_sched_attr attr; > unsigned int flags; > #define I915_SCHED_HAS_SEMAPHORE_CHAIN BIT(0) > +#define I915_SCHED_HAS_EXTERNAL_CHAINBIT(1) > intel_engine_mask_t semaphores; > }; > > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Prevent using semaphores to chain up to external fences
Quoting Mika Kuoppala (2020-05-08 16:37:15) > Chris Wilson writes: > > > The downside of using semaphores is that we lose metadata passing > > along the signaling chain. This is particularly nasty when we > > need to pass along a fatal error such as EFAULT or EDEADLK. For > > fatal errors we want to scrub the request before it is executed, > > which means that we cannot preload the request onto HW and have > > it wait upon a semaphore. > > b is waiting on a, a fails and we want to release b with error? Yes. B is submitted before A, and if B is relying on A to setup GPU page tables or other interesting things, we can't let B run if A dies. For the EDEADLK if B is waiting on A, but then A is submitted with a wait on B -- we have to untangle that mess. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Prevent using semaphores to chain up to external fences
Chris Wilson writes: > Quoting Mika Kuoppala (2020-05-08 16:37:15) >> Chris Wilson writes: >> >> > The downside of using semaphores is that we lose metadata passing >> > along the signaling chain. This is particularly nasty when we >> > need to pass along a fatal error such as EFAULT or EDEADLK. For >> > fatal errors we want to scrub the request before it is executed, >> > which means that we cannot preload the request onto HW and have >> > it wait upon a semaphore. >> >> b is waiting on a, a fails and we want to release b with error? > > Yes. B is submitted before A, and if B is relying on A to setup GPU page I guess this has to be A is before B. > tables or other interesting things, we can't let B run if A dies. For > the EDEADLK if B is waiting on A, but then A is submitted with a wait on > B -- we have to untangle that mess. Avoiding the hw semaphore makes sense Reviewed-by: Mika Kuoppala > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Tidy awaiting on dma-fences
Chris Wilson writes: > Just tidy up the return handling for completed dma-fences. While it may > return errors for invalid fence, we already know that we have a good > fence and the only error will be an already signaled fence. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_sw_fence.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c > b/drivers/gpu/drm/i915/i915_sw_fence.c > index 7daf81f55c90..295b9829e2da 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.c > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c > @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct > i915_sw_fence *fence, > cb->fence = fence; > i915_sw_fence_await(fence); > > - ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake); > - if (ret == 0) { > - ret = 1; > - } else { > + ret = 1; > + if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) { > + /* fence already signaled */ This seems to hold water now. Perhaps for eternity. But how about if (dma_fence_add_callback() == -ENOENT) ret = 0; else GEM_BUG_ON()? -Mika > __dma_i915_sw_fence_wake(dma, &cb->base); > - if (ret == -ENOENT) /* fence already signaled */ > - ret = 0; > + ret = 0; > } > > return ret; > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Prevent using semaphores to chain up to external fences
Quoting Mika Kuoppala (2020-05-08 16:44:48) > Chris Wilson writes: > > > Quoting Mika Kuoppala (2020-05-08 16:37:15) > >> Chris Wilson writes: > >> > >> > The downside of using semaphores is that we lose metadata passing > >> > along the signaling chain. This is particularly nasty when we > >> > need to pass along a fatal error such as EFAULT or EDEADLK. For > >> > fatal errors we want to scrub the request before it is executed, > >> > which means that we cannot preload the request onto HW and have > >> > it wait upon a semaphore. > >> > >> b is waiting on a, a fails and we want to release b with error? > > > > Yes. B is submitted before A, and if B is relying on A to setup GPU page > > I guess this has to be A is before B. > > > tables or other interesting things, we can't let B run if A dies. For > > the EDEADLK if B is waiting on A, but then A is submitted with a wait on > > B -- we have to untangle that mess. > > Avoiding the hw semaphore makes sense It's an unfortunate necessity, unless we can work in a conditional jump after the semaphore. But it also requires sampling all the possible error signals :( -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Tidy awaiting on dma-fences
Quoting Mika Kuoppala (2020-05-08 16:50:22) > Chris Wilson writes: > > > Just tidy up the return handling for completed dma-fences. While it may > > return errors for invalid fence, we already know that we have a good > > fence and the only error will be an already signaled fence. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_sw_fence.c | 10 -- > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c > > b/drivers/gpu/drm/i915/i915_sw_fence.c > > index 7daf81f55c90..295b9829e2da 100644 > > --- a/drivers/gpu/drm/i915/i915_sw_fence.c > > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c > > @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct > > i915_sw_fence *fence, > > cb->fence = fence; > > i915_sw_fence_await(fence); > > > > - ret = dma_fence_add_callback(dma, &cb->base, > > __dma_i915_sw_fence_wake); > > - if (ret == 0) { > > - ret = 1; > > - } else { > > + ret = 1; > > + if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) > > { > > + /* fence already signaled */ > > This seems to hold water now. Perhaps for eternity. > > But how about if (dma_fence_add_callback() == -ENOENT) ret = 0; else > GEM_BUG_ON()? Because that's just ugly. If we do not install the callback, we need to signal the callback. The only question is whether or not an error there is moot -- we either have the fence, or we would have exploded. The fence callback will be propagating errors along the fence. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gt: Replace zero-length array with flexible-array
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/i915/display/intel_vbt_defs.h |4 ++-- drivers/gpu/drm/i915/gt/intel_lrc.c |2 +- drivers/gpu/drm/i915/i915_gpu_error.h |2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h index 05c7cbe32eb4..aef7fe932d1a 100644 --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h @@ -462,7 +462,7 @@ struct bdb_general_definitions { * number = (block_size - sizeof(bdb_general_definitions))/ * defs->child_dev_size; */ - u8 devices[0]; + u8 devices[]; } __packed; /* @@ -839,7 +839,7 @@ struct bdb_mipi_config { struct bdb_mipi_sequence { u8 version; - u8 data[0]; /* up to 6 variable length blocks */ + u8 data[]; /* up to 6 variable length blocks */ } __packed; /* diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 683014e7bc51..f42c99da2580 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -216,7 +216,7 @@ struct virtual_engine { /* And finally, which physical engines this virtual engine maps onto. */ unsigned int num_siblings; - struct intel_engine_cs *siblings[0]; + struct intel_engine_cs *siblings[]; }; static struct virtual_engine *to_virtual_engine(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h index 0d1f6c8ff355..5a6561f7a210 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.h +++ b/drivers/gpu/drm/i915/i915_gpu_error.h @@ -42,7 +42,7 @@ struct i915_vma_coredump { int num_pages; int page_count; int unused; - u32 *pages[0]; + u32 *pages[]; }; struct i915_request_coredump { ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Fix page flip ioctl format check
On Fri, Apr 17, 2020 at 09:28:34PM +0300, Ville Syrjälä wrote: > On Fri, Apr 17, 2020 at 08:10:26PM +0200, Daniel Vetter wrote: > > On Fri, Apr 17, 2020 at 5:43 PM Ville Syrjälä > > wrote: > > > > > > On Fri, Apr 17, 2020 at 05:23:10PM +0200, Daniel Vetter wrote: > > > > On Thu, Apr 16, 2020 at 08:04:20PM +0300, Ville Syrjala wrote: > > > > > From: Ville Syrjälä > > > > > > > > > > Revert back to comparing fb->format->format instead fb->format for the > > > > > page flip ioctl. This check was originally only here to disallow pixel > > > > > format changes, but when we changed it to do the pointer comparison > > > > > we potentially started to reject some (but definitely not all) > > > > > modifier > > > > > changes as well. In fact the current behaviour depends on whether the > > > > > driver overrides the format info for a specific format+modifier combo. > > > > > Eg. on i915 this now rejects compression vs. no compression changes > > > > > but > > > > > does not reject any other tiling changes. That's just inconsistent > > > > > nonsense. > > > > > > > > > > The main reason we have to go back to the old behaviour is to fix page > > > > > flipping with Xorg. At some point Xorg got its atomic rights taken > > > > > away > > > > > and since then we can't page flip between compressed and > > > > > non-compressed > > > > > fbs on i915. Currently we get no page flipping for any games pretty > > > > > much > > > > > since Mesa likes to use compressed buffers. Not sure how compositors > > > > > are > > > > > working around this (don't use one myself). I guess they must be doing > > > > > something to get non-compressed buffers instead. Either that or > > > > > somehow no one noticed the tearing from the blit fallback. > > > > > > > > Mesa only uses compressed buffers if you enable modifiers, and there's a > > > > _lt_ more that needs to be fixed in Xorg to enable that for > > > > real. Like real atomic support. > > > > > > Why would you need atomic for modifiers? Xorg doesn't even have > > > any sensible framework for atomic and I suspect it never will. > > > > Frankly if no one cares about atomic in X I don't think we should do > > work-arounds for lack of atomic in X. > > > > > > Without modifiers all you get is X tiling, > > > > and that works just fine. > > > > > > > > Which would also fix this issue here you're papering over. > > > > > > > > So if this is the entire reason for this, I'm inclined to not do this. > > > > Current Xorg is toast wrt modifiers, that's not news. > > > > > > Works just fine. Also pretty sure modifiers are even enabled by > > > default now in modesetting. > > > > Y/CSS is harder to scan out, you need to verify with TEST_ONLY whether > > it works. Otherwise good chances for some oddball black screens on > > configurations that worked before. Which is why all non-atomic > > compositors reverted modifiers by default again. > > Y alone is hard to scanout also, and yet we do nothing to reject that. > It's just an inconsistent mess. > > If we really want to keep this check then we should rewrite it > to be explicit: > > if (old_fb->format->format != new_fb->format->format || > is_ccs(old_fb->modifier) != is_ccs(new_fb->modifier)) > return -EINVAL; > > Now it's just a random thing that may even stop doing what it's > currently doing if anyone touches their .get_format_info() > implementation. > > > > > > And as stated the current check doesn't have consistent behaviour > > > anyway. You can still flip between different modifiers as long a the > > > driver doesn't override .get_format_info() for one of them. The *only* > > > case where that happens is CCS on i915. There is no valid reason to > > > special case that one. > > > > The thing is, you need atomic to make CCS work reliably enough for > > compositors and distros to dare enabling it by default. > > If it's not enabled by default then there is no harm in letting people > explicitly enable it and get better performance. > > > CCS flipping > > works with atomic. I really see no point in baking this in with as > > uapi. > > It's just going back to the original intention of the check. > Heck, the debug message doesn't even match what it's doing now. > > > Just fix Xorg. > > Be serious. No one is going to rewrite all the randr code to be atomic. I fully understand Daniel's concern here, but I also believe this won't be done so soon at least. Meanwhile would it be acceptable to have a comment with the code /* XXX: Xorg blah... */ or /* FIXME: After Xorg blah.. */ ? > > > -Daniel > > > > > > > > > -Daniel > > > > > > > > > > > > > > Looking back at the original discussion on this change we pretty much > > > > > just did it in the name of skipping a few extra pointer dereferences. > > > > > However, I've decided not to revert the whole thing in case someone > > > > > has since started to depend on these changes. None of the other checks > > > > > are relevant for i915 anyways. > > > > > > > > >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Replace zero-length array with flexible-array
== Series Details == Series: drm/i915/gt: Replace zero-length array with flexible-array URL : https://patchwork.freedesktop.org/series/77080/ State : success == Summary == CI Bug Log - changes from CI_DRM_8453 -> Patchwork_17612 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/index.html Known issues Here are the changes found in Patchwork_17612 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gt_timelines: - fi-bwr-2160:[PASS][1] -> [INCOMPLETE][2] ([i915#489]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/fi-bwr-2160/igt@i915_selftest@live@gt_timelines.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/fi-bwr-2160/igt@i915_selftest@live@gt_timelines.html [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489 Participating hosts (48 -> 41) -- Additional (1): fi-kbl-7560u Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-byt-clapper fi-bdw-samus Build changes - * CI: CI-20190529 -> None * Linux: CI_DRM_8453 -> Patchwork_17612 CI-20190529: 20190529 CI_DRM_8453: bd2e8a4803db758fcbc558acbf5ad89e3a1779b0 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5642: d1ce4abb01c70f7be6e777b6d45442663c4b830e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_17612: efdf66834e31052f2218abc1bc1dbb87a0278e9b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == efdf66834e31 drm/i915/gt: Replace zero-length array with flexible-array == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tests/gem_exec_nop: Kill obsolete pass/fail metric
Quoting Janusz Krzysztofik (2020-05-08 14:56:30) > Commit 870c774b866c ("igt/gem_exec_nop: Add expectancy of independent > execution between engines") extended a "basic" subtest (now > "basic-series") with a pass/fail metric based on comparison of parallel > execution time to be less than an average * 2. Since then, that limit > has been raised quite a few times: > - by commit 41a26b5152a5 ("igt/gem_exec_nop: Relax parallel assertion > for short rings") to maximum + minimum, > - by commit 7bd4f918c461 ("igt/gem_exec_nop: Explain the parallel > execution assertion") to maximum + minimum * 10/9, > - by commit a0eebbddecaa ("igt/gem_exec_nop: Relax assertion for > parallel execution") to sum * 2. > > With the criteria relaxed up to that extent, the purpose of that check > has been limited to a showcase for an old GuC failure. Since that is > now obsolete, kill that assert. > > Suggested-by: Chris Wilson > Signed-off-by: Janusz Krzysztofik Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] tests/gem_exec_nop: Remove submission batching
Quoting Janusz Krzysztofik (2020-05-08 14:56:31) > static double nop_on_ring(int fd, uint32_t handle, > const struct intel_execution_engine2 *e, int > timeout, > - unsigned long *out) > + unsigned long *count) > { > struct drm_i915_gem_execbuffer2 execbuf; > struct drm_i915_gem_exec_object2 obj; > struct timespec start, now; > - unsigned long count; > + unsigned long total; > + > + igt_assert(*count); > > memset(&obj, 0, sizeof(obj)); > obj.handle = handle; > @@ -93,18 +95,18 @@ static double nop_on_ring(int fd, uint32_t handle, > } > intel_detect_and_clear_missed_interrupts(fd); > > - count = 0; > + total = 0; > clock_gettime(CLOCK_MONOTONIC, &start); > do { > - for (int loop = 0; loop < 1024; loop++) > + for (int loop = 0; loop < *count; loop++) This unnerves me. I expect to get this wrong when writing new callers. There's no great reason to even have 1024 here, we can survive with doing clock_gettime() every iteration, and just accept it as part of the systematic cost. > gem_execbuf(fd, &execbuf); > > - count += 1024; > + total += *count; > clock_gettime(CLOCK_MONOTONIC, &now); > } while (elapsed(&start, &now) < timeout); > igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0); > > - *out = count; > + *count = total; > return elapsed(&start, &now); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 53/59] drm/arc: Move to drm/tiny
On Fri, May 8, 2020 at 3:56 PM Alexey Brodkin wrote: > > Hi Daniel, > > > > Looking at this patch series, feels a bit like hand-rolling of bridge > > > code, badly. We should get away from that. > > > > > > Once you have that I think the end result is tiny enough that it can > > > stay, bridges intergrate quite well into simple display pipe drivers. > > > > > > > BTW should I pull that series in my tree and send you a pull-request > > > > or that kind of change needs to go through another tree? > > > > > > > > Also I'd like to test the change we discuss here to make sure stuff > > > > still works. Once we do that I'll send an update. Any hint on > > > > when that change needs to be acked/nacked? > > > > > > Simplest is if this can all land through drm-misc, is arc not > > > maintained in there? And there's plenty of time for testing, I'm just > > > slowly crawling through the tree to get everything polished and > > > cleaned up in this area. > > > > Any updates on testing this pile here? First patch landed now, and I've > > started to push driver patches. So would be good to get this sorted out > > too. > > Sorry we're in the middle of 2 long weekends so missed this one. > I guess we'll be able to test it in a week or two from now. > > Is that OK? This aren't high-priority, so totally ok. As long as you don't land a driver rewrite and I have to rebase everything :-) -Daniel > > -Alexey > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Peel dma-fence-chains for await
From: Lionel Landwerlin To allow faster engine to engine synchronization, peel the layer of dma-fence-chain to expose potential i915 fences so that the i915_request code can emit HW semaphore wait/signal operations in the ring which is faster than waking up the host to submit unblocked workloads after interrupt notification. This is similar to the peeling we do for e.g. dma_fence_array. Signed-off-by: Lionel Landwerlin --- drivers/gpu/drm/i915/i915_request.c | 30 - 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 94189c7d43cd..88a491078fee 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -1068,13 +1069,40 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) } static int -i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +__i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) { return i915_sw_fence_await_dma_fence(&rq->submit, fence, fence->context ? I915_FENCE_TIMEOUT : 0, I915_FENCE_GFP); } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + struct dma_fence *iter; + int err = 0; + + if (!to_dma_fence_chain(fence)) + return __i915_request_await_external(rq, fence); + + dma_fence_chain_for_each(iter, fence) { + struct dma_fence_chain *chain = to_dma_fence_chain(iter); + GEM_BUG_ON(!chain); + + if (!dma_fence_is_i915(chain->fence)) { + err =__i915_request_await_external(rq, chain->fence); + break; + } + + err = i915_request_await_request(rq, to_request(chain->fence)); + if (err < 0) + break; + } + + dma_fence_put(iter); + return err; +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Peel dma-fence-chains for await
From: Lionel Landwerlin To allow faster engine to engine synchronization, peel the layer of dma-fence-chain to expose potential i915 fences so that the i915_request code can emit HW semaphore wait/signal operations in the ring which is faster than waking up the host to submit unblocked workloads after interrupt notification. This is similar to the peeling we do for e.g. dma_fence_array. Signed-off-by: Lionel Landwerlin --- I wanted to avoid the recursion, but the timeline squashing is useful here. --- drivers/gpu/drm/i915/i915_request.c | 27 ++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 94189c7d43cd..d0321e990ad1 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -1068,13 +1069,37 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) } static int -i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +__i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) { return i915_sw_fence_await_dma_fence(&rq->submit, fence, fence->context ? I915_FENCE_TIMEOUT : 0, I915_FENCE_GFP); } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + struct dma_fence *iter; + + if (!to_dma_fence_chain(fence)) + return __i915_request_await_external(rq, fence); + + dma_fence_chain_for_each(iter, fence) { + struct dma_fence_chain *chain = to_dma_fence_chain(iter); + int err; + + GEM_BUG_ON(!chain); + + err = i915_request_await_dma_fence(rq, chain->fence); + if (err) { + dma_fence_put(iter); + return err; + } + } + + return 0; +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: Replace zero-length array with flexible-array
== Series Details == Series: drm/i915/gt: Replace zero-length array with flexible-array URL : https://patchwork.freedesktop.org/series/77080/ State : success == Summary == CI Bug Log - changes from CI_DRM_8453_full -> Patchwork_17612_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_17612_full that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_dc@dc6-psr: - shard-skl: [PASS][1] -> [FAIL][2] ([i915#454]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl2/igt@i915_pm...@dc6-psr.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-skl3/igt@i915_pm...@dc6-psr.html * igt@i915_suspend@forcewake: - shard-kbl: [PASS][3] -> [DMESG-WARN][4] ([i915#180]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-kbl6/igt@i915_susp...@forcewake.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-kbl4/igt@i915_susp...@forcewake.html * igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen: - shard-kbl: [PASS][5] -> [FAIL][6] ([i915#54] / [i915#93] / [i915#95]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-kbl2/igt@kms_cursor_...@pipe-a-cursor-64x64-offscreen.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-kbl3/igt@kms_cursor_...@pipe-a-cursor-64x64-offscreen.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [PASS][7] -> [FAIL][8] ([i915#1188]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl3/igt@kms_...@bpc-switch-dpms.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-skl6/igt@kms_...@bpc-switch-dpms.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][9] -> [FAIL][10] ([fdo#108145] / [i915#265]) +2 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl9/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-skl7/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html * igt@kms_psr@psr2_suspend: - shard-iclb: [PASS][11] -> [SKIP][12] ([fdo#109441]) +2 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-iclb2/igt@kms_psr@psr2_suspend.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-iclb8/igt@kms_psr@psr2_suspend.html * igt@kms_vblank@pipe-b-ts-continuation-suspend: - shard-apl: [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +2 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-apl6/igt@kms_vbl...@pipe-b-ts-continuation-suspend.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-apl6/igt@kms_vbl...@pipe-b-ts-continuation-suspend.html Possible fixes * igt@gem_exec_suspend@basic-s3: - shard-skl: [INCOMPLETE][15] ([i915#69]) -> [PASS][16] +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl1/igt@gem_exec_susp...@basic-s3.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-skl6/igt@gem_exec_susp...@basic-s3.html * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled: - shard-skl: [FAIL][17] ([i915#52] / [i915#54]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl6/igt@kms_draw_...@draw-method-rgb565-mmap-wc-ytiled.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-skl1/igt@kms_draw_...@draw-method-rgb565-mmap-wc-ytiled.html * {igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2}: - shard-glk: [FAIL][19] ([i915#79]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interrupti...@ab-hdmi-a1-hdmi-a2.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interrupti...@ab-hdmi-a1-hdmi-a2.html * {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}: - shard-apl: [DMESG-WARN][21] ([i915#180]) -> [PASS][22] +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-apl4/igt@kms_flip@flip-vs-suspend-interrupti...@c-dp1.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-apl8/igt@kms_flip@flip-vs-suspend-interrupti...@c-dp1.html * {igt@kms_flip@flip-vs-suspend@b-hdmi-a1}: - shard-hsw: [INCOMPLETE][23] ([i915#61]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-hsw4/igt@kms_flip@flip-vs-susp...@b-hdmi-a1.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17612/shard-hsw1/igt@kms_flip@flip-vs-susp...@b-hdmi-a1.html * igt@kms_hdr@bpc-switch-suspen
[Intel-gfx] [PATCH] drm/i915: Peel dma-fence-chains for await
From: Lionel Landwerlin To allow faster engine to engine synchronization, peel the layer of dma-fence-chain to expose potential i915 fences so that the i915_request code can emit HW semaphore wait/signal operations in the ring which is faster than waking up the host to submit unblocked workloads after interrupt notification. This is similar to the peeling we do for e.g. dma_fence_array. Signed-off-by: Lionel Landwerlin --- drivers/gpu/drm/i915/i915_request.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 94189c7d43cd..9df2cb8d66d0 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -1068,13 +1069,39 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) } static int -i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +__i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) { return i915_sw_fence_await_dma_fence(&rq->submit, fence, fence->context ? I915_FENCE_TIMEOUT : 0, I915_FENCE_GFP); } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + struct dma_fence *iter; + int err = 0; + + if (!to_dma_fence_chain(fence)) + return __i915_request_await_external(rq, fence); + + dma_fence_chain_for_each(iter, fence) { + struct dma_fence_chain *chain = to_dma_fence_chain(iter); + + if (!dma_fence_is_i915(chain->fence)) { + err = __i915_request_await_external(rq, iter); + break; + } + + err = i915_request_await_dma_fence(rq, chain->fence); + if (err < 0) + break; + } + + dma_fence_put(iter); + return err; +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Peel dma-fence-chains for await (rev3)
== Series Details == Series: drm/i915: Peel dma-fence-chains for await (rev3) URL : https://patchwork.freedesktop.org/series/77081/ State : success == Summary == CI Bug Log - changes from CI_DRM_8453 -> Patchwork_17613 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/index.html Known issues Here are the changes found in Patchwork_17613 that come from known issues: ### IGT changes ### Warnings * igt@i915_pm_rpm@module-reload: - fi-kbl-x1275: [SKIP][1] ([fdo#109271]) -> [FAIL][2] ([i915#62]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 Participating hosts (48 -> 43) -- Additional (1): fi-kbl-7560u Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * CI: CI-20190529 -> None * Linux: CI_DRM_8453 -> Patchwork_17613 CI-20190529: 20190529 CI_DRM_8453: bd2e8a4803db758fcbc558acbf5ad89e3a1779b0 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5642: d1ce4abb01c70f7be6e777b6d45442663c4b830e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_17613: f7542d436cfcb22ac4769e8277fa6a555bdcb44b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == f7542d436cfc drm/i915: Peel dma-fence-chains for await == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Peel dma-fence-chains for await (rev3)
== Series Details == Series: drm/i915: Peel dma-fence-chains for await (rev3) URL : https://patchwork.freedesktop.org/series/77081/ State : success == Summary == CI Bug Log - changes from CI_DRM_8453_full -> Patchwork_17613_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_17613_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_workarounds@suspend-resume: - shard-apl: [PASS][1] -> [DMESG-WARN][2] ([i915#180]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-apl8/igt@gem_workarou...@suspend-resume.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-apl4/igt@gem_workarou...@suspend-resume.html * igt@gen9_exec_parse@allowed-single: - shard-skl: [PASS][3] -> [DMESG-WARN][4] ([i915#716]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl9/igt@gen9_exec_pa...@allowed-single.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl8/igt@gen9_exec_pa...@allowed-single.html * igt@i915_pm_dc@dc6-psr: - shard-skl: [PASS][5] -> [FAIL][6] ([i915#454]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl2/igt@i915_pm...@dc6-psr.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl7/igt@i915_pm...@dc6-psr.html * igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen: - shard-kbl: [PASS][7] -> [FAIL][8] ([i915#54] / [i915#93] / [i915#95]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-kbl2/igt@kms_cursor_...@pipe-a-cursor-64x64-offscreen.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-kbl7/igt@kms_cursor_...@pipe-a-cursor-64x64-offscreen.html * igt@kms_cursor_edge_walk@pipe-a-64x64-bottom-edge: - shard-snb: [PASS][9] -> [SKIP][10] ([fdo#109271]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-snb5/igt@kms_cursor_edge_w...@pipe-a-64x64-bottom-edge.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-snb6/igt@kms_cursor_edge_w...@pipe-a-64x64-bottom-edge.html * igt@kms_hdr@bpc-switch: - shard-skl: [PASS][11] -> [FAIL][12] ([i915#1188]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl8/igt@kms_...@bpc-switch.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl10/igt@kms_...@bpc-switch.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: - shard-kbl: [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-kbl7/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-kbl4/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145] / [i915#265]) +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl9/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl9/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html * igt@kms_psr@psr2_suspend: - shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-iclb2/igt@kms_psr@psr2_suspend.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-iclb7/igt@kms_psr@psr2_suspend.html Possible fixes * igt@gem_exec_suspend@basic-s3: - shard-skl: [INCOMPLETE][19] ([i915#69]) -> [PASS][20] +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl1/igt@gem_exec_susp...@basic-s3.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl3/igt@gem_exec_susp...@basic-s3.html * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled: - shard-skl: [FAIL][21] ([i915#52] / [i915#54]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-skl6/igt@kms_draw_...@draw-method-rgb565-mmap-wc-ytiled.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-skl6/igt@kms_draw_...@draw-method-rgb565-mmap-wc-ytiled.html * {igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2}: - shard-glk: [FAIL][23] ([i915#79]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8453/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interrupti...@ab-hdmi-a1-hdmi-a2.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17613/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interrupti...@ab-hdmi-a1-hdmi-a2.html * {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}: - shar