From 8c994de768f64b6697798c7b08f25b05485195c9 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 30 Aug 2021 15:30:41 +0200 Subject: [PATCH 588/634] drm/vc4: Wait for the commit before starting our clock request Several DRM/KMS atomic commits can run in parallel if they affect different CRTC. These commits share the global HVS state, so we have some code to make sure we run commits in sequence. This synchronization code is one of the first thing that runs in vc4_atomic_commit_tail(). Another constraints we have is that we need to make sure the HVS clock gets a boost during the commit. That code relies on clk_requests and will remove the old requests and start a new one. We also need another, temporary, request for the duration of the commit. The algorithm is thus to start a temporary request, drop the previous one, do the commit, start a new request for the current mode, and finally drop the temporary request. However, the part that takes a temporary request and drops the older one runs before the commit synchronization code. Thus, under the proper conditions, we can end up dropping twice the old request, resulting in an use-after-free. To avoid it, let's move the clock setup in the protected section. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_kms.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index cc905b8c3282..8c63b962b2e6 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -369,20 +369,6 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state) vc4_hvs_mask_underrun(dev, vc4_crtc_state->assigned_channel); } - if (vc4->hvs && vc4->hvs->hvs5) { - unsigned long core_rate = max_t(unsigned long, - 500000000, - new_hvs_state->core_clock_rate); - - core_req = clk_request_start(hvs->core_clk, core_rate); - - /* - * And remove the previous one based on the HVS - * requirements if any. - */ - clk_request_done(hvs->core_req); - } - for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(old_crtc_state); @@ -407,6 +393,26 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state) drm_crtc_commit_put(commit); } + if (vc4->hvs && vc4->hvs->hvs5) { + unsigned long core_rate = max_t(unsigned long, + 500000000, + new_hvs_state->core_clock_rate); + + drm_dbg(dev, "Raising the core clock at %lu Hz\n", core_rate); + + /* + * Do a temporary request on the core clock during the + * modeset. + */ + core_req = clk_request_start(hvs->core_clk, core_rate); + + /* + * And remove the previous one based on the HVS + * requirements if any. + */ + clk_request_done(hvs->core_req); + } + drm_atomic_helper_commit_modeset_disables(dev, state); vc4_ctm_commit(vc4, state); -- 2.33.1