Drop vsync_event and vsync_event_work handlers as they are unnecessary.
Signed-off-by: Jessica Zhang <[email protected]>
---
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)
---
base-commit: eab616ad7f56cafc8af85e9774816f0901e1efa2
change-id: 20230721-encoder-cleanup-49887b263c6c
Best regards,
--
Jessica Zhang <[email protected]>
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 <[email protected]>
> ---
> 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,
--
With best wishes
Dmitry
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 <[email protected]>
>> ---
>> 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
>