[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPY8ntAALKiTEG6qyFO=qVqSLUW9x8AMfPMc_TUwC3z8tJ7Kzw@mail.gmail.com>
Date: Tue, 5 Dec 2023 15:39:00 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: linux-kernel@...r.kernel.org,
Amarula patchwork <linux-amarula@...rulasolutions.com>,
michael@...rulasolutions.com,
Andrzej Hajda <andrzej.hajda@...el.com>,
Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...il.com>,
Frieder Schrempf <frieder.schrempf@...tron.de>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Jonas Karlman <jonas@...boo.se>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
Hi Dario
On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
<dario.binacchi@...rulasolutions.com> wrote:
>
> The patch fixes the code for finding the next bridge with the
> "pre_enable_prev_first" flag set to false. In case this condition is
> not verified, i. e. there is no subsequent bridge with the flag set to
> false, the whole bridge list is traversed, invalidating the "next"
> variable.
>
> The use of a new iteration variable (i. e. "iter") ensures that the value
> of the "next" variable is not invalidated.
We already have https://patchwork.freedesktop.org/patch/529288/ that
has been reviewed (but not applied) to resolve this. What does this
version do differently and why?
Dave
> Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order")
> Co-developed-by: Michael Trimarchi <michael@...rulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@...rulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/drm_bridge.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f66bf4925dd8..2e5781bf192e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -662,7 +662,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
> struct drm_atomic_state *old_state)
> {
> struct drm_encoder *encoder;
> - struct drm_bridge *next, *limit;
> + struct drm_bridge *iter, *next, *limit;
>
> if (!bridge)
> return;
> @@ -680,14 +680,15 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
> * was enabled first, so disabled last
> */
> limit = next;
> + iter = next;
>
> /* Find the next bridge that has NOT requested
> * prev to be enabled first / disabled last
> */
> - list_for_each_entry_from(next, &encoder->bridge_chain,
> + list_for_each_entry_from(iter, &encoder->bridge_chain,
> chain_node) {
> - if (!next->pre_enable_prev_first) {
> - next = list_prev_entry(next, chain_node);
> + if (!iter->pre_enable_prev_first) {
> + next = list_prev_entry(iter, chain_node);
> limit = next;
> break;
> }
> --
> 2.43.0
>
Powered by blists - more mailing lists