Since DPU 8.x Watchdog timer settings were moved from the TOP to the
INTF block. Support programming the timer in the INTF block. Fixes tag
points to the commit which removed register access to thos registers on
DPU 8.x+ (and which also should have added proper support for WD timer
on those devices).
Fixes: 43e3293fc614 ("drm/msm/dpu: add support for MDP_TOP blackhole")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 48 +++++++++++++++++++++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 7 -----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 7 +++++
5 files changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3921c15aee98..058a7c8727f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -775,13 +775,13 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
}
vsync_cfg.vsync_source = disp_info->vsync_source;
+ vsync_cfg.frame_rate = drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode);
if (hw_mdptop->ops.setup_vsync_source) {
for (i = 0; i < dpu_enc->num_phys_encs; i++)
vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
vsync_cfg.pp_count = dpu_enc->num_phys_encs;
- vsync_cfg.frame_rate = drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode);
hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg);
}
@@ -791,7 +791,7 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
if (phys_enc->has_intf_te && phys_enc->hw_intf->ops.vsync_sel)
phys_enc->hw_intf->ops.vsync_sel(phys_enc->hw_intf,
- vsync_cfg.vsync_source);
+ &vsync_cfg);
}
}
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 a80ac82a9625..7967d9bd2f44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -67,6 +67,10 @@
#define INTF_MISR_CTRL 0x180
#define INTF_MISR_SIGNATURE 0x184
+#define INTF_WD_TIMER_0_CTL 0x230
+#define INTF_WD_TIMER_0_CTL2 0x234
+#define INTF_WD_TIMER_0_LOAD_VALUE 0x238
+
#define INTF_MUX 0x25C
#define INTF_STATUS 0x26C
#define INTF_AVR_CONTROL 0x270
@@ -475,7 +479,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf *intf,
}
static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
- enum dpu_vsync_source vsync_source)
+ struct dpu_vsync_source_cfg *cfg)
{
struct dpu_hw_blk_reg_map *c;
@@ -484,7 +488,42 @@ static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
c = &intf->hw;
- DPU_REG_WRITE(c, INTF_TEAR_MDP_VSYNC_SEL, (vsync_source & 0xf));
+ DPU_REG_WRITE(c, INTF_TEAR_MDP_VSYNC_SEL, (cfg->vsync_source & 0xf));
+}
+
+static void dpu_hw_intf_vsync_sel_v8(struct dpu_hw_intf *intf,
+ struct dpu_vsync_source_cfg *cfg)
+{
+ struct dpu_hw_blk_reg_map *c;
+
+ if (!intf)
+ return;
+
+ c = &intf->hw;
+
+ if (cfg->vsync_source >= DPU_VSYNC_SOURCE_WD_TIMER_4 &&
+ cfg->vsync_source <= DPU_VSYNC_SOURCE_WD_TIMER_1) {
+ pr_warn_once("DPU 8.x supports only GPIOs and timer0 as TE sources\n");
+ return;
+ }
+
+ if (cfg->vsync_source == DPU_VSYNC_SOURCE_WD_TIMER_0) {
+ u32 reg;
+
+ DPU_REG_WRITE(c, INTF_WD_TIMER_0_LOAD_VALUE,
+ CALCULATE_WD_LOAD_VALUE(cfg->frame_rate));
+
+ DPU_REG_WRITE(c, INTF_WD_TIMER_0_CTL, BIT(0)); /* clear timer */
+ reg = DPU_REG_READ(c, INTF_WD_TIMER_0_CTL2);
+ reg |= BIT(8); /* enable heartbeat timer */
+ reg |= BIT(0); /* enable WD timer */
+ DPU_REG_WRITE(c, INTF_WD_TIMER_0_CTL2, reg);
+
+ /* make sure that timers are enabled/disabled for vsync state */
+ wmb();
+ }
+
+ dpu_hw_intf_vsync_sel(intf, cfg);
}
static void dpu_hw_intf_disable_autorefresh(struct dpu_hw_intf *intf,
@@ -598,7 +637,10 @@ struct dpu_hw_intf *dpu_hw_intf_init(struct drm_device *dev,
c->ops.enable_tearcheck = dpu_hw_intf_enable_te;
c->ops.disable_tearcheck = dpu_hw_intf_disable_te;
c->ops.connect_external_te = dpu_hw_intf_connect_external_te;
- c->ops.vsync_sel = dpu_hw_intf_vsync_sel;
+ if (mdss_rev->core_major_ver >= 8)
+ c->ops.vsync_sel = dpu_hw_intf_vsync_sel_v8;
+ else
+ c->ops.vsync_sel = dpu_hw_intf_vsync_sel;
c->ops.disable_autorefresh = dpu_hw_intf_disable_autorefresh;
}
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 f31067a9aaf1..e84ab849d71a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -12,6 +12,7 @@
#include "dpu_hw_util.h"
struct dpu_hw_intf;
+struct dpu_vsync_source_cfg;
/* intf timing settings */
struct dpu_hw_intf_timing_params {
@@ -107,7 +108,7 @@ struct dpu_hw_intf_ops {
int (*connect_external_te)(struct dpu_hw_intf *intf, bool enable_external_te);
- void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source vsync_source);
+ void (*vsync_sel)(struct dpu_hw_intf *intf, struct dpu_vsync_source_cfg *cfg);
/**
* Disable autorefresh if enabled
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index 96dc10589bee..1ebd75d4f9be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -22,13 +22,6 @@
#define TRAFFIC_SHAPER_WR_CLIENT(num) (0x060 + (num * 4))
#define TRAFFIC_SHAPER_FIXPOINT_FACTOR 4
-#define MDP_TICK_COUNT 16
-#define XO_CLK_RATE 19200
-#define MS_TICKS_IN_SEC 1000
-
-#define CALCULATE_WD_LOAD_VALUE(fps) \
- ((uint32_t)((MS_TICKS_IN_SEC * XO_CLK_RATE)/(MDP_TICK_COUNT * fps)))
-
static void dpu_hw_setup_split_pipe(struct dpu_hw_mdp *mdp,
struct split_pipe_cfg *cfg)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index 67b08e99335d..6fe65bc3bff4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -21,6 +21,13 @@
#define TO_S15D16(_x_)((_x_) << 7)
+#define MDP_TICK_COUNT 16
+#define XO_CLK_RATE 19200
+#define MS_TICKS_IN_SEC 1000
+
+#define CALCULATE_WD_LOAD_VALUE(fps) \
+ ((uint32_t)((MS_TICKS_IN_SEC * XO_CLK_RATE)/(MDP_TICK_COUNT * fps)))
+
extern const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L;
extern const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L;
extern const struct dpu_csc_cfg dpu_csc10_rgb2yuv_601l;
--
2.47.3On 2025-12-28 05:57:12, Dmitry Baryshkov wrote:
> Since DPU 8.x Watchdog timer settings were moved from the TOP to the
> INTF block. Support programming the timer in the INTF block. Fixes tag
> points to the commit which removed register access to thos registers on
thos -> those
> DPU 8.x+ (and which also should have added proper support for WD timer
> on those devices).
Right, yes. Commit 2f69e5458447 ("drm/msm/dpu: skip watchdog timer programming
through TOP on >= SM8450") was already a fixup of that (though marked as fixing
the followup commit 100d7ef ("drm/msm/dpu: add support for SM8450") for being
the first to use the new DPU_MDP_PERIPH_0_REMOVED flag).
>
> Fixes: 43e3293fc614 ("drm/msm/dpu: add support for MDP_TOP blackhole")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
...
> @@ -791,7 +791,7 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
>
> if (phys_enc->has_intf_te && phys_enc->hw_intf->ops.vsync_sel)
> phys_enc->hw_intf->ops.vsync_sel(phys_enc->hw_intf,
> - vsync_cfg.vsync_source);
> + &vsync_cfg);
In some way this makes me wonder if we simply need another struct, in favour
of not missing fields that are never / not-yet read, although resulting in more
clutter.
(Just a nit / question, not a request)
...
> + if (cfg->vsync_source == DPU_VSYNC_SOURCE_WD_TIMER_0) {
> + u32 reg;
> +
> + DPU_REG_WRITE(c, INTF_WD_TIMER_0_LOAD_VALUE,
> + CALCULATE_WD_LOAD_VALUE(cfg->frame_rate));
> +
> + DPU_REG_WRITE(c, INTF_WD_TIMER_0_CTL, BIT(0)); /* clear timer */
> + reg = DPU_REG_READ(c, INTF_WD_TIMER_0_CTL2);
> + reg |= BIT(8); /* enable heartbeat timer */
> + reg |= BIT(0); /* enable WD timer */
My downstream also sets BIT(1) for "select default 16 clock ticks":
https://github.com/sonyxperiadev/kernel-techpack-display-driver/blob/61a727e1ce1fda617a73793b2cbb76b5ca846ea2/msm/sde/sde_hw_intf.c#L511
Although it doesn't read back the current register value. Do we need that; or
maybe you are inferring this "missing" BIT(1) via this readback?
After all downstream removed the readback exactly in favour of setting BIT(1)
though because of a "default value change" since MDSS 9.x.x:
https://github.com/sonyxperiadev/kernel-techpack-display-driver/commit/e55c68138b04770d51067c158f92de526e0c926e
- Marijn
On Mon, Dec 29, 2025 at 12:39:13PM +0100, Marijn Suijten wrote:
> On 2025-12-28 05:57:12, Dmitry Baryshkov wrote:
> > Since DPU 8.x Watchdog timer settings were moved from the TOP to the
> > INTF block. Support programming the timer in the INTF block. Fixes tag
> > points to the commit which removed register access to thos registers on
>
> thos -> those
>
> > DPU 8.x+ (and which also should have added proper support for WD timer
> > on those devices).
>
> Right, yes. Commit 2f69e5458447 ("drm/msm/dpu: skip watchdog timer programming
> through TOP on >= SM8450") was already a fixup of that (though marked as fixing
> the followup commit 100d7ef ("drm/msm/dpu: add support for SM8450") for being
> the first to use the new DPU_MDP_PERIPH_0_REMOVED flag).
>
> >
> > Fixes: 43e3293fc614 ("drm/msm/dpu: add support for MDP_TOP blackhole")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> ...
> > @@ -791,7 +791,7 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
> >
> > if (phys_enc->has_intf_te && phys_enc->hw_intf->ops.vsync_sel)
> > phys_enc->hw_intf->ops.vsync_sel(phys_enc->hw_intf,
> > - vsync_cfg.vsync_source);
> > + &vsync_cfg);
>
> In some way this makes me wonder if we simply need another struct, in favour
> of not missing fields that are never / not-yet read, although resulting in more
> clutter.
>
> (Just a nit / question, not a request)
>
> ...
> > + if (cfg->vsync_source == DPU_VSYNC_SOURCE_WD_TIMER_0) {
> > + u32 reg;
> > +
> > + DPU_REG_WRITE(c, INTF_WD_TIMER_0_LOAD_VALUE,
> > + CALCULATE_WD_LOAD_VALUE(cfg->frame_rate));
> > +
> > + DPU_REG_WRITE(c, INTF_WD_TIMER_0_CTL, BIT(0)); /* clear timer */
> > + reg = DPU_REG_READ(c, INTF_WD_TIMER_0_CTL2);
> > + reg |= BIT(8); /* enable heartbeat timer */
> > + reg |= BIT(0); /* enable WD timer */
>
> My downstream also sets BIT(1) for "select default 16 clock ticks":
>
> https://github.com/sonyxperiadev/kernel-techpack-display-driver/blob/61a727e1ce1fda617a73793b2cbb76b5ca846ea2/msm/sde/sde_hw_intf.c#L511
>
> Although it doesn't read back the current register value. Do we need that; or
> maybe you are inferring this "missing" BIT(1) via this readback?
>
> After all downstream removed the readback exactly in favour of setting BIT(1)
> though because of a "default value change" since MDSS 9.x.x:
Ack, thanks for the note.
>
> https://github.com/sonyxperiadev/kernel-techpack-display-driver/commit/e55c68138b04770d51067c158f92de526e0c926e
>
> - Marijn
--
With best wishes
Dmitry
© 2016 - 2026 Red Hat, Inc.