On Wed, Sep 19, 2018 at 03:13:12PM -0400, Sean Paul wrote: > On Wed, Sep 19, 2018 at 11:44:14AM -0400, Bruce Wang wrote: > > All checks for _dpu_crtc_power_enable are not true, so the function > > can never return an error code. All calls of the function have also > > been changed so that they don't expect a return value. > > > > Signed-off-by: Bruce Wang <bzwang@xxxxxxxxxxxx> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 ++++-------------------- > > 1 file changed, 5 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 017d16131dac..f0b52281c46f 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -60,37 +60,16 @@ static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) > > return to_dpu_kms(priv->kms); > > } > > > > -static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable) > > +static inline void _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable) > > { > > - struct drm_crtc *crtc; > > - struct msm_drm_private *priv; > > - struct dpu_kms *dpu_kms; > > - > > - if (!dpu_crtc) { > > - DPU_ERROR("invalid dpu crtc\n"); > > - return -EINVAL; > > - } > > - > > - crtc = &dpu_crtc->base; > > - if (!crtc->dev || !crtc->dev->dev_private) { > > - DPU_ERROR("invalid drm device\n"); > > - return -EINVAL; > > - } > > - > > - priv = crtc->dev->dev_private; > > - if (!priv->kms) { > > - DPU_ERROR("invalid kms\n"); > > - return -EINVAL; > > - } > > - > > - dpu_kms = to_dpu_kms(priv->kms); > > + struct drm_crtc *crtc = &dpu_crtc->base; > > + struct msm_drm_private *priv = crtc->dev->dev_private; > > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > > > > if (enable) > > pm_runtime_get_sync(&dpu_kms->pdev->dev); > > else > > pm_runtime_put_sync(&dpu_kms->pdev->dev); > > - > > - return 0; > > } > > > > static void dpu_crtc_destroy(struct drm_crtc *crtc) > > @@ -849,14 +828,10 @@ static int _dpu_crtc_vblank_enable_no_lock( > > I think this can go to void now too since the if (!dpu_crtc) check at the top is > a no-op. > > This is a good thing, too since this function is only called from void functions > with one exception where its return value isn't passed on! In light of patch 5/5, Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > > Sean > > > dev = crtc->dev; > > > > if (enable) { > > - int ret; > > - > > /* drop lock since power crtc cb may try to re-acquire lock */ > > mutex_unlock(&dpu_crtc->crtc_lock); > > - ret = _dpu_crtc_power_enable(dpu_crtc, true); > > + _dpu_crtc_power_enable(dpu_crtc, true); > > mutex_lock(&dpu_crtc->crtc_lock); > > - if (ret) > > - return ret; > > > > list_for_each_entry(enc, &dev->mode_config.encoder_list, head) { > > if (enc->crtc != crtc) > > -- > > 2.19.0.397.gdd90340f6a-goog > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS -- Sean Paul, Software Engineer, Google / Chromium OS