Re: [Freedreno] [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder
On 2018-11-16 10:42, Sean Paul wrote: From: Sean Paul The indirection of registering a callback and opaque pointer isn't reall useful when there's only one callsite. So instead of having the vblank_cb registration, just give encoder a crtc and let it directly call the vblank handler. In a later patch, we'll make use of this further. Changes in v2: - None Cc: Jeykumar Sankaran Signed-off-by: Sean Paul Reviewed-by: Jeykumar Sankaran --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 8 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 - 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 141ed1b0e90a..4d7f9ff1e9f4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc *crtc) return INTF_MODE_NONE; } -static void dpu_crtc_vblank_cb(void *data) +void dpu_crtc_vblank_callback(struct drm_crtc *crtc) { - struct drm_crtc *crtc = (struct drm_crtc *)data; struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); /* keep statistics on vblank callback - with auto reset via debugfs */ @@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock( DRMID(enc), enable, dpu_crtc); - dpu_encoder_register_vblank_callback(enc, - dpu_crtc_vblank_cb, (void *)crtc); + dpu_encoder_assign_crtc(enc, crtc); } } else { list_for_each_entry(enc, >mode_config.encoder_list, head) { @@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock( DRMID(enc), enable, dpu_crtc); - dpu_encoder_register_vblank_callback(enc, NULL, NULL); + dpu_encoder_assign_crtc(enc, NULL); } } } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 93d21a61a040..54595cc29be5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc) */ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en); +/** + * dpu_crtc_vblank_callback - called on vblank irq, issues completion events + * @crtc: Pointer to drm crtc object + */ +void dpu_crtc_vblank_callback(struct drm_crtc *crtc); + /** * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc * @crtc: Pointer to drm crtc object diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index d89ac520f7e6..fd6514f681ae 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -142,9 +142,11 @@ enum dpu_enc_rc_states { * @intfs_swapped Whether or not the phys_enc interfaces have been swapped * for partial update right-only cases, such as pingpong * split where virtual pingpong does not generate IRQs - * @crtc_vblank_cb:Callback into the upper layer / CRTC for - * notification of the VBLANK - * @crtc_vblank_cb_data: Data from upper layer for VBLANK notification + * @crtc: Pointer to the currently assigned crtc. Normally you + * would use crtc->state->encoder_mask to determine the + * link between encoder/crtc. However in this case we need + * to track crtc in the disable() hook which is called + * _after_ encoder_mask is cleared. * @crtc_kickoff_cb: Callback into CRTC that will flush & start * all CTL paths * @crtc_kickoff_cb_data: Opaque user data given to crtc_kickoff_cb @@ -186,8 +188,7 @@ struct dpu_encoder_virt { bool intfs_swapped; - void (*crtc_vblank_cb)(void *); - void *crtc_vblank_cb_data; + struct drm_crtc *crtc; struct dentry *debugfs_root; struct mutex enc_lock; @@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, dpu_enc = to_dpu_encoder_virt(drm_enc); spin_lock_irqsave(_enc->enc_spinlock, lock_flags); - if (dpu_enc->crtc_vblank_cb) - dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data); + if (dpu_enc->crtc) + dpu_crtc_vblank_callback(dpu_enc->crtc); spin_unlock_irqrestore(_enc->enc_spinlock, lock_flags); atomic_inc(_enc->vsync_cnt); @@
[Freedreno] [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder
From: Sean Paul The indirection of registering a callback and opaque pointer isn't reall useful when there's only one callsite. So instead of having the vblank_cb registration, just give encoder a crtc and let it directly call the vblank handler. In a later patch, we'll make use of this further. Changes in v2: - None Cc: Jeykumar Sankaran Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 8 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 - 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 141ed1b0e90a..4d7f9ff1e9f4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc *crtc) return INTF_MODE_NONE; } -static void dpu_crtc_vblank_cb(void *data) +void dpu_crtc_vblank_callback(struct drm_crtc *crtc) { - struct drm_crtc *crtc = (struct drm_crtc *)data; struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); /* keep statistics on vblank callback - with auto reset via debugfs */ @@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock( DRMID(enc), enable, dpu_crtc); - dpu_encoder_register_vblank_callback(enc, - dpu_crtc_vblank_cb, (void *)crtc); + dpu_encoder_assign_crtc(enc, crtc); } } else { list_for_each_entry(enc, >mode_config.encoder_list, head) { @@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock( DRMID(enc), enable, dpu_crtc); - dpu_encoder_register_vblank_callback(enc, NULL, NULL); + dpu_encoder_assign_crtc(enc, NULL); } } } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 93d21a61a040..54595cc29be5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc) */ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en); +/** + * dpu_crtc_vblank_callback - called on vblank irq, issues completion events + * @crtc: Pointer to drm crtc object + */ +void dpu_crtc_vblank_callback(struct drm_crtc *crtc); + /** * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc * @crtc: Pointer to drm crtc object diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index d89ac520f7e6..fd6514f681ae 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -142,9 +142,11 @@ enum dpu_enc_rc_states { * @intfs_swapped Whether or not the phys_enc interfaces have been swapped * for partial update right-only cases, such as pingpong * split where virtual pingpong does not generate IRQs - * @crtc_vblank_cb:Callback into the upper layer / CRTC for - * notification of the VBLANK - * @crtc_vblank_cb_data: Data from upper layer for VBLANK notification + * @crtc: Pointer to the currently assigned crtc. Normally you + * would use crtc->state->encoder_mask to determine the + * link between encoder/crtc. However in this case we need + * to track crtc in the disable() hook which is called + * _after_ encoder_mask is cleared. * @crtc_kickoff_cb: Callback into CRTC that will flush & start * all CTL paths * @crtc_kickoff_cb_data: Opaque user data given to crtc_kickoff_cb @@ -186,8 +188,7 @@ struct dpu_encoder_virt { bool intfs_swapped; - void (*crtc_vblank_cb)(void *); - void *crtc_vblank_cb_data; + struct drm_crtc *crtc; struct dentry *debugfs_root; struct mutex enc_lock; @@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, dpu_enc = to_dpu_encoder_virt(drm_enc); spin_lock_irqsave(_enc->enc_spinlock, lock_flags); - if (dpu_enc->crtc_vblank_cb) - dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data); + if (dpu_enc->crtc) + dpu_crtc_vblank_callback(dpu_enc->crtc); spin_unlock_irqrestore(_enc->enc_spinlock, lock_flags); atomic_inc(_enc->vsync_cnt); @@ -1262,15 +1263,14 @@ static void dpu_encoder_underrun_callback(struct