Messages in this thread | | | Date | Tue, 23 Feb 2021 20:41:48 +0530 | From | kalyan_t@codeauro ... | Subject | Re: [Freedreno] [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver |
| |
On 2021-02-22 21:38, Rob Clark wrote: > On Thu, Feb 18, 2021 at 4:36 AM Kalyan Thota <kalyan_t@codeaurora.org> > wrote: >> >> Set the flag vblank_disable_immediate = true to turn off vblank irqs >> immediately as soon as drm_vblank_put is requested so that there are >> no irqs triggered during idle state. This will reduce cpu wakeups >> and help in power saving. >> >> To enable vblank_disable_immediate flag the underlying KMS driver >> needs to support high precision vblank timestamping and also a >> reliable way of providing vblank counter which is incrementing >> at the leading edge of vblank. >> >> This patch also brings in changes to support vblank_disable_immediate >> requirement in dpu driver. >> >> Changes in v1: >> - Specify reason to add vblank timestamp support. (Rob). >> - Add changes to provide vblank counter from dpu driver. >> >> Changes in v2: >> - Fix warn stack reported by Rob Clark with v2 patch. >> >> Changes in v3: >> - Move back to HW frame counter (Rob). >> > > could you let me know what the delta was in v4? (No need to resend > yet, if needed I can amend the commit msg when applying) > - Frame count mismatch was causing a DRM WARN stack spew. DPU HW will increment the frame count at the end of the sync, where as vblank will be triggered at the fetch_start counter which is calculated as v_total - vfp. This is to start fetching early for panels with low vbp w.r.t hw latency lines.
Add logic to detect the line count if it falls between vactive and v_total then return incremented frame count value. > BR, > -R > >> Signed-off-by: Kalyan Thota <kalyan_t@codeaurora.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 80 >> ++++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 30 ++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 11 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 1 + >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 26 +++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++ >> 8 files changed, 155 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index d4662e8..9a80981 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -65,6 +65,83 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc) >> kfree(dpu_crtc); >> } >> >> +static struct drm_encoder *get_encoder_from_crtc(struct drm_crtc >> *crtc) >> +{ >> + struct drm_device *dev = crtc->dev; >> + struct drm_encoder *encoder; >> + >> + drm_for_each_encoder(encoder, dev) >> + if (encoder->crtc == crtc) >> + return encoder; >> + >> + return NULL; >> +} >> + >> +static u32 dpu_crtc_get_vblank_counter(struct drm_crtc *crtc) >> +{ >> + struct drm_encoder *encoder; >> + >> + encoder = get_encoder_from_crtc(crtc); >> + if (!encoder) { >> + DRM_ERROR("no encoder found for crtc %d\n", >> crtc->index); >> + return false; >> + } >> + >> + return dpu_encoder_get_frame_count(encoder); >> +} >> + >> +static bool dpu_crtc_get_scanout_position(struct drm_crtc *crtc, >> + bool in_vblank_irq, >> + int *vpos, int *hpos, >> + ktime_t *stime, ktime_t >> *etime, >> + const struct >> drm_display_mode *mode) >> +{ >> + unsigned int pipe = crtc->index; >> + struct drm_encoder *encoder; >> + int line, vsw, vbp, vactive_start, vactive_end, vfp_end; >> + >> + encoder = get_encoder_from_crtc(crtc); >> + if (!encoder) { >> + DRM_ERROR("no encoder found for crtc %d\n", pipe); >> + return false; >> + } >> + >> + vsw = mode->crtc_vsync_end - mode->crtc_vsync_start; >> + vbp = mode->crtc_vtotal - mode->crtc_vsync_end; >> + >> + /* >> + * the line counter is 1 at the start of the VSYNC pulse and >> VTOTAL at >> + * the end of VFP. Translate the porch values relative to the >> line >> + * counter positions. >> + */ >> + >> + vactive_start = vsw + vbp + 1; >> + vactive_end = vactive_start + mode->crtc_vdisplay; >> + >> + /* last scan line before VSYNC */ >> + vfp_end = mode->crtc_vtotal; >> + >> + if (stime) >> + *stime = ktime_get(); >> + >> + line = dpu_encoder_get_linecount(encoder); >> + >> + if (line < vactive_start) >> + line -= vactive_start; >> + else if (line > vactive_end) >> + line = line - vfp_end - vactive_start; >> + else >> + line -= vactive_start; >> + >> + *vpos = line; >> + *hpos = 0; >> + >> + if (etime) >> + *etime = ktime_get(); >> + >> + return true; >> +} >> + >> static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer, >> struct dpu_plane_state *pstate, struct dpu_format >> *format) >> { >> @@ -1243,6 +1320,8 @@ static const struct drm_crtc_funcs >> dpu_crtc_funcs = { >> .early_unregister = dpu_crtc_early_unregister, >> .enable_vblank = msm_crtc_enable_vblank, >> .disable_vblank = msm_crtc_disable_vblank, >> + .get_vblank_timestamp = >> drm_crtc_vblank_helper_get_vblank_timestamp, >> + .get_vblank_counter = dpu_crtc_get_vblank_counter, >> }; >> >> static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { >> @@ -1251,6 +1330,7 @@ static const struct drm_crtc_helper_funcs >> dpu_crtc_helper_funcs = { >> .atomic_check = dpu_crtc_atomic_check, >> .atomic_begin = dpu_crtc_atomic_begin, >> .atomic_flush = dpu_crtc_atomic_flush, >> + .get_scanout_position = dpu_crtc_get_scanout_position, >> }; >> >> /* initialize crtc */ >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index f7f5c25..5cd3f31 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -425,6 +425,36 @@ int dpu_encoder_helper_unregister_irq(struct >> dpu_encoder_phys *phys_enc, >> return 0; >> } >> >> +int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc) >> +{ >> + struct dpu_encoder_virt *dpu_enc; >> + struct dpu_encoder_phys *phys; >> + int framecount = 0; >> + >> + dpu_enc = to_dpu_encoder_virt(drm_enc); >> + phys = dpu_enc ? dpu_enc->cur_master : NULL; >> + >> + if (phys && phys->ops.get_frame_count) >> + framecount = phys->ops.get_frame_count(phys); >> + >> + return framecount; >> +} >> + >> +int dpu_encoder_get_linecount(struct drm_encoder *drm_enc) >> +{ >> + struct dpu_encoder_virt *dpu_enc; >> + struct dpu_encoder_phys *phys; >> + int linecount = 0; >> + >> + dpu_enc = to_dpu_encoder_virt(drm_enc); >> + phys = dpu_enc ? dpu_enc->cur_master : NULL; >> + >> + if (phys && phys->ops.get_line_count) >> + linecount = phys->ops.get_line_count(phys); >> + >> + return linecount; >> +} >> + >> void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc, >> struct dpu_encoder_hw_resources >> *hw_res) >> { >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> index b491346..99a5d73 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> @@ -156,5 +156,16 @@ void dpu_encoder_prepare_commit(struct >> drm_encoder *drm_enc); >> */ >> void dpu_encoder_set_idle_timeout(struct drm_encoder *drm_enc, >> u32 >> idle_timeout); >> +/** >> + * dpu_encoder_get_linecount - get interface line count for the >> encoder. >> + * @drm_enc: Pointer to previously created drm encoder structure >> + */ >> +int dpu_encoder_get_linecount(struct drm_encoder *drm_enc); >> + >> +/** >> + * dpu_encoder_get_frame_count - get interface frame count for the >> encoder. >> + * @drm_enc: Pointer to previously created drm encoder structure >> + */ >> +int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc); >> >> #endif /* __DPU_ENCODER_H__ */ >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> index f8f2515..ecbc4be 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> @@ -143,6 +143,7 @@ struct dpu_encoder_phys_ops { >> void (*prepare_idle_pc)(struct dpu_encoder_phys *phys_enc); >> void (*restore)(struct dpu_encoder_phys *phys); >> int (*get_line_count)(struct dpu_encoder_phys *phys); >> + int (*get_frame_count)(struct dpu_encoder_phys *phys); >> }; >> >> /** >> 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 9a69fad..0e06b7e 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 >> @@ -658,6 +658,31 @@ static int dpu_encoder_phys_vid_get_line_count( >> return >> phys_enc->hw_intf->ops.get_line_count(phys_enc->hw_intf); >> } >> >> +static int dpu_encoder_phys_vid_get_frame_count( >> + struct dpu_encoder_phys *phys_enc) >> +{ >> + struct intf_status s = {0}; >> + u32 fetch_start = 0; >> + struct drm_display_mode mode = phys_enc->cached_mode; >> + >> + if (!dpu_encoder_phys_vid_is_master(phys_enc)) >> + return -EINVAL; >> + >> + if (!phys_enc->hw_intf || !phys_enc->hw_intf->ops.get_status) >> + return -EINVAL; >> + >> + phys_enc->hw_intf->ops.get_status(phys_enc->hw_intf, &s); >> + >> + if (s.is_prog_fetch_en && s.is_en) { >> + fetch_start = mode.vtotal - (mode.vsync_start - >> mode.vdisplay); >> + if ((s.line_count > fetch_start) && >> + (s.line_count <= mode.vtotal)) >> + return s.frame_count + 1; >> + } >> + >> + return s.frame_count; >> +} >> + >> static void dpu_encoder_phys_vid_init_ops(struct dpu_encoder_phys_ops >> *ops) >> { >> ops->is_master = dpu_encoder_phys_vid_is_master; >> @@ -676,6 +701,7 @@ static void dpu_encoder_phys_vid_init_ops(struct >> dpu_encoder_phys_ops *ops) >> ops->handle_post_kickoff = >> dpu_encoder_phys_vid_handle_post_kickoff; >> ops->needs_single_flush = >> dpu_encoder_phys_vid_needs_single_flush; >> ops->get_line_count = dpu_encoder_phys_vid_get_line_count; >> + ops->get_frame_count = dpu_encoder_phys_vid_get_frame_count; >> } >> >> struct dpu_encoder_phys *dpu_encoder_phys_vid_init( >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> index 6f0f545..717178b 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> @@ -256,6 +256,7 @@ static void dpu_hw_intf_get_status( >> struct dpu_hw_blk_reg_map *c = &intf->hw; >> >> s->is_en = DPU_REG_READ(c, INTF_TIMING_ENGINE_EN); >> + s->is_prog_fetch_en = !!(DPU_REG_READ(c, INTF_CONFIG) & >> BIT(31)); >> if (s->is_en) { >> s->frame_count = DPU_REG_READ(c, INTF_FRAME_COUNT); >> s->line_count = DPU_REG_READ(c, INTF_LINE_COUNT); >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h >> index 0ead64d..3568be8 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h >> @@ -40,6 +40,7 @@ struct intf_prog_fetch { >> >> struct intf_status { >> u8 is_en; /* interface timing engine is enabled >> or not */ >> + u8 is_prog_fetch_en; /* interface prog fetch counter is >> enabled or not */ >> u32 frame_count; /* frame count since timing engine >> enabled */ >> u32 line_count; /* current line count including >> blanking */ >> }; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> index 374b0e8..ed636f1 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> @@ -14,6 +14,7 @@ >> >> #include <drm/drm_crtc.h> >> #include <drm/drm_file.h> >> +#include <drm/drm_vblank.h> >> >> #include "msm_drv.h" >> #include "msm_mmu.h" >> @@ -1020,6 +1021,10 @@ static int dpu_kms_hw_init(struct msm_kms *kms) >> */ >> dev->mode_config.allow_fb_modifiers = true; >> >> + dev->max_vblank_count = 0xffffffff; >> + /* Disable vblank irqs aggressively for power-saving */ >> + dev->vblank_disable_immediate = true; >> + >> /* >> * _dpu_kms_drm_obj_init should create the DRM related objects >> * i.e. CRTCs, planes, encoders, connectors and so forth >> -- >> 2.7.4 >> > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno
| |