From patchwork Mon Sep 4 04:16:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 13373464 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 84DD3C83F33 for ; Mon, 4 Sep 2023 04:16:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1607810E288; Mon, 4 Sep 2023 04:16:49 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 01A2A10E287; Mon, 4 Sep 2023 04:16:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693801005; x=1725337005; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=l82/oFi4MHKbE72huJXg77fwQqfakLlpWVcH/cf76/g=; b=grNvGSRJo7I5fcK+mxMPzt6c70AKOccRCto7eWG1U7fBjn4Ho5XvmSfj +GXt6L6gTorrtDrtbdX7L+j7QGVDOD688wAkZ7Mqc2q1ymYEDmiu4Y0Jo 65rFu0v4gyGRKZ1/Hg+kvDiLqWJtHZ4V531R0j4Wan2KqxZ2VSqP+p0qu ZwPuzwyrwO8MveHL/cPtwJnPMezkDgab/rSbaInIOXyfnix/KevDt2aLj Hg400368qVtQLL+qrZHn9tWu9o/Zt1tWlSrCskg61QIM5JLY3DFlXq00F INoeTnxOj8+Ot5EyTc08+E31d15XoOGdfEtttAv7jPQNGl6nD3WkMdP1j g==; X-IronPort-AV: E=McAfee;i="6600,9927,10822"; a="361531439" X-IronPort-AV: E=Sophos;i="6.02,225,1688454000"; d="scan'208";a="361531439" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Sep 2023 21:16:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10822"; a="883893539" X-IronPort-AV: E=Sophos;i="6.02,225,1688454000"; d="scan'208";a="883893539" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.153]) by fmsmga001.fm.intel.com with SMTP; 03 Sep 2023 21:16:36 -0700 Received: by stinkbox (sSMTP sendmail emulation); Mon, 04 Sep 2023 07:16:40 +0300 From: Ville Syrjala To: intel-gfx@lists.freedesktop.org Subject: [PATCH v2 1/2] drm/i915: Use vblank worker to unpin old legacy cursor fb safely Date: Mon, 4 Sep 2023 07:16:39 +0300 Message-ID: <20230904041640.31297-1-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä The cursor hardware only does sync updates, and thus the hardware will be scanning out from the old fb until the next start of vblank. So in order to make the legacy cursor fastpath actually safe we should not unpin the old fb until we're sure the hardware has ceased accessing it. The simplest approach is to just use a vblank work here to do the delayed unpin. Not 100% sure it's a good idea to put this onto the same high priority vblank worker as eg. our timing critical gamma updates. But let's keep it simple for now, and it we later discover that this is causing problems we can think about adding a lower priority worker for such things. v2: wait for cursor unpins before turning off the vblank irq Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Reviewed-by: Juha-Pekka Heikkila --- drivers/gpu/drm/i915/display/intel_cursor.c | 36 +++++++++++++++++-- drivers/gpu/drm/i915/display/intel_cursor.h | 2 ++ drivers/gpu/drm/i915/display/intel_display.c | 3 ++ .../drm/i915/display/intel_display_types.h | 4 +++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index b342fad180ca..625540fd1dab 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -603,6 +603,26 @@ static bool intel_cursor_format_mod_supported(struct drm_plane *_plane, return format == DRM_FORMAT_ARGB8888; } +static void intel_cursor_unpin_work(struct kthread_work *base) +{ + struct drm_vblank_work *work = to_drm_vblank_work(base); + struct intel_plane_state *plane_state = + container_of(work, typeof(*plane_state), unpin_work); + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); + + intel_plane_unpin_fb(plane_state); + intel_plane_destroy_state(&plane->base, &plane_state->uapi); + + if (atomic_dec_and_test(&plane->cursor.pending_unpins)) + wake_up_var(&plane->cursor.pending_unpins); +} + +void intel_cursor_wait_unpin_works(struct intel_plane *plane) +{ + wait_var_event(&plane->cursor.pending_unpins, + !atomic_read(&plane->cursor.pending_unpins)); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -730,14 +750,26 @@ intel_legacy_cursor_update(struct drm_plane *_plane, local_irq_enable(); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->hw.fb != new_plane_state->hw.fb) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, + intel_cursor_unpin_work); + + atomic_inc(&plane->cursor.pending_unpins); + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->base) + 1, + false); + + old_plane_state = NULL; + } else { + intel_plane_unpin_fb(old_plane_state); + } out_free: if (new_crtc_state) intel_crtc_destroy_state(&crtc->base, &new_crtc_state->uapi); if (ret) intel_plane_destroy_state(&plane->base, &new_plane_state->uapi); - else + else if (old_plane_state) intel_plane_destroy_state(&plane->base, &old_plane_state->uapi); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_cursor.h b/drivers/gpu/drm/i915/display/intel_cursor.h index ce333bf4c2d5..e778aff77129 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.h +++ b/drivers/gpu/drm/i915/display/intel_cursor.h @@ -14,4 +14,6 @@ struct intel_plane * intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe); +void intel_cursor_wait_unpin_works(struct intel_plane *plane); + #endif diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index f6397462e4c2..90c1ed61ba0e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -63,6 +63,7 @@ #include "intel_crt.h" #include "intel_crtc.h" #include "intel_crtc_state_dump.h" +#include "intel_cursor.h" #include "intel_ddi.h" #include "intel_de.h" #include "intel_display_driver.h" @@ -6618,6 +6619,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) intel_pre_plane_update(state, crtc); intel_crtc_disable_planes(state, crtc); + + intel_cursor_wait_unpin_works(to_intel_plane(crtc->base.cursor)); } /* Only disable port sync and MST slaves */ diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index c21064794f32..f6ca86f1d834 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -702,6 +702,9 @@ struct intel_plane_state { struct intel_fb_view view; + /* for legacy cursor fb unpin */ + struct drm_vblank_work unpin_work; + /* Plane pxp decryption state */ bool decrypt; @@ -1515,6 +1518,7 @@ struct intel_plane { struct { u32 base, cntl, size; + atomic_t pending_unpins; } cursor; struct intel_fbc *fbc; From patchwork Mon Sep 4 04:16:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 13373465 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AB15EC83F33 for ; Mon, 4 Sep 2023 04:16:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 05AC610E287; Mon, 4 Sep 2023 04:16:57 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 53B3B10E287; Mon, 4 Sep 2023 04:16:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693801007; x=1725337007; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=q0FNLZFCv31DhObWokrUuGARIHxCY7P9GHenMrzVxMc=; b=kokwkvQrV2ngKdK8ARFV8ZB6zynTb2oMrLWVU2UQP8subC4XQersX9iL LEUWiosCTX+TiRBcAonhwMsD8NBMOfRodHDMoLCv8MQx2C8PmYmTH+CrE D8skfsFIaPavycc8TDeeT0LFNM6+Skv2XwA2CdTJfuTGVizPyINxRVMcD lqi7nCVnjVuT0tWrWylxoKu3tdTaiHfbTB/fhLJgy2yTui3Q5ly6hj3ay UD4bHYgMJIzPg7B/DBaQ2TpSu/DsA4ak4eGpJ60diyji65hppEpZc4WFQ /Gs7FIpEEVeOLSwIACjJVku/S5Uc4X+roamqI5liPd7h76I3Fj/azvX42 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10822"; a="361531448" X-IronPort-AV: E=Sophos;i="6.02,225,1688454000"; d="scan'208";a="361531448" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Sep 2023 21:16:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10822"; a="883893548" X-IronPort-AV: E=Sophos;i="6.02,225,1688454000"; d="scan'208";a="883893548" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.153]) by fmsmga001.fm.intel.com with SMTP; 03 Sep 2023 21:16:40 -0700 Received: by stinkbox (sSMTP sendmail emulation); Mon, 04 Sep 2023 07:16:43 +0300 From: Ville Syrjala To: intel-gfx@lists.freedesktop.org Subject: [PATCH v2 2/2] drm/vblank: Warn when silently cancelling vblank works Date: Mon, 4 Sep 2023 07:16:40 +0300 Message-ID: <20230904041640.31297-2-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230904041640.31297-1-ville.syrjala@linux.intel.com> References: <20230904041640.31297-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä Silently cancelling vblank works is a bit rude, especially if said works do any resource management (eg. free memory). WARN if we ever hit this. TODO: Maybe drm_crtc_vblank_off() should wait for any pending work to reach its target vblank before actually doing anything drastic? Cc: Lyude Paul Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_vblank_work.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c index bd481fdd6b87..43cd5c0f4f6f 100644 --- a/drivers/gpu/drm/drm_vblank_work.c +++ b/drivers/gpu/drm/drm_vblank_work.c @@ -73,6 +73,9 @@ void drm_vblank_cancel_pending_works(struct drm_vblank_crtc *vblank) assert_spin_locked(&vblank->dev->event_lock); + drm_WARN_ONCE(vblank->dev, !list_empty(&vblank->pending_work), + "Cancelling pending vblank works!\n"); + list_for_each_entry_safe(work, next, &vblank->pending_work, node) { list_del_init(&work->node); drm_vblank_put(vblank->dev, vblank->pipe);