[Freedreno] [PATCH v3 2/2] drm/msm/mdp5: Return error code in mdp5_mixer_release when deadlock is detected

[Rob Clark]

On Thu, May 5, 2022 at 2:41 PM Jessica Zhang <quic_jesszhan at quicinc.com> wrote:
>
> There is a possibility for mdp5_get_global_state to return
> -EDEADLK when acquiring the modeset lock, but currently global_state in
> mdp5_mixer_release doesn't check for if an error is returned.
>
> To avoid a NULL dereference error, let's have mdp5_mixer_release
> check if an error is returned and propagate that error.
>
> Reported-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>

Fixes: 7907a0d77cb4 ("drm/msm/mdp5: Use the new private_obj state")
Reviewed-by: Rob Clark <robdclark at gmail.com>

> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c  | 10 ++++++++--
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 15 +++++++++++----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.h |  4 ++--
>  3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b966cd69f99d..fe2922c8d21b 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -612,9 +612,15 @@ static int mdp5_crtc_setup_pipeline(struct drm_crtc *crtc,
>                 if (ret)
>                         return ret;
>
> -               mdp5_mixer_release(new_crtc_state->state, old_mixer);
> +               ret = mdp5_mixer_release(new_crtc_state->state, old_mixer);
> +               if (ret)
> +                       return ret;
> +
>                 if (old_r_mixer) {
> -                       mdp5_mixer_release(new_crtc_state->state, old_r_mixer);
> +                       ret = mdp5_mixer_release(new_crtc_state->state, old_r_mixer);
> +                       if (ret)
> +                               return ret;
> +
>                         if (!need_right_mixer)
>                                 pipeline->r_mixer = NULL;
>                 }
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> index 954db683ae44..2536def2a000 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> @@ -116,21 +116,28 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
>         return 0;
>  }
>
> -void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
> +int mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
>  {
>         struct mdp5_global_state *global_state = mdp5_get_global_state(s);
> -       struct mdp5_hw_mixer_state *new_state = &global_state->hwmixer;
> +       struct mdp5_hw_mixer_state *new_state;
>
>         if (!mixer)
> -               return;
> +               return 0;
> +
> +       if (IS_ERR(global_state))
> +               return PTR_ERR(global_state);
> +
> +       new_state = &global_state->hwmixer;
>
>         if (WARN_ON(!new_state->hwmixer_to_crtc[mixer->idx]))
> -               return;
> +               return -EINVAL;
>
>         DBG("%s: release from crtc %s", mixer->name,
>             new_state->hwmixer_to_crtc[mixer->idx]->name);
>
>         new_state->hwmixer_to_crtc[mixer->idx] = NULL;
> +
> +       return 0;
>  }
>
>  void mdp5_mixer_destroy(struct mdp5_hw_mixer *mixer)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.h
> index 43c9ba43ce18..545ee223b9d7 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.h
> @@ -30,7 +30,7 @@ void mdp5_mixer_destroy(struct mdp5_hw_mixer *lm);
>  int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
>                       uint32_t caps, struct mdp5_hw_mixer **mixer,
>                       struct mdp5_hw_mixer **r_mixer);
> -void mdp5_mixer_release(struct drm_atomic_state *s,
> -                       struct mdp5_hw_mixer *mixer);
> +int mdp5_mixer_release(struct drm_atomic_state *s,
> +                      struct mdp5_hw_mixer *mixer);
>
>  #endif /* __MDP5_LM_H__ */
> --
> 2.35.1
>