Re: [Intel-gfx] [PATCH v7 17/17] drm/i915: Add HDCP 1.4 support for MST connectors
On 2020-08-11 at 13:28:46 -0400, Sean Paul wrote: > On Thu, Jul 9, 2020 at 8:40 AM Anshuman Gupta > wrote: > > > > \snip > > > > > +static int > > > > +intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port > > > > *intel_dig_port, > > > > + enum transcoder cpu_transcoder, > > > > + bool enable) > > > > +{ > > > > + struct drm_i915_private *i915 = > > > > to_i915(intel_dig_port->base.base.dev); > > > > + int ret; > > > > + > > > > + if (!enable) > > > > + usleep_range(6, 60); /* Bspec says >= 6us */ > > > > + > > > > + ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, > > > > + cpu_transcoder, enable); > > > Sean, > > > > > > This function toggles the TRANS_DDI_HDCP_SIGNALLING (9th)bit of > > > TRANS_DDI_FUNC_CTL(tran) > > > But in the hw specification this bit is mentioned to be ignored for non > > > HDMI/DVI modes of the TRANS DDI. > > > > > > Any reason why we need this? Did you try with out this function? > > > > > Under "Authentication Part 1 for Multi-stream DisplayPort", bspec says: > 2. Select HDCP for the desired stream using the Pipe DDI Function > Control register. This is the 5th bit (Multistream HDCP Select) of Pipe DDI Function Control register i.e TRANS_DDI_FUNC_CTL register. This bit ensures HDCP encryption to this transcoder stream when used in multistream DisplayPort mode. Unfortunately public specs of Gen11 has discrepency and doesn't describe this bit. https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol02c-commandreference-registers-part2_0.pdf Page No.1026 TRANS_DDI_FUNC_CTL > > > > Apart from that Patch looks good to me. > > IMHO it seems we are still missing to enable the Multistream HDCP Select > > bit (5) in TRANS_DDI_FUNC_CTL register which is required to enable the > > stream encryption. > > > > Could you send me some docs on this? I don't see have info on this bit. This bit is not described in above mentioned public spec, but neither bit TRANS_DDI_HDCP_SIGNALLING bit (9th) which used in this patch, what is the source of public B.spec you are following? Thanks, Anshuman Gupta. > > Sean > > > Thanks, > > Anshuman Gupta. > > > > > > -Ram > > > > > > > + if (ret) > > > > + drm_dbg_kms(&i915->drm, "%s HDCP signalling failed (%d)\n", > > > > + enable ? "Enable" : "Disable", ret); > > > > + return ret; > > > > +} > > > > + > > > > +static > > > > +bool intel_dp_mst_hdcp_check_link(struct intel_digital_port > > > > *intel_dig_port, > > > > + struct intel_connector *connector) > > > > +{ > > > > + struct drm_i915_private *i915 = > > > > to_i915(intel_dig_port->base.base.dev); > > > > + struct intel_dp *intel_dp = &intel_dig_port->dp; > > > > + struct drm_dp_query_stream_enc_status_ack_reply reply; > > > > + int ret; > > > > + > > > > + if (!intel_dp_hdcp_check_link(intel_dig_port, connector)) > > > > + return false; > > > > + > > > > + ret = drm_dp_send_query_stream_enc_status(&intel_dp->mst_mgr, > > > > + connector->port, &reply); > > > > + if (ret) { > > > > + drm_dbg_kms(&i915->drm, > > > > + "[CONNECTOR:%d:%s] failed QSES ret=%d\n", > > > > + connector->base.base.id, connector->base.name, > > > > ret); > > > > + return false; > > > > + } > > > > + > > > > + return reply.auth_completed && reply.encryption_enabled; > > > > +} > > > > + > > > > +static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > + .write_an_aksv = intel_dp_hdcp_write_an_aksv, > > > > + .read_bksv = intel_dp_hdcp_read_bksv, > > > > + .read_bstatus = intel_dp_hdcp_read_bstatus, > > > > + .repeater_present = intel_dp_hdcp_repeater_present, > > > > + .read_ri_prime = intel_dp_hdcp_read_ri_prime, > > > > + .read_ksv_ready = intel_dp_hdcp_read_ksv_ready, > > > > + .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo, > > > > + .read_v_prime_part = intel_dp_hdcp_read_v_prime_part, > > > > + .toggle_signalling = intel_dp_mst_hdcp_toggle_signalling, > > > > + .check_link = intel_dp_mst_hdcp_check_link, > > > > + .hdcp_capable = intel_dp_hdcp_capable, > > > > + > > > > + .protocol = HDCP_PROTOCOL_DP, > > > > +}; > > > > + > > > > int intel_dp_init_hdcp(struct intel_digital_port *intel_dig_port, > > > >struct intel_connector *intel_connector) > > > > { > > > > @@ -630,7 +691,10 @@ int intel_dp_init_hdcp(struct intel_digital_port > > > > *intel_dig_port, > > > > if (!is_hdcp_supported(dev_priv, port)) > > > > return 0; > > > > > > > > - if (!intel_dp_is_edp(intel_dp)) > > > > + if (intel_connector->mst_port) > > > > + return intel_hdcp_init(intel_connector, port, > > > > + &intel_dp_mst_hdcp_shim); > > > > + else if (!intel_dp_is_ed
Re: [Intel-gfx] [PATCH 01/24] Revert "drm/i915/gem: Async GPU relocations only"
Quoting Maarten Lankhorst (2020-08-10 11:30:40) > This reverts commit 9e0f9464e2ab36b864359a59b0e9058fdef0ce47, > and related commit 7ac2d2536dfa7 ("drm/i915/gem: Delete unused code"). > > Async GPU relocations are not the path forward, we want to remove > GPU accelerated relocation support eventually when userspace is fixed > to use VM_BIND, and this is the first step towards that. We will keep > async gpu relocations around for now, until userspace is fixed. As it stands you are exposing an attack vector with your code that allows one user to create an arbitrary lock holdtime and to pass that lock onto a high priority consumer forcing them to wait upon you. You may keep on denying that, and also keep denying that you have been shown the bug and how to avoid it. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 15/17] drm/mst: Add support for QUERY_STREAM_ENCRYPTION_STATUS MST sideband message
On 2020-08-11 at 13:21:38 -0400, Sean Paul wrote: > On Thu, Jul 9, 2020 at 9:09 AM Anshuman Gupta > wrote: > > > > On 2020-07-02 at 20:07:36 +0530, Anshuman Gupta wrote: > > > On 2020-06-30 at 12:48:34 -0400, Sean Paul wrote: > > > > On Tue, Jun 30, 2020 at 10:21 AM Anshuman Gupta > > > > wrote: > > > > > > > > > > On 2020-06-23 at 21:29:05 +0530, Sean Paul wrote: > > > > > Hi Sean, > > > > > I am new to DP MST stuff, I am looking to DP MST spec DP v1.2a. > > > > > I have looked the entire series, i will take up this opportunity to > > > > > review > > > > > the series from HDCP over DP MST POV. > > > > > I think theoretically this series should work or Gen12 as well, as DP > > > > > MST streams > > > > > are getting encrypted by QUERY_STREAM_ENCRYPTION_STATUS reply > > > > > tranaction msg > > > > > (generating Stream State Signature L’). > > > > > I will test this on Gen12 H/W with DP MST support and will provide my > > > > > inputs. > > > > > > > > > > Meanwhile while going through DP MST v1.2a specs(Page 262) came to > > > > > know about > > > > > a DP irq vector LINK_SERVICE_IRQ_VECTOR_ESI0 (02005h), > > > > > Bit 2 : STREAM_STATUS_CHANGED. > > > > > When this bit set to ‘1’ indicates the source must re-check the > > > > > Stream Status > > > > > with the QUERY_STREAM_ENCRYPTION_STATUS message. > > > > > Currently i feel this irq support is missing, do we require to support > > > > > above IRQ vector for DP MST stream encryption. > > > > > > > > > > > > > Hi Anshuman, > > > > Thank you for your comments. > > > > > > > > QUERY_STREAM_ENCRYPTION_STATUS is not necessary for HDCP 1.x, I added > > > > this as a safety check to ensure that the streams were being > > > > encrypted. As such, the existing integrity checks in place for DP are > > > > sufficient to satisfy spec. When HDCP 2.2 support is added for MST, > > > > handling QSES will need to be addressed to meet spec. Note also that > > > > we're not validating the QSES signature for the same reason. > > > Thanks sean for the explanation, > > > overall patch looks good to me but i have couple of doubt see below. > > > > > > > > Sean > > > > > > > > > > > > > Thanks, > > > > > Anshuman Gupta. > > > > > > > > > > > From: Sean Paul > > > > > > > > > > > > Used to query whether an MST stream is encrypted or not. > > > > > > > > > > > > Signed-off-by: Sean Paul > > > > > > > > > > > > Link: > > > > > > https://patchwork.freedesktop.org/patch/msgid/20200218220242.107265-14-s...@poorly.run > > > > > > #v4 > > > > > > Link: > > > > > > https://patchwork.freedesktop.org/patch/msgid/20200305201236.152307-15-s...@poorly.run > > > > > > #v5 > > > > > > Link: > > > > > > https://patchwork.freedesktop.org/patch/msgid/20200429195502.39919-15-s...@poorly.run > > > > > > #v6 > > > > > > > > > > > > Changes in v4: > > > > > > -Added to the set > > > > > > Changes in v5: > > > > > > -None > > > > > > Changes in v6: > > > > > > -Use FIELD_PREP to generate request buffer bitfields (Lyude) > > > > > > -Add mst selftest and dump/decode_sideband_req for QSES (Lyude) > > > > > > Changes in v7: > > > > > > -None > > > > > > --- > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 142 > > > > > > ++ > > > > > > .../drm/selftests/test-drm_dp_mst_helper.c| 17 +++ > > > > > > include/drm/drm_dp_helper.h | 3 + > > > > > > include/drm/drm_dp_mst_helper.h | 44 ++ > > > > > > 4 files changed, 206 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > index b2f5a84b4cfb..fc68478eaeb4 100644 > > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > @@ -20,11 +20,13 @@ > > > > > > * OF THIS SOFTWARE. > > > > > > */ > > > > > > > > > > > > +#include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > +#include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > @@ -419,6 +421,22 @@ drm_dp_encode_sideband_req(const struct > > > > > > drm_dp_sideband_msg_req_body *req, > > > > > > memcpy(&buf[idx], req->u.i2c_write.bytes, > > > > > > req->u.i2c_write.num_bytes); > > > > > > idx += req->u.i2c_write.num_bytes; > > > > > > break; > > > > > > + case DP_QUERY_STREAM_ENC_STATUS: { > > > > > > + const struct drm_dp_query_stream_enc_status *msg; > > > > > > + > > > > > > + msg = &req->u.enc_status; > > > > > > + buf[idx] = msg->stream_id; > > > > > > + idx++; > > > > > > + memcpy(&buf[idx], msg->client_id, > > > > > > sizeof(msg->client_id)); > > > > > > + idx += sizeof(msg->client_id); > > > > > > + buf[idx] = 0; > > > > > > + buf[idx] |= FIELD_PREP(GENMASK(1, 0), > > > > > > msg->stream_event);
Re: [Intel-gfx] [PULL] drm-intel-next-fixes
On Mon, 10 Aug 2020, Dave Airlie wrote: > Like this stuff has been getting past me for years but I'm not happy > about it anymore, I'm going to just be grumpy asshole going forward. > If we introduced scheduler races in linux-next, I want to see reverts, > and reverts only until we stopped introducing them. I'll redo the fixes once we have dinq sorted out. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-misc-fixes
drm-misc-fixes-2020-08-12: drm-misc-fixes for v5.9-rc1: - Add missing dma_fence_put() in virtio_gpu_execbuffer_ioctl(). - Fix memory leak in virtio_gpu_cleanup_object(). The following changes since commit b5ac98cbb8e5e30c34ebc837d1e5a3982d2b5f5c: drm: Added orientation quirk for ASUS tablet model T103HAF (2020-08-04 11:45:23 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2020-08-12 for you to fetch changes up to 836b194d65782aaec4485a07d2aab52d3f698505: drm/virtio: fix memory leak in virtio_gpu_cleanup_object() (2020-08-07 13:59:08 +0200) drm-misc-fixes for v5.9-rc1: - Add missing dma_fence_put() in virtio_gpu_execbuffer_ioctl(). - Fix memory leak in virtio_gpu_cleanup_object(). Qi Liu (1): drm/virtio: fix missing dma_fence_put() in virtio_gpu_execbuffer_ioctl() Xin He (1): drm/virtio: fix memory leak in virtio_gpu_cleanup_object() drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + drivers/gpu/drm/virtio/virtgpu_object.c | 1 + 2 files changed, 2 insertions(+) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i915] flip_done timed out errors with i7-1065G7 on kernel > 4.19
Hello there, I am seeing this error after recently installing Archlinux on my Razer Blade Stealth with the kernel version 5.7.12. I have seen it as well on 4.19, 5.4.55 and 5.8.0. The error happens during boot time where it significantly increases boot time. It starts with a time out in |drm_atomic_helper_wait_for_flip_done followed by more time out in |||drm_atomic_helper_wait_for_dependencies and |drm_atomic_helper_wait_for_flip_done, each of them taking about 10 seconds (which is the value in the source code as far as I can see). There are 11-12 time out at each boot.||| || |||A similar issue was already discovered in previous version of the kernel, but the selected solution of adding video=SVIDEO-1:d to the kernel parameters has proved unsuccessful so far.||| Note that Xorg also fails to launch, but this may be due to a misconfiguration from my side, as I start it with startx. There is more details (including a dmesg output for the kernel version 5.8.0) in my post on the Archlinux forums: https://bbs.archlinux.org/viewtopic.php?id=258051 Thank you for any time you may spend on this problem, Vincent Guenat ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 17/17] drm/i915: Add HDCP 1.4 support for MST connectors
On Wed, Aug 12, 2020 at 3:15 AM Anshuman Gupta wrote: > > On 2020-08-11 at 13:28:46 -0400, Sean Paul wrote: > > On Thu, Jul 9, 2020 at 8:40 AM Anshuman Gupta > > wrote: > > > > > > > \snip > > > > > > > +static int > > > > > +intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port > > > > > *intel_dig_port, > > > > > + enum transcoder cpu_transcoder, > > > > > + bool enable) > > > > > +{ > > > > > + struct drm_i915_private *i915 = > > > > > to_i915(intel_dig_port->base.base.dev); > > > > > + int ret; > > > > > + > > > > > + if (!enable) > > > > > + usleep_range(6, 60); /* Bspec says >= 6us */ > > > > > + > > > > > + ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, > > > > > + cpu_transcoder, enable); > > > > Sean, > > > > > > > > This function toggles the TRANS_DDI_HDCP_SIGNALLING (9th)bit of > > > > TRANS_DDI_FUNC_CTL(tran) > > > > But in the hw specification this bit is mentioned to be ignored for non > > > > HDMI/DVI modes of the TRANS DDI. > > > > > > > > Any reason why we need this? Did you try with out this function? > > > > > > > > Under "Authentication Part 1 for Multi-stream DisplayPort", bspec says: > > 2. Select HDCP for the desired stream using the Pipe DDI Function > > Control register. > This is the 5th bit (Multistream HDCP Select) of Pipe DDI Function Control > register i.e > TRANS_DDI_FUNC_CTL register. This bit ensures HDCP encryption to this > transcoder stream when used in > multistream DisplayPort mode. > Unfortunately public specs of Gen11 has discrepency and doesn't describe this > bit. > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol02c-commandreference-registers-part2_0.pdf > Page No.1026 TRANS_DDI_FUNC_CTL > > > > > > Apart from that Patch looks good to me. > > > IMHO it seems we are still missing to enable the Multistream HDCP Select > > > bit (5) in TRANS_DDI_FUNC_CTL register which is required to enable the > > > stream encryption. > > > > > > > Could you send me some docs on this? I don't see have info on this bit. > This bit is not described in above mentioned public spec, but neither bit > TRANS_DDI_HDCP_SIGNALLING bit (9th) > which used in this patch, what is the source of public B.spec you are > following? I think it's been removed from public now, but there was a public doc on Intel's site when I originally wrote these which described the TRANS_DDI_FUNC_CTL register. So do I just need to set bit 5 at the same time as the HDCP SIGNALLING bit? Sean > Thanks, > Anshuman Gupta. > > > > Sean > > > > > Thanks, > > > Anshuman Gupta. > > > > > > > > -Ram > > > > > > > > > + if (ret) > > > > > + drm_dbg_kms(&i915->drm, "%s HDCP signalling failed > > > > > (%d)\n", > > > > > + enable ? "Enable" : "Disable", ret); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static > > > > > +bool intel_dp_mst_hdcp_check_link(struct intel_digital_port > > > > > *intel_dig_port, > > > > > + struct intel_connector *connector) > > > > > +{ > > > > > + struct drm_i915_private *i915 = > > > > > to_i915(intel_dig_port->base.base.dev); > > > > > + struct intel_dp *intel_dp = &intel_dig_port->dp; > > > > > + struct drm_dp_query_stream_enc_status_ack_reply reply; > > > > > + int ret; > > > > > + > > > > > + if (!intel_dp_hdcp_check_link(intel_dig_port, connector)) > > > > > + return false; > > > > > + > > > > > + ret = drm_dp_send_query_stream_enc_status(&intel_dp->mst_mgr, > > > > > + connector->port, > > > > > &reply); > > > > > + if (ret) { > > > > > + drm_dbg_kms(&i915->drm, > > > > > + "[CONNECTOR:%d:%s] failed QSES ret=%d\n", > > > > > + connector->base.base.id, > > > > > connector->base.name, ret); > > > > > + return false; > > > > > + } > > > > > + > > > > > + return reply.auth_completed && reply.encryption_enabled; > > > > > +} > > > > > + > > > > > +static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > > + .write_an_aksv = intel_dp_hdcp_write_an_aksv, > > > > > + .read_bksv = intel_dp_hdcp_read_bksv, > > > > > + .read_bstatus = intel_dp_hdcp_read_bstatus, > > > > > + .repeater_present = intel_dp_hdcp_repeater_present, > > > > > + .read_ri_prime = intel_dp_hdcp_read_ri_prime, > > > > > + .read_ksv_ready = intel_dp_hdcp_read_ksv_ready, > > > > > + .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo, > > > > > + .read_v_prime_part = intel_dp_hdcp_read_v_prime_part, > > > > > + .toggle_signalling = intel_dp_mst_hdcp_toggle_signalling, > > > > > + .check_link = intel_dp_mst_hdcp_check_link, > > > > > + .hdcp_capable = intel_dp_hdcp_capable, > > > > > + > > > > > + .protocol = HDCP_PROTOCOL_DP, > > > > > +}; > > > > > + > > > > > int intel_dp_init_hdcp(struct intel_digita
Re: [Intel-gfx] [PATCH 13/24] drm/i915: Pin engine before pinning all objects, v5.
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: We want to lock all gem objects, including the engine context objects, rework the throttling to ensure that we can do this. Now we only throttle once, but can take eb_pin_engine while acquiring objects. This means we will have to drop the lock to wait. If we don't have to throttle we can still take the fastpath, if not we will take the slowpath and wait for the throttle request while unlocked. The engine has to be pinned as first step, otherwise gpu relocations won't work. Changes since v1: - Only need to get a throttled request in the fastpath, no need for a global flag any more. - Always free the waited request correctly. Changes since v2: - Use intel_engine_pm_get()/put() to keeep engine pool alive during EDEADLK handling. Changes since v3: - Fix small rq leak. Changes since v4: - Use a single reloc_context, for intel_context_pin_ww(). Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/24] drm/i915: Rework intel_context pinning to do everything outside of pin_mutex
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: Instead of doing everything inside of pin_mutex, we move all pinning outside. Because i915_active has its own reference counting and pinning is also having the same issues vs mutexes, we make sure everything is pinned first, so the pinning in i915_active only needs to bump refcounts. This allows us to take pin refcounts correctly all the time. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/gt/intel_context.c | 232 +++--- drivers/gpu/drm/i915/gt/intel_context_types.h | 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 34 ++- .../gpu/drm/i915/gt/intel_ring_submission.c | 13 +- drivers/gpu/drm/i915/gt/mock_engine.c | 13 +- 5 files changed, 190 insertions(+), 106 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 52db2bde44a3..efe9a7a89ede 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -93,79 +93,6 @@ static void intel_context_active_release(struct intel_context *ce) i915_active_release(&ce->active); } -int __intel_context_do_pin(struct intel_context *ce) -{ - int err; - - if (unlikely(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags))) { - err = intel_context_alloc_state(ce); - if (err) - return err; - } - - err = i915_active_acquire(&ce->active); - if (err) - return err; - - if (mutex_lock_interruptible(&ce->pin_mutex)) { - err = -EINTR; - goto out_release; - } - - if (unlikely(intel_context_is_closed(ce))) { - err = -ENOENT; - goto out_unlock; - } - - if (likely(!atomic_add_unless(&ce->pin_count, 1, 0))) { - err = intel_context_active_acquire(ce); - if (unlikely(err)) - goto out_unlock; - - err = ce->ops->pin(ce); - if (unlikely(err)) - goto err_active; - - CE_TRACE(ce, "pin ring:{start:%08x, head:%04x, tail:%04x}\n", -i915_ggtt_offset(ce->ring->vma), -ce->ring->head, ce->ring->tail); - - smp_mb__before_atomic(); /* flush pin before it is visible */ - atomic_inc(&ce->pin_count); - } - - GEM_BUG_ON(!intel_context_is_pinned(ce)); /* no overflow! */ - GEM_BUG_ON(i915_active_is_idle(&ce->active)); - goto out_unlock; - -err_active: - intel_context_active_release(ce); -out_unlock: - mutex_unlock(&ce->pin_mutex); -out_release: - i915_active_release(&ce->active); - return err; -} - -void intel_context_unpin(struct intel_context *ce) -{ - if (!atomic_dec_and_test(&ce->pin_count)) - return; - - CE_TRACE(ce, "unpin\n"); - ce->ops->unpin(ce); - - /* -* Once released, we may asynchronously drop the active reference. -* As that may be the only reference keeping the context alive, -* take an extra now so that it is not freed before we finish -* dereferencing it. -*/ - intel_context_get(ce); - intel_context_active_release(ce); - intel_context_put(ce); -} - static int __context_pin_state(struct i915_vma *vma) { unsigned int bias = i915_ggtt_pin_bias(vma) | PIN_OFFSET_BIAS; @@ -225,6 +152,138 @@ static void __ring_retire(struct intel_ring *ring) intel_ring_unpin(ring); } +static int intel_context_pre_pin(struct intel_context *ce) +{ + int err; + + CE_TRACE(ce, "active\n"); + + err = __ring_active(ce->ring); + if (err) + return err; + + err = intel_timeline_pin(ce->timeline); + if (err) + goto err_ring; + + if (!ce->state) + return 0; + + err = __context_pin_state(ce->state); + if (err) + goto err_timeline; + + + return 0; + +err_timeline: + intel_timeline_unpin(ce->timeline); +err_ring: + __ring_retire(ce->ring); + return err; +} + +static void intel_context_post_unpin(struct intel_context *ce) +{ + if (ce->state) + __context_unpin_state(ce->state); + + intel_timeline_unpin(ce->timeline); + __ring_retire(ce->ring); +} + +int __intel_context_do_pin(struct intel_context *ce) +{ + bool handoff = false; + void *vaddr; + int err = 0; + + if (unlikely(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags))) { + err = intel_context_alloc_state(ce); + if (err) + return err; + } + + /* +* We always pin the context/ring/timeline here, to ensure a pin +* refcount for __intel_context_active(), which prevent a lock +* inversion of ce->pin_mutex vs dma_resv_lock(). +*/ + err = intel_context_pre_pin(ce); +
Re: [Intel-gfx] [PATCH 15/24] drm/i915: Make sure execbuffer always passes ww state to i915_vma_pin.
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: As a preparation step for full object locking and wait/wound handling during pin and object mapping, ensure that we always pass the ww context in i915_gem_execbuffer.c to i915_vma_pin, use lockdep to ensure this happens. This also requires changing the order of eb_parse slightly, to ensure we pass ww at a point where we could still handle -EDEADLK safely. Signed-off-by: Maarten Lankhorst I'm a bit curious as how we handle the lifetime on the contending locks since we often return through the call tree before doing the ww transaction relaxation (the slow lock). Has that been a problem? --- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_context.c | 4 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 140 ++ .../i915/gem/selftests/i915_gem_execbuffer.c | 4 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 4 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.h | 4 +- drivers/gpu/drm/i915/gt/intel_context.c | 65 +--- drivers/gpu/drm/i915/gt/intel_context.h | 13 ++ drivers/gpu/drm/i915/gt/intel_context_types.h | 3 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt.c| 2 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 5 +- drivers/gpu/drm/i915/gt/intel_renderstate.c | 2 +- drivers/gpu/drm/i915/gt/intel_ring.c | 10 +- drivers/gpu/drm/i915/gt/intel_ring.h | 3 +- .../gpu/drm/i915/gt/intel_ring_submission.c | 15 +- drivers/gpu/drm/i915/gt/intel_timeline.c | 12 +- drivers/gpu/drm/i915/gt/intel_timeline.h | 3 +- drivers/gpu/drm/i915/gt/mock_engine.c | 3 +- drivers/gpu/drm/i915/gt/selftest_lrc.c| 2 +- drivers/gpu/drm/i915/gt/selftest_timeline.c | 4 +- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 2 +- drivers/gpu/drm/i915/i915_drv.h | 13 +- drivers/gpu/drm/i915/i915_gem.c | 11 +- drivers/gpu/drm/i915/i915_vma.c | 13 +- drivers/gpu/drm/i915/i915_vma.h | 13 +- 26 files changed, 217 insertions(+), 137 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5b4434289117..aa5a88340d10 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3451,7 +3451,7 @@ initial_plane_vma(struct drm_i915_private *i915, if (IS_ERR(vma)) goto err_obj; - if (i915_ggtt_pin(vma, 0, PIN_MAPPABLE | PIN_OFFSET_FIXED | base)) + if (i915_ggtt_pin(vma, NULL, 0, PIN_MAPPABLE | PIN_OFFSET_FIXED | base)) goto err_obj; if (i915_gem_object_is_tiled(obj) && diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 34c8b0dd85e0..cf5ecbde9e06 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1154,7 +1154,7 @@ static int context_barrier_task(struct i915_gem_context *ctx, i915_gem_ww_ctx_init(&ww, true); retry: - err = intel_context_pin(ce); + err = intel_context_pin_ww(ce, &ww); if (err) goto err; @@ -1247,7 +1247,7 @@ static int pin_ppgtt_update(struct intel_context *ce, struct i915_gem_ww_ctx *ww if (!HAS_LOGICAL_RING_CONTEXTS(vm->i915)) /* ppGTT is not part of the legacy context image */ - return gen6_ppgtt_pin(i915_vm_to_ppgtt(vm)); + return gen6_ppgtt_pin(i915_vm_to_ppgtt(vm), ww); return 0; } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 604e26adea23..94bfdc54f035 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -437,16 +437,17 @@ eb_pin_vma(struct i915_execbuffer *eb, pin_flags |= PIN_GLOBAL; /* Attempt to reuse the current location if available */ - if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags))) { + /* TODO: Add -EDEADLK handling here */ + if (unlikely(i915_vma_pin_ww(vma, &eb->ww, 0, 0, pin_flags))) { if (entry->flags & EXEC_OBJECT_PINNED) return false; /* Failing that pick any _free_ space if suitable */ - if (unlikely(i915_vma_pin(vma, - entry->pad_to_size, - entry->alignment, - eb_pin_flags(entry, ev->flags) | - PIN_USER | PIN_NOEVICT))) + if (unlikely(i915_vma_pin_ww(vma, &eb->ww, +entry->pad_to_size, +entry->alignment, +
Re: [Intel-gfx] [PATCH 16/24] drm/i915: Convert i915_gem_object/client_blt.c to use ww locking as well, v2.
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: This is the last part outside of selftests that still don't use the correct lock ordering of timeline->mutex vs resv_lock. With gem fixed, there are a few places that still get locking wrong: - gvt/scheduler.c - i915_perf.c - Most if not all selftests. Changes since v1: - Add intel_engine_pm_get/put() calls to fix use-after-free when using intel_engine_get_pool(). Signed-off-by: Maarten Lankhorst LGTM. Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/24] drm/i915: Kill last user of intel_context_create_request outside of selftests
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: Instead of using intel_context_create_request(), use intel_context_pin() and i915_create_request directly. Now all those calls are gone outside of selftests. :) Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/24] drm/i915: Convert i915_perf to ww locking as well
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: We have the ordering of timeline->mutex vs resv_lock wrong, convert the i915_pin_vma and intel_context_pin as well to future-proof this. We may need to do future changes to do this more transaction-like, and only get down to a single i915_gem_ww_ctx, but for now this should work. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/i915_perf.c | 57 +++- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index c6f6370283cf..e94976976571 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1195,24 +1195,39 @@ static struct intel_context *oa_pin_context(struct i915_perf_stream *stream) struct i915_gem_engines_iter it; struct i915_gem_context *ctx = stream->ctx; struct intel_context *ce; - int err; + struct i915_gem_ww_ctx ww; + int err = -ENODEV; for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { if (ce->engine != stream->engine) /* first match! */ continue; - /* -* As the ID is the gtt offset of the context's vma we -* pin the vma to ensure the ID remains fixed. -*/ - err = intel_context_pin(ce); - if (err == 0) { - stream->pinned_ctx = ce; - break; - } + err = 0; + break; } i915_gem_context_unlock_engines(ctx); + if (err) + return ERR_PTR(err); + + i915_gem_ww_ctx_init(&ww, true); +retry: + /* +* As the ID is the gtt offset of the context's vma we +* pin the vma to ensure the ID remains fixed. +*/ + err = intel_context_pin_ww(ce, &ww); + if (err == -EDEADLK) { + err = i915_gem_ww_ctx_backoff(&ww); + if (!err) + goto retry; + } + i915_gem_ww_ctx_fini(&ww); + Hmm. Didn't we keep an intel_context_pin() that does exactly the above without recoding the whole ww transaction? Or do you plan to remove that? With that taken into account, Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/24] drm/i915: Dirty hack to fix selftests locking inversion
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: Some i915 selftests still use i915_vma_lock() as inner lock, and intel_context_create_request() intel_timeline->mutex as outer lock. Fortunately for selftests this is not an issue, they should be fixed but we can move ahead and cleanify lockdep now. Signed-off-by: Maarten Lankhorst Ugh. We should probably fix this properly as soon as possible to avoid copy-pasting of self-tests that aren't fixed yet. For the hack: Acked-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 20/24] drm/i915/selftests: Fix locking inversion in lrc selftest.
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: This function does not use intel_context_create_request, so it has to use the same locking order as normal code. This is required to shut up lockdep in selftests. Signed-off-by: Maarten Lankhorst LGTM. Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/24] drm/i915: Use ww pinning for intel_context_create_request()
On 8/10/20 12:31 PM, Maarten Lankhorst wrote: We want to get rid of intel_context_pin(), This answers a previous review question. convert intel_context_create_request() first. :) Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 22/24] drm/i915: Move i915_vma_lock in the selftests to avoid lock inversion, v3.
On 8/10/20 12:31 PM, Maarten Lankhorst wrote: Make sure vma_lock is not used as inner lock when kernel context is used, and add ww handling where appropriate. Ensure that execbuf selftests keep passing by using ww handling. Changes since v2: - Fix i915_gem_context finally. Signed-off-by: Maarten Lankhorst LGTM. Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 23/24] drm/i915: Add ww locking to vm_fault_gtt
On 8/10/20 12:31 PM, Maarten Lankhorst wrote: Signed-off-by: Maarten Lankhorst Commit message, please. Otherwise, looks good. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/24] drm/i915: Make sure execbuffer always passes ww state to i915_vma_pin.
On 8/12/20 9:32 PM, Thomas Hellström (Intel) wrote: + if (!err && ce->ring->vma->obj) + err = i915_gem_object_lock(ce->ring->vma->obj, ww); + if (!err && ce->state) + err = i915_gem_object_lock(ce->state->obj, ww); Could these three locks be made interruptible? Never mind, I see now how that's done. /Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 24/24] drm/i915: Add ww locking to pin_to_display_plane
On 8/10/20 12:31 PM, Maarten Lankhorst wrote: Signed-off-by: Maarten Lankhorst Commit message, please. --- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 65 -- drivers/gpu/drm/i915/gem/i915_gem_object.h | 1 + 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index 8ebceebd11b0..c0d153284984 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -37,6 +37,12 @@ void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj) i915_gem_object_unlock(obj); } +void i915_gem_object_flush_if_display_locked(struct drm_i915_gem_object *obj) +{ + if (i915_gem_object_is_framebuffer(obj)) + __i915_gem_object_flush_for_display(obj); +} + /** * Moves a single object to the WC read, and possibly write domain. * @obj: object to act on @@ -197,18 +203,12 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, if (ret) return ret; - ret = i915_gem_object_lock_interruptible(obj, NULL); - if (ret) - return ret; - /* Always invalidate stale cachelines */ if (obj->cache_level != cache_level) { i915_gem_object_set_cache_coherency(obj, cache_level); obj->cache_dirty = true; } - i915_gem_object_unlock(obj); - /* The cache-level will be applied when each vma is rebound. */ return i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE | @@ -255,6 +255,7 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_caching *args = data; struct drm_i915_gem_object *obj; enum i915_cache_level level; + struct i915_gem_ww_ctx ww; int ret = 0; switch (args->caching) { @@ -293,7 +294,18 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, goto out; } - ret = i915_gem_object_set_cache_level(obj, level); + i915_gem_ww_ctx_init(&ww, true); +retry: + ret = i915_gem_object_lock(obj, &ww); + if (!ret) + ret = i915_gem_object_set_cache_level(obj, level); + + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } + i915_gem_ww_ctx_fini(&ww); Why a ww transaction for a single lock? /Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/24] drm/i915: Use per object locking in execbuf, v12.
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: Now that we changed execbuf submission slightly to allow us to do all pinning in one place, we can now simply add ww versions on top of struct_mutex. All we have to do is a separate path for -EDEADLK handling, which needs to unpin all gem bo's before dropping the lock, then starting over. This finally allows us to do parallel submission, but because not all of the pinning code uses the ww ctx yet, we cannot completely drop struct_mutex yet. Changes since v1: - Keep struct_mutex for now. :( Changes since v2: - Make sure we always lock the ww context in slowpath. Changes since v3: - Don't call __eb_unreserve_vma in eb_move_to_gpu now; this can be done on normal unlock path. - Unconditionally release vmas and context. Changes since v4: - Rebased on top of struct_mutex reduction. Changes since v5: - Remove training wheels. Changes since v6: - Fix accidentally broken -ENOSPC handling. Changes since v7: - Handle gt buffer pool better. Changes since v8: - Properly clear variables, to make -EDEADLK handling not BUG. Change since v9: - Fix unpinning fence on pnv and below. Changes since v10: - Make relocation gpu chaining working again. Changes since v11: - Remove relocation chaining, pain to make it work. Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Apply Wa_14011264657:gen11+
Add minimum width to planes, variable with specific formats for gen11+ to reflect recent bspec changes. Signed-off-by: Matt Atwood --- drivers/gpu/drm/i915/display/intel_display.c | 54 +--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 2ddabf92adde..b5ebcff8d56e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3762,6 +3762,44 @@ static int glk_max_plane_width(const struct drm_framebuffer *fb, } } +static int icl_min_plane_width(const struct drm_framebuffer *fb) +{ + /* Wa_14011264657, Wa_14011050563: gen11+ */ + switch (fb->format->format) { + case DRM_FORMAT_C8: + return 18; + case DRM_FORMAT_RGB565: + return 10; + case DRM_FORMAT_XRGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_ABGR: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_ARGB2101010: + case DRM_FORMAT_ABGR2101010: + case DRM_FORMAT_XVYU2101010: + case DRM_FORMAT_Y212: + case DRM_FORMAT_Y216: + return 6; + case DRM_FORMAT_NV12: + return 20; + case DRM_FORMAT_P010: + case DRM_FORMAT_P012: + case DRM_FORMAT_P016: + return 12; + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ARGB16161616F: + case DRM_FORMAT_ABGR16161616F: + case DRM_FORMAT_XVYU12_16161616: + case DRM_FORMAT_XVYU16161616: + return 4; + default: + return 1; + } +} + static int icl_max_plane_width(const struct drm_framebuffer *fb, int color_plane, unsigned int rotation) @@ -3844,15 +3882,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) int y = plane_state->uapi.src.y1 >> 16; int w = drm_rect_width(&plane_state->uapi.src) >> 16; int h = drm_rect_height(&plane_state->uapi.src) >> 16; - int max_width; - int max_height; - u32 alignment; - u32 offset; + int max_width, min_width = 1, max_height; + u32 alignment, offset; int aux_plane = intel_main_to_aux_plane(fb, 0); u32 aux_offset = plane_state->color_plane[aux_plane].offset; - if (INTEL_GEN(dev_priv) >= 11) + if (INTEL_GEN(dev_priv) >= 11) { max_width = icl_max_plane_width(fb, 0, rotation); + min_width = icl_min_plane_width(fb); + } else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) max_width = glk_max_plane_width(fb, 0, rotation); else @@ -3863,10 +3901,10 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) else max_height = skl_max_plane_height(); - if (w > max_width || h > max_height) { + if (w > max_width || w < min_width || h > max_height) { drm_dbg_kms(&dev_priv->drm, - "requested Y/RGB source size %dx%d too big (limit %dx%d)\n", - w, h, max_width, max_height); + "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n", + w, h, min_width, max_width, max_height); return -EINVAL; } -- 2.21.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/24] Revert "drm/i915/gem: Split eb_vma into its own allocation"
On 8/10/20 12:30 PM, Maarten Lankhorst wrote: This reverts commit 0f1dd02295f35dcdcbaafcbcbbec0753884ab974. With the WW locking, we will drop all references only at the end, so refcounting can be removed. Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Exercise cleanup after disabling heartbeats
We expose the heartbeat interval on each engine, allowing the sysadim to disable them if they prefer avoiding any interruption for their GPU tasks. A caveat to allowing the contexts to run without checks is that we require such contexts to be non-persistent and so cleaned up on closure (including abnormal process termination). However, we also need to flush any persistent contexts that are still inflight at that time, lest they continue to run unchecked. Signed-off-by: Chris Wilson --- tests/i915/gem_ctx_persistence.c | 92 1 file changed, 92 insertions(+) diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c index e73a3e6a0..ca676d845 100644 --- a/tests/i915/gem_ctx_persistence.c +++ b/tests/i915/gem_ctx_persistence.c @@ -426,6 +426,87 @@ static void test_nohangcheck_hang(int i915) close(dir); } +static bool set_heartbeat(int i915, const char *name, unsigned int value) +{ + return gem_engine_property_printf(i915, name, + "heartbeat_interval_ms", + "%d", value) > 0; +} + +static void test_noheartbeat_many(int i915, int count, unsigned int flags) +{ + cleanup(i915); + + /* +* If the user disables the heartbeat, after leaving behind +* a number of long running *persistent* contexts, check they get +* cleaned up. +*/ + + for_each_engine(e, i915) { + igt_spin_t *spin[count]; + + if (!set_heartbeat(i915, e->full_name, 100)) + continue; + + for (int n = 0; n < ARRAY_SIZE(spin); n++) { + uint32_t ctx; + + ctx = gem_context_create(i915); + spin[n] = igt_spin_new(i915, ctx, .engine = eb_ring(e), + .flags = (IGT_SPIN_FENCE_OUT | +flags)); + gem_context_destroy(i915, ctx); + } + + if (set_heartbeat(i915, e->full_name, 0)) { + for (int n = 0; n < ARRAY_SIZE(spin); n++) { + igt_assert_eq(wait_for_status(i915, spin[n]->out_fence, reset_timeout_ms), + -EIO); + } + } + + for (int n = 0; n < ARRAY_SIZE(spin); n++) + igt_spin_free(i915, spin[n]); + + set_heartbeat(i915, e->full_name, 2500); + cleanup(i915); + } +} + +static void test_noheartbeat_close(int i915, unsigned int flags) +{ + cleanup(i915); + + /* +* Check that non-persistent contexts are also cleaned up if we +* close the context while they are active, but the engine's +* heartbeat has already been disabled. +*/ + + for_each_engine(e, i915) { + igt_spin_t *spin; + uint32_t ctx; + + if (!set_heartbeat(i915, e->full_name, 0)) + continue; + + ctx = gem_context_create(i915); + gem_context_set_persistence(i915, ctx, false); + spin = igt_spin_new(i915, ctx, .engine = eb_ring(e), + .flags = (IGT_SPIN_FENCE_OUT | flags)); + gem_context_destroy(i915, ctx); + + set_heartbeat(i915, e->full_name, 2500); + + igt_assert_eq(wait_for_status(i915, spin->out_fence, reset_timeout_ms), + -EIO); + + igt_spin_free(i915, spin); + cleanup(i915); + } +} + static void test_nonpersistent_file(int i915) { int debugfs = i915; @@ -1157,6 +1238,17 @@ igt_main igt_subtest("hang") test_nohangcheck_hang(i915); + igt_subtest("heartbeat-stop") + test_noheartbeat_many(i915, 1, 0); + igt_subtest("heartbeat-hang") + test_noheartbeat_many(i915, 1, IGT_SPIN_NO_PREEMPTION); + igt_subtest("heartbeat-many") + test_noheartbeat_many(i915, 16, 0); + igt_subtest("heartbeat-close") + test_noheartbeat_close(i915, 0); + igt_subtest("heartbeat-hostile") + test_noheartbeat_close(i915, IGT_SPIN_NO_PREEMPTION); + igt_subtest_group { igt_fixture gem_require_contexts(i915); -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] i915/gem_ctx_persistence: Exercise cleanup after disabling heartbeats
We expose the heartbeat interval on each engine, allowing the sysadim to disable them if they prefer avoiding any interruption for their GPU tasks. A caveat to allowing the contexts to run without checks is that we require such contexts to be non-persistent and so cleaned up on closure (including abnormal process termination). However, we also need to flush any persistent contexts that are still inflight at that time, lest they continue to run unchecked. Signed-off-by: Chris Wilson --- git add --- tests/i915/gem_ctx_persistence.c | 92 1 file changed, 92 insertions(+) diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c index e73a3e6a0..d9c972130 100644 --- a/tests/i915/gem_ctx_persistence.c +++ b/tests/i915/gem_ctx_persistence.c @@ -426,6 +426,87 @@ static void test_nohangcheck_hang(int i915) close(dir); } +static bool set_heartbeat(int i915, const char *name, unsigned int value) +{ + return gem_engine_property_printf(i915, name, + "heartbeat_interval_ms", + "%d", value) > 0; +} + +static void test_noheartbeat_many(int i915, int count, unsigned int flags) +{ + cleanup(i915); + + /* +* If the user disables the heartbeat, after leaving behind +* a number of long running *persistent* contexts, check they get +* cleaned up. +*/ + + for_each_engine(e, i915) { + igt_spin_t *spin[count]; + + if (!set_heartbeat(i915, e->full_name, 100)) + continue; + + for (int n = 0; n < ARRAY_SIZE(spin); n++) { + uint32_t ctx; + + ctx = gem_context_create(i915); + spin[n] = igt_spin_new(i915, ctx, .engine = eb_ring(e), + .flags = (IGT_SPIN_FENCE_OUT | +flags)); + gem_context_destroy(i915, ctx); + } + + if (set_heartbeat(i915, e->full_name, 0)) { + for (int n = 0; n < ARRAY_SIZE(spin); n++) { + igt_assert_eq(wait_for_status(spin[n]->out_fence, reset_timeout_ms), + -EIO); + } + } + + for (int n = 0; n < ARRAY_SIZE(spin); n++) + igt_spin_free(i915, spin[n]); + + set_heartbeat(i915, e->full_name, 2500); + cleanup(i915); + } +} + +static void test_noheartbeat_close(int i915, unsigned int flags) +{ + cleanup(i915); + + /* +* Check that non-persistent contexts are also cleaned up if we +* close the context while they are active, but the engine's +* heartbeat has already been disabled. +*/ + + for_each_engine(e, i915) { + igt_spin_t *spin; + uint32_t ctx; + + if (!set_heartbeat(i915, e->full_name, 0)) + continue; + + ctx = gem_context_create(i915); + gem_context_set_persistence(i915, ctx, false); + spin = igt_spin_new(i915, ctx, .engine = eb_ring(e), + .flags = (IGT_SPIN_FENCE_OUT | flags)); + gem_context_destroy(i915, ctx); + + set_heartbeat(i915, e->full_name, 2500); + + igt_assert_eq(wait_for_status(spin->out_fence, reset_timeout_ms), + -EIO); + + igt_spin_free(i915, spin); + cleanup(i915); + } +} + static void test_nonpersistent_file(int i915) { int debugfs = i915; @@ -1157,6 +1238,17 @@ igt_main igt_subtest("hang") test_nohangcheck_hang(i915); + igt_subtest("heartbeat-stop") + test_noheartbeat_many(i915, 1, 0); + igt_subtest("heartbeat-hang") + test_noheartbeat_many(i915, 1, IGT_SPIN_NO_PREEMPTION); + igt_subtest("heartbeat-many") + test_noheartbeat_many(i915, 16, 0); + igt_subtest("heartbeat-close") + test_noheartbeat_close(i915, 0); + igt_subtest("heartbeat-hostile") + test_noheartbeat_close(i915, IGT_SPIN_NO_PREEMPTION); + igt_subtest_group { igt_fixture gem_require_contexts(i915); -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/gt: Implement WA_1406941453
On Wed, Aug 05, 2020 at 04:29:20PM -0700, clinton.a.tay...@intel.com wrote: > From: Clint Taylor > > Enable HW Default flip for small PL. > > bspec: 52890 > bspec: 53508 > bspec: 53273 > > v2: rebase to drm-tip > Reviewed-by: Matt Atwood > Signed-off-by: Clint Taylor > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index cef1c122696f..cb02813c5e92 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -639,6 +639,9 @@ static void tgl_ctx_workarounds_init(struct > intel_engine_cs *engine, > FF_MODE2_GS_TIMER_MASK | FF_MODE2_TDS_TIMER_MASK, > FF_MODE2_GS_TIMER_224 | FF_MODE2_TDS_TIMER_128, > 0); > + > + /* Wa_1406941453:gen12 */ > + WA_SET_BIT_MASKED(GEN10_SAMPLER_MODE, ENABLE_SMALLPL); Is this register part of the engine context on gen12? I see it in the context for ICL (bspec 18907), but not for TGL (46255). So I think this should either be a GT or engine workaround, not a context workaround, right? > } > > static void > @@ -1522,6 +1525,9 @@ static void icl_whitelist_build(struct intel_engine_cs > *engine) > whitelist_reg_ext(w, PS_INVOCATION_COUNT, > RING_FORCE_TO_NONPRIV_ACCESS_RD | > RING_FORCE_TO_NONPRIV_RANGE_4); > + > + /* Wa_1406941453:gen12 */ > + whitelist_reg(w, GEN10_SAMPLER_MODE); Do we need to whitelist this? If we're applying the workaround in the kernel then the UMD shouldn't need to worry about it if they don't otherwise have access or need to change the register value. Matt > break; > > case VIDEO_DECODE_CLASS: > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 2b403df03404..494b2e1e358e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9314,6 +9314,7 @@ enum { > #define GEN11_LSN_UNSLCVC_GAFS_HALF_SF_MAXALLOC(1 << 7) > > #define GEN10_SAMPLER_MODE _MMIO(0xE18C) > +#define ENABLE_SMALLPL REG_BIT(15) > #define GEN11_SAMPLER_ENABLE_HEADLESS_MSG REG_BIT(5) > > /* IVYBRIDGE DPF */ > -- > 2.27.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Apply Wa_14011264657:gen11+ (rev2)
== Series Details == Series: drm/i915: Apply Wa_14011264657:gen11+ (rev2) URL : https://patchwork.freedesktop.org/series/78430/ State : warning == Summary == $ dim checkpatch origin/drm-tip 9ee56d53b00b drm/i915: Apply Wa_14011264657:gen11+ -:74: CHECK:BRACES: braces {} should be used on all arms of this statement #74: FILE: drivers/gpu/drm/i915/display/intel_display.c:3890: + if (INTEL_GEN(dev_priv) >= 11) { [...] else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) [...] total: 0 errors, 0 warnings, 1 checks, 77 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine
We only allow persistent requests to remain on the GPU past the closure of their containing context (and process) so long as they are continuously checked for hangs or allow other requests to preempt them, as we need to ensure forward progress of the system. If we allow persistent contexts to remain on the system after the the hangcheck mechanism is disabled, the system may grind to a halt. On disabling the mechanism, we sent a pulse along the engine to remove all executing contexts from the engine which would check for hung contexts -- but we did not prevent those contexts from being resubmitted if they survived the final hangcheck. Fixes: 9a40bddd47ca ("drm/i915/gt: Expose heartbeat interval via sysfs") Testcase: igt/gem_ctx_persistence/heartbeat-stop Signed-off-by: Chris Wilson Cc: # v5.7+ --- drivers/gpu/drm/i915/gt/intel_engine.h | 9 + drivers/gpu/drm/i915/i915_request.c| 5 + 2 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 08e2c000dcc3..a90cb91c8246 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -337,4 +337,13 @@ intel_engine_has_preempt_reset(const struct intel_engine_cs *engine) return intel_engine_has_preemption(engine); } +static inline bool +intel_engine_has_heartbeat(const struct intel_engine_cs *engine) +{ + if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL)) + return false; + + return engine->props.heartbeat_interval_ms; +} + #endif /* _INTEL_RINGBUFFER_H_ */ diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 0208e917d14a..92efca606f91 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -542,8 +542,13 @@ bool __i915_request_submit(struct i915_request *request) if (i915_request_completed(request)) goto xfer; + if (unlikely(intel_context_is_closed(request->context) && +!intel_engine_has_heartbeat(engine))) + intel_context_set_banned(request->context); + if (unlikely(intel_context_is_banned(request->context))) i915_request_set_error_once(request, -EIO); + if (unlikely(fatal_error(request->fence.error))) __i915_request_skip(request); -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/gt: Always send a pulse down the engine after disabling heartbeat
Currently, we check we can send a pulse prior to disabling the heartbeat to verify that we can change the heartbeat, but since we may re-evaluate execution upon changing the heartbeat interval we need another pulse afterwards to refresh execution. Fixes: 9a40bddd47ca ("drm/i915/gt: Expose heartbeat interval via sysfs") Signed-off-by: Chris Wilson Cc: # v5.7+ --- drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c index 8ffdf676c0a0..d09df370f7cd 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c @@ -192,10 +192,12 @@ int intel_engine_set_heartbeat(struct intel_engine_cs *engine, WRITE_ONCE(engine->props.heartbeat_interval_ms, delay); if (intel_engine_pm_get_if_awake(engine)) { - if (delay) + if (delay) { intel_engine_unpark_heartbeat(engine); - else + } else { intel_engine_park_heartbeat(engine); + intel_engine_pulse(engine); /* recheck execution */ + } intel_engine_pm_put(engine); } -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/gem: Always test execution status on closing the context
Verify that if a context is active at the time it is closed, that it is either persistent and preemptible (with hangcheck running) or it shall be removed from execution. Fixes: 9a40bddd47ca ("drm/i915/gt: Expose heartbeat interval via sysfs") Testcase: igt/gem_ctx_persistence/heartbeat-close Signed-off-by: Chris Wilson Cc: # v5.7+ --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 24 - 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index db893f6c516b..49715ae71386 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -431,8 +431,7 @@ static bool __cancel_engine(struct intel_engine_cs *engine) * kill the banned context, we fallback to doing a local reset * instead. */ - if (IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT) && - !intel_engine_pulse(engine)) + if (intel_engine_pulse(engine) == 0) return true; /* If we are unable to send a pulse, try resetting this engine. */ @@ -493,7 +492,7 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce) return engine; } -static void kill_engines(struct i915_gem_engines *engines) +static void kill_engines(struct i915_gem_engines *engines, bool ban) { struct i915_gem_engines_iter it; struct intel_context *ce; @@ -508,7 +507,7 @@ static void kill_engines(struct i915_gem_engines *engines) for_each_gem_engine(ce, engines, it) { struct intel_engine_cs *engine; - if (intel_context_set_banned(ce)) + if (ban && intel_context_set_banned(ce)) continue; /* @@ -531,8 +530,10 @@ static void kill_engines(struct i915_gem_engines *engines) } } -static void kill_stale_engines(struct i915_gem_context *ctx) +static void kill_context(struct i915_gem_context *ctx) { + bool ban = (!i915_gem_context_is_persistent(ctx) || + !ctx->i915->params.enable_hangcheck); struct i915_gem_engines *pos, *next; spin_lock_irq(&ctx->stale.lock); @@ -545,7 +546,7 @@ static void kill_stale_engines(struct i915_gem_context *ctx) spin_unlock_irq(&ctx->stale.lock); - kill_engines(pos); + kill_engines(pos, ban); spin_lock_irq(&ctx->stale.lock); GEM_BUG_ON(i915_sw_fence_signaled(&pos->fence)); @@ -557,11 +558,6 @@ static void kill_stale_engines(struct i915_gem_context *ctx) spin_unlock_irq(&ctx->stale.lock); } -static void kill_context(struct i915_gem_context *ctx) -{ - kill_stale_engines(ctx); -} - static void engines_idle_release(struct i915_gem_context *ctx, struct i915_gem_engines *engines) { @@ -596,7 +592,7 @@ static void engines_idle_release(struct i915_gem_context *ctx, kill: if (list_empty(&engines->link)) /* raced, already closed */ - kill_engines(engines); + kill_engines(engines, true); i915_sw_fence_commit(&engines->fence); } @@ -654,9 +650,7 @@ static void context_close(struct i915_gem_context *ctx) * case we opt to forcibly kill off all remaining requests on * context close. */ - if (!i915_gem_context_is_persistent(ctx) || - !ctx->i915->params.enable_hangcheck) - kill_context(ctx); + kill_context(ctx); i915_gem_context_put(ctx); } -- 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: Apply Wa_14011264657:gen11+ (rev2)
== Series Details == Series: drm/i915: Apply Wa_14011264657:gen11+ (rev2) URL : https://patchwork.freedesktop.org/series/78430/ State : success == Summary == CI Bug Log - changes from CI_DRM_8879 -> Patchwork_18349 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/index.html Known issues Here are the changes found in Patchwork_18349 that come from known issues: ### IGT changes ### Issues hit * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-byt-j1900: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-byt-j1900/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-byt-j1900/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html Possible fixes * igt@kms_busy@basic@flip: - fi-kbl-x1275: [DMESG-WARN][3] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@kms_busy@ba...@flip.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-kbl-x1275/igt@kms_busy@ba...@flip.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [DMESG-WARN][5] ([i915#2203]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html * igt@kms_flip@basic-flip-vs-wf_vblank@d-dsi1: - {fi-tgl-dsi}: [DMESG-WARN][7] ([i915#1982]) -> [PASS][8] +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-wf_vbl...@d-dsi1.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-wf_vbl...@d-dsi1.html Warnings * igt@gem_exec_suspend@basic-s0: - fi-kbl-x1275: [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][10] ([i915#62] / [i915#92]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html * igt@i915_pm_rpm@module-reload: - fi-kbl-x1275: [SKIP][11] ([fdo#109271]) -> [DMESG-FAIL][12] ([i915#62]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-kbl-x1275/igt@i915_pm_...@module-reload.html * igt@kms_force_connector_basic@force-edid: - fi-kbl-x1275: [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#62] / [i915#92] / [i915#95]) +4 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.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#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (43 -> 37) -- Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * Linux: CI_DRM_8879 -> Patchwork_18349 CI-20190529: 20190529 CI_DRM_8879: 383f247a5884778b21e17dc9537e0bdeae503953 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5768: 4ce2f30715836214d7b4452ada8c63b23eba73b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18349: 9ee56d53b00b040e07daff2d2b805c8d4992a091 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 9ee56d53b00b drm/i915: Apply Wa_14011264657:gen11+ == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gem: Always test execution status on closing the context
Verify that if a context is active at the time it is closed, that it is either persistent and preemptible (with hangcheck running) or it shall be removed from execution. Fixes: 9a40bddd47ca ("drm/i915/gt: Expose heartbeat interval via sysfs") Testcase: igt/gem_ctx_persistence/heartbeat-close Signed-off-by: Chris Wilson Cc: # v5.7+ --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 26 - 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index db893f6c516b..ba8ef1225f58 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -431,8 +431,7 @@ static bool __cancel_engine(struct intel_engine_cs *engine) * kill the banned context, we fallback to doing a local reset * instead. */ - if (IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT) && - !intel_engine_pulse(engine)) + if (intel_engine_pulse(engine) == 0) return true; /* If we are unable to send a pulse, try resetting this engine. */ @@ -493,7 +492,7 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce) return engine; } -static void kill_engines(struct i915_gem_engines *engines) +static void kill_engines(struct i915_gem_engines *engines, bool ban) { struct i915_gem_engines_iter it; struct intel_context *ce; @@ -508,7 +507,7 @@ static void kill_engines(struct i915_gem_engines *engines) for_each_gem_engine(ce, engines, it) { struct intel_engine_cs *engine; - if (intel_context_set_banned(ce)) + if (ban && intel_context_set_banned(ce)) continue; /* @@ -521,7 +520,7 @@ static void kill_engines(struct i915_gem_engines *engines) engine = active_engine(ce); /* First attempt to gracefully cancel the context */ - if (engine && !__cancel_engine(engine)) + if (engine && !__cancel_engine(engine) && ban) /* * If we are unable to send a preemptive pulse to bump * the context from the GPU, we have to resort to a full @@ -531,8 +530,10 @@ static void kill_engines(struct i915_gem_engines *engines) } } -static void kill_stale_engines(struct i915_gem_context *ctx) +static void kill_context(struct i915_gem_context *ctx) { + bool ban = (!i915_gem_context_is_persistent(ctx) || + !ctx->i915->params.enable_hangcheck); struct i915_gem_engines *pos, *next; spin_lock_irq(&ctx->stale.lock); @@ -545,7 +546,7 @@ static void kill_stale_engines(struct i915_gem_context *ctx) spin_unlock_irq(&ctx->stale.lock); - kill_engines(pos); + kill_engines(pos, ban); spin_lock_irq(&ctx->stale.lock); GEM_BUG_ON(i915_sw_fence_signaled(&pos->fence)); @@ -557,11 +558,6 @@ static void kill_stale_engines(struct i915_gem_context *ctx) spin_unlock_irq(&ctx->stale.lock); } -static void kill_context(struct i915_gem_context *ctx) -{ - kill_stale_engines(ctx); -} - static void engines_idle_release(struct i915_gem_context *ctx, struct i915_gem_engines *engines) { @@ -596,7 +592,7 @@ static void engines_idle_release(struct i915_gem_context *ctx, kill: if (list_empty(&engines->link)) /* raced, already closed */ - kill_engines(engines); + kill_engines(engines, true); i915_sw_fence_commit(&engines->fence); } @@ -654,9 +650,7 @@ static void context_close(struct i915_gem_context *ctx) * case we opt to forcibly kill off all remaining requests on * context close. */ - if (!i915_gem_context_is_persistent(ctx) || - !ctx->i915->params.enable_hangcheck) - kill_context(ctx); + kill_context(ctx); i915_gem_context_put(ctx); } -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine
== Series Details == Series: series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine URL : https://patchwork.freedesktop.org/series/80573/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +drivers/gpu/drm/i915/gt/intel_reset.c:1311:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block +drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040 +drivers/gpu/drm/i915/i915_perf.c:1425:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1479:15: warning: memset with byte count of 16777216 +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block ___ 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 [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine
== Series Details == Series: series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine URL : https://patchwork.freedesktop.org/series/80573/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8879 -> Patchwork_18350 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18350 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18350, 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_18350/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18350: ### IGT changes ### Possible regressions * igt@gem_close_race@basic-threads: - fi-bwr-2160:[PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bwr-2160/igt@gem_close_r...@basic-threads.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bwr-2160/igt@gem_close_r...@basic-threads.html * igt@gem_ctx_create@basic-files: - fi-byt-j1900: [PASS][3] -> [FAIL][4] +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-byt-j1900/igt@gem_ctx_cre...@basic-files.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-byt-j1900/igt@gem_ctx_cre...@basic-files.html - fi-snb-2600:[PASS][5] -> [FAIL][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-snb-2600/igt@gem_ctx_cre...@basic-files.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-snb-2600/igt@gem_ctx_cre...@basic-files.html - fi-bsw-n3050: [PASS][7] -> [FAIL][8] +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-n3050/igt@gem_ctx_cre...@basic-files.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bsw-n3050/igt@gem_ctx_cre...@basic-files.html - fi-ivb-3770:[PASS][9] -> [FAIL][10] +3 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-ivb-3770/igt@gem_ctx_cre...@basic-files.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-ivb-3770/igt@gem_ctx_cre...@basic-files.html * igt@gem_exec_parallel@engines@contexts: - fi-bdw-gvtdvm: [PASS][11] -> [FAIL][12] +2 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bdw-gvtdvm/igt@gem_exec_parallel@engi...@contexts.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bdw-gvtdvm/igt@gem_exec_parallel@engi...@contexts.html - fi-bsw-nick:[PASS][13] -> [FAIL][14] +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-nick/igt@gem_exec_parallel@engi...@contexts.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bsw-nick/igt@gem_exec_parallel@engi...@contexts.html - fi-snb-2520m: [PASS][15] -> [FAIL][16] +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-snb-2520m/igt@gem_exec_parallel@engi...@contexts.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-snb-2520m/igt@gem_exec_parallel@engi...@contexts.html - fi-ilk-650: [PASS][17] -> [FAIL][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-ilk-650/igt@gem_exec_parallel@engi...@contexts.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-ilk-650/igt@gem_exec_parallel@engi...@contexts.html - fi-elk-e7500: [PASS][19] -> [FAIL][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-elk-e7500/igt@gem_exec_parallel@engi...@contexts.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-elk-e7500/igt@gem_exec_parallel@engi...@contexts.html * igt@gem_exec_parallel@engines@fds: - fi-hsw-4770:[PASS][21] -> [FAIL][22] +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-hsw-4770/igt@gem_exec_parallel@engi...@fds.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-hsw-4770/igt@gem_exec_parallel@engi...@fds.html - fi-bdw-5557u: [PASS][23] -> [FAIL][24] +2 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bdw-5557u/igt@gem_exec_parallel@engi...@fds.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bdw-5557u/igt@gem_exec_parallel@engi...@fds.html - fi-bsw-nick:[PASS][25] -> [INCOMPLETE][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-nick/igt@gem_exec_parallel@engi...@fds.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18350/fi-bsw-nick/igt@gem_exec_parallel@engi...@fds.html - fi-bsw-kefka: [PASS][27] -> [FAI
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine (rev2)
== Series Details == Series: series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine (rev2) URL : https://patchwork.freedesktop.org/series/80573/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +drivers/gpu/drm/i915/gt/intel_reset.c:1311:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block +drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040 +drivers/gpu/drm/i915/i915_perf.c:1425:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1479:15: warning: memset with byte count of 16777216 +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block +./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block ___ 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 [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine (rev2)
== Series Details == Series: series starting with [1/3] drm/i915: Cancel outstanding work after disabling heartbeats on an engine (rev2) URL : https://patchwork.freedesktop.org/series/80573/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8879 -> Patchwork_18351 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18351 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18351, 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_18351/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18351: ### IGT changes ### Possible regressions * igt@i915_selftest@live@gem_contexts: - fi-bsw-kefka: [PASS][1] -> [DMESG-FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-kefka/igt@i915_selftest@live@gem_contexts.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-bsw-kefka/igt@i915_selftest@live@gem_contexts.html - fi-bsw-n3050: [PASS][3] -> [DMESG-FAIL][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-n3050/igt@i915_selftest@live@gem_contexts.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-bsw-n3050/igt@i915_selftest@live@gem_contexts.html - fi-bsw-nick:[PASS][5] -> [DMESG-FAIL][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-nick/igt@i915_selftest@live@gem_contexts.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-bsw-nick/igt@i915_selftest@live@gem_contexts.html - fi-bdw-5557u: [PASS][7] -> [DMESG-FAIL][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html Known issues Here are the changes found in Patchwork_18351 that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rpm@basic-pci-d3-state: - fi-bsw-n3050: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html Possible fixes * igt@i915_selftest@live@execlists: - fi-icl-y: [INCOMPLETE][11] ([i915#2276]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-icl-y/igt@i915_selftest@l...@execlists.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-icl-y/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@gem_contexts: - fi-tgl-u2: [INCOMPLETE][13] ([i915#2045]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html * igt@kms_busy@basic@flip: - fi-kbl-x1275: [DMESG-WARN][15] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@kms_busy@ba...@flip.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-kbl-x1275/igt@kms_busy@ba...@flip.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [DMESG-WARN][17] ([i915#2203]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html * igt@kms_flip@basic-flip-vs-wf_vblank@d-dsi1: - {fi-tgl-dsi}: [DMESG-WARN][19] ([i915#1982]) -> [PASS][20] +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-wf_vbl...@d-dsi1.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-wf_vbl...@d-dsi1.html Warnings * igt@gem_exec_suspend@basic-s0: - fi-kbl-x1275: [DMESG-WARN][21] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][22] ([i915#62] / [i915#92]) +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18351/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence: - fi-kbl-x1275: [DMESG-WARN][
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Apply Wa_14011264657:gen11+ (rev2)
== Series Details == Series: drm/i915: Apply Wa_14011264657:gen11+ (rev2) URL : https://patchwork.freedesktop.org/series/78430/ State : success == Summary == CI Bug Log - changes from CI_DRM_8879_full -> Patchwork_18349_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18349_full that come from known issues: ### IGT changes ### Issues hit * igt@kms_concurrent@pipe-c: - shard-skl: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) +11 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-skl10/igt@kms_concurr...@pipe-c.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-skl1/igt@kms_concurr...@pipe-c.html * igt@kms_cursor_crc@pipe-c-cursor-suspend: - shard-iclb: [PASS][3] -> [INCOMPLETE][4] ([i915#1185]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-iclb4/igt@kms_cursor_...@pipe-c-cursor-suspend.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-iclb3/igt@kms_cursor_...@pipe-c-cursor-suspend.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1: - shard-glk: [PASS][5] -> [FAIL][6] ([i915#79]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interrupti...@b-hdmi-a1.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interrupti...@b-hdmi-a1.html * igt@kms_flip@flip-vs-suspend@c-dp1: - shard-kbl: [PASS][7] -> [DMESG-WARN][8] ([i915#180]) +4 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-kbl4/igt@kms_flip@flip-vs-susp...@c-dp1.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-kbl1/igt@kms_flip@flip-vs-susp...@c-dp1.html * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1: - shard-skl: [PASS][9] -> [FAIL][10] ([i915#2122]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-skl7/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-skl8/igt@kms_flip@plain-flip-ts-check-interrupti...@b-edp1.html * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu: - shard-kbl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-kbl7/igt@kms_frontbuffer_track...@fbc-rgb101010-draw-mmap-cpu.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-kbl7/igt@kms_frontbuffer_track...@fbc-rgb101010-draw-mmap-cpu.html * igt@kms_frontbuffer_tracking@psr-shrfb-scaledprimary: - shard-iclb: [PASS][13] -> [DMESG-WARN][14] ([i915#1982]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-iclb4/igt@kms_frontbuffer_track...@psr-shrfb-scaledprimary.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-iclb3/igt@kms_frontbuffer_track...@psr-shrfb-scaledprimary.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [PASS][15] -> [FAIL][16] ([i915#1188]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-skl10/igt@kms_...@bpc-switch-dpms.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-skl1/igt@kms_...@bpc-switch-dpms.html * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min: - shard-skl: [PASS][17] -> [FAIL][18] ([fdo#108145] / [i915#265]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-skl8/igt@kms_plane_alpha_bl...@pipe-c-constant-alpha-min.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-skl5/igt@kms_plane_alpha_bl...@pipe-c-constant-alpha-min.html * igt@kms_plane_scaling@pipe-b-scaler-with-rotation: - shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#109052]) +4 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-iclb5/igt@kms_plane_scal...@pipe-b-scaler-with-rotation.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-iclb7/igt@kms_plane_scal...@pipe-b-scaler-with-rotation.html * igt@kms_plane_scaling@pipe-d-scaler-with-pixel-format: - shard-tglb: [PASS][21] -> [FAIL][22] ([i915#1214]) +6 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-tglb8/igt@kms_plane_scal...@pipe-d-scaler-with-pixel-format.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18349/shard-tglb1/igt@kms_plane_scal...@pipe-d-scaler-with-pixel-format.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-iclb: [PASS][23] -> [SKIP][24] ([fdo#109441]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8879/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html [24]: https://intel-gfx-ci.01.org
[Intel-gfx] [Regression] "drm/i915: Implement display w/a #1143" breaks HDMI on ASUS GL552VW
Hi, There's a regression reported that HDMI output stops working after os upgrade: https://bugs.launchpad.net/bugs/1871721 Here's the bisect result: 0519c102f5285476d7868a387bdb6c58385e4074 is the first bad commit commit 0519c102f5285476d7868a387bdb6c58385e4074 Author: Ville Syrjälä Date: Mon Jan 22 19:41:31 2018 +0200 drm/i915: Implement display w/a #1143 Apparently SKL/KBL/CFL need some manual help to get the programmed HDMI vswing to stick. Implement the relevant workaround (display w/a #1143). Note that the relevant chicken bits live in a transcoder register even though the bits affect a specific DDI port rather than a specific transcoder. Hence we must pick the correct transcoder register instance based on the port rather than based on the cpu_transcoder. Also note that for completeness I included support for DDI A/E in the code even though we never have HDMI on those ports. v2: CFL needs the w/a as well (Rodrigo and Art) Cc: Rodrigo Vivi Cc: Art Runyan Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180122174131.28046-1-ville.syrj...@linux.intel.com Reviewed-by: Rodrigo Vivi dmesg from drm-tip with drm.debug=0xe can be found here: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/64 Kai-Heng ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/5] xen/gntdev: Fix dmabuf import with non-zero sgt offset
From: Oleksandr Andrushchenko It is possible that the scatter-gather table during dmabuf import has non-zero offset of the data, but user-space doesn't expect that. Fix this by failing the import, so user-space doesn't access wrong data. Fixes: bf8dc55b1358 ("xen/gntdev: Implement dma-buf import functionality") Signed-off-by: Oleksandr Andrushchenko Acked-by: Juergen Gross Cc: --- drivers/xen/gntdev-dmabuf.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 75d3bb948bf3..b1b6eebafd5d 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -613,6 +613,14 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev, goto fail_detach; } + /* Check that we have zero offset. */ + if (sgt->sgl->offset) { + ret = ERR_PTR(-EINVAL); + pr_debug("DMA buffer has %d bytes offset, user-space expects 0\n", +sgt->sgl->offset); + goto fail_unmap; + } + /* Check number of pages that imported buffer has. */ if (attach->dmabuf->size != gntdev_dmabuf->nr_pages << PAGE_SHIFT) { ret = ERR_PTR(-EINVAL); -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/5] drm/xen-front: Fix misused IS_ERR_OR_NULL checks
From: Oleksandr Andrushchenko The patch c575b7eeb89f: "drm/xen-front: Add support for Xen PV display frontend" from Apr 3, 2018, leads to the following static checker warning: drivers/gpu/drm/xen/xen_drm_front_gem.c:140 xen_drm_front_gem_create() warn: passing zero to 'ERR_CAST' drivers/gpu/drm/xen/xen_drm_front_gem.c 133 struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev, 134 size_t size) 135 { 136 struct xen_gem_object *xen_obj; 137 138 xen_obj = gem_create(dev, size); 139 if (IS_ERR_OR_NULL(xen_obj)) 140 return ERR_CAST(xen_obj); Fix this and the rest of misused places with IS_ERR_OR_NULL in the driver. Fixes: c575b7eeb89f: "drm/xen-front: Add support for Xen PV display frontend" Signed-off-by: Oleksandr Andrushchenko Reported-by: Dan Carpenter Reviewed-by: Dan Carpenter Cc: --- drivers/gpu/drm/xen/xen_drm_front.c | 4 ++-- drivers/gpu/drm/xen/xen_drm_front_gem.c | 8 drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index 1fd458e877ca..51818e76facd 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -400,8 +400,8 @@ static int xen_drm_drv_dumb_create(struct drm_file *filp, args->size = args->pitch * args->height; obj = xen_drm_front_gem_create(dev, args->size); - if (IS_ERR_OR_NULL(obj)) { - ret = PTR_ERR_OR_ZERO(obj); + if (IS_ERR(obj)) { + ret = PTR_ERR(obj); goto fail; } diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c index f0b85e094111..4ec8a49241e1 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -83,7 +83,7 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size) size = round_up(size, PAGE_SIZE); xen_obj = gem_create_obj(dev, size); - if (IS_ERR_OR_NULL(xen_obj)) + if (IS_ERR(xen_obj)) return xen_obj; if (drm_info->front_info->cfg.be_alloc) { @@ -117,7 +117,7 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size) */ xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE); xen_obj->pages = drm_gem_get_pages(&xen_obj->base); - if (IS_ERR_OR_NULL(xen_obj->pages)) { + if (IS_ERR(xen_obj->pages)) { ret = PTR_ERR(xen_obj->pages); xen_obj->pages = NULL; goto fail; @@ -136,7 +136,7 @@ struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev, struct xen_gem_object *xen_obj; xen_obj = gem_create(dev, size); - if (IS_ERR_OR_NULL(xen_obj)) + if (IS_ERR(xen_obj)) return ERR_CAST(xen_obj); return &xen_obj->base; @@ -194,7 +194,7 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev, size = attach->dmabuf->size; xen_obj = gem_create_obj(dev, size); - if (IS_ERR_OR_NULL(xen_obj)) + if (IS_ERR(xen_obj)) return ERR_CAST(xen_obj); ret = gem_alloc_pages_array(xen_obj, size); diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c index 78096bbcd226..ef11b1e4de39 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c @@ -60,7 +60,7 @@ fb_create(struct drm_device *dev, struct drm_file *filp, int ret; fb = drm_gem_fb_create_with_funcs(dev, filp, mode_cmd, &fb_funcs); - if (IS_ERR_OR_NULL(fb)) + if (IS_ERR(fb)) return fb; gem_obj = fb->obj[0]; -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/5] drm/xen-front: Add YUYV to supported formats
From: Oleksandr Andrushchenko Add YUYV to supported formats, so the frontend can work with the formats used by cameras and other HW. Signed-off-by: Oleksandr Andrushchenko Acked-by: Noralf Trønnes --- drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.c b/drivers/gpu/drm/xen/xen_drm_front_conn.c index 459702fa990e..44f1f70c0aed 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_conn.c +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.c @@ -33,6 +33,7 @@ static const u32 plane_formats[] = { DRM_FORMAT_ARGB, DRM_FORMAT_XRGB1555, DRM_FORMAT_ARGB1555, + DRM_FORMAT_YUYV, }; const u32 *xen_drm_front_conn_get_formats(int *format_count) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/5] xen: Sync up with the canonical protocol definition in Xen
From: Oleksandr Andrushchenko This is the sync up with the canonical definition of the display protocol in Xen. 1. Add protocol version as an integer Version string, which is in fact an integer, is hard to handle in the code that supports different protocol versions. To simplify that also add the version as an integer. 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE There are cases when display data buffer is created with non-zero offset to the data start. Handle such cases and provide that offset while creating a display buffer. 3. Add XENDISPL_OP_GET_EDID command Add an optional request for reading Extended Display Identification Data (EDID) structure which allows better configuration of the display connectors over the configuration set in XenStore. With this change connectors may have multiple resolutions defined with respect to detailed timing definitions and additional properties normally provided by displays. If this request is not supported by the backend then visible area is defined by the relevant XenStore's "resolution" property. If backend provides extended display identification data (EDID) with XENDISPL_OP_GET_EDID request then EDID values must take precedence over the resolutions defined in XenStore. 4. Bump protocol version to 2. Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Juergen Gross --- include/xen/interface/io/displif.h | 91 +- 1 file changed, 88 insertions(+), 3 deletions(-) diff --git a/include/xen/interface/io/displif.h b/include/xen/interface/io/displif.h index fdc279dc4a88..d43ca0361f86 100644 --- a/include/xen/interface/io/displif.h +++ b/include/xen/interface/io/displif.h @@ -38,7 +38,8 @@ * Protocol version ** */ -#define XENDISPL_PROTOCOL_VERSION "1" +#define XENDISPL_PROTOCOL_VERSION "2" +#define XENDISPL_PROTOCOL_VERSION_INT 2 /* ** @@ -202,6 +203,9 @@ * Width and height of the connector in pixels separated by * XENDISPL_RESOLUTION_SEPARATOR. This defines visible area of the * display. + * If backend provides extended display identification data (EDID) with + * XENDISPL_OP_GET_EDID request then EDID values must take precedence + * over the resolutions defined here. * *-- Connector Request Transport Parameters --- * @@ -349,6 +353,8 @@ #define XENDISPL_OP_FB_DETACH 0x13 #define XENDISPL_OP_SET_CONFIG 0x14 #define XENDISPL_OP_PG_FLIP0x15 +/* The below command is available in protocol version 2 and above. */ +#define XENDISPL_OP_GET_EDID 0x16 /* ** @@ -377,6 +383,10 @@ #define XENDISPL_FIELD_BE_ALLOC"be-alloc" #define XENDISPL_FIELD_UNIQUE_ID "unique-id" +#define XENDISPL_EDID_BLOCK_SIZE 128 +#define XENDISPL_EDID_BLOCK_COUNT 256 +#define XENDISPL_EDID_MAX_SIZE (XENDISPL_EDID_BLOCK_SIZE * XENDISPL_EDID_BLOCK_COUNT) + /* ** * STATUS RETURN CODES @@ -451,7 +461,9 @@ * +++++ * | gref_directory | 40 * +++++ - * | reserved | 44 + * | data_ofs | 44 + * +++++ + * | reserved | 48 * +++++ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| * +++++ @@ -494,6 +506,7 @@ * buffer size (buffer_sz) exceeds what can be addressed by this single page, * then reference to the next page must be supplied (see gref_dir_next_page * below) + * data_ofs - uint32_t, offset of the data in the buffer, octets */ #define XENDISPL_DBUF_FLG_REQ_ALLOC(1 << 0) @@ -506,6 +519,7 @@ struct xendispl_dbuf_create_req { uint32_t buffer_sz; uint32_t flags; grant_ref_t gref_directory; + uint32_t data_ofs; }; /* @@ -731,6 +745,44 @@ struct xendispl_page_flip_req { uint64_t fb_cookie; }; +/* + * Request EDID - request EDID describing current connector: + * 01 2 3octet + * +++++ + * | id| _OP_GET_EDID | reserved | 4 + * ++--
[Intel-gfx] [PATCH v2 0/5] Fixes and improvements for Xen pvdrm
From: Oleksandr Andrushchenko Hello, This series contains an assorted set of fixes and improvements for the Xen para-virtualized display driver and grant device driver which I have collected over the last couple of months: 1. Minor fixes to grant device driver and drm/xen-front. 2. New format (YUYV) added to the list of the PV DRM supported formats which allows the driver to be used in zero-copying use-cases when a camera device is the source of the dma-bufs. 3. Synchronization with the latest para-virtualized protocol definition in Xen [1]. 4. SGT offset is now propagated to the backend: while importing a dmabuf it is possible that the data of the buffer is put with offset which is indicated by the SGT offset. This is needed for some GPUs which have non-zero offset. Thank you, Oleksandr Andrushchenko [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0 Changes since v1: = 1. Removed patch which adds EDID to PV DRM as it needs more time for review: "5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID request which allows frontends to request EDID structure per connector. This request is optional and if not supported by the backend then visible area is still defined by the relevant XenStore's "resolution" property. If backend provides EDID with XENDISPL_OP_GET_EDID request then its values must take precedence over the resolutions defined in XenStore." I will send this as a dedicated patch. 2. Added missing CC stable for the patches with fixes Oleksandr Andrushchenko (5): xen/gntdev: Fix dmabuf import with non-zero sgt offset drm/xen-front: Fix misused IS_ERR_OR_NULL checks drm/xen-front: Add YUYV to supported formats xen: Sync up with the canonical protocol definition in Xen drm/xen-front: Pass dumb buffer data offset to the backend drivers/gpu/drm/xen/xen_drm_front.c | 10 +-- drivers/gpu/drm/xen/xen_drm_front.h | 2 +- drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 + drivers/gpu/drm/xen/xen_drm_front_gem.c | 11 +-- drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +- drivers/xen/gntdev-dmabuf.c | 8 +++ include/xen/interface/io/displif.h | 91 +++- 7 files changed, 111 insertions(+), 14 deletions(-) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/5] drm/xen-front: Pass dumb buffer data offset to the backend
From: Oleksandr Andrushchenko While importing a dmabuf it is possible that the data of the buffer is put with offset which is indicated by the SGT offset. Respect the offset value and forward it to the backend. Signed-off-by: Oleksandr Andrushchenko Acked-by: Noralf Trønnes --- drivers/gpu/drm/xen/xen_drm_front.c | 6 -- drivers/gpu/drm/xen/xen_drm_front.h | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index 51818e76facd..3dd56794593a 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -157,7 +157,8 @@ int xen_drm_front_mode_set(struct xen_drm_front_drm_pipeline *pipeline, int xen_drm_front_dbuf_create(struct xen_drm_front_info *front_info, u64 dbuf_cookie, u32 width, u32 height, - u32 bpp, u64 size, struct page **pages) + u32 bpp, u64 size, u32 offset, + struct page **pages) { struct xen_drm_front_evtchnl *evtchnl; struct xen_drm_front_dbuf *dbuf; @@ -194,6 +195,7 @@ int xen_drm_front_dbuf_create(struct xen_drm_front_info *front_info, req->op.dbuf_create.gref_directory = xen_front_pgdir_shbuf_get_dir_start(&dbuf->shbuf); req->op.dbuf_create.buffer_sz = size; + req->op.dbuf_create.data_ofs = offset; req->op.dbuf_create.dbuf_cookie = dbuf_cookie; req->op.dbuf_create.width = width; req->op.dbuf_create.height = height; @@ -408,7 +410,7 @@ static int xen_drm_drv_dumb_create(struct drm_file *filp, ret = xen_drm_front_dbuf_create(drm_info->front_info, xen_drm_front_dbuf_to_cookie(obj), args->width, args->height, args->bpp, - args->size, + args->size, 0, xen_drm_front_gem_get_pages(obj)); if (ret) goto fail_backend; diff --git a/drivers/gpu/drm/xen/xen_drm_front.h b/drivers/gpu/drm/xen/xen_drm_front.h index f92c258350ca..54486d89650e 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.h +++ b/drivers/gpu/drm/xen/xen_drm_front.h @@ -145,7 +145,7 @@ int xen_drm_front_mode_set(struct xen_drm_front_drm_pipeline *pipeline, int xen_drm_front_dbuf_create(struct xen_drm_front_info *front_info, u64 dbuf_cookie, u32 width, u32 height, - u32 bpp, u64 size, struct page **pages); + u32 bpp, u64 size, u32 offset, struct page **pages); int xen_drm_front_fb_attach(struct xen_drm_front_info *front_info, u64 dbuf_cookie, u64 fb_cookie, u32 width, diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c index 4ec8a49241e1..39ff95b75357 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -210,7 +210,8 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev, ret = xen_drm_front_dbuf_create(drm_info->front_info, xen_drm_front_dbuf_to_cookie(&xen_obj->base), - 0, 0, 0, size, xen_obj->pages); + 0, 0, 0, size, sgt->sgl->offset, + xen_obj->pages); if (ret < 0) return ERR_PTR(ret); -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fixes and improvements for Xen pvdrm (rev2)
== Series Details == Series: Fixes and improvements for Xen pvdrm (rev2) URL : https://patchwork.freedesktop.org/series/80141/ State : warning == Summary == $ dim checkpatch origin/drm-tip 18bd08b8bf52 xen/gntdev: Fix dmabuf import with non-zero sgt offset edc113ee2576 drm/xen-front: Fix misused IS_ERR_OR_NULL checks -:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #14: 133 struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev, total: 0 errors, 1 warnings, 0 checks, 50 lines checked 8ba79afe8343 drm/xen-front: Add YUYV to supported formats 76f71f3be317 xen: Sync up with the canonical protocol definition in Xen -:109: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t' #109: FILE: include/xen/interface/io/displif.h:522: + uint32_t data_ofs; -:151: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t' #151: FILE: include/xen/interface/io/displif.h:782: + uint32_t buffer_sz; -:187: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t' #187: FILE: include/xen/interface/io/displif.h:833: + uint32_t edid_sz; -:209: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u8' over 'uint8_t' #209: FILE: include/xen/interface/io/displif.h:900: + uint8_t reserved1[56]; total: 0 errors, 0 warnings, 4 checks, 157 lines checked 62487e5fb7ce drm/xen-front: Pass dumb buffer data offset to the backend ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx