From patchwork Sun Feb 4 19:52:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chaitanya Kumar Borah X-Patchwork-Id: 13544770 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 8F5F4C4828F for ; Sun, 4 Feb 2024 19:59:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2CAC610E898; Sun, 4 Feb 2024 19:58:57 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="JTL7LAR2"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id E20C610E8FF for ; Sun, 4 Feb 2024 19:58:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1707076736; x=1738612736; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=FVHeRRsbAh3MBjfI69pcnEC8kQ8cDY8StoHuUtsrduE=; b=JTL7LAR2jY0NiGE7MrPVTy7A5AYlHyEaIedH/PTsqM+LdVbLrY1z+rJi a1BSStrSXME2uTV2PfBsOMkOMK54JfAieX+Ckn59xzRANKPKXQS6WgXs3 FfeodVDb3NisWXN7BhSh7T4RhqXfveQ8d0E/sRn7MCRXqGDRZ82lRWy9S VcOSpRrJOMd0OGDCpP5/pYzWWyhdsVaX1Xka5JQOLQmMGwbKuWq7g72T3 +f2YuX0KTKCX2kXffqtQBGwgbmQaS7vMFUZbe2tvNMmviBqUKmd0/Cq2T qvuCYpUGX+HTtjobhOKHcP1YklvNkph+HidfVij6sKQpmIz2FSn8SFBei g==; X-IronPort-AV: E=McAfee;i="6600,9927,10973"; a="311179" X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="311179" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2024 11:58:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="5160727" Received: from dut-2a59.iind.intel.com ([10.190.239.113]) by fmviesa005.fm.intel.com with ESMTP; 04 Feb 2024 11:58:53 -0800 From: Chaitanya Kumar Borah To: intel-gfx@lists.freedesktop.org Cc: uma.shankar@intel.com, chaitanya.kumar.borah@intel.com, maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 1/5] drm/i915: Use vblank worker to unpin old legacy cursor fb safely Date: Mon, 5 Feb 2024 01:22:03 +0530 Message-Id: <20240204195207.3616932-2-chaitanya.kumar.borah@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> References: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" 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. This patch is slightly reworked by Maarten Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_cursor.c | 26 +++++++++++++++++-- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ .../drm/i915/display/intel_display_types.h | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index f8b33999d43f..9021c0c1683d 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,6 +654,17 @@ 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); +} + static int intel_legacy_cursor_update(struct drm_plane *_plane, struct drm_crtc *_crtc, @@ -797,14 +808,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane, intel_psr_unlock(crtc_state); - intel_plane_unpin_fb(old_plane_state); + if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) { + drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base, + intel_cursor_unpin_work); + + 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_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7fb0f71652ac..bf684c4d1732 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -64,6 +64,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" @@ -6780,6 +6781,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_crtc_disable_planes(state, crtc); + + drm_vblank_work_flush_all(&crtc->base); } /* 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 ae2e8cff9d69..5fdb9eccab5a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -713,6 +713,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; From patchwork Sun Feb 4 19:52:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chaitanya Kumar Borah X-Patchwork-Id: 13544771 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 74C3CC48286 for ; Sun, 4 Feb 2024 19:59:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 28FCD10E87A; Sun, 4 Feb 2024 19:58:59 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="ENegF2oD"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9ED7710E87A for ; Sun, 4 Feb 2024 19:58:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1707076738; x=1738612738; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=WoAnVrBPfkFEeKxpHZPuK1liFX9FXzdebad4asuYSAQ=; b=ENegF2oDIaQ/3syELPfh8aMSxicrcggHjHQf803TT+qGVEpkfaKV1wVE XngFhn4CaHbHMQss/zcPe6LJ/3NDpj69heRHyP/7uEDfTDHnbeKY+FEWK V/baSIH4LQGcSJCMCYYjuVFOjy35GD9nDWNb3nT5P5xsgwGNW8njLpKb1 n7cAvmlil1fJAaFEGQXEgjfVsck2aYJabPvSjzBPOQS0+M7YFcO7qy+XG 7SP9Tk/EXNIZxnTTOeHgT6AuZo89fquvIAgaJhnLgZt02b0pqF03XXQKV Vit7pkpA1pk0cDy4fK1KM4nh53JXwAqYCV0nLKETHQgmSEnmM6kJSYFud A==; X-IronPort-AV: E=McAfee;i="6600,9927,10973"; a="311181" X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="311181" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2024 11:58:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="5160730" Received: from dut-2a59.iind.intel.com ([10.190.239.113]) by fmviesa005.fm.intel.com with ESMTP; 04 Feb 2024 11:58:55 -0800 From: Chaitanya Kumar Borah To: intel-gfx@lists.freedesktop.org Cc: uma.shankar@intel.com, chaitanya.kumar.borah@intel.com, maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 2/5] drm/i915: Use the same vblank worker for atomic unpin Date: Mon, 5 Feb 2024 01:22:04 +0530 Message-Id: <20240204195207.3616932-3-chaitanya.kumar.borah@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> References: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Maarten Lankhorst In case of legacy cursor update, the cursor VMA needs to be unpinned only after vblank. This exceeds the lifetime of the whole atomic commit. Any trick I attempted to keep the atomic commit alive didn't work, as drm_atomic_helper_setup_commit() force throttles on any old commit that wasn't cleaned up. The only option remaining is to remove the plane from the atomic commit, and use the same path as the legacy cursor update to clean the state after vblank. Signed-off-by: Maarten Lankhorst --- .../gpu/drm/i915/display/intel_atomic_plane.c | 28 ++++++++++++++++++- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_crtc.c | 27 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- drivers/gpu/drm/i915/display/intel_cursor.h | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 76d77d5a0409..06c5d8262443 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -42,6 +42,7 @@ #include "i915_reg.h" #include "intel_atomic_plane.h" #include "intel_cdclk.h" +#include "intel_cursor.h" #include "intel_display_rps.h" #include "intel_display_trace.h" #include "intel_display_types.h" @@ -1163,7 +1164,21 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_display_rps_mark_interactive(dev_priv, state, false); - /* Should only be called after a successful intel_prepare_plane_fb()! */ + /* + * This branch can only ever be called after plane update is succesful, + * the error path will not cause unpin_work to be set. + */ + if (old_plane_state->unpin_work.vblank) { + int i = drm_plane_index(old_plane_state->uapi.plane); + + /* + * Remove plane from atomic commit, + * free is done from vblank worker + */ + memset(&state->base.planes[i], 0, sizeof(*state->base.planes)); + return; + } + intel_plane_unpin_fb(old_plane_state); } @@ -1176,3 +1191,14 @@ void intel_plane_helper_add(struct intel_plane *plane) { drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); } + +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, + struct intel_plane_state *new_plane_state) +{ + if (!old_plane_state->ggtt_vma || + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma) + return; + + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state->uapi.crtc, + intel_cursor_unpin_work); +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 191dad0efc8e..5a897cf6fa02 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); void intel_plane_helper_add(struct intel_plane *plane); +void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state, + struct intel_plane_state *new_plane_state); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 25593f6aae7d..a7fb7f5ace07 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -501,6 +501,18 @@ void intel_pipe_update_start(struct intel_atomic_state *state, intel_crtc_vblank_work_init(new_crtc_state); intel_vblank_evade_init(old_crtc_state, new_crtc_state, &evade); + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state, *new_plane_state; + int i; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, + new_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base) + intel_plane_init_cursor_vblank_work(old_plane_state, + new_plane_state); + } + } if (drm_WARN_ON(&dev_priv->drm, drm_crtc_vblank_get(&crtc->base))) goto irq_disable; @@ -616,6 +628,21 @@ void intel_pipe_update_end(struct intel_atomic_state *state, new_crtc_state->uapi.event = NULL; } + if (state->base.legacy_cursor_update) { + struct intel_plane *plane; + struct intel_plane_state *old_plane_state; + int i; + + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { + if (old_plane_state->uapi.crtc == &crtc->base && + old_plane_state->unpin_work.vblank) { + drm_vblank_work_schedule(&old_plane_state->unpin_work, + drm_crtc_accurate_vblank_count(&crtc->base) + 1, + false); + } + } + } + /* * Send VRR Push to terminate Vblank. If we are already in vblank * this has to be done _after_ sampling the frame counter, as diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index 9021c0c1683d..dbb26a212800 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -654,7 +654,7 @@ 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) +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 = diff --git a/drivers/gpu/drm/i915/display/intel_cursor.h b/drivers/gpu/drm/i915/display/intel_cursor.h index ce333bf4c2d5..e2d9ec710a86 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.h +++ b/drivers/gpu/drm/i915/display/intel_cursor.h @@ -9,9 +9,12 @@ enum pipe; struct drm_i915_private; struct intel_plane; +struct kthread_work; struct intel_plane * intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe); +void intel_cursor_unpin_work(struct kthread_work *base); + #endif From patchwork Sun Feb 4 19:52:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chaitanya Kumar Borah X-Patchwork-Id: 13544774 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 930B4C48297 for ; Sun, 4 Feb 2024 19:59:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2D42C10EB71; Sun, 4 Feb 2024 19:59:01 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="fyJcsC9Q"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3F97A10E9BB for ; Sun, 4 Feb 2024 19:58:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1707076739; x=1738612739; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=pREDIC8p3h4esMH7kiMi9LZNYxvYQOsucxJZZ8Q5DcA=; b=fyJcsC9QbNS6jX5pz8INoiXC4abFwhNaSFTNcJ18hXNw4hk8xSC5bLYH yIrjKDPf8BkTpV4LWNoVf8XDeDKIeSF3rqz0cwL55LEklJHHOFj8O67Pe aYt/ZZzYfo14Zmp1uKN5hEEZCk+1/ue/px03GLQzzVejQUdWgJVyISTKU /bUTJXYMTcNpH4e+O6/4DfkvPVA3TROWjM+c0Oh++YFKbMvIleDs1B5by NJ+yL8f8dR3slED1n5BkknZm63TeBH6iY1H2UUfKf0904Quq6S8tEnClg oqF2aotJKFq9x0XGsNznky761wFGsx2QFLzcjfTMlJr2uz6jxkKPfw/rY Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10973"; a="311185" X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="311185" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2024 11:58:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="5160733" Received: from dut-2a59.iind.intel.com ([10.190.239.113]) by fmviesa005.fm.intel.com with ESMTP; 04 Feb 2024 11:58:57 -0800 From: Chaitanya Kumar Borah To: intel-gfx@lists.freedesktop.org Cc: uma.shankar@intel.com, chaitanya.kumar.borah@intel.com, maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 3/5] drm/i915: do not destroy plane state if cursor unpin worker is scheduled Date: Mon, 5 Feb 2024 01:22:05 +0530 Message-Id: <20240204195207.3616932-4-chaitanya.kumar.borah@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> References: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" The plane destroy hook can be called asynchronously even when vblank worker responsible for unpinning the cursor fb is scheduled. Since the vblank worker destroys the plane state, do not destroy the plane state if it is scheduled. Signed-off-by: Chaitanya Kumar Borah --- .../gpu/drm/i915/display/intel_atomic_plane.c | 19 +++++++++++++++++++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++ drivers/gpu/drm/i915/display/intel_cursor.c | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 06c5d8262443..a585e4aca309 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -136,6 +136,25 @@ intel_plane_destroy_state(struct drm_plane *plane, { struct intel_plane_state *plane_state = to_intel_plane_state(state); + /* Do not proceed if vblank unpin worker is yet to be executed */ + if (plane_state->unpin_work.vblank) + return; + + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); + drm_WARN_ON(plane->dev, plane_state->dpt_vma); + + __drm_atomic_helper_plane_destroy_state(&plane_state->uapi); + if (plane_state->hw.fb) + drm_framebuffer_put(plane_state->hw.fb); + kfree(plane_state); +} + +void +intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane_state *plane_state = to_intel_plane_state(state); + drm_WARN_ON(plane->dev, plane_state->ggtt_vma); drm_WARN_ON(plane->dev, plane_state->dpt_vma); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5a897cf6fa02..1e165b709a80 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -45,6 +45,8 @@ void intel_plane_free(struct intel_plane *plane); struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void intel_cursor_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); void intel_crtc_planes_update_noarm(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_crtc_planes_update_arm(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index dbb26a212800..32f9bb753331 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -662,7 +662,7 @@ void intel_cursor_unpin_work(struct kthread_work *base) 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); + intel_cursor_destroy_state(&plane->base, &plane_state->uapi); } static int From patchwork Sun Feb 4 19:52:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chaitanya Kumar Borah X-Patchwork-Id: 13544773 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 84232C4829A for ; Sun, 4 Feb 2024 19:59:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 502B610E9BB; Sun, 4 Feb 2024 19:59:02 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="ku+p9MYZ"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 15DE910EB59 for ; Sun, 4 Feb 2024 19:59:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1707076741; x=1738612741; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=OsycmJ9Rop7DNXJLxzVXISd7MwqwsvTPYuaXFmMJEes=; b=ku+p9MYZttMfhF+SrQ/PRi8WZANEzCvcguo3wE/zXgf10xPbr+PP/lQ+ 7PbiXrGs23jMESWCN9VfjCqnYPxiEgZX6e2GloWvsaVprkRHvNExWVybc 7w8HjeOyMsqVon78iKZJLZu6KV0HVSHzTNneRpL6YT7PMm7Woi6qV7eLL F9hXaff6M36fodf5nhKD9RxonOXZfuuSb9IQdVtCGAmf9rVYqOXh+diei woQfg7oi6hu4WKrwEf4DaglVdL+5zWMKwb9DTR24go+162qvH4gWmmurA Tg3gd9TPzhb6JpdSM/t9zQJpxYSVfGMjmrpVWb6pwEZvys/IlQVrmKJVH g==; X-IronPort-AV: E=McAfee;i="6600,9927,10973"; a="311193" X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="311193" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2024 11:59:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="5160736" Received: from dut-2a59.iind.intel.com ([10.190.239.113]) by fmviesa005.fm.intel.com with ESMTP; 04 Feb 2024 11:58:59 -0800 From: Chaitanya Kumar Borah To: intel-gfx@lists.freedesktop.org Cc: uma.shankar@intel.com, chaitanya.kumar.borah@intel.com, maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 4/5] drm/i915: Add sanity checks before accessing fb buffer object Date: Mon, 5 Feb 2024 01:22:06 +0530 Message-Id: <20240204195207.3616932-5-chaitanya.kumar.borah@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> References: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Now that cursor plane fb unpinning can be deferred to vblank work check if plane state and corresponding fb pointers are still valid before cleanup. Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index a585e4aca309..a0217fef9920 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -1171,12 +1171,18 @@ static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *_old_plane_state) { + struct drm_i915_private *i915 = to_i915(_old_plane_state->plane->dev); struct intel_plane_state *old_plane_state = to_intel_plane_state(_old_plane_state); struct intel_atomic_state *state = to_intel_atomic_state(old_plane_state->uapi.state); struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_i915_gem_object *obj = intel_fb_obj(old_plane_state->hw.fb); + struct drm_i915_gem_object *obj = NULL; + + if (old_plane_state && old_plane_state->hw.fb) + obj = intel_fb_obj(old_plane_state->hw.fb); + else + drm_err_ratelimited(&i915->drm, "Invalid plane state or fb\n"); if (!obj) return; From patchwork Sun Feb 4 19:52:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chaitanya Kumar Borah X-Patchwork-Id: 13544775 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 5C6EFC48299 for ; Sun, 4 Feb 2024 19:59:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5680F10EB59; Sun, 4 Feb 2024 19:59:04 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="SI3d8KgX"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1F5E810EB59 for ; Sun, 4 Feb 2024 19:59:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1707076743; x=1738612743; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=V+OQQND8EfGszRHhG597lpXEf3IWF4ojHfZD9c5+Y7w=; b=SI3d8KgXVbgjK8qt5+yaSGxefmrsPfQgM9nlydTn39MqHV7FOnE3KAbu aXEIEd9pr1TZyPp3Bnran1D8pTJ3hs9ilwFW6FvU8DXeL9+czi9EDjqrs zmf3sj3lvT6jB+5TwlKcpCaXlqKGdTy1dU2LURfUMvtRvlXnPRYkpNgQl VdEOMQZXc00R7hfPWK39Myb80T2XQ7ZctNa7tsnBGXz+EfxKMHWXWc7K1 Pnz0YTF71LiqTjMTXkLYu9g/auj1EM1Vknzcwtn+ebJmqV7MEkXpyRzQT 8SJZjmfIMZ3ha6YrMEpAHHNy1l72uwkY/NmaN4UGjejVBUOFtdmh0XK7Q Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10973"; a="311201" X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="311201" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2024 11:59:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,242,1701158400"; d="scan'208";a="5160739" Received: from dut-2a59.iind.intel.com ([10.190.239.113]) by fmviesa005.fm.intel.com with ESMTP; 04 Feb 2024 11:59:01 -0800 From: Chaitanya Kumar Borah To: intel-gfx@lists.freedesktop.org Cc: uma.shankar@intel.com, chaitanya.kumar.borah@intel.com, maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com Subject: [PATCH 5/5] drm/i915: do not defer cleanup work Date: Mon, 5 Feb 2024 01:22:07 +0530 Message-Id: <20240204195207.3616932-6-chaitanya.kumar.borah@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> References: <20240204195207.3616932-1-chaitanya.kumar.borah@intel.com> MIME-Version: 1.0 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" After we move the cursor fb unpin to a vblank work, we encounter race conditions between the vblank work and the atomic clean up work leading to dump stacks[1]. Let's serialize the clean up to avoid theses races. [1] [ 278.748767] Workqueue: events_highpri intel_atomic_cleanup_work [i915] [ 278.749115] RIP: 0010:intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749425] Code: 92 cb 20 e1 e9 49 ff ff ff 5b 48 89 ef 5d 41 5c e9 11 23 44 e1 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 <38> 96 a5 05 00 00 74 2a 55 48 89 f5 0f b6 f2 53 48 8b bf 40 37 00 [ 278.749428] RSP: 0018:ffffc9000029fdc8 EFLAGS: 00010246 [ 278.749433] RAX: 0000000000000060 RBX: 0000000000000000 RCX: 0000000000000000 [ 278.749435] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff888124d70000 [ 278.749438] RBP: ffff88810394c000 R08: 0000000000000000 R09: ffffc9000029fc80 [ 278.749441] R10: 0000000000f6d950 R11: 0000000000f6da18 R12: ffff888124d70000 [ 278.749443] R13: ffff88814c952000 R14: ffff8881000aac05 R15: ffff8881059baf10 [ 278.749446] FS: 0000000000000000(0000) GS:ffff88817bd80000(0000) knlGS:0000000000000000 [ 278.749449] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 278.749452] CR2: 00000000000005a5 CR3: 0000000104078000 CR4: 0000000000350ef0 [ 278.749454] Call Trace: [ 278.749458] [ 278.749461] ? __die_body+0x1a/0x60 [ 278.749469] ? page_fault_oops+0x156/0x450 [ 278.749474] ? do_user_addr_fault+0x65/0x9e0 [ 278.749479] ? exc_page_fault+0x68/0x1a0 [ 278.749486] ? asm_exc_page_fault+0x26/0x30 [ 278.749494] ? intel_display_rps_mark_interactive+0x4/0x40 [i915] [ 278.749802] intel_cleanup_plane_fb+0x6f/0xc0 [i915] [ 278.750114] drm_atomic_helper_cleanup_planes+0x42/0x60 [ 278.750122] intel_atomic_cleanup_work+0x70/0xc0 [i915] [ 278.750433] ? process_scheduled_works+0x264/0x530 [ 278.750438] process_scheduled_works+0x2db/0x530 [ 278.750444] ? __pfx_worker_thread+0x10/0x10 [ 278.750448] worker_thread+0x18c/0x350 [ 278.750452] ? __pfx_worker_thread+0x10/0x10 [ 278.750455] kthread+0xfe/0x130 [ 278.750460] ? __pfx_kthread+0x10/0x10 [ 278.750464] ret_from_fork+0x2c/0x50 [ 278.750468] ? __pfx_kthread+0x10/0x10 [ 278.750472] ret_from_fork_asm+0x1b/0x30 Signed-off-by: Chaitanya Kumar Borah --- drivers/gpu/drm/i915/display/intel_display.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bf684c4d1732..b0e89036508e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7006,10 +7006,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } } -static void intel_atomic_cleanup_work(struct work_struct *work) +static void intel_atomic_cleanup_work(struct intel_atomic_state *state) { - struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_crtc_state *old_crtc_state; struct intel_crtc *crtc; @@ -7283,8 +7281,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + + intel_atomic_cleanup_work(state); } static void intel_atomic_commit_work(struct work_struct *work)