Re: [Freedreno] [PATCH 3/3] drm/msm: dpu: Don't store/deref pointers in trace ringbuffer
On Wed, Sep 19, 2018 at 01:04:42PM -0700, Abhinav Kumar wrote:
> On 2018-09-19 11:33, Sean Paul wrote:
> > From: Sean Paul
> >
> > TP_printk is not synchronous, so storing pointers and then later
> > derferencing them is a Bad Idea. This patch stores everything locally to
> minor typo "dereferencing",
> > avoid display stomped memory.
> >
> > Signed-off-by: Sean Paul
> After fixing the minor nit please add,
> Reviewed-by: Abhinav Kumar
Thanks for the reviews, Abhinav, I've stuffed the set in dpu-staging/for-next.
Sean
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 98 +--
> > 1 file changed, 55 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > index ae6b0c51ba52..e12c4cefb742 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > @@ -686,37 +686,41 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
> > TP_STRUCT__entry(
> > __field(uint32_t, crtc_id )
> > __field(uint32_t, plane_id)
> > - __field(struct drm_plane_state*,state )
> > - __field(struct dpu_plane_state*,pstate )
> > + __field(uint32_t, fb_id )
> > + __field_struct( struct drm_rect,src_rect)
> > + __field_struct( struct drm_rect,dst_rect)
> > __field(uint32_t, stage_idx )
> > + __field(enum dpu_stage, stage )
> > __field(enum dpu_sspp, sspp)
> > + __field(uint32_t, multirect_idx )
> > + __field(uint32_t, multirect_mode )
> > __field(uint32_t, pixel_format)
> > __field(uint64_t, modifier)
> > ),
> > TP_fast_assign(
> > __entry->crtc_id = crtc_id;
> > __entry->plane_id = plane_id;
> > - __entry->state = state;
> > - __entry->pstate = pstate;
> > + __entry->fb_id = state ? state->fb->base.id : 0;
> > + __entry->src_rect = drm_plane_state_src(state);
> > + __entry->dst_rect = drm_plane_state_dest(state);
> > __entry->stage_idx = stage_idx;
> > + __entry->stage = pstate->stage;
> > __entry->sspp = sspp;
> > + __entry->multirect_idx = pstate->multirect_index;
> > + __entry->multirect_mode = pstate->multirect_mode;
> > __entry->pixel_format = pixel_format;
> > __entry->modifier = modifier;
> > ),
> > - TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:{%ux%u+%ux%u} "
> > - "dst:{%ux%u+%ux%u} stage_idx:%u stage:%d, sspp:%d "
> > + TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT
> > + " dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d "
> > "multirect_index:%d multirect_mode:%u pix_format:%u "
> > "modifier:%llu",
> > - __entry->crtc_id, __entry->plane_id,
> > - __entry->state->fb ? __entry->state->fb->base.id : -1,
> > - __entry->state->src_w >> 16, __entry->state->src_h >> 16,
> > - __entry->state->src_x >> 16, __entry->state->src_y >> 16,
> > - __entry->state->crtc_w, __entry->state->crtc_h,
> > - __entry->state->crtc_x, __entry->state->crtc_y,
> > - __entry->stage_idx, __entry->pstate->stage, __entry->sspp,
> > - __entry->pstate->multirect_index,
> > - __entry->pstate->multirect_mode, __entry->pixel_format,
> > - __entry->modifier)
> > + __entry->crtc_id, __entry->plane_id, __entry->fb_id,
> > + DRM_RECT_FP_ARG(&__entry->src_rect),
> > + DRM_RECT_ARG(&__entry->dst_rect),
> > + __entry->stage_idx, __entry->stage, __entry->sspp,
> > + __entry->multirect_idx, __entry->multirect_mode,
> > + __entry->pixel_format, __entry->modifier)
> > );
> >
> > TRACE_EVENT(dpu_crtc_setup_lm_bounds,
> > @@ -725,15 +729,15 @@ TRACE_EVENT(dpu_crtc_setup_lm_bounds,
> > TP_STRUCT__entry(
> > __field(uint32_t, drm_id )
> > __field(int,mixer )
> > - __field(struct drm_rect *, bounds )
> > + __field_struct( struct drm_rect,bounds )
> > ),
> > TP_fast_assign(
> > __entry->drm_id = drm_id;
> > __entry->mixer = mixer;
> > - __entry->bounds = bounds;
> > + __entry->bounds = *bounds;
> > ),
> > TP_printk("id:%u mixer:%d bounds:" DRM_RECT_FMT, __entry->drm_id,
> > - __entry->mixer,
Re: [Freedreno] [PATCH 2/3] drm/msm: dpu: Add extra_flush_bits to trigger_flush trace
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul
It's useful to know which bits of the flush come from
"extra_flush_bits"
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 +-
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index e56f29190121..8f6880db5c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1444,7 +1444,8 @@ static inline void
_dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
ret = ctl->ops.get_pending_flush(ctl);
trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx,
- pending_kickoff_cnt, ctl->idx, ret);
+ pending_kickoff_cnt, ctl->idx,
+ extra_flush_bits, ret);
}
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 0be51db02f2e..ae6b0c51ba52 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -468,14 +468,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
TRACE_EVENT(dpu_enc_trigger_flush,
TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx,
-int pending_kickoff_cnt, int ctl_idx, u32 pending_flush_ret),
+int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits,
+u32 pending_flush_ret),
TP_ARGS(drm_id, intf_idx, pending_kickoff_cnt, ctl_idx,
- pending_flush_ret),
+ extra_flush_bits, pending_flush_ret),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
__field(enum dpu_intf, intf_idx)
__field(int,pending_kickoff_cnt )
__field(int,ctl_idx )
+ __field(u32,extra_flush_bits)
__field(u32,pending_flush_ret )
),
TP_fast_assign(
@@ -483,12 +485,14 @@ TRACE_EVENT(dpu_enc_trigger_flush,
__entry->intf_idx = intf_idx;
__entry->pending_kickoff_cnt = pending_kickoff_cnt;
__entry->ctl_idx = ctl_idx;
+ __entry->extra_flush_bits = extra_flush_bits;
__entry->pending_flush_ret = pending_flush_ret;
),
TP_printk("id=%u, intf_idx=%d, pending_kickoff_cnt=%d ctl_idx=%d "
- "pending_flush_ret=%u", __entry->drm_id,
- __entry->intf_idx, __entry->pending_kickoff_cnt,
- __entry->ctl_idx, __entry->pending_flush_ret)
+ "extra_flush_bits=0x%x pending_flush_ret=0x%x",
+ __entry->drm_id, __entry->intf_idx,
+ __entry->pending_kickoff_cnt, __entry->ctl_idx,
+ __entry->extra_flush_bits, __entry->pending_flush_ret)
);
DECLARE_EVENT_CLASS(dpu_enc_ktime_template,
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/3] drm/msm: dpu: Clear frame_busy_mask bit after trace
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul
We're printing the frame_busy_mask in a trace, but after it's been
cleared. This, as it turns out, is pretty pointless.
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index c2e8985b4c54..e56f29190121 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1361,9 +1361,9 @@ static void dpu_encoder_frame_done_callback(
/* One of the physical encoders has become idle */
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
if (dpu_enc->phys_encs[i] == ready_phys) {
- clear_bit(i, dpu_enc->frame_busy_mask);
trace_dpu_enc_frame_done_cb(DRMID(drm_enc), i,
dpu_enc->frame_busy_mask[0]);
+ clear_bit(i, dpu_enc->frame_busy_mask);
}
}
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
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
> > ---
> > 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 = _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 = _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(_kms->pdev->dev);
> > else
> > pm_runtime_put_sync(_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
>
> Sean
>
> > dev = crtc->dev;
> >
> > if (enable) {
> > - int ret;
> > -
> > /* drop lock since power crtc cb may try to re-acquire lock */
> > mutex_unlock(_crtc->crtc_lock);
> > - ret = _dpu_crtc_power_enable(dpu_crtc, true);
> > + _dpu_crtc_power_enable(dpu_crtc, true);
> > mutex_lock(_crtc->crtc_lock);
> > - if (ret)
> > - return ret;
> >
> > list_for_each_entry(enc, >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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
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
> ---
> 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 = _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 = _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(_kms->pdev->dev);
> else
> pm_runtime_put_sync(_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(_crtc->crtc_lock);
> - ret = _dpu_crtc_power_enable(dpu_crtc, true);
> + _dpu_crtc_power_enable(dpu_crtc, true);
> mutex_lock(_crtc->crtc_lock);
> - if (ret)
> - return ret;
>
> list_for_each_entry(enc, >mode_config.encoder_list, head) {
> if (enc->crtc != crtc)
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 3/5] drm/msm/dpu: Remove impossible checks
On Wed, Sep 19, 2018 at 11:44:13AM -0400, Bruce Wang wrote:
> Removes additional impossible checks in dpu_plane.c and 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
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 121 +++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +--
The dpu_plane changes should probably get rolled into the dpu_plane removal
patch, then rename the subject of this patch "Remove impossible checks from
dpu_crtc".
> 2 files changed, 17 insertions(+), 120 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..017d16131dac 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -55,17 +55,7 @@ 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;
> - }
> + struct msm_drm_private *priv = crtc->dev->dev_private;
>
> return to_dpu_kms(priv->kms);
Might as well just slap the whole thing in the arg and save a local:
return to_dpu_kms(crtc->dev->dev_private->kms);
> }
> @@ -177,28 +167,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 = _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 = _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 +196,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 +236,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,7 +344,6 @@ 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;
> @@ -400,11 +366,6 @@ static void dpu_crtc_frame_event_work(struct
> kthread_work *work)
This is collapsed, but the if (!work) condition is also bogus.
The fevent null checks are also suspicious. Checking fevent->crtc->state
is downright dangerous since it's unlocked access. Fortunately it doesn't
look like the function is using state, so let's remove that check. Given that
you can remove a bunch more, most of these assignments can be folded up top.
> dpu_crtc = to_dpu_crtc(crtc);
>
> dpu_kms = _dpu_crtc_get_kms(crtc);
> - if
Re: [Freedreno] [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update
On Wed, Sep 19, 2018 at 11:44:12AM -0400, Bruce Wang wrote:
> Removes unnecessary checks from dpu_plane_atomic_disable, old_state
> argument for both dpu_plane_atomic_disable and
> dpu_plane_sspp_atomic_update is removed as it is no longer used.
>
> Signed-off-by: Bruce Wang
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +--
> 1 file changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index a8d00f57f06a..7f6046166626 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1157,8 +1157,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool
> error)
> pdpu->is_error = error;
> }
>
> -static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> - struct drm_plane_state *old_state)
> +static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> {
> uint32_t src_flags;
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> @@ -1271,27 +1270,11 @@ static void dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
> _dpu_plane_set_qos_remap(plane);
> }
>
> -static void _dpu_plane_atomic_disable(struct drm_plane *plane,
> - struct drm_plane_state *old_state)
> +static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> {
> - struct dpu_plane *pdpu;
> - struct drm_plane_state *state;
> - struct dpu_plane_state *pstate;
> -
> - if (!plane) {
> - DPU_ERROR("invalid plane\n");
> - return;
> - } else if (!plane->state) {
> - DPU_ERROR("invalid plane state\n");
> - return;
> - } else if (!old_state) {
> - DPU_ERROR("invalid old state\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> - state = plane->state;
> - pstate = to_dpu_plane_state(state);
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> + struct drm_plane_state *state = plane->state;
> + struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>
> trace_dpu_plane_disable(DRMID(plane), is_dpu_plane_virtual(plane),
> pstate->multirect_mode);
> @@ -1315,9 +1298,9 @@ static void dpu_plane_atomic_update(struct drm_plane
> *plane,
> DPU_DEBUG_PLANE(pdpu, "\n");
>
> if (!state->visible) {
> - _dpu_plane_atomic_disable(plane, old_state);
> + _dpu_plane_atomic_disable(plane);
> } else {
> - dpu_plane_sspp_atomic_update(plane, old_state);
> + dpu_plane_sspp_atomic_update(plane);
> }
> }
>
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane
On Wed, Sep 19, 2018 at 11:44:11AM -0400, Bruce Wang wrote:
> Removes some checks from dpu_plane.c that will never result in an error.
> Subsequent variable assignments become part of the initialization wherever
> possible.
>
> Signed-off-by: Bruce Wang
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++
> 1 file changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 1ce76460d710..a8d00f57f06a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -229,19 +229,11 @@ static u64 _dpu_plane_get_qos_lut(const struct
> dpu_qos_lut_tbl *tbl,
> static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
> struct drm_framebuffer *fb)
> {
> - struct dpu_plane *pdpu;
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> const struct dpu_format *fmt = NULL;
> u64 qos_lut;
> u32 total_fl = 0, lut_usage;
>
> - if (!plane || !fb) {
> - DPU_ERROR("invalid arguments plane %d fb %d\n",
> - plane != 0, fb != 0);
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> -
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
As we discussed, it looks like we can remove all of these too \o/
> DPU_ERROR("invalid arguments\n");
> return;
> @@ -290,17 +282,10 @@ static void _dpu_plane_set_qos_lut(struct drm_plane
> *plane,
> static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
> struct drm_framebuffer *fb)
> {
> - struct dpu_plane *pdpu;
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> const struct dpu_format *fmt = NULL;
> u32 danger_lut, safe_lut;
>
> - if (!plane || !fb) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> -
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
and here
> DPU_ERROR("invalid arguments\n");
> return;
> @@ -361,14 +346,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane
> *plane,
> static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
> bool enable, u32 flags)
> {
> - struct dpu_plane *pdpu;
> -
> - if (!plane) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
>
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
and here
> DPU_ERROR("invalid arguments\n");
> @@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane
> *plane,
> {
> struct dpu_plane *pdpu;
> struct dpu_vbif_set_ot_params ot_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
> struct dpu_kms *dpu_kms;
>
> - if (!plane || !plane->dev || !crtc) {
> - DPU_ERROR("invalid arguments plane %d crtc %d\n",
> - plane != 0, crtc != 0);
> - return;
> - }
> -
> - priv = plane->dev->dev_private;
> if (!priv || !priv->kms) {
These are also guaranteed to be non-NULL (dev_private checked in msm_drv and kms
is checked in dpu_kms.c (or maybe msm_kms.c, but either way)).
> DPU_ERROR("invalid KMS reference\n");
> return;
> @@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane
> *plane)
> {
> struct dpu_plane *pdpu;
> struct dpu_vbif_set_qos_params qos_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
> struct dpu_kms *dpu_kms;
>
> - if (!plane || !plane->dev) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - priv = plane->dev->dev_private;
> if (!priv || !priv->kms) {
> DPU_ERROR("invalid KMS reference\n");
> return;
> @@ -1737,17 +1702,11 @@ struct drm_plane *dpu_plane_init(struct drm_device
> *dev,
> struct drm_plane *plane = NULL, *master_plane = NULL;
> const struct dpu_format_extended *format_list;
> struct dpu_plane *pdpu;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = dev->dev_private;
> struct dpu_kms *kms;
> int zpos_max = DPU_ZPOS_MAX;
> int ret = -EINVAL;
>
> - if (!dev) {
> - DPU_ERROR("[%u]device is NULL\n", pipe);
> - goto exit;
> - }
> -
> - priv = dev->dev_private;
> if (!priv) {
So let's do another pass of all dev_private checks along with kms,
pipe_hw/sblk/etc.
Sean
> DPU_ERROR("[%u]private data is NULL\n", pipe);
> goto exit;
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
[Freedreno] [PATCH 2/2] drm/msm: dpu: Make legacy cursor updates asynchronous
From: Sean Paul
This patch sprinkles a few async/legacy_cursor_update checks
through commit to ensure that cursor updates aren't blocked on vsync.
There are 2 main components to this, the first is that we don't want to
wait_for_commit_done in msm_atomic before returning from atomic_complete.
The second is that in dpu we don't want to wait for frame_done events when
updating the cursor.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 44 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-
drivers/gpu/drm/msm/msm_atomic.c| 3 +-
6 files changed, 48 insertions(+), 34 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..b23f00a2554b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -816,7 +816,7 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc
*crtc)
return rc;
}
-void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
+void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async)
{
struct drm_encoder *encoder;
struct drm_device *dev;
@@ -862,27 +862,30 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
* Encoder will flush/start now, unless it has a tx pending.
* If so, it may delay and flush at an irq event (e.g. ppdone)
*/
- dpu_encoder_prepare_for_kickoff(encoder, );
+ dpu_encoder_prepare_for_kickoff(encoder, , async);
}
- /* wait for frame_event_done completion */
- DPU_ATRACE_BEGIN("wait_for_frame_done_event");
- ret = _dpu_crtc_wait_for_frame_done(crtc);
- DPU_ATRACE_END("wait_for_frame_done_event");
- if (ret) {
- DPU_ERROR("crtc%d wait for frame done failed;frame_pending%d\n",
- crtc->base.id,
- atomic_read(_crtc->frame_pending));
- goto end;
- }
- if (atomic_inc_return(_crtc->frame_pending) == 1) {
- /* acquire bandwidth and other resources */
- DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
- } else
- DPU_DEBUG("crtc%d commit\n", crtc->base.id);
+ if (!async) {
+ /* wait for frame_event_done completion */
+ DPU_ATRACE_BEGIN("wait_for_frame_done_event");
+ ret = _dpu_crtc_wait_for_frame_done(crtc);
+ DPU_ATRACE_END("wait_for_frame_done_event");
+ if (ret) {
+ DPU_ERROR("crtc%d wait for frame done
failed;frame_pending%d\n",
+ crtc->base.id,
+ atomic_read(_crtc->frame_pending));
+ goto end;
+ }
+
+ if (atomic_inc_return(_crtc->frame_pending) == 1) {
+ /* acquire bandwidth and other resources */
+ DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
+ } else
+ DPU_DEBUG("crtc%d commit\n", crtc->base.id);
- dpu_crtc->play_count++;
+ dpu_crtc->play_count++;
+ }
dpu_vbif_clear_errors(dpu_kms);
@@ -890,11 +893,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
if (encoder->crtc != crtc)
continue;
- dpu_encoder_kickoff(encoder);
+ dpu_encoder_kickoff(encoder, async);
}
end:
- reinit_completion(_crtc->frame_done_comp);
+ if (!async)
+ reinit_completion(_crtc->frame_done_comp);
DPU_ATRACE_END("crtc_commit");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 3723b4830335..67c9f59997cf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -285,8 +285,9 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
/**
* dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc
* @crtc: Pointer to drm crtc object
+ * @async: true if the commit is asynchronous, false otherwise
*/
-void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
+void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async);
/**
* dpu_crtc_complete_commit - callback signalling completion of current commit
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index c2e8985b4c54..fc729fc8dd8c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1410,7 +1410,7 @@ static void dpu_encoder_off_work(struct kthread_work
*work)
* extra_flush_bits: Additional bit mask to include in flush
[Freedreno] [PATCH 1/2] drm/msm: dpu: Only check flush register against pending flushes
From: Sean Paul There exists a case where a flush of a plane/dma may have been triggered & started from an async commit. If that plane/dma is subsequently disabled by the next commit, the flush register will continue to hold the flush bit for the disabled plane. Since the bit remains active, pending_kickoff_cnt will never decrement and we'll miss frame_done events. This patch limits the check of flush_register to include only those bits which have been updated with the latest commit. Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 84de385a9f62..60f146f02b77 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -331,7 +331,7 @@ static void dpu_encoder_phys_vid_vblank_irq(void *arg, int irq_idx) if (hw_ctl && hw_ctl->ops.get_flush_register) flush_register = hw_ctl->ops.get_flush_register(hw_ctl); - if (flush_register == 0) + if (!(flush_register & hw_ctl->ops.get_pending_flush(hw_ctl))) new_cnt = atomic_add_unless(_enc->pending_kickoff_cnt, -1, 0); spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags); -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 0/2] drm/msm: dpu: Fix cursor updates
From: Sean Paul Hey all, So here's the async cursor patchset, it works pretty well on my device (video mode dsi). I do have a couple of concerns that hopefully people might have solutions for. The first patch masks off the flush register to exclude inactive bits. I think it'd be nicer to clear those bits if the plane was being disabled, but it wasn't obvious how I could do this. Any suggestions? The second patch seems a bit more spread out than I would like. I'd rather not propagate the async checks all over, but this seemed like the minimum amount in order to get things working reliably. If you have better suggestions, please speak up. Thanks, Sean Sean Paul (2): drm/msm: dpu: Only check flush register against pending flushes drm/msm: dpu: Make legacy cursor updates asynchronous drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 44 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++- drivers/gpu/drm/msm/msm_atomic.c | 3 +- 7 files changed, 49 insertions(+), 35 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 3/3] drm/msm: dpu: Don't store/deref pointers in trace ringbuffer
From: Sean Paul
TP_printk is not synchronous, so storing pointers and then later
derferencing them is a Bad Idea. This patch stores everything locally to
avoid display stomped memory.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 98 +--
1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index ae6b0c51ba52..e12c4cefb742 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -686,37 +686,41 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
TP_STRUCT__entry(
__field(uint32_t, crtc_id )
__field(uint32_t, plane_id)
- __field(struct drm_plane_state*,state )
- __field(struct dpu_plane_state*,pstate )
+ __field(uint32_t, fb_id )
+ __field_struct( struct drm_rect,src_rect)
+ __field_struct( struct drm_rect,dst_rect)
__field(uint32_t, stage_idx )
+ __field(enum dpu_stage, stage )
__field(enum dpu_sspp, sspp)
+ __field(uint32_t, multirect_idx )
+ __field(uint32_t, multirect_mode )
__field(uint32_t, pixel_format)
__field(uint64_t, modifier)
),
TP_fast_assign(
__entry->crtc_id = crtc_id;
__entry->plane_id = plane_id;
- __entry->state = state;
- __entry->pstate = pstate;
+ __entry->fb_id = state ? state->fb->base.id : 0;
+ __entry->src_rect = drm_plane_state_src(state);
+ __entry->dst_rect = drm_plane_state_dest(state);
__entry->stage_idx = stage_idx;
+ __entry->stage = pstate->stage;
__entry->sspp = sspp;
+ __entry->multirect_idx = pstate->multirect_index;
+ __entry->multirect_mode = pstate->multirect_mode;
__entry->pixel_format = pixel_format;
__entry->modifier = modifier;
),
- TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:{%ux%u+%ux%u} "
- "dst:{%ux%u+%ux%u} stage_idx:%u stage:%d, sspp:%d "
+ TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT
+ " dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d "
"multirect_index:%d multirect_mode:%u pix_format:%u "
"modifier:%llu",
- __entry->crtc_id, __entry->plane_id,
- __entry->state->fb ? __entry->state->fb->base.id : -1,
- __entry->state->src_w >> 16, __entry->state->src_h >> 16,
- __entry->state->src_x >> 16, __entry->state->src_y >> 16,
- __entry->state->crtc_w, __entry->state->crtc_h,
- __entry->state->crtc_x, __entry->state->crtc_y,
- __entry->stage_idx, __entry->pstate->stage, __entry->sspp,
- __entry->pstate->multirect_index,
- __entry->pstate->multirect_mode, __entry->pixel_format,
- __entry->modifier)
+ __entry->crtc_id, __entry->plane_id, __entry->fb_id,
+ DRM_RECT_FP_ARG(&__entry->src_rect),
+ DRM_RECT_ARG(&__entry->dst_rect),
+ __entry->stage_idx, __entry->stage, __entry->sspp,
+ __entry->multirect_idx, __entry->multirect_mode,
+ __entry->pixel_format, __entry->modifier)
);
TRACE_EVENT(dpu_crtc_setup_lm_bounds,
@@ -725,15 +729,15 @@ TRACE_EVENT(dpu_crtc_setup_lm_bounds,
TP_STRUCT__entry(
__field(uint32_t, drm_id )
__field(int,mixer )
- __field(struct drm_rect *, bounds )
+ __field_struct( struct drm_rect,bounds )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->mixer = mixer;
- __entry->bounds = bounds;
+ __entry->bounds = *bounds;
),
TP_printk("id:%u mixer:%d bounds:" DRM_RECT_FMT, __entry->drm_id,
- __entry->mixer, DRM_RECT_ARG(__entry->bounds))
+ __entry->mixer, DRM_RECT_ARG(&__entry->bounds))
);
TRACE_EVENT(dpu_crtc_vblank_enable,
@@ -744,21 +748,25 @@ TRACE_EVENT(dpu_crtc_vblank_enable,
__field(uint32_t, drm_id )
__field(uint32_t, enc_id )
__field(bool,
[Freedreno] [PATCH 2/3] drm/msm: dpu: Add extra_flush_bits to trigger_flush trace
From: Sean Paul
It's useful to know which bits of the flush come from "extra_flush_bits"
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 +-
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index e56f29190121..8f6880db5c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1444,7 +1444,8 @@ static inline void _dpu_encoder_trigger_flush(struct
drm_encoder *drm_enc,
ret = ctl->ops.get_pending_flush(ctl);
trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx,
- pending_kickoff_cnt, ctl->idx, ret);
+ pending_kickoff_cnt, ctl->idx,
+ extra_flush_bits, ret);
}
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 0be51db02f2e..ae6b0c51ba52 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -468,14 +468,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
TRACE_EVENT(dpu_enc_trigger_flush,
TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx,
-int pending_kickoff_cnt, int ctl_idx, u32 pending_flush_ret),
+int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits,
+u32 pending_flush_ret),
TP_ARGS(drm_id, intf_idx, pending_kickoff_cnt, ctl_idx,
- pending_flush_ret),
+ extra_flush_bits, pending_flush_ret),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
__field(enum dpu_intf, intf_idx)
__field(int,pending_kickoff_cnt )
__field(int,ctl_idx )
+ __field(u32,extra_flush_bits)
__field(u32,pending_flush_ret )
),
TP_fast_assign(
@@ -483,12 +485,14 @@ TRACE_EVENT(dpu_enc_trigger_flush,
__entry->intf_idx = intf_idx;
__entry->pending_kickoff_cnt = pending_kickoff_cnt;
__entry->ctl_idx = ctl_idx;
+ __entry->extra_flush_bits = extra_flush_bits;
__entry->pending_flush_ret = pending_flush_ret;
),
TP_printk("id=%u, intf_idx=%d, pending_kickoff_cnt=%d ctl_idx=%d "
- "pending_flush_ret=%u", __entry->drm_id,
- __entry->intf_idx, __entry->pending_kickoff_cnt,
- __entry->ctl_idx, __entry->pending_flush_ret)
+ "extra_flush_bits=0x%x pending_flush_ret=0x%x",
+ __entry->drm_id, __entry->intf_idx,
+ __entry->pending_kickoff_cnt, __entry->ctl_idx,
+ __entry->extra_flush_bits, __entry->pending_flush_ret)
);
DECLARE_EVENT_CLASS(dpu_enc_ktime_template,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 1/3] drm/msm: dpu: Clear frame_busy_mask bit after trace
From: Sean Paul
We're printing the frame_busy_mask in a trace, but after it's been
cleared. This, as it turns out, is pretty pointless.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index c2e8985b4c54..e56f29190121 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1361,9 +1361,9 @@ static void dpu_encoder_frame_done_callback(
/* One of the physical encoders has become idle */
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
if (dpu_enc->phys_encs[i] == ready_phys) {
- clear_bit(i, dpu_enc->frame_busy_mask);
trace_dpu_enc_frame_done_cb(DRMID(drm_enc), i,
dpu_enc->frame_busy_mask[0]);
+ clear_bit(i, dpu_enc->frame_busy_mask);
}
}
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane
Removes some checks from dpu_plane.c that will never result in an error.
Subsequent variable assignments become part of the initialization wherever
possible.
Signed-off-by: Bruce Wang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++
1 file changed, 6 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1ce76460d710..a8d00f57f06a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -229,19 +229,11 @@ static u64 _dpu_plane_get_qos_lut(const struct
dpu_qos_lut_tbl *tbl,
static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
struct drm_framebuffer *fb)
{
- struct dpu_plane *pdpu;
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
const struct dpu_format *fmt = NULL;
u64 qos_lut;
u32 total_fl = 0, lut_usage;
- if (!plane || !fb) {
- DPU_ERROR("invalid arguments plane %d fb %d\n",
- plane != 0, fb != 0);
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
DPU_ERROR("invalid arguments\n");
return;
@@ -290,17 +282,10 @@ static void _dpu_plane_set_qos_lut(struct drm_plane
*plane,
static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
struct drm_framebuffer *fb)
{
- struct dpu_plane *pdpu;
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
const struct dpu_format *fmt = NULL;
u32 danger_lut, safe_lut;
- if (!plane || !fb) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
DPU_ERROR("invalid arguments\n");
return;
@@ -361,14 +346,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane
*plane,
static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
bool enable, u32 flags)
{
- struct dpu_plane *pdpu;
-
- if (!plane) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
DPU_ERROR("invalid arguments\n");
@@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane
*plane,
{
struct dpu_plane *pdpu;
struct dpu_vbif_set_ot_params ot_params;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = plane->dev->dev_private;
struct dpu_kms *dpu_kms;
- if (!plane || !plane->dev || !crtc) {
- DPU_ERROR("invalid arguments plane %d crtc %d\n",
- plane != 0, crtc != 0);
- return;
- }
-
- priv = plane->dev->dev_private;
if (!priv || !priv->kms) {
DPU_ERROR("invalid KMS reference\n");
return;
@@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane
*plane)
{
struct dpu_plane *pdpu;
struct dpu_vbif_set_qos_params qos_params;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = plane->dev->dev_private;
struct dpu_kms *dpu_kms;
- if (!plane || !plane->dev) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- priv = plane->dev->dev_private;
if (!priv || !priv->kms) {
DPU_ERROR("invalid KMS reference\n");
return;
@@ -1737,17 +1702,11 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
struct drm_plane *plane = NULL, *master_plane = NULL;
const struct dpu_format_extended *format_list;
struct dpu_plane *pdpu;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = dev->dev_private;
struct dpu_kms *kms;
int zpos_max = DPU_ZPOS_MAX;
int ret = -EINVAL;
- if (!dev) {
- DPU_ERROR("[%u]device is NULL\n", pipe);
- goto exit;
- }
-
- priv = dev->dev_private;
if (!priv) {
DPU_ERROR("[%u]private data is NULL\n", pipe);
goto exit;
--
2.19.0.397.gdd90340f6a-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update
Removes unnecessary checks from dpu_plane_atomic_disable, old_state
argument for both dpu_plane_atomic_disable and
dpu_plane_sspp_atomic_update is removed as it is no longer used.
Signed-off-by: Bruce Wang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +--
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index a8d00f57f06a..7f6046166626 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1157,8 +1157,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool
error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
-struct drm_plane_state *old_state)
+static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
{
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
@@ -1271,27 +1270,11 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane,
_dpu_plane_set_qos_remap(plane);
}
-static void _dpu_plane_atomic_disable(struct drm_plane *plane,
- struct drm_plane_state *old_state)
+static void _dpu_plane_atomic_disable(struct drm_plane *plane)
{
- struct dpu_plane *pdpu;
- struct drm_plane_state *state;
- struct dpu_plane_state *pstate;
-
- if (!plane) {
- DPU_ERROR("invalid plane\n");
- return;
- } else if (!plane->state) {
- DPU_ERROR("invalid plane state\n");
- return;
- } else if (!old_state) {
- DPU_ERROR("invalid old state\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
- state = plane->state;
- pstate = to_dpu_plane_state(state);
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct drm_plane_state *state = plane->state;
+ struct dpu_plane_state *pstate = to_dpu_plane_state(state);
trace_dpu_plane_disable(DRMID(plane), is_dpu_plane_virtual(plane),
pstate->multirect_mode);
@@ -1315,9 +1298,9 @@ static void dpu_plane_atomic_update(struct drm_plane
*plane,
DPU_DEBUG_PLANE(pdpu, "\n");
if (!state->visible) {
- _dpu_plane_atomic_disable(plane, old_state);
+ _dpu_plane_atomic_disable(plane);
} else {
- dpu_plane_sspp_atomic_update(plane, old_state);
+ dpu_plane_sspp_atomic_update(plane);
}
}
--
2.19.0.397.gdd90340f6a-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
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
---
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 = _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 = _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(_kms->pdev->dev);
else
pm_runtime_put_sync(_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(_crtc->crtc_lock);
- ret = _dpu_crtc_power_enable(dpu_crtc, true);
+ _dpu_crtc_power_enable(dpu_crtc, true);
mutex_lock(_crtc->crtc_lock);
- if (ret)
- return ret;
list_for_each_entry(enc, >mode_config.encoder_list, head) {
if (enc->crtc != crtc)
--
2.19.0.397.gdd90340f6a-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 0/5] drm/msm/dpu: Clean up dpu code
Removes unneeded checks and unused variables. Changes some functions that do not need return values to void. Bruce Wang (5): [patch 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane [patch 2/5] drm/msm/dpu: Clean up plane atomic disable/update [patch 3/5] drm/msm/dpu: Remove impossible checks [patch 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void [patch 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock to void drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 198 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 100 ++- 2 files changed, 42 insertions(+), 256 deletions(-) -- 2.19.0.397.gdd90340f6a-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock to void
Removes redundant tests for _dpu_crtc_vblank_enable_no_lock.
Function return type is now void and all function calls have
been changed accordingly.
Signed-off-by: Bruce Wang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 42
1 file changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f0b52281c46f..8ce24a85e70b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -809,24 +809,14 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
* _dpu_crtc_vblank_enable_no_lock - update power resource and vblank request
* @dpu_crtc: Pointer to dpu crtc structure
* @enable: Whether to enable/disable vblanks
- *
- * @Return: error code
*/
-static int _dpu_crtc_vblank_enable_no_lock(
+static void _dpu_crtc_vblank_enable_no_lock(
struct dpu_crtc *dpu_crtc, bool enable)
{
- struct drm_device *dev;
- struct drm_crtc *crtc;
+ struct drm_crtc *crtc = _crtc->base;
+ struct drm_device *dev = crtc->dev;
struct drm_encoder *enc;
- if (!dpu_crtc) {
- DPU_ERROR("invalid crtc\n");
- return -EINVAL;
- }
-
- crtc = _crtc->base;
- dev = crtc->dev;
-
if (enable) {
/* drop lock since power crtc cb may try to re-acquire lock */
mutex_unlock(_crtc->crtc_lock);
@@ -861,8 +851,6 @@ static int _dpu_crtc_vblank_enable_no_lock(
_dpu_crtc_power_enable(dpu_crtc, false);
mutex_lock(_crtc->crtc_lock);
}
-
- return 0;
}
/**
@@ -873,7 +861,6 @@ 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 = to_dpu_crtc(crtc);
- int ret = 0;
DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
@@ -888,10 +875,7 @@ static void _dpu_crtc_set_suspend(struct drm_crtc *crtc,
bool enable)
DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n",
crtc->base.id, enable);
else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
}
dpu_crtc->suspend = enable;
@@ -1000,7 +984,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
struct drm_display_mode *mode;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
- int ret;
unsigned long flags;
if (!crtc || !crtc->dev || !crtc->dev->dev_private || !crtc->state) {
@@ -1031,10 +1014,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
if (dpu_crtc->enabled && !dpu_crtc->suspend &&
dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
}
dpu_crtc->enabled = false;
@@ -1080,7 +1060,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
- int ret;
if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
DPU_ERROR("invalid crtc\n");
@@ -1102,10 +1081,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
if (!dpu_crtc->enabled && !dpu_crtc->suspend &&
dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
}
dpu_crtc->enabled = true;
@@ -1360,7 +1336,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
struct dpu_crtc *dpu_crtc;
- int ret;
if (!crtc) {
DPU_ERROR("invalid crtc\n");
@@ -1371,10 +1346,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(_crtc->base), en, dpu_crtc);
if (dpu_crtc->enabled && !dpu_crtc->suspend) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
- if (ret)
-
[Freedreno] [PATCH 3/5] drm/msm/dpu: Remove impossible checks
Removes additional impossible checks in dpu_plane.c and 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
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 121 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +--
2 files changed, 17 insertions(+), 120 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..017d16131dac 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -55,17 +55,7 @@ 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;
- }
+ struct msm_drm_private *priv = crtc->dev->dev_private;
return to_dpu_kms(priv->kms);
}
@@ -177,28 +167,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 = _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 = _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 +196,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 +236,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,7 +344,6 @@ 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;
@@ -400,11 +366,6 @@ static void dpu_crtc_frame_event_work(struct kthread_work
*work)
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 +431,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 +539,12 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
struct
