| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ceef94d5-7033-49d2-9fe6-e9fcb67eaf91@suse.de>
Date: Wed, 4 Jun 2025 10:21:49 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Aradhya Bhatia <aradhya.bhatia@...ux.dev>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>
Cc: Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
Devarsh Thakkar <devarsht@...com>, Praneeth Bajjuri <praneeth@...com>,
Udit Kumar <u-kumar1@...com>, Jayesh Choudhary <j-choudhary@...com>,
Alexander Sverdlin <alexander.sverdlin@...mens.com>,
DRI Development List <dri-devel@...ts.freedesktop.org>,
Linux Kernel List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v12 1/5] drm/atomic-helper: Refactor crtc & encoder-bridge
op loops into separate functions
Am 06.04.25 um 15:16 schrieb Aradhya Bhatia:
> From: Aradhya Bhatia <a-bhatia1@...com>
>
> 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 <tomi.valkeinen@...asonboard.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Tested-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> Signed-off-by: Aradhya Bhatia <a-bhatia1@...com>
> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@...ux.dev>
Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
> ---
> 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_atomic_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,
> }
>
> static void
> -disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
> +encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)
> {
> 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;
>
> @@ -1227,6 +1226,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
>
> 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;
>
> 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_atomic_state *state)
> }
> }
>
> +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 state
> * @dev: DRM device
> @@ -1483,28 +1498,12 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
> }
> }
>
> -/**
> - * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
> - * @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 function
> - * does. But drivers with different needs can group the modeset commits together
> - * and do the plane commits at the end. This is useful for drivers doing runtime
> - * PM since planes updates then only happen when the CRTC is actually enabled.
> - */
> -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;
>
> 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 *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) {
> const struct drm_encoder_helper_funcs *funcs;
> @@ -1565,6 +1572,28 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>
> drm_atomic_bridge_chain_enable(bridge, state);
> }
> +}
> +
> +/**
> + * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
> + * @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 function
> + * does. But drivers with different needs can group the modeset commits together
> + * and do the plane commits at the end. This is useful for drivers doing runtime
> + * PM since planes updates then only happen when the CRTC is actually enabled.
> + */
> +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);
>
> drm_atomic_helper_commit_writebacks(dev, state);
> }
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Powered by blists - more mailing lists