Received-SPF: pass (google.com: domain of linux-kernel+bounces-29700-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240118015916.2296741-1-hsinyi@chromium.org>
In-Reply-To: <20240118015916.2296741-1-hsinyi@chromium.org>
From: Pin-yen Lin <treapking@chromium.org>
Date: Thu, 18 Jan 2024 12:05:21 +0800
Message-ID: <CAEXTbpc2JUh8-_4sqJEGYXH-4QCuTJgRDviPVwv=3Kz_tfxjow@mail.gmail.com>
Subject: Re: [PATCH] drm/bridge: anx7625: Ensure bridge is suspended in disable()
To: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>, dri-devel@lists.freedesktop.org, 
	Andrzej Hajda <andrzej.hajda@intel.com>, Neil Armstrong <neil.armstrong@linaro.org>, 
	Robert Foss <rfoss@kernel.org>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, 
	Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@gmail.com>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, 
	Xin Ji <xji@analogixsemi.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Hsin-Yi,

On Thu, Jan 18, 2024 at 9:59=E2=80=AFAM Hsin-Yi Wang <hsinyi@chromium.org> =
wrote:
>
> Similar to commit 26db46bc9c67 ("drm/bridge: parade-ps8640: Ensure bridge
> is suspended in .post_disable()"). Add a mutex to ensure that aux transfe=
r
> won't race with atomic_disable by holding the PM reference and prevent
> the bridge from suspend.
>
> Also we need to use pm_runtime_put_sync_suspend() to suspend the bridge
> instead of idle with pm_runtime_put_sync().
>
> Fixes: 3203e497eb76 ("drm/bridge: anx7625: Synchronously run runtime susp=
end.")
> Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through a=
ux channel")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Tested-by: Xuxin Xiong <xuxinxiong@huaqin.corp-partner.google.com>
> ---
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 7 ++++++-
>  drivers/gpu/drm/bridge/analogix/anx7625.h | 2 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/=
bridge/analogix/anx7625.c
> index ef31033439bc..29d91493b101 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1762,6 +1762,7 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_a=
ux *aux,
>         u8 request =3D msg->request & ~DP_AUX_I2C_MOT;
>         int ret =3D 0;
>
> +       mutex_lock(&ctx->aux_lock);
>         pm_runtime_get_sync(dev);
>         msg->reply =3D 0;
>         switch (request) {
> @@ -1778,6 +1779,7 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_a=
ux *aux,
>                                         msg->size, msg->buffer);
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_put_autosuspend(dev);
> +       mutex_unlock(&ctx->aux_lock);
>
>         return ret;
>  }
> @@ -2474,7 +2476,9 @@ static void anx7625_bridge_atomic_disable(struct dr=
m_bridge *bridge,
>         ctx->connector =3D NULL;
>         anx7625_dp_stop(ctx);
>
> -       pm_runtime_put_sync(dev);
> +       mutex_lock(&ctx->aux_lock);
> +       pm_runtime_put_sync_suspend(dev);
> +       mutex_unlock(&ctx->aux_lock);
>  }
>
>  static enum drm_connector_status
> @@ -2668,6 +2672,7 @@ static int anx7625_i2c_probe(struct i2c_client *cli=
ent)
>
>         mutex_init(&platform->lock);
>         mutex_init(&platform->hdcp_wq_lock);
> +       mutex_init(&platform->aux_lock);
>
>         INIT_DELAYED_WORK(&platform->hdcp_work, hdcp_check_work_func);
>         platform->hdcp_workqueue =3D create_workqueue("hdcp workqueue");
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/=
bridge/analogix/anx7625.h
> index 66ebee7f3d83..39ed35d33836 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -475,6 +475,8 @@ struct anx7625_data {
>         struct workqueue_struct *hdcp_workqueue;
>         /* Lock for hdcp work queue */
>         struct mutex hdcp_wq_lock;
> +       /* Lock for aux transfer and disable */
> +       struct mutex aux_lock;
>         char edid_block;
>         struct display_timing dt;
>         u8 display_timing_valid;
> --
> 2.43.0.381.gb435a96ce8-goog
>

Reviewed-by: Pin-yen Lin <treapking@chromium.org>