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

On Thu, May 5, 2022 at 2:41 PM Jessica Zhang quic_jesszhan@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@collabora.com Signed-off-by: Jessica Zhang quic_jesszhan@quicinc.com

Fixes: 7907a0d77cb4 ("drm/msm/mdp5: Use the new private_obj state") Reviewed-by: Rob Clark robdclark@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