* [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
@ 2021-02-18 12:35 Kalyan Thota
2021-02-22 16:08 ` Rob Clark
0 siblings, 1 reply; 3+ messages in thread
From: Kalyan Thota @ 2021-02-18 12:35 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: Kalyan Thota, linux-kernel, robdclark, seanpaul, hoegsberg,
dianders, mkrishn, swboyd, abhinavk, ddavenport
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).
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
2021-02-18 12:35 [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver Kalyan Thota
@ 2021-02-22 16:08 ` Rob Clark
2021-02-23 15:11 ` [Freedreno] " kalyan_t
0 siblings, 1 reply; 3+ messages in thread
From: Rob Clark @ 2021-02-22 16:08 UTC (permalink / raw)
To: Kalyan Thota
Cc: dri-devel, linux-arm-msm, freedreno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Sean Paul, Kristian H. Kristensen,
Douglas Anderson, Krishna Manikandan, Stephen Boyd,
Abhinav Kumar, Drew Davenport
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)
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Freedreno] [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
2021-02-22 16:08 ` Rob Clark
@ 2021-02-23 15:11 ` kalyan_t
0 siblings, 0 replies; 3+ messages in thread
From: kalyan_t @ 2021-02-23 15:11 UTC (permalink / raw)
To: Rob Clark
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Krishna Manikandan, linux-arm-msm, Linux Kernel Mailing List,
dri-devel, Douglas Anderson, Sean Paul, Abhinav Kumar,
Drew Davenport, Kristian H. Kristensen, Stephen Boyd, freedreno
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-23 15:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 12:35 [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver Kalyan Thota
2021-02-22 16:08 ` Rob Clark
2021-02-23 15:11 ` [Freedreno] " kalyan_t
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).