From nobody Fri Dec 19 00:03:23 2025 Received: from out-177.mta1.migadu.com (out-177.mta1.migadu.com [95.215.58.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 49195276052 for ; Thu, 5 Jun 2025 17:15:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143750; cv=none; b=MsWloPbnjuXufuAosqWFXMoMENtWzE1S31zHwY5HMKO6eysWlOMrpHx8NVF+D8QvylCQUK/+Os7bBWXzI/N46t9WvzWycVv7jRoQuVus8tdhXtRvBCC7CrwEd8sWpibBhC7I+TUrGCa3Kal+jwSGXN492ZcNjLhWE9yXV/tcJF4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143750; c=relaxed/simple; bh=Lmif4NFDCSu1EvRLZfSV2REMCV0xoUU/SUlegqmnfuw=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=bfarBe8MAT2vJikRtV+NLCKYSwdKEYcmxbxuWsKdH9QJCMuhsWjdui6It4VnbC+3mRgj1ldYnppu43Z7B1IoVIGqNQN2ddiFLFCL+enS0mx/cS9joCBNz1Aj9ITAbiUFAP1RWLuPgU0avLtTSMRj1exbRjvZOHa+nSH8x6++t7o= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=GA+m20Rc; arc=none smtp.client-ip=95.215.58.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="GA+m20Rc" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1749143744; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nkQLXsIMtWJzKMLpD3iULaJ1aOfCE4JOXHQInL7DKXU=; b=GA+m20RcD6EbyWaeXA3BXy2JqliLtMYybtBn2uel5sEAKJA8jlAoQkLaBpFQ6tXK0Gstci FzsfiAapyFYnDeA1u1849z39pICZk/4NSDL61C6Ed0u86zfruB3Te3hrNBBWpL+0T2wiun jjO5/KQWKoNheyNLfjIt6hnqxtU8Phc= From: Aradhya Bhatia To: Tomi Valkeinen , Dmitry Baryshkov , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: DRI Development List , Linux Kernel List , Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Jayesh Choudhary , Aradhya Bhatia , Alexander Sverdlin Subject: [PATCH v13 1/4] drm/atomic-helper: Refactor crtc & encoder-bridge op loops into separate functions Date: Thu, 5 Jun 2025 22:45:21 +0530 Message-Id: <20250605171524.27222-2-aradhya.bhatia@linux.dev> In-Reply-To: <20250605171524.27222-1-aradhya.bhatia@linux.dev> References: <20250605171524.27222-1-aradhya.bhatia@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" From: Aradhya Bhatia The way any singular display pipeline, in need of a modeset, gets enabled is as follows - crtc enable (all) bridge pre-enable encoder enable (all) bridge enable - and the disable sequence is exactly the reverse of this. The crtc operations occur by looping over the old and new crtc states, while the encoder and bridge operations occur together, by looping over the connector states of the display pipelines. Refactor these operations - crtc enable/disable, and encoder & bridge (pre/post) enable/disable - into separate functions each, to make way for the re-ordering of the enable/disable sequences. This patch doesn't alter the sequence of crtc/encoder/bridge operations in any way, but helps to cleanly pave the way for the next two patches, by maintaining logical bisectability. Reviewed-by: Tomi Valkeinen Reviewed-by: Dmitry Baryshkov Reviewed-by: Thomas Zimmermann Tested-by: Tomi Valkeinen Tested-by: Alexander Sverdlin Signed-off-by: Aradhya Bhatia Signed-off-by: Aradhya Bhatia Tested-by: Devarsh Thakkar --- drivers/gpu/drm/drm_atomic_helper.c | 69 ++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atom= ic_helper.c index ee64ca1b1bec..d185486071c5 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1160,11 +1160,10 @@ crtc_needs_disable(struct drm_crtc_state *old_state, } =20 static void -disable_outputs(struct drm_device *dev, struct drm_atomic_state *state) +encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *st= ate) { struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; - struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; =20 @@ -1227,6 +1226,14 @@ disable_outputs(struct drm_device *dev, struct drm_a= tomic_state *state) =20 drm_atomic_bridge_chain_post_disable(bridge, state); } +} + +static void +crtc_disable(struct drm_device *dev, struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + int i; =20 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state= , i) { const struct drm_crtc_helper_funcs *funcs; @@ -1274,6 +1281,14 @@ disable_outputs(struct drm_device *dev, struct drm_a= tomic_state *state) } } =20 +static void +disable_outputs(struct drm_device *dev, struct drm_atomic_state *state) +{ + encoder_bridge_disable(dev, state); + + crtc_disable(dev, state); +} + /** * drm_atomic_helper_update_legacy_modeset_state - update legacy modeset s= tate * @dev: DRM device @@ -1483,28 +1498,12 @@ static void drm_atomic_helper_commit_writebacks(str= uct drm_device *dev, } } =20 -/** - * drm_atomic_helper_commit_modeset_enables - modeset commit to enable out= puts - * @dev: DRM device - * @state: atomic state object being committed - * - * This function enables all the outputs with the new configuration which = had to - * be turned off for the update. - * - * For compatibility with legacy CRTC helpers this should be called after - * drm_atomic_helper_commit_planes(), which is what the default commit fun= ction - * does. But drivers with different needs can group the modeset commits to= gether - * and do the plane commits at the end. This is useful for drivers doing r= untime - * PM since planes updates then only happen when the CRTC is actually enab= led. - */ -void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, - struct drm_atomic_state *state) +static void +crtc_enable(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; struct drm_crtc_state *new_crtc_state; - struct drm_connector *connector; - struct drm_connector_state *new_conn_state; int i; =20 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state= , i) { @@ -1528,6 +1527,14 @@ void drm_atomic_helper_commit_modeset_enables(struct= drm_device *dev, funcs->commit(crtc); } } +} + +static void +encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *sta= te) +{ + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; + int i; =20 for_each_new_connector_in_state(state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; @@ -1565,6 +1572,28 @@ void drm_atomic_helper_commit_modeset_enables(struct= drm_device *dev, =20 drm_atomic_bridge_chain_enable(bridge, state); } +} + +/** + * drm_atomic_helper_commit_modeset_enables - modeset commit to enable out= puts + * @dev: DRM device + * @state: atomic state object being committed + * + * This function enables all the outputs with the new configuration which = had to + * be turned off for the update. + * + * For compatibility with legacy CRTC helpers this should be called after + * drm_atomic_helper_commit_planes(), which is what the default commit fun= ction + * does. But drivers with different needs can group the modeset commits to= gether + * and do the plane commits at the end. This is useful for drivers doing r= untime + * PM since planes updates then only happen when the CRTC is actually enab= led. + */ +void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, + struct drm_atomic_state *state) +{ + crtc_enable(dev, state); + + encoder_bridge_enable(dev, state); =20 drm_atomic_helper_commit_writebacks(dev, state); } --=20 2.34.1 From nobody Fri Dec 19 00:03:23 2025 Received: from out-189.mta1.migadu.com (out-189.mta1.migadu.com [95.215.58.189]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DAFDF149C7B for ; Thu, 5 Jun 2025 17:15:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.189 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143752; cv=none; b=gz/iXj8tEOnPZHSyAtLx1EIeFgOFkUTuBB9MiCVlmSoXRbxFycNkYbeW2qxs88N6fG6aamZ/jJD5sM8akq4oscFWQVCiCab9Kb2Nip2PqUaiiixwhuA7s4xDMTCaDTziPA6FAEj7CMzfKI/nunuu1ZPTbLZ3GYFhItglRko5S98= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143752; c=relaxed/simple; bh=q0+2m760AEDdzhpiq5ZzK16VK3dhnc51lKmJc09PnTA=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=f+7v3kZUELxlqeJi7+8iUEF18OWohCHnNJQP32v9pqzO4Y80QAsR/MFGeMpnIqCS3CYED+2X5OqBY1IF/oPonaK/3+OYNUjKRH35reO7LoZlc++V4LkRNnnct1liTp4stLV43ycDtExoIYdO0on+HVmFYG8VtGHb+HFDTRvWTw8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=X45UZ0Bm; arc=none smtp.client-ip=95.215.58.189 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="X45UZ0Bm" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1749143748; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=j7rgZ0BS12OWD8vbtUruCamyU9orBoA03q/2SPzJLhM=; b=X45UZ0BmRe7VZbDlv3WIVRoxC7jQFc/PN5tV2juaAGWkElrCOnEOsejSB+xSxKa8jM8sZ6 3X71iIXRJ0ftUTrGskcDLYjNPzx5td2yHmTLv+F6X2J6hzLKi+8Z6FSI1hu+lYh53CVlZG UkxrV+tAufPHG/kyM2TuxHiv0xGZPAo= From: Aradhya Bhatia To: Tomi Valkeinen , Dmitry Baryshkov , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: DRI Development List , Linux Kernel List , Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Jayesh Choudhary , Aradhya Bhatia , Alexander Sverdlin Subject: [PATCH v13 2/4] drm/atomic-helper: Separate out bridge pre_enable/post_disable from enable/disable Date: Thu, 5 Jun 2025 22:45:22 +0530 Message-Id: <20250605171524.27222-3-aradhya.bhatia@linux.dev> In-Reply-To: <20250605171524.27222-1-aradhya.bhatia@linux.dev> References: <20250605171524.27222-1-aradhya.bhatia@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" The encoder-bridge ops occur by looping over the new connector states of the display pipelines. The enable sequence runs as follows - - pre_enable(bridge), - enable(encoder), - enable(bridge), while the disable sequnce runs as follows - - disable(bridge), - disable(encoder), - post_disable(bridge). Separate out the pre_enable(bridge), and the post_disable(bridge) operations into separate functions each. This patch keeps the sequence same for any singular disaplay pipe, but changes the sequence across multiple display pipelines. This patch is meant to be an interim patch, to cleanly pave the way for the sequence re-ordering patch, and maintain bisectability in the process. Reviewed-by: Dmitry Baryshkov Reviewed-by: Jayesh Choudhary Reviewed-by: Tomi Valkeinen Reviewed-by: Thomas Zimmermann Tested-by: Tomi Valkeinen Tested-by: Alexander Sverdlin Signed-off-by: Aradhya Bhatia Tested-by: Devarsh Thakkar --- drivers/gpu/drm/drm_atomic_helper.c | 91 ++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atom= ic_helper.c index d185486071c5..539b7f072c72 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1223,8 +1223,6 @@ encoder_bridge_disable(struct drm_device *dev, struct= drm_atomic_state *state) else if (funcs->dpms) funcs->dpms(encoder, DRM_MODE_DPMS_OFF); } - - drm_atomic_bridge_chain_post_disable(bridge, state); } } =20 @@ -1281,11 +1279,65 @@ crtc_disable(struct drm_device *dev, struct drm_ato= mic_state *state) } } =20 +static void +encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_stat= e *state) +{ + struct drm_connector *connector; + struct drm_connector_state *old_conn_state, *new_conn_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + int i; + + for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_= conn_state, i) { + struct drm_encoder *encoder; + struct drm_bridge *bridge; + + /* + * Shut down everything that's in the changeset and currently + * still on. So need to check the old, saved state. + */ + if (!old_conn_state->crtc) + continue; + + old_crtc_state =3D drm_atomic_get_old_crtc_state(state, old_conn_state->= crtc); + + if (new_conn_state->crtc) + new_crtc_state =3D drm_atomic_get_new_crtc_state(state, + new_conn_state->crtc); + else + new_crtc_state =3D NULL; + + if (!crtc_needs_disable(old_crtc_state, new_crtc_state) || + !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state)) + continue; + + encoder =3D old_conn_state->best_encoder; + + /* + * We shouldn't get this far if we didn't previously have + * an encoder.. but WARN_ON() rather than explode. + */ + if (WARN_ON(!encoder)) + continue; + + drm_dbg_atomic(dev, "post-disabling bridges [ENCODER:%d:%s]\n", + encoder->base.id, encoder->name); + + /* + * Each encoder has at most one connector (since we always steal + * it away), so we won't call disable hooks twice. + */ + bridge =3D drm_bridge_chain_get_first_bridge(encoder); + drm_atomic_bridge_chain_post_disable(bridge, state); + } +} + static void disable_outputs(struct drm_device *dev, struct drm_atomic_state *state) { encoder_bridge_disable(dev, state); =20 + encoder_bridge_post_disable(dev, state); + crtc_disable(dev, state); } =20 @@ -1498,6 +1550,38 @@ static void drm_atomic_helper_commit_writebacks(stru= ct drm_device *dev, } } =20 +static void +encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state = *state) +{ + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; + int i; + + for_each_new_connector_in_state(state, connector, new_conn_state, i) { + struct drm_encoder *encoder; + struct drm_bridge *bridge; + + if (!new_conn_state->best_encoder) + continue; + + if (!new_conn_state->crtc->state->active || + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) + continue; + + encoder =3D new_conn_state->best_encoder; + + drm_dbg_atomic(dev, "pre-enabling bridges [ENCODER:%d:%s]\n", + encoder->base.id, encoder->name); + + /* + * Each encoder has at most one connector (since we always steal + * it away), so we won't call enable hooks twice. + */ + bridge =3D drm_bridge_chain_get_first_bridge(encoder); + drm_atomic_bridge_chain_pre_enable(bridge, state); + } +} + static void crtc_enable(struct drm_device *dev, struct drm_atomic_state *state) { @@ -1559,7 +1643,6 @@ encoder_bridge_enable(struct drm_device *dev, struct = drm_atomic_state *state) * it away), so we won't call enable hooks twice. */ bridge =3D drm_bridge_chain_get_first_bridge(encoder); - drm_atomic_bridge_chain_pre_enable(bridge, state); =20 if (funcs) { if (funcs->atomic_enable) @@ -1593,6 +1676,8 @@ void drm_atomic_helper_commit_modeset_enables(struct = drm_device *dev, { crtc_enable(dev, state); =20 + encoder_bridge_pre_enable(dev, state); + encoder_bridge_enable(dev, state); =20 drm_atomic_helper_commit_writebacks(dev, state); --=20 2.34.1 From nobody Fri Dec 19 00:03:23 2025 Received: from out-173.mta1.migadu.com (out-173.mta1.migadu.com [95.215.58.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 005C1278153 for ; Thu, 5 Jun 2025 17:15:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143759; cv=none; b=sNC9AiLhpLTiuRZf9L4XUKEHE5ozZcIqlingPHp2FscxQ1oNwlD9fkhicpxF0Nsi4yzLaVPyFyUaTfe104mt6+xA7oW+Vyn+rBkJPyal4C1oDuHLaY1TvxAboWt+h1NUPwnRV03xFyqqa3Y148SSkcmmgA0rbLe7wsxMKZJLoUY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143759; c=relaxed/simple; bh=7AU2DpTMKPRxa5aj1whluARpJstGea8LAmvDZ4DS9Es=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=chlSQXFiLytuG7UP9opbYIvOUoJOSh87NtHtoTkk2SfwbYEGbtRVfWj8i+QiwgdxVD4cs2ybDULeYujpbP7qIWNKiBj19TFCMtSjwDVavbxlUTbh4tnZrKAgzj7CG+Z2HIFQYLxoGeOiIS7Khg1a/tK3VOJSJpbLC1uB/jX4yHU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=ksgcCf3l; arc=none smtp.client-ip=95.215.58.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="ksgcCf3l" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1749143753; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OSsoSA/a0rn2E8TexkvkkgZ5Jp5q4Qs8pTgbtALLktQ=; b=ksgcCf3lfy7wnb3bPpDEfXJwoU1VXRbZyJXiIOLvVwaPgS56ixWRzlLC8t1JnheozRmAw1 +3GMG5JrrHHzENUsRB1SzQFReokOPhlHbCSWmFu3mQ79a1ZrwgUFXGGBCFczroc8hmKFUf 7F1HWuUkiCmUvAoAxccwvFmLj1GJyjg= From: Aradhya Bhatia To: Tomi Valkeinen , Dmitry Baryshkov , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: DRI Development List , Linux Kernel List , Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Jayesh Choudhary , Aradhya Bhatia , Alexander Sverdlin Subject: [PATCH v13 3/4] drm/atomic-helper: Re-order bridge chain pre-enable and post-disable Date: Thu, 5 Jun 2025 22:45:23 +0530 Message-Id: <20250605171524.27222-4-aradhya.bhatia@linux.dev> In-Reply-To: <20250605171524.27222-1-aradhya.bhatia@linux.dev> References: <20250605171524.27222-1-aradhya.bhatia@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" From: Aradhya Bhatia Move the bridge pre_enable call before crtc enable, and the bridge post_disable call after the crtc disable. The sequence of enable after this patch will look like: bridge[n]_pre_enable ... bridge[1]_pre_enable crtc_enable encoder_enable bridge[1]_enable ... bridge[n]_enable And, the disable sequence for the display pipeline will look like: bridge[n]_disable ... bridge[1]_disable encoder_disable crtc_disable bridge[1]_post_disable ... bridge[n]_post_disable The definition of bridge pre_enable hook says that, "The display pipe (i.e. clocks and timing signals) feeding this bridge will not yet be running when this callback is called". Since CRTC is also a source feeding the bridge, it should not be enabled before the bridges in the pipeline are pre_enabled. Fix that by re-ordering the sequence of bridge pre_enable and bridge post_disable. While at it, update the drm bridge API documentation as well. Acked-by: Dmitry Baryshkov Reviewed-by: Tomi Valkeinen Reviewed-by: Thomas Zimmermann Tested-by: Tomi Valkeinen Tested-by: Alexander Sverdlin Signed-off-by: Aradhya Bhatia Signed-off-by: Aradhya Bhatia Tested-by: Devarsh Thakkar --- drivers/gpu/drm/drm_atomic_helper.c | 8 +- include/drm/drm_bridge.h | 249 ++++++++++++++++++++-------- 2 files changed, 187 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atom= ic_helper.c index 539b7f072c72..2fe6c91910a1 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1336,9 +1336,9 @@ disable_outputs(struct drm_device *dev, struct drm_at= omic_state *state) { encoder_bridge_disable(dev, state); =20 - encoder_bridge_post_disable(dev, state); - crtc_disable(dev, state); + + encoder_bridge_post_disable(dev, state); } =20 /** @@ -1674,10 +1674,10 @@ encoder_bridge_enable(struct drm_device *dev, struc= t drm_atomic_state *state) void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_atomic_state *state) { - crtc_enable(dev, state); - encoder_bridge_pre_enable(dev, state); =20 + crtc_enable(dev, state); + encoder_bridge_enable(dev, state); =20 drm_atomic_helper_commit_writebacks(dev, state); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 0af5db244db8..ecdeb90e5586 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -165,17 +165,33 @@ struct drm_bridge_funcs { /** * @disable: * - * This callback should disable the bridge. It is called right before - * the preceding element in the display pipe is disabled. If the - * preceding element is a bridge this means it's called before that - * bridge's @disable vfunc. If the preceding element is a &drm_encoder - * it's called right before the &drm_encoder_helper_funcs.disable, - * &drm_encoder_helper_funcs.prepare or &drm_encoder_helper_funcs.dpms - * hook. + * The @disable callback should disable the bridge. * * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is still running when this callback is called. * + * + * If the preceding element is a &drm_bridge, then this is called before + * that bridge is disabled via one of: + * + * - &drm_bridge_funcs.disable + * - &drm_bridge_funcs.atomic_disable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called before the encoder is disabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_disable + * - &drm_encoder_helper_funcs.prepare + * - &drm_encoder_helper_funcs.disable + * - &drm_encoder_helper_funcs.dpms + * + * and the CRTC is disabled via one of: + * + * - &drm_crtc_helper_funcs.prepare + * - &drm_crtc_helper_funcs.atomic_disable + * - &drm_crtc_helper_funcs.disable + * - &drm_crtc_helper_funcs.dpms. + * * The @disable callback is optional. * * NOTE: @@ -188,17 +204,34 @@ struct drm_bridge_funcs { /** * @post_disable: * - * This callback should disable the bridge. It is called right after the - * preceding element in the display pipe is disabled. If the preceding - * element is a bridge this means it's called after that bridge's - * @post_disable function. If the preceding element is a &drm_encoder - * it's called right after the encoder's - * &drm_encoder_helper_funcs.disable, &drm_encoder_helper_funcs.prepare - * or &drm_encoder_helper_funcs.dpms hook. - * * The bridge must assume that the display pipe (i.e. clocks and timing - * signals) feeding it is no longer running when this callback is - * called. + * signals) feeding this bridge is no longer running when the + * @post_disable is called. + * + * This callback should perform all the actions required by the hardware + * after it has stopped receiving signals from the preceding element. + * + * If the preceding element is a &drm_bridge, then this is called after + * that bridge is post-disabled (unless marked otherwise by the + * @pre_enable_prev_first flag) via one of: + * + * - &drm_bridge_funcs.post_disable + * - &drm_bridge_funcs.atomic_post_disable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called after the encoder is disabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_disable + * - &drm_encoder_helper_funcs.prepare + * - &drm_encoder_helper_funcs.disable + * - &drm_encoder_helper_funcs.dpms + * + * and the CRTC is disabled via one of: + * + * - &drm_crtc_helper_funcs.prepare + * - &drm_crtc_helper_funcs.atomic_disable + * - &drm_crtc_helper_funcs.disable + * - &drm_crtc_helper_funcs.dpms * * The @post_disable callback is optional. * @@ -241,18 +274,30 @@ struct drm_bridge_funcs { /** * @pre_enable: * - * This callback should enable the bridge. It is called right before - * the preceding element in the display pipe is enabled. If the - * preceding element is a bridge this means it's called before that - * bridge's @pre_enable function. If the preceding element is a - * &drm_encoder it's called right before the encoder's - * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or - * &drm_encoder_helper_funcs.dpms hook. - * * The display pipe (i.e. clocks and timing signals) feeding this bridge - * will not yet be running when this callback is called. The bridge must - * not enable the display link feeding the next bridge in the chain (if - * there is one) when this callback is called. + * will not yet be running when the @pre_enable is called. + * + * This callback should perform all the necessary actions to prepare the + * bridge to accept signals from the preceding element. + * + * If the preceding element is a &drm_bridge, then this is called before + * that bridge is pre-enabled (unless marked otherwise by + * @pre_enable_prev_first flag) via one of: + * + * - &drm_bridge_funcs.pre_enable + * - &drm_bridge_funcs.atomic_pre_enable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called before the CRTC is enabled via one of: + * + * - &drm_crtc_helper_funcs.atomic_enable + * - &drm_crtc_helper_funcs.commit + * + * and the encoder is enabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_enable + * - &drm_encoder_helper_funcs.enable + * - &drm_encoder_helper_funcs.commit * * The @pre_enable callback is optional. * @@ -266,19 +311,31 @@ struct drm_bridge_funcs { /** * @enable: * - * This callback should enable the bridge. It is called right after - * the preceding element in the display pipe is enabled. If the - * preceding element is a bridge this means it's called after that - * bridge's @enable function. If the preceding element is a - * &drm_encoder it's called right after the encoder's - * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or - * &drm_encoder_helper_funcs.dpms hook. + * The @enable callback should enable the bridge. * * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is running when this callback is called. This * callback must enable the display link feeding the next bridge in the * chain if there is one. * + * If the preceding element is a &drm_bridge, then this is called after + * that bridge is enabled via one of: + * + * - &drm_bridge_funcs.enable + * - &drm_bridge_funcs.atomic_enable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called after the CRTC is enabled via one of: + * + * - &drm_crtc_helper_funcs.atomic_enable + * - &drm_crtc_helper_funcs.commit + * + * and the encoder is enabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_enable + * - &drm_encoder_helper_funcs.enable + * - drm_encoder_helper_funcs.commit + * * The @enable callback is optional. * * NOTE: @@ -291,17 +348,30 @@ struct drm_bridge_funcs { /** * @atomic_pre_enable: * - * This callback should enable the bridge. It is called right before - * the preceding element in the display pipe is enabled. If the - * preceding element is a bridge this means it's called before that - * bridge's @atomic_pre_enable or @pre_enable function. If the preceding - * element is a &drm_encoder it's called right before the encoder's - * &drm_encoder_helper_funcs.atomic_enable hook. - * * The display pipe (i.e. clocks and timing signals) feeding this bridge - * will not yet be running when this callback is called. The bridge must - * not enable the display link feeding the next bridge in the chain (if - * there is one) when this callback is called. + * will not yet be running when the @atomic_pre_enable is called. + * + * This callback should perform all the necessary actions to prepare the + * bridge to accept signals from the preceding element. + * + * If the preceding element is a &drm_bridge, then this is called before + * that bridge is pre-enabled (unless marked otherwise by + * @pre_enable_prev_first flag) via one of: + * + * - &drm_bridge_funcs.pre_enable + * - &drm_bridge_funcs.atomic_pre_enable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called before the CRTC is enabled via one of: + * + * - &drm_crtc_helper_funcs.atomic_enable + * - &drm_crtc_helper_funcs.commit + * + * and the encoder is enabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_enable + * - &drm_encoder_helper_funcs.enable + * - &drm_encoder_helper_funcs.commit * * The @atomic_pre_enable callback is optional. */ @@ -311,18 +381,31 @@ struct drm_bridge_funcs { /** * @atomic_enable: * - * This callback should enable the bridge. It is called right after - * the preceding element in the display pipe is enabled. If the - * preceding element is a bridge this means it's called after that - * bridge's @atomic_enable or @enable function. If the preceding element - * is a &drm_encoder it's called right after the encoder's - * &drm_encoder_helper_funcs.atomic_enable hook. + * The @atomic_enable callback should enable the bridge. * * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is running when this callback is called. This * callback must enable the display link feeding the next bridge in the * chain if there is one. * + * If the preceding element is a &drm_bridge, then this is called after + * that bridge is enabled via one of: + * + * - &drm_bridge_funcs.enable + * - &drm_bridge_funcs.atomic_enable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called after the CRTC is enabled via one of: + * + * - &drm_crtc_helper_funcs.atomic_enable + * - &drm_crtc_helper_funcs.commit + * + * and the encoder is enabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_enable + * - &drm_encoder_helper_funcs.enable + * - drm_encoder_helper_funcs.commit + * * The @atomic_enable callback is optional. */ void (*atomic_enable)(struct drm_bridge *bridge, @@ -330,16 +413,32 @@ struct drm_bridge_funcs { /** * @atomic_disable: * - * This callback should disable the bridge. It is called right before - * the preceding element in the display pipe is disabled. If the - * preceding element is a bridge this means it's called before that - * bridge's @atomic_disable or @disable vfunc. If the preceding element - * is a &drm_encoder it's called right before the - * &drm_encoder_helper_funcs.atomic_disable hook. + * The @atomic_disable callback should disable the bridge. * * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is still running when this callback is called. * + * If the preceding element is a &drm_bridge, then this is called before + * that bridge is disabled via one of: + * + * - &drm_bridge_funcs.disable + * - &drm_bridge_funcs.atomic_disable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called before the encoder is disabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_disable + * - &drm_encoder_helper_funcs.prepare + * - &drm_encoder_helper_funcs.disable + * - &drm_encoder_helper_funcs.dpms + * + * and the CRTC is disabled via one of: + * + * - &drm_crtc_helper_funcs.prepare + * - &drm_crtc_helper_funcs.atomic_disable + * - &drm_crtc_helper_funcs.disable + * - &drm_crtc_helper_funcs.dpms. + * * The @atomic_disable callback is optional. */ void (*atomic_disable)(struct drm_bridge *bridge, @@ -348,16 +447,34 @@ struct drm_bridge_funcs { /** * @atomic_post_disable: * - * This callback should disable the bridge. It is called right after the - * preceding element in the display pipe is disabled. If the preceding - * element is a bridge this means it's called after that bridge's - * @atomic_post_disable or @post_disable function. If the preceding - * element is a &drm_encoder it's called right after the encoder's - * &drm_encoder_helper_funcs.atomic_disable hook. - * * The bridge must assume that the display pipe (i.e. clocks and timing - * signals) feeding it is no longer running when this callback is - * called. + * signals) feeding this bridge is no longer running when the + * @atomic_post_disable is called. + * + * This callback should perform all the actions required by the hardware + * after it has stopped receiving signals from the preceding element. + * + * If the preceding element is a &drm_bridge, then this is called after + * that bridge is post-disabled (unless marked otherwise by the + * @pre_enable_prev_first flag) via one of: + * + * - &drm_bridge_funcs.post_disable + * - &drm_bridge_funcs.atomic_post_disable + * + * If the preceding element of the bridge is a display controller, then + * this callback is called after the encoder is disabled via one of: + * + * - &drm_encoder_helper_funcs.atomic_disable + * - &drm_encoder_helper_funcs.prepare + * - &drm_encoder_helper_funcs.disable + * - &drm_encoder_helper_funcs.dpms + * + * and the CRTC is disabled via one of: + * + * - &drm_crtc_helper_funcs.prepare + * - &drm_crtc_helper_funcs.atomic_disable + * - &drm_crtc_helper_funcs.disable + * - &drm_crtc_helper_funcs.dpms * * The @atomic_post_disable callback is optional. */ --=20 2.34.1 From nobody Fri Dec 19 00:03:23 2025 Received: from out-189.mta1.migadu.com (out-189.mta1.migadu.com [95.215.58.189]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4D9F727A916 for ; Thu, 5 Jun 2025 17:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.189 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143762; cv=none; b=NgV094mbTWY5ckMYMewopEZF9uxpDFQIxk3AKZCQZ9Ve6OvypSWuzVQQVLMpQk+kc60RGel93ehhednD+ReFekzdKf6Gt4DxZIZHcl3waicQhUnyWwDcetHPWLrU7ovSrnGlEmP8JW77rCLRI7KrqLVxt7CWZqdKJLAGmFFvtVc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749143762; c=relaxed/simple; bh=mOpT+lrla1VIMKYk0PDD8P+0Babfo/nD7siBASuBgtk=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=WI4y7fix8meVj8mL1K2zuXDonzUBXDnZruhT2j57InLlfuwbJEhsOL2wLoZqwrnBtIQMFpvl9CepkVQ8DUzhQUbtSIVT0R5oFpwTk2zPFYzm1rJW5vhpl8u4K3q2w0GzuawH3wLApH6QyTO+o6Sk9cG00JSzC3s+9JEKQwSA7zk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=eyZxkkSl; arc=none smtp.client-ip=95.215.58.189 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="eyZxkkSl" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1749143758; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=M9NCllNipIETn5Ms9WhRG3GuOeYbw61QE2xDj2iUzqI=; b=eyZxkkSlw2Z0ykcnsGYqDRsPgtzUWs6gynk2GHupp3Oy60ueleeEQt7y1YVKTP6S/M1/4U eQvlzRKuvEGallFw8S2I04n5SnxuTvP1AQUF/f/u4THVjCA4EzzU9eeICxAwMRLM9MZoRR ARaZ3CCrcvIt6vWWbnChzFOx/mlhhjI= From: Aradhya Bhatia To: Tomi Valkeinen , Dmitry Baryshkov , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: DRI Development List , Linux Kernel List , Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Jayesh Choudhary , Aradhya Bhatia , Alexander Sverdlin Subject: [PATCH v13 4/4] drm/bridge: cdns-dsi: Use pre_enable/post_disable to enable/disable Date: Thu, 5 Jun 2025 22:45:24 +0530 Message-Id: <20250605171524.27222-5-aradhya.bhatia@linux.dev> In-Reply-To: <20250605171524.27222-1-aradhya.bhatia@linux.dev> References: <20250605171524.27222-1-aradhya.bhatia@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" From: Aradhya Bhatia The cdns-dsi controller requires that it be turned on completely before the input DPI's source has begun streaming[0]. Not having that, allows for a small window before cdns-dsi enable and after cdns-dsi disable where the previous entity (in this case tidss's videoport) to continue streaming DPI video signals. This small window where cdns-dsi is disabled but is still receiving signals causes the input FIFO of cdns-dsi to get corrupted. This causes the colors to shift on the output display. The colors can either shift by one color component (R->G, G->B, B->R), or by two color components (R->B, G->R, B->G). Since tidss's videoport starts streaming via crtc enable hooks, we need cdns-dsi to be up and running before that. Now that the bridges are pre_enabled before crtc is enabled, and post_disabled after crtc is disabled, use the pre_enable and post_disable hooks to get cdns-dsi ready and running before the tidss videoport to get pass the color shift issues. [0]: See section 12.6.5.7.3 "Start-up Procedure" in J721E SoC TRM TRM Link: http://www.ti.com/lit/pdf/spruil1 Reviewed-by: Tomi Valkeinen Tested-by: Tomi Valkeinen Signed-off-by: Aradhya Bhatia Signed-off-by: Aradhya Bhatia Tested-by: Devarsh Thakkar --- .../gpu/drm/bridge/cadence/cdns-dsi-core.c | 64 ++++++++++--------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/d= rm/bridge/cadence/cdns-dsi-core.c index 7604574da666..a57ca8c3bdae 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c @@ -670,13 +670,28 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge, return MODE_OK; } =20 -static void cdns_dsi_bridge_atomic_disable(struct drm_bridge *bridge, - struct drm_atomic_state *state) +static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input =3D bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi =3D input_to_dsi(input); u32 val; =20 + /* + * The cdns-dsi controller needs to be disabled after it's DPI source + * has stopped streaming. If this is not followed, there is a brief + * window before DPI source is disabled and after cdns-dsi controller + * has been disabled where the DPI stream is still on, but the cdns-dsi + * controller is not ready anymore to accept the incoming signals. This + * is one of the reasons why a shift in pixel colors is observed on + * displays that have cdns-dsi as one of the bridges. + * + * To mitigate this, disable this bridge from the bridge post_disable() + * hook, instead of the bridge _disable() hook. The bridge post_disable() + * hook gets called after the CRTC disable, where often many DPI sources + * disable their streams. + */ + val =3D readl(dsi->regs + MCTL_MAIN_DATA_CTL); val &=3D ~(IF_VID_SELECT_MASK | IF_VID_MODE | VID_EN | HOST_EOT_GEN | DISP_EOT_GEN); @@ -688,15 +703,6 @@ static void cdns_dsi_bridge_atomic_disable(struct drm_= bridge *bridge, if (dsi->platform_ops && dsi->platform_ops->disable) dsi->platform_ops->disable(dsi); =20 - pm_runtime_put(dsi->base.dev); -} - -static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge, - struct drm_atomic_state *state) -{ - struct cdns_dsi_input *input =3D bridge_to_cdns_dsi_input(bridge); - struct cdns_dsi *dsi =3D input_to_dsi(input); - dsi->phy_initialized =3D false; dsi->link_initialized =3D false; phy_power_off(dsi->dphy); @@ -774,8 +780,8 @@ static void cdns_dsi_init_link(struct cdns_dsi *dsi) dsi->link_initialized =3D true; } =20 -static void cdns_dsi_bridge_atomic_enable(struct drm_bridge *bridge, - struct drm_atomic_state *state) +static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input =3D bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi =3D input_to_dsi(input); @@ -792,6 +798,21 @@ static void cdns_dsi_bridge_atomic_enable(struct drm_b= ridge *bridge, u32 tmp, reg_wakeup, div, status; int nlanes; =20 + /* + * The cdns-dsi controller needs to be enabled before it's DPI source + * has begun streaming. If this is not followed, there is a brief window + * after DPI source enable and before cdns-dsi controller enable where + * the DPI stream is on, but the cdns-dsi controller is not ready to + * accept the incoming signals. This is one of the reasons why a shift + * in pixel colors is observed on displays that have cdns-dsi as one of + * the bridges. + * + * To mitigate this, enable this bridge from the bridge pre_enable() + * hook, instead of the bridge _enable() hook. The bridge pre_enable() + * hook gets called before the CRTC enable, where often many DPI sources + * enable their streams. + */ + if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0)) return; =20 @@ -811,8 +832,8 @@ static void cdns_dsi_bridge_atomic_enable(struct drm_br= idge *bridge, mode =3D &crtc_state->adjusted_mode; nlanes =3D output->dev->lanes; =20 - cdns_dsi_hs_init(dsi); cdns_dsi_init_link(dsi); + cdns_dsi_hs_init(dsi); =20 /* * Now that the DSI Link and DSI Phy are initialized, @@ -941,19 +962,6 @@ static void cdns_dsi_bridge_atomic_enable(struct drm_b= ridge *bridge, writel(tmp, dsi->regs + MCTL_MAIN_EN); } =20 -static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge, - struct drm_atomic_state *state) -{ - struct cdns_dsi_input *input =3D bridge_to_cdns_dsi_input(bridge); - struct cdns_dsi *dsi =3D input_to_dsi(input); - - if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0)) - return; - - cdns_dsi_init_link(dsi); - cdns_dsi_hs_init(dsi); -} - static u32 *cdns_dsi_bridge_get_input_bus_fmts(struct drm_bridge *bridge, struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state, @@ -1048,9 +1056,7 @@ cdns_dsi_bridge_atomic_reset(struct drm_bridge *bridg= e) static const struct drm_bridge_funcs cdns_dsi_bridge_funcs =3D { .attach =3D cdns_dsi_bridge_attach, .mode_valid =3D cdns_dsi_bridge_mode_valid, - .atomic_disable =3D cdns_dsi_bridge_atomic_disable, .atomic_pre_enable =3D cdns_dsi_bridge_atomic_pre_enable, - .atomic_enable =3D cdns_dsi_bridge_atomic_enable, .atomic_post_disable =3D cdns_dsi_bridge_atomic_post_disable, .atomic_check =3D cdns_dsi_bridge_atomic_check, .atomic_reset =3D cdns_dsi_bridge_atomic_reset, --=20 2.34.1