Message ID | 20180920164924.225847-4-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 Thu, Sep 20, 2018 at 12:49:20PM -0400, Bruce Wang wrote: > Removes impossible checks in dpu_crtc.c. > Variable assignments are moved up to be initializations where > possible. Some variables are no longer used, these are removed. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 170 +++-------------------- > 1 file changed, 23 insertions(+), 147 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index a8f2dd7a37c7..a9adb16eac6f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -55,19 +55,8 @@ static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state *cstate, > > static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) > { > - struct msm_drm_private *priv; > - > - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { > - DPU_ERROR("invalid crtc\n"); > - return NULL; > - } > - priv = crtc->dev->dev_private; > - if (!priv || !priv->kms) { > - DPU_ERROR("invalid kms\n"); > - return NULL; > - } > - > - return to_dpu_kms(priv->kms); > + return to_dpu_kms(((struct msm_drm_private *) crtc->dev->dev_private) > + ->kms); Hmm, I didn't realize the cast was needed in order to inline everything. In light of that, I'd prefer going back to your v1. With that fixed, Reviewed-by: Sean Paul <seanpaul@chromium.org> > } > > static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable) > @@ -177,28 +166,17 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > struct drm_plane *plane; > struct drm_framebuffer *fb; > struct drm_plane_state *state; > - struct dpu_crtc_state *cstate; > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); > struct dpu_plane_state *pstate = NULL; > struct dpu_format *format; > - struct dpu_hw_ctl *ctl; > - struct dpu_hw_mixer *lm; > - struct dpu_hw_stage_cfg *stage_cfg; > + struct dpu_hw_ctl *ctl = mixer->lm_ctl; > + struct dpu_hw_stage_cfg *stage_cfg = &dpu_crtc->stage_cfg; > > u32 flush_mask; > uint32_t stage_idx, lm_idx; > int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 }; > bool bg_alpha_enable = false; > > - if (!dpu_crtc || !mixer) { > - DPU_ERROR("invalid dpu_crtc or mixer\n"); > - return; > - } > - > - ctl = mixer->lm_ctl; > - lm = mixer->hw_lm; > - stage_cfg = &dpu_crtc->stage_cfg; > - cstate = to_dpu_crtc_state(crtc->state); > - > drm_atomic_crtc_for_each_plane(plane, crtc) { > state = plane->state; > if (!state) > @@ -217,10 +195,6 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > state->fb ? state->fb->base.id : -1); > > format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); > - if (!format) { > - DPU_ERROR("invalid format\n"); > - return; > - } > > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > bg_alpha_enable = true; > @@ -261,21 +235,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > */ > static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) > { > - struct dpu_crtc *dpu_crtc; > - struct dpu_crtc_state *cstate; > - struct dpu_crtc_mixer *mixer; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); > + struct dpu_crtc_mixer *mixer = cstate->mixers; > struct dpu_hw_ctl *ctl; > struct dpu_hw_mixer *lm; > - > int i; > > - if (!crtc) > - return; > - > - dpu_crtc = to_dpu_crtc(crtc); > - cstate = to_dpu_crtc_state(crtc->state); > - mixer = cstate->mixers; > - > DPU_DEBUG("%s\n", dpu_crtc->name); > > for (i = 0; i < cstate->num_mixers; i++) { > @@ -377,34 +343,13 @@ static void dpu_crtc_vblank_cb(void *data) > > static void dpu_crtc_frame_event_work(struct kthread_work *work) > { > - struct msm_drm_private *priv; > - struct dpu_crtc_frame_event *fevent; > - struct drm_crtc *crtc; > - struct dpu_crtc *dpu_crtc; > - struct dpu_kms *dpu_kms; > + struct dpu_crtc_frame_event *fevent = container_of(work, > + struct dpu_crtc_frame_event, work); > + struct drm_crtc *crtc = fevent->crtc; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > unsigned long flags; > bool frame_done = false; > > - if (!work) { > - DPU_ERROR("invalid work handle\n"); > - return; > - } > - > - fevent = container_of(work, struct dpu_crtc_frame_event, work); > - if (!fevent->crtc || !fevent->crtc->state) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > - > - crtc = fevent->crtc; > - dpu_crtc = to_dpu_crtc(crtc); > - > - dpu_kms = _dpu_crtc_get_kms(crtc); > - if (!dpu_kms) { > - DPU_ERROR("invalid kms handle\n"); > - return; > - } > - priv = dpu_kms->dev->dev_private; > DPU_ATRACE_BEGIN("crtc_frame_event"); > > DRM_DEBUG_KMS("crtc%d event:%u ts:%lld\n", crtc->base.id, fevent->event, > @@ -470,11 +415,6 @@ static void dpu_crtc_frame_event_cb(void *data, u32 event) > unsigned long flags; > u32 crtc_id; > > - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { > - DPU_ERROR("invalid parameters\n"); > - return; > - } > - > /* Nothing to do on idle event */ > if (event & DPU_ENCODER_FRAME_EVENT_IDLE) > return; > @@ -583,23 +523,12 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc) > static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc, > struct drm_crtc_state *state) > { > - struct dpu_crtc *dpu_crtc; > - struct dpu_crtc_state *cstate; > - struct drm_display_mode *adj_mode; > - u32 crtc_split_width; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > + struct drm_display_mode *adj_mode = &state->adjusted_mode; > + u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode); > int i; > > - if (!crtc || !state) { > - DPU_ERROR("invalid args\n"); > - return; > - } > - > - dpu_crtc = to_dpu_crtc(crtc); > - cstate = to_dpu_crtc_state(state); > - > - adj_mode = &state->adjusted_mode; > - crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode); > - > for (i = 0; i < cstate->num_mixers; i++) { > struct drm_rect *r = &cstate->lm_bounds[i]; > r->x1 = crtc_split_width * i; > @@ -693,11 +622,6 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc, > unsigned long flags; > struct dpu_crtc_state *cstate; > > - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > - > if (!crtc->state->enable) { > DPU_DEBUG("crtc%d -> enable %d, skip atomic_flush\n", > crtc->base.id, crtc->state->enable); > @@ -790,15 +714,9 @@ static void dpu_crtc_destroy_state(struct drm_crtc *crtc, > > static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) > { > - struct dpu_crtc *dpu_crtc; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > int ret, rc = 0; > > - if (!crtc) { > - DPU_ERROR("invalid argument\n"); > - return -EINVAL; > - } > - dpu_crtc = to_dpu_crtc(crtc); > - > if (!atomic_read(&dpu_crtc->frame_pending)) { > DPU_DEBUG("no frames pending\n"); > return 0; > @@ -819,29 +737,12 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > { > struct drm_encoder *encoder; > - struct drm_device *dev; > - struct dpu_crtc *dpu_crtc; > - struct msm_drm_private *priv; > - struct dpu_kms *dpu_kms; > - struct dpu_crtc_state *cstate; > + struct drm_device *dev = crtc->dev; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > + struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); > int ret; > > - if (!crtc) { > - DPU_ERROR("invalid argument\n"); > - return; > - } > - dev = crtc->dev; > - dpu_crtc = to_dpu_crtc(crtc); > - dpu_kms = _dpu_crtc_get_kms(crtc); > - > - if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev_private) { > - DPU_ERROR("invalid argument\n"); > - return; > - } > - > - priv = dpu_kms->dev->dev_private; > - cstate = to_dpu_crtc_state(crtc->state); > - > /* > * If no mixers has been allocated in dpu_crtc_atomic_check(), > * it means we are trying to start a CRTC whose state is disabled: > @@ -969,24 +870,9 @@ static int _dpu_crtc_vblank_enable_no_lock( > */ > static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable) > { > - struct dpu_crtc *dpu_crtc; > - struct msm_drm_private *priv; > - struct dpu_kms *dpu_kms; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > int ret = 0; > > - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > - dpu_crtc = to_dpu_crtc(crtc); > - priv = crtc->dev->dev_private; > - > - if (!priv->kms) { > - DPU_ERROR("invalid crtc kms\n"); > - return; > - } > - dpu_kms = to_dpu_kms(priv->kms); > - > DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable); > > mutex_lock(&dpu_crtc->crtc_lock); > @@ -1079,16 +965,8 @@ static void dpu_crtc_reset(struct drm_crtc *crtc) > static void dpu_crtc_handle_power_event(u32 event_type, void *arg) > { > struct drm_crtc *crtc = arg; > - struct dpu_crtc *dpu_crtc; > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > struct drm_encoder *encoder; > - struct dpu_crtc_state *cstate; > - > - if (!crtc) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > - dpu_crtc = to_dpu_crtc(crtc); > - cstate = to_dpu_crtc_state(dpu_crtc->base.state); > > mutex_lock(&dpu_crtc->crtc_lock); > > @@ -1673,8 +1551,6 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc) > dpu_crtc = to_dpu_crtc(crtc); > > dpu_kms = _dpu_crtc_get_kms(crtc); > - if (!dpu_kms) > - return -EINVAL; > > dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name, > crtc->dev->primary->debugfs_root); > -- > 2.19.0.444.g18242da7ef-goog > > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index a8f2dd7a37c7..a9adb16eac6f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -55,19 +55,8 @@ static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state *cstate, static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) { - struct msm_drm_private *priv; - - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { - DPU_ERROR("invalid crtc\n"); - return NULL; - } - priv = crtc->dev->dev_private; - if (!priv || !priv->kms) { - DPU_ERROR("invalid kms\n"); - return NULL; - } - - return to_dpu_kms(priv->kms); + return to_dpu_kms(((struct msm_drm_private *) crtc->dev->dev_private) + ->kms); } static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable) @@ -177,28 +166,17 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, struct drm_plane *plane; struct drm_framebuffer *fb; struct drm_plane_state *state; - struct dpu_crtc_state *cstate; + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); struct dpu_plane_state *pstate = NULL; struct dpu_format *format; - struct dpu_hw_ctl *ctl; - struct dpu_hw_mixer *lm; - struct dpu_hw_stage_cfg *stage_cfg; + struct dpu_hw_ctl *ctl = mixer->lm_ctl; + struct dpu_hw_stage_cfg *stage_cfg = &dpu_crtc->stage_cfg; u32 flush_mask; uint32_t stage_idx, lm_idx; int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 }; bool bg_alpha_enable = false; - if (!dpu_crtc || !mixer) { - DPU_ERROR("invalid dpu_crtc or mixer\n"); - return; - } - - ctl = mixer->lm_ctl; - lm = mixer->hw_lm; - stage_cfg = &dpu_crtc->stage_cfg; - cstate = to_dpu_crtc_state(crtc->state); - drm_atomic_crtc_for_each_plane(plane, crtc) { state = plane->state; if (!state) @@ -217,10 +195,6 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, state->fb ? state->fb->base.id : -1); format = to_dpu_format(msm_framebuffer_format(pstate->base.fb)); - if (!format) { - DPU_ERROR("invalid format\n"); - return; - } if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; @@ -261,21 +235,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, */ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) { - struct dpu_crtc *dpu_crtc; - struct dpu_crtc_state *cstate; - struct dpu_crtc_mixer *mixer; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); + struct dpu_crtc_mixer *mixer = cstate->mixers; struct dpu_hw_ctl *ctl; struct dpu_hw_mixer *lm; - int i; - if (!crtc) - return; - - dpu_crtc = to_dpu_crtc(crtc); - cstate = to_dpu_crtc_state(crtc->state); - mixer = cstate->mixers; - DPU_DEBUG("%s\n", dpu_crtc->name); for (i = 0; i < cstate->num_mixers; i++) { @@ -377,34 +343,13 @@ static void dpu_crtc_vblank_cb(void *data) static void dpu_crtc_frame_event_work(struct kthread_work *work) { - struct msm_drm_private *priv; - struct dpu_crtc_frame_event *fevent; - struct drm_crtc *crtc; - struct dpu_crtc *dpu_crtc; - struct dpu_kms *dpu_kms; + struct dpu_crtc_frame_event *fevent = container_of(work, + struct dpu_crtc_frame_event, work); + struct drm_crtc *crtc = fevent->crtc; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); unsigned long flags; bool frame_done = false; - if (!work) { - DPU_ERROR("invalid work handle\n"); - return; - } - - fevent = container_of(work, struct dpu_crtc_frame_event, work); - if (!fevent->crtc || !fevent->crtc->state) { - DPU_ERROR("invalid crtc\n"); - return; - } - - crtc = fevent->crtc; - dpu_crtc = to_dpu_crtc(crtc); - - dpu_kms = _dpu_crtc_get_kms(crtc); - if (!dpu_kms) { - DPU_ERROR("invalid kms handle\n"); - return; - } - priv = dpu_kms->dev->dev_private; DPU_ATRACE_BEGIN("crtc_frame_event"); DRM_DEBUG_KMS("crtc%d event:%u ts:%lld\n", crtc->base.id, fevent->event, @@ -470,11 +415,6 @@ static void dpu_crtc_frame_event_cb(void *data, u32 event) unsigned long flags; u32 crtc_id; - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { - DPU_ERROR("invalid parameters\n"); - return; - } - /* Nothing to do on idle event */ if (event & DPU_ENCODER_FRAME_EVENT_IDLE) return; @@ -583,23 +523,12 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc) static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc, struct drm_crtc_state *state) { - struct dpu_crtc *dpu_crtc; - struct dpu_crtc_state *cstate; - struct drm_display_mode *adj_mode; - u32 crtc_split_width; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); + struct drm_display_mode *adj_mode = &state->adjusted_mode; + u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode); int i; - if (!crtc || !state) { - DPU_ERROR("invalid args\n"); - return; - } - - dpu_crtc = to_dpu_crtc(crtc); - cstate = to_dpu_crtc_state(state); - - adj_mode = &state->adjusted_mode; - crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode); - for (i = 0; i < cstate->num_mixers; i++) { struct drm_rect *r = &cstate->lm_bounds[i]; r->x1 = crtc_split_width * i; @@ -693,11 +622,6 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc, unsigned long flags; struct dpu_crtc_state *cstate; - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { - DPU_ERROR("invalid crtc\n"); - return; - } - if (!crtc->state->enable) { DPU_DEBUG("crtc%d -> enable %d, skip atomic_flush\n", crtc->base.id, crtc->state->enable); @@ -790,15 +714,9 @@ static void dpu_crtc_destroy_state(struct drm_crtc *crtc, static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) { - struct dpu_crtc *dpu_crtc; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); int ret, rc = 0; - if (!crtc) { - DPU_ERROR("invalid argument\n"); - return -EINVAL; - } - dpu_crtc = to_dpu_crtc(crtc); - if (!atomic_read(&dpu_crtc->frame_pending)) { DPU_DEBUG("no frames pending\n"); return 0; @@ -819,29 +737,12 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) { struct drm_encoder *encoder; - struct drm_device *dev; - struct dpu_crtc *dpu_crtc; - struct msm_drm_private *priv; - struct dpu_kms *dpu_kms; - struct dpu_crtc_state *cstate; + struct drm_device *dev = crtc->dev; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); + struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); int ret; - if (!crtc) { - DPU_ERROR("invalid argument\n"); - return; - } - dev = crtc->dev; - dpu_crtc = to_dpu_crtc(crtc); - dpu_kms = _dpu_crtc_get_kms(crtc); - - if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev_private) { - DPU_ERROR("invalid argument\n"); - return; - } - - priv = dpu_kms->dev->dev_private; - cstate = to_dpu_crtc_state(crtc->state); - /* * If no mixers has been allocated in dpu_crtc_atomic_check(), * it means we are trying to start a CRTC whose state is disabled: @@ -969,24 +870,9 @@ static int _dpu_crtc_vblank_enable_no_lock( */ static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable) { - struct dpu_crtc *dpu_crtc; - struct msm_drm_private *priv; - struct dpu_kms *dpu_kms; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); int ret = 0; - if (!crtc || !crtc->dev || !crtc->dev->dev_private) { - DPU_ERROR("invalid crtc\n"); - return; - } - dpu_crtc = to_dpu_crtc(crtc); - priv = crtc->dev->dev_private; - - if (!priv->kms) { - DPU_ERROR("invalid crtc kms\n"); - return; - } - dpu_kms = to_dpu_kms(priv->kms); - DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable); mutex_lock(&dpu_crtc->crtc_lock); @@ -1079,16 +965,8 @@ static void dpu_crtc_reset(struct drm_crtc *crtc) static void dpu_crtc_handle_power_event(u32 event_type, void *arg) { struct drm_crtc *crtc = arg; - struct dpu_crtc *dpu_crtc; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); struct drm_encoder *encoder; - struct dpu_crtc_state *cstate; - - if (!crtc) { - DPU_ERROR("invalid crtc\n"); - return; - } - dpu_crtc = to_dpu_crtc(crtc); - cstate = to_dpu_crtc_state(dpu_crtc->base.state); mutex_lock(&dpu_crtc->crtc_lock); @@ -1673,8 +1551,6 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc) dpu_crtc = to_dpu_crtc(crtc); dpu_kms = _dpu_crtc_get_kms(crtc); - if (!dpu_kms) - return -EINVAL; dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name, crtc->dev->primary->debugfs_root);
Removes impossible checks in dpu_crtc.c. Variable assignments are moved up to be initializations where possible. Some variables are no longer used, these are removed. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 170 +++-------------------- 1 file changed, 23 insertions(+), 147 deletions(-)