Re: [PATCH] drm/bridge: anx7625: Ensure bridge is suspended in disable()

[Pin-yen Lin]

Hi Hsin-Yi,

On Thu, Jan 18, 2024 at 9:59 AM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> Similar to commit 26db46bc9c67 ("drm/bridge: parade-ps8640: Ensure bridge
> is suspended in .post_disable()"). Add a mutex to ensure that aux transfer
> 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 suspend.")
> Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel")
> Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> Tested-by: Xuxin Xiong <xuxinxiong@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
>  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_aux *aux,
>         u8 request = msg->request & ~DP_AUX_I2C_MOT;
>         int ret = 0;
>
> +       mutex_lock(&ctx->aux_lock);
>         pm_runtime_get_sync(dev);
>         msg->reply = 0;
>         switch (request) {
> @@ -1778,6 +1779,7 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_aux *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 drm_bridge *bridge,
>         ctx->connector = 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 *client)
>
>         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 = 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@xxxxxxxxxxxx>