| Message ID | 20180919154415.131292-5-bzwang@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | drm/msm/dpu: Clean up dpu code | expand |
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@chromium.org> > --- > 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! 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 >
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@chromium.org> > > --- > > 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@chromium.org> > > 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
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( 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)
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@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 ++++-------------------- 1 file changed, 5 insertions(+), 30 deletions(-)