Message ID | 20230801-encoder-cleanup-v1-1-f9e37fe27997@quicinc.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/msm/dpu: Drop encoder vsync_event | expand |
On 01/08/2023 23:18, Jessica Zhang wrote: > Drop vsync_event and vsync_event_work handlers as they are unnecessary. > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 65 +---------------------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4 -- > 2 files changed, 1 insertion(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index f0a2a1dca741..d34e684a4178 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -152,7 +152,6 @@ enum dpu_enc_rc_states { > * @crtc_frame_event_cb_data: callback handler private data > * @frame_done_timeout_ms: frame done timeout in ms > * @frame_done_timer: watchdog timer for frame done event > - * @vsync_event_timer: vsync timer > * @disp_info: local copy of msm_display_info struct > * @idle_pc_supported: indicate if idle power collaps is supported > * @rc_lock: resource control mutex lock to protect > @@ -160,7 +159,6 @@ enum dpu_enc_rc_states { > * @rc_state: resource controller state > * @delayed_off_work: delayed worker to schedule disabling of > * clks and resources after IDLE_TIMEOUT time. > - * @vsync_event_work: worker to handle vsync event for autorefresh > * @topology: topology of the display > * @idle_timeout: idle timeout duration in milliseconds > * @wide_bus_en: wide bus is enabled on this interface > @@ -194,7 +192,6 @@ struct dpu_encoder_virt { > > atomic_t frame_done_timeout_ms; > struct timer_list frame_done_timer; > - struct timer_list vsync_event_timer; > > struct msm_display_info disp_info; > > @@ -202,7 +199,6 @@ struct dpu_encoder_virt { > struct mutex rc_lock; > enum dpu_enc_rc_states rc_state; > struct delayed_work delayed_off_work; > - struct kthread_work vsync_event_work; > struct msm_display_topology topology; > > u32 idle_timeout; > @@ -1770,49 +1766,6 @@ int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t *wakeup_time) > return 0; > } > > -static void dpu_encoder_vsync_event_handler(struct timer_list *t) > -{ > - struct dpu_encoder_virt *dpu_enc = from_timer(dpu_enc, t, > - vsync_event_timer); > - struct drm_encoder *drm_enc = &dpu_enc->base; > - struct msm_drm_private *priv; > - struct msm_drm_thread *event_thread; > - > - if (!drm_enc->dev || !drm_enc->crtc) { > - DPU_ERROR("invalid parameters\n"); > - return; > - } > - > - priv = drm_enc->dev->dev_private; > - > - if (drm_enc->crtc->index >= ARRAY_SIZE(priv->event_thread)) { > - DPU_ERROR("invalid crtc index\n"); > - return; > - } > - event_thread = &priv->event_thread[drm_enc->crtc->index]; > - if (!event_thread) { > - DPU_ERROR("event_thread not found for crtc:%d\n", > - drm_enc->crtc->index); > - return; > - } > - > - del_timer(&dpu_enc->vsync_event_timer); > -} > - > -static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work) > -{ > - struct dpu_encoder_virt *dpu_enc = container_of(work, > - struct dpu_encoder_virt, vsync_event_work); > - ktime_t wakeup_time; > - > - if (dpu_encoder_vsync_time(&dpu_enc->base, &wakeup_time)) > - return; > - > - trace_dpu_enc_vsync_event_work(DRMID(&dpu_enc->base), wakeup_time); > - mod_timer(&dpu_enc->vsync_event_timer, > - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); > -} > - > static u32 > dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc, > u32 enc_ip_width) > @@ -1972,7 +1925,6 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc) > { > struct dpu_encoder_virt *dpu_enc; > struct dpu_encoder_phys *phys; > - ktime_t wakeup_time; > unsigned long timeout_ms; > unsigned int i; > > @@ -1998,14 +1950,6 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc) > phys->ops.handle_post_kickoff(phys); > } > > - if (dpu_enc->disp_info.intf_type == INTF_DSI && > - !dpu_encoder_vsync_time(drm_enc, &wakeup_time)) { > - trace_dpu_enc_early_kickoff(DRMID(drm_enc), > - ktime_to_ms(wakeup_time)); > - mod_timer(&dpu_enc->vsync_event_timer, > - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); > - } > - > DPU_ATRACE_END("encoder_kickoff"); > } > > @@ -2439,11 +2383,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > timer_setup(&dpu_enc->frame_done_timer, > dpu_encoder_frame_done_timeout, 0); > > - if (disp_info->intf_type == INTF_DSI) > - timer_setup(&dpu_enc->vsync_event_timer, > - dpu_encoder_vsync_event_handler, > - 0); > - else if (disp_info->intf_type == INTF_DP) > + if (disp_info->intf_type == INTF_DP) > dpu_enc->wide_bus_en = msm_dp_wide_bus_available( > priv->dp[disp_info->h_tile_instance[0]]); > > @@ -2451,9 +2391,6 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > dpu_encoder_off_work); > dpu_enc->idle_timeout = IDLE_TIMEOUT; > > - kthread_init_work(&dpu_enc->vsync_event_work, > - dpu_encoder_vsync_event_work_handler); > - > memcpy(&dpu_enc->disp_info, disp_info, sizeof(*disp_info)); > > DPU_DEBUG_ENC(dpu_enc, "created\n"); > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > index 1a92d21094f4..7352903eab04 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > @@ -467,10 +467,6 @@ DECLARE_EVENT_CLASS(dpu_enc_ktime_template, > TP_printk("id=%u, time=%lld", __entry->drm_id, > ktime_to_ms(__entry->time)) > ); > -DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_vsync_event_work, > - TP_PROTO(uint32_t drm_id, ktime_t time), > - TP_ARGS(drm_id, time) > -); > DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_early_kickoff, > TP_PROTO(uint32_t drm_id, ktime_t time), > TP_ARGS(drm_id, time) This trace event and corresponding event class can also be removed. Other than that LGTM. > > --- > base-commit: eab616ad7f56cafc8af85e9774816f0901e1efa2 > change-id: 20230721-encoder-cleanup-49887b263c6c > > Best regards,
On 8/1/2023 1:37 PM, Dmitry Baryshkov wrote: > On 01/08/2023 23:18, Jessica Zhang wrote: >> Drop vsync_event and vsync_event_work handlers as they are unnecessary. >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 65 >> +---------------------------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4 -- >> 2 files changed, 1 insertion(+), 68 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index f0a2a1dca741..d34e684a4178 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -152,7 +152,6 @@ enum dpu_enc_rc_states { >> * @crtc_frame_event_cb_data: callback handler private data >> * @frame_done_timeout_ms: frame done timeout in ms >> * @frame_done_timer: watchdog timer for frame done event >> - * @vsync_event_timer: vsync timer >> * @disp_info: local copy of msm_display_info struct >> * @idle_pc_supported: indicate if idle power collaps is >> supported >> * @rc_lock: resource control mutex lock to protect >> @@ -160,7 +159,6 @@ enum dpu_enc_rc_states { >> * @rc_state: resource controller state >> * @delayed_off_work: delayed worker to schedule disabling of >> * clks and resources after IDLE_TIMEOUT time. >> - * @vsync_event_work: worker to handle vsync event for >> autorefresh >> * @topology: topology of the display >> * @idle_timeout: idle timeout duration in milliseconds >> * @wide_bus_en: wide bus is enabled on this interface >> @@ -194,7 +192,6 @@ struct dpu_encoder_virt { >> atomic_t frame_done_timeout_ms; >> struct timer_list frame_done_timer; >> - struct timer_list vsync_event_timer; >> struct msm_display_info disp_info; >> @@ -202,7 +199,6 @@ struct dpu_encoder_virt { >> struct mutex rc_lock; >> enum dpu_enc_rc_states rc_state; >> struct delayed_work delayed_off_work; >> - struct kthread_work vsync_event_work; >> struct msm_display_topology topology; >> u32 idle_timeout; >> @@ -1770,49 +1766,6 @@ int dpu_encoder_vsync_time(struct drm_encoder >> *drm_enc, ktime_t *wakeup_time) >> return 0; >> } >> -static void dpu_encoder_vsync_event_handler(struct timer_list *t) >> -{ >> - struct dpu_encoder_virt *dpu_enc = from_timer(dpu_enc, t, >> - vsync_event_timer); >> - struct drm_encoder *drm_enc = &dpu_enc->base; >> - struct msm_drm_private *priv; >> - struct msm_drm_thread *event_thread; >> - >> - if (!drm_enc->dev || !drm_enc->crtc) { >> - DPU_ERROR("invalid parameters\n"); >> - return; >> - } >> - >> - priv = drm_enc->dev->dev_private; >> - >> - if (drm_enc->crtc->index >= ARRAY_SIZE(priv->event_thread)) { >> - DPU_ERROR("invalid crtc index\n"); >> - return; >> - } >> - event_thread = &priv->event_thread[drm_enc->crtc->index]; >> - if (!event_thread) { >> - DPU_ERROR("event_thread not found for crtc:%d\n", >> - drm_enc->crtc->index); >> - return; >> - } >> - >> - del_timer(&dpu_enc->vsync_event_timer); >> -} >> - >> -static void dpu_encoder_vsync_event_work_handler(struct kthread_work >> *work) >> -{ >> - struct dpu_encoder_virt *dpu_enc = container_of(work, >> - struct dpu_encoder_virt, vsync_event_work); >> - ktime_t wakeup_time; >> - >> - if (dpu_encoder_vsync_time(&dpu_enc->base, &wakeup_time)) >> - return; >> - >> - trace_dpu_enc_vsync_event_work(DRMID(&dpu_enc->base), wakeup_time); >> - mod_timer(&dpu_enc->vsync_event_timer, >> - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); >> -} >> - >> static u32 >> dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc, >> u32 enc_ip_width) >> @@ -1972,7 +1925,6 @@ void dpu_encoder_kickoff(struct drm_encoder >> *drm_enc) >> { >> struct dpu_encoder_virt *dpu_enc; >> struct dpu_encoder_phys *phys; >> - ktime_t wakeup_time; >> unsigned long timeout_ms; >> unsigned int i; >> @@ -1998,14 +1950,6 @@ void dpu_encoder_kickoff(struct drm_encoder >> *drm_enc) >> phys->ops.handle_post_kickoff(phys); >> } >> - if (dpu_enc->disp_info.intf_type == INTF_DSI && >> - !dpu_encoder_vsync_time(drm_enc, &wakeup_time)) { >> - trace_dpu_enc_early_kickoff(DRMID(drm_enc), >> - ktime_to_ms(wakeup_time)); >> - mod_timer(&dpu_enc->vsync_event_timer, >> - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); >> - } >> - >> DPU_ATRACE_END("encoder_kickoff"); >> } >> @@ -2439,11 +2383,7 @@ struct drm_encoder *dpu_encoder_init(struct >> drm_device *dev, >> timer_setup(&dpu_enc->frame_done_timer, >> dpu_encoder_frame_done_timeout, 0); >> - if (disp_info->intf_type == INTF_DSI) >> - timer_setup(&dpu_enc->vsync_event_timer, >> - dpu_encoder_vsync_event_handler, >> - 0); >> - else if (disp_info->intf_type == INTF_DP) >> + if (disp_info->intf_type == INTF_DP) >> dpu_enc->wide_bus_en = msm_dp_wide_bus_available( >> priv->dp[disp_info->h_tile_instance[0]]); >> @@ -2451,9 +2391,6 @@ struct drm_encoder *dpu_encoder_init(struct >> drm_device *dev, >> dpu_encoder_off_work); >> dpu_enc->idle_timeout = IDLE_TIMEOUT; >> - kthread_init_work(&dpu_enc->vsync_event_work, >> - dpu_encoder_vsync_event_work_handler); >> - >> memcpy(&dpu_enc->disp_info, disp_info, sizeof(*disp_info)); >> DPU_DEBUG_ENC(dpu_enc, "created\n"); >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h >> index 1a92d21094f4..7352903eab04 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h >> @@ -467,10 +467,6 @@ DECLARE_EVENT_CLASS(dpu_enc_ktime_template, >> TP_printk("id=%u, time=%lld", __entry->drm_id, >> ktime_to_ms(__entry->time)) >> ); >> -DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_vsync_event_work, >> - TP_PROTO(uint32_t drm_id, ktime_t time), >> - TP_ARGS(drm_id, time) >> -); >> DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_early_kickoff, >> TP_PROTO(uint32_t drm_id, ktime_t time), >> TP_ARGS(drm_id, time) > > This trace event and corresponding event class can also be removed. > Other than that LGTM. Hi Dmitry, Acked. Thanks, Jessica Zhang > >> >> --- >> base-commit: eab616ad7f56cafc8af85e9774816f0901e1efa2 >> change-id: 20230721-encoder-cleanup-49887b263c6c >> >> Best regards, > > -- > With best wishes > Dmitry >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..d34e684a4178 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -152,7 +152,6 @@ enum dpu_enc_rc_states { * @crtc_frame_event_cb_data: callback handler private data * @frame_done_timeout_ms: frame done timeout in ms * @frame_done_timer: watchdog timer for frame done event - * @vsync_event_timer: vsync timer * @disp_info: local copy of msm_display_info struct * @idle_pc_supported: indicate if idle power collaps is supported * @rc_lock: resource control mutex lock to protect @@ -160,7 +159,6 @@ enum dpu_enc_rc_states { * @rc_state: resource controller state * @delayed_off_work: delayed worker to schedule disabling of * clks and resources after IDLE_TIMEOUT time. - * @vsync_event_work: worker to handle vsync event for autorefresh * @topology: topology of the display * @idle_timeout: idle timeout duration in milliseconds * @wide_bus_en: wide bus is enabled on this interface @@ -194,7 +192,6 @@ struct dpu_encoder_virt { atomic_t frame_done_timeout_ms; struct timer_list frame_done_timer; - struct timer_list vsync_event_timer; struct msm_display_info disp_info; @@ -202,7 +199,6 @@ struct dpu_encoder_virt { struct mutex rc_lock; enum dpu_enc_rc_states rc_state; struct delayed_work delayed_off_work; - struct kthread_work vsync_event_work; struct msm_display_topology topology; u32 idle_timeout; @@ -1770,49 +1766,6 @@ int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t *wakeup_time) return 0; } -static void dpu_encoder_vsync_event_handler(struct timer_list *t) -{ - struct dpu_encoder_virt *dpu_enc = from_timer(dpu_enc, t, - vsync_event_timer); - struct drm_encoder *drm_enc = &dpu_enc->base; - struct msm_drm_private *priv; - struct msm_drm_thread *event_thread; - - if (!drm_enc->dev || !drm_enc->crtc) { - DPU_ERROR("invalid parameters\n"); - return; - } - - priv = drm_enc->dev->dev_private; - - if (drm_enc->crtc->index >= ARRAY_SIZE(priv->event_thread)) { - DPU_ERROR("invalid crtc index\n"); - return; - } - event_thread = &priv->event_thread[drm_enc->crtc->index]; - if (!event_thread) { - DPU_ERROR("event_thread not found for crtc:%d\n", - drm_enc->crtc->index); - return; - } - - del_timer(&dpu_enc->vsync_event_timer); -} - -static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work) -{ - struct dpu_encoder_virt *dpu_enc = container_of(work, - struct dpu_encoder_virt, vsync_event_work); - ktime_t wakeup_time; - - if (dpu_encoder_vsync_time(&dpu_enc->base, &wakeup_time)) - return; - - trace_dpu_enc_vsync_event_work(DRMID(&dpu_enc->base), wakeup_time); - mod_timer(&dpu_enc->vsync_event_timer, - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); -} - static u32 dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc, u32 enc_ip_width) @@ -1972,7 +1925,6 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc) { struct dpu_encoder_virt *dpu_enc; struct dpu_encoder_phys *phys; - ktime_t wakeup_time; unsigned long timeout_ms; unsigned int i; @@ -1998,14 +1950,6 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc) phys->ops.handle_post_kickoff(phys); } - if (dpu_enc->disp_info.intf_type == INTF_DSI && - !dpu_encoder_vsync_time(drm_enc, &wakeup_time)) { - trace_dpu_enc_early_kickoff(DRMID(drm_enc), - ktime_to_ms(wakeup_time)); - mod_timer(&dpu_enc->vsync_event_timer, - nsecs_to_jiffies(ktime_to_ns(wakeup_time))); - } - DPU_ATRACE_END("encoder_kickoff"); } @@ -2439,11 +2383,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(&dpu_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) - timer_setup(&dpu_enc->vsync_event_timer, - dpu_encoder_vsync_event_handler, - 0); - else if (disp_info->intf_type == INTF_DP) + if (disp_info->intf_type == INTF_DP) dpu_enc->wide_bus_en = msm_dp_wide_bus_available( priv->dp[disp_info->h_tile_instance[0]]); @@ -2451,9 +2391,6 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, dpu_encoder_off_work); dpu_enc->idle_timeout = IDLE_TIMEOUT; - kthread_init_work(&dpu_enc->vsync_event_work, - dpu_encoder_vsync_event_work_handler); - memcpy(&dpu_enc->disp_info, disp_info, sizeof(*disp_info)); DPU_DEBUG_ENC(dpu_enc, "created\n"); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 1a92d21094f4..7352903eab04 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -467,10 +467,6 @@ DECLARE_EVENT_CLASS(dpu_enc_ktime_template, TP_printk("id=%u, time=%lld", __entry->drm_id, ktime_to_ms(__entry->time)) ); -DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_vsync_event_work, - TP_PROTO(uint32_t drm_id, ktime_t time), - TP_ARGS(drm_id, time) -); DEFINE_EVENT(dpu_enc_ktime_template, dpu_enc_early_kickoff, TP_PROTO(uint32_t drm_id, ktime_t time), TP_ARGS(drm_id, time)
Drop vsync_event and vsync_event_work handlers as they are unnecessary. Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 65 +---------------------------- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4 -- 2 files changed, 1 insertion(+), 68 deletions(-) --- base-commit: eab616ad7f56cafc8af85e9774816f0901e1efa2 change-id: 20230721-encoder-cleanup-49887b263c6c Best regards,