* [Intel-gfx] [RFC 0/3] Implement CMRR Support
@ 2023-11-15 6:30 Mitul Golani
2023-11-15 6:30 ` [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers Mitul Golani
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Mitul Golani @ 2023-11-15 6:30 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala
CMRR is a display feature that uses adaptive sync
framework to vary Vtotal slightly to match the
content rate exactly without frame drops. This
feature is a variation of VRR where it varies Vtotal
slightly (between additional 0 and 1 Vtotal scanlines)
to match content rate exactly without frame drops
using the adaptive sync framework.
enable this feature by programing new registers for
CMRR enable, CMRR_M, CMRR_N, vmin=vmax=flipline.The
CMRR_M/CMRR_N ratio represents the fractional part
in (actual refresh rate/target refresh rate) * origVTotal.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Mitul Golani (3):
drm/i915: Define and compute Transcoder CMRR registers
drm/i915: Add Enable/Disable for CMRR based on VRR state
drm/i915: Compute CMRR and calculate vtotal
.../drm/i915/display/intel_crtc_state_dump.c | 4 +-
drivers/gpu/drm/i915/display/intel_display.c | 48 +++++-
.../drm/i915/display/intel_display_device.h | 1 +
.../drm/i915/display/intel_display_types.h | 6 +
drivers/gpu/drm/i915/display/intel_vrr.c | 141 +++++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 14 ++
6 files changed, 191 insertions(+), 23 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
@ 2023-11-15 6:30 ` Mitul Golani
2023-11-15 8:47 ` Jani Nikula
2023-11-15 6:30 ` [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state Mitul Golani
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Mitul Golani @ 2023-11-15 6:30 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala
Add register definitions for Transcoder Fixed Average
Vtotal mode/CMRR function, with the necessary bitfields.
Compute these registers when CMRR is enabled, extending
Adaptive refresh rate capabilities.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++-
.../drm/i915/display/intel_display_types.h | 6 +++++
drivers/gpu/drm/i915/display/intel_vrr.c | 25 ++++++++++++++++++-
drivers/gpu/drm/i915/i915_reg.h | 14 +++++++++++
4 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 125903007a29..f99d2de840bc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -921,6 +921,13 @@ static bool vrr_params_changed(const struct intel_crtc_state *old_crtc_state,
old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full;
}
+static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state,
+ const struct intel_crtc_state *new_crtc_state)
+{
+ return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m ||
+ old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n;
+}
+
static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
const struct intel_crtc_state *new_crtc_state)
{
@@ -5067,6 +5074,16 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
} \
} while (0)
+#define PIPE_CONF_CHECK_LLI(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_mismatch(fastset, crtc, __stringify(name), \
+ "(expected %lli, found %lli)", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
+
#define PIPE_CONF_CHECK_BOOL(name) do { \
if (current_config->name != pipe_config->name) { \
pipe_config_mismatch(fastset, crtc, __stringify(name), \
@@ -5447,10 +5464,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(vrr.flipline);
PIPE_CONF_CHECK_I(vrr.pipeline_full);
PIPE_CONF_CHECK_I(vrr.guardband);
+ PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
+ PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
}
#undef PIPE_CONF_CHECK_X
#undef PIPE_CONF_CHECK_I
+#undef PIPE_CONF_CHECK_LLI
#undef PIPE_CONF_CHECK_BOOL
#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
#undef PIPE_CONF_CHECK_P
@@ -6790,7 +6810,8 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
intel_crtc_needs_fastset(new_crtc_state))
icl_set_pipe_chicken(new_crtc_state);
- if (vrr_params_changed(old_crtc_state, new_crtc_state))
+ if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
+ cmrr_params_changed(old_crtc_state, new_crtc_state))
intel_vrr_set_transcoder_timings(new_crtc_state);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9a44350ba05d..e42a0807227b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1406,6 +1406,12 @@ struct intel_crtc_state {
u16 flipline, vmin, vmax, guardband;
} vrr;
+ /* Content Match Refresh Rate state */
+ struct {
+ bool enable;
+ u64 cmrr_n, cmrr_m;
+ } cmrr;
+
/* Stream Splitter for eDP MSO */
struct {
bool enable;
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5d905f932cb4..4aeccbbf1d2a 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -199,6 +199,19 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
return;
}
+ if (crtc_state->cmrr.enable) {
+ intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
+ VRR_CTL_CMRR_ENABLE | trans_vrr_ctl(crtc_state));
+ intel_de_write(dev_priv, TRANS_CMRR_M_HI(cpu_transcoder),
+ upper_32_bits(crtc_state->cmrr.cmrr_m));
+ intel_de_write(dev_priv, TRANS_CMRR_M_LO(cpu_transcoder),
+ lower_32_bits(crtc_state->cmrr.cmrr_m));
+ intel_de_write(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder),
+ upper_32_bits(crtc_state->cmrr.cmrr_n));
+ intel_de_write(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder),
+ lower_32_bits(crtc_state->cmrr.cmrr_n));
+ }
+
intel_de_write(dev_priv, TRANS_VRR_VMIN(cpu_transcoder), crtc_state->vrr.vmin - 1);
intel_de_write(dev_priv, TRANS_VRR_VMAX(cpu_transcoder), crtc_state->vrr.vmax - 1);
intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder), trans_vrr_ctl(crtc_state));
@@ -263,12 +276,22 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- u32 trans_vrr_ctl;
+ u32 trans_vrr_ctl, cmrr_m_hi, cmrr_m_lo, cmrr_n_hi, cmrr_n_lo;
trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+ if (crtc_state->cmrr.enable) {
+ cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
+ cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
+ cmrr_m_hi = intel_de_read(dev_priv, TRANS_CMRR_M_HI(cpu_transcoder));
+ cmrr_m_lo = intel_de_read(dev_priv, TRANS_CMRR_M_LO(cpu_transcoder));
+
+ crtc_state->cmrr.cmrr_n = ((u64)cmrr_n_hi << 32) | (u64)cmrr_n_lo;
+ crtc_state->cmrr.cmrr_m = ((u64)cmrr_m_hi << 32) | (u64)cmrr_m_lo;
+ }
+
if (DISPLAY_VER(dev_priv) >= 13)
crtc_state->vrr.guardband =
REG_FIELD_GET(XELPD_VRR_CTL_VRR_GUARDBAND_MASK, trans_vrr_ctl);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 135e8d8dbdf0..8554083ebf98 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2013,6 +2013,7 @@
#define VRR_CTL_VRR_ENABLE REG_BIT(31)
#define VRR_CTL_IGN_MAX_SHIFT REG_BIT(30)
#define VRR_CTL_FLIP_LINE_EN REG_BIT(29)
+#define VRR_CTL_CMRR_ENABLE REG_BIT(27)
#define VRR_CTL_PIPELINE_FULL_MASK REG_GENMASK(10, 3)
#define VRR_CTL_PIPELINE_FULL(x) REG_FIELD_PREP(VRR_CTL_PIPELINE_FULL_MASK, (x))
#define VRR_CTL_PIPELINE_FULL_OVERRIDE REG_BIT(0)
@@ -2089,6 +2090,19 @@
#define TRANS_VRR_STATUS2(trans) _MMIO_TRANS2(trans, _TRANS_VRR_STATUS2_A)
#define VRR_STATUS2_VERT_LN_CNT_MASK REG_GENMASK(19, 0)
+#define _TRANS_CMRR_M_HI_A 0x604F4
+#define TRANS_CMRR_M_HI(trans) _MMIO_TRANS2(trans, \
+ _TRANS_CMRR_M_HI_A)
+#define _TRANS_CMRR_N_HI_A 0x604FC
+#define TRANS_CMRR_N_HI(trans) _MMIO_TRANS2(trans, \
+ _TRANS_CMRR_N_HI_A)
+#define _TRANS_CMRR_M_LO_A 0x604F0
+#define TRANS_CMRR_M_LO(trans) _MMIO_TRANS2(trans, \
+ _TRANS_CMRR_M_LO_A)
+#define _TRANS_CMRR_N_LO_A 0x604F8
+#define TRANS_CMRR_N_LO(trans) _MMIO_TRANS2(trans, \
+ _TRANS_CMRR_N_LO_A)
+
#define _TRANS_PUSH_A 0x60A70
#define _TRANS_PUSH_B 0x61A70
#define _TRANS_PUSH_C 0x62A70
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
2023-11-15 6:30 ` [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers Mitul Golani
@ 2023-11-15 6:30 ` Mitul Golani
2023-11-15 8:55 ` Jani Nikula
2023-11-15 6:30 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Mitul Golani @ 2023-11-15 6:30 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala
Add CMRR/Fixed Average Vtotal mode enable and disable
functions based on change in VRR mode of operation.
When Adaptive Sync Vtotal is enabled, Fixed Average Vtotal
mode is disabled and vice versa. With this commit setting
the stage for subsequent CMRR enablement.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
.../drm/i915/display/intel_crtc_state_dump.c | 4 +-
drivers/gpu/drm/i915/display/intel_display.c | 24 ++++++++++--
drivers/gpu/drm/i915/display/intel_vrr.c | 37 +++++++++++++------
3 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 2d15e82c0b3d..908a4c4ccb00 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -299,7 +299,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
intel_dump_buffer(i915, "ELD: ", pipe_config->eld,
drm_eld_size(pipe_config->eld));
- drm_dbg_kms(&i915->drm, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
+ drm_dbg_kms(&i915->drm,
+ "cmrr: %s, vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d, flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
+ str_yes_no(pipe_config->cmrr.enable),
str_yes_no(pipe_config->vrr.enable),
pipe_config->vrr.vmin, pipe_config->vrr.vmax,
pipe_config->vrr.pipeline_full, pipe_config->vrr.guardband,
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f99d2de840bc..f5a69309e65a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -937,6 +937,12 @@ static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
vrr_params_changed(old_crtc_state, new_crtc_state)));
}
+static bool cmrr_enabling(const struct intel_crtc_state *old_crtc_state,
+ const struct intel_crtc_state *new_crtc_state)
+{
+ return is_enabling(cmrr.enable, old_crtc_state, new_crtc_state);
+}
+
static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
const struct intel_crtc_state *new_crtc_state)
{
@@ -946,6 +952,12 @@ static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
vrr_params_changed(old_crtc_state, new_crtc_state)));
}
+static bool cmrr_disabling(const struct intel_crtc_state *old_crtc_state,
+ const struct intel_crtc_state *new_crtc_state)
+{
+ return is_disabling(cmrr.enable, old_crtc_state, new_crtc_state);
+}
+
#undef is_disabling
#undef is_enabling
@@ -1064,7 +1076,8 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
intel_atomic_get_new_crtc_state(state, crtc);
enum pipe pipe = crtc->pipe;
- if (vrr_disabling(old_crtc_state, new_crtc_state)) {
+ if (vrr_disabling(old_crtc_state, new_crtc_state) ||
+ cmrr_disabling(old_crtc_state, new_crtc_state)) {
intel_vrr_disable(old_crtc_state);
intel_crtc_update_active_timings(old_crtc_state, false);
}
@@ -6754,7 +6767,8 @@ static void commit_pipe_post_planes(struct intel_atomic_state *state,
!intel_crtc_needs_modeset(new_crtc_state))
skl_detach_scalers(new_crtc_state);
- if (vrr_enabling(old_crtc_state, new_crtc_state))
+ if (vrr_enabling(old_crtc_state, new_crtc_state) ||
+ cmrr_enabling(old_crtc_state, new_crtc_state))
intel_vrr_enable(new_crtc_state);
}
@@ -6851,9 +6865,11 @@ static void intel_update_crtc(struct intel_atomic_state *state,
* FIXME Should be synchronized with the start of vblank somehow...
*/
if (vrr_enabling(old_crtc_state, new_crtc_state) ||
- new_crtc_state->update_m_n || new_crtc_state->update_lrr)
+ new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
+ cmrr_enabling(old_crtc_state, new_crtc_state))
intel_crtc_update_active_timings(new_crtc_state,
- new_crtc_state->vrr.enable);
+ new_crtc_state->vrr.enable |
+ new_crtc_state->cmrr.enable);
/*
* We usually enable FIFO underrun interrupts as part of the
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 4aeccbbf1d2a..1e33661062b3 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -224,7 +224,7 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- if (!crtc_state->vrr.enable)
+ if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
return;
intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder),
@@ -237,7 +237,7 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- if (!crtc_state->vrr.enable)
+ if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
return false;
return intel_de_read(dev_priv, TRANS_PUSH(cpu_transcoder)) & TRANS_PUSH_SEND;
@@ -245,15 +245,30 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- if (!crtc_state->vrr.enable)
- return;
- intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
- intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
- VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
+ if (!crtc_state->cmrr.enable && !crtc_state->vrr.enable) {
+ return;
+ } else if (crtc_state->vrr.enable && crtc_state->cmrr.enable) {
+ drm_WARN_ON(&dev_priv->drm, crtc_state->vrr.enable &&
+ crtc_state->cmrr.enable);
+ } else {
+ if (!crtc_state->vrr.enable && crtc_state->cmrr.enable) {
+ intel_de_write(dev_priv,
+ TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
+ intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
+ VRR_CTL_VRR_ENABLE | VRR_CTL_CMRR_ENABLE |
+ trans_vrr_ctl(crtc_state));
+ } else {
+ intel_de_write(dev_priv,
+ TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
+ intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
+ VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
+ }
+ }
}
void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
@@ -262,7 +277,7 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
- if (!old_crtc_state->vrr.enable)
+ if (!(old_crtc_state->vrr.enable | old_crtc_state->cmrr.enable))
return;
intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
@@ -280,8 +295,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
- crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
-
if (crtc_state->cmrr.enable) {
cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
@@ -306,6 +319,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
crtc_state->vrr.vmin = intel_de_read(dev_priv, TRANS_VRR_VMIN(cpu_transcoder)) + 1;
}
- if (crtc_state->vrr.enable)
+ if (crtc_state->vrr.enable | crtc_state->cmrr.enable)
crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
2023-11-15 6:30 ` [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers Mitul Golani
2023-11-15 6:30 ` [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state Mitul Golani
@ 2023-11-15 6:30 ` Mitul Golani
2023-11-15 6:57 ` Ville Syrjälä
2023-11-15 9:14 ` Jani Nikula
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement CMRR Support Patchwork
` (2 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Mitul Golani @ 2023-11-15 6:30 UTC (permalink / raw)
To: intel-gfx; +Cc: ville.syrjala
Compute Fixed Average Vtotal/CMRR with resepect to
userspace VRR enablement. Also calculate required
parameters in case of CMRR is enabled. During
intel_vrr_compute_config, CMRR is getting enabled
based on userspace has enabled Adaptive Sync Vtotal
mode (Legacy VRR) or not.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 1 +
.../drm/i915/display/intel_display_device.h | 1 +
drivers/gpu/drm/i915/display/intel_vrr.c | 81 +++++++++++++++++--
3 files changed, 78 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f5a69309e65a..d61790f8ebb4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5479,6 +5479,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(vrr.guardband);
PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
+ PIPE_CONF_CHECK_BOOL(cmrr.enable);
}
#undef PIPE_CONF_CHECK_X
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 4299cc452e05..66cbc3a6bbe8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -68,6 +68,7 @@ struct drm_printer;
#define HAS_TRANSCODER(i915, trans) ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
BIT(trans)) != 0)
#define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
+#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
#define INTEL_NUM_PIPES(i915) (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
#define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)->has_hotplug)
#define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 1e33661062b3..4a056a71b68d 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state)
return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state);
}
+static int
+is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
+{
+ int target_refresh_k, actual_refresh_k;
+ struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+ target_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
+ actual_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+ adjusted_mode->crtc_htotal) * 1000;
+ actual_refresh_k /= adjusted_mode->crtc_vtotal;
+
+ if (actual_refresh_k == target_refresh_k)
+ return false;
+
+ return true;
+}
+
+static unsigned int
+cmrr_get_vtotal(struct intel_crtc_state *crtc_state)
+{
+ unsigned int muliplierM = 1, muliplierN = 1, vtotal;
+ unsigned int actual_refresh_rate, desired_refresh_rate;
+ unsigned long long actual_pixel_rate;
+ struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+ actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+ adjusted_mode->crtc_htotal) * 1000;
+ actual_refresh_rate /= adjusted_mode->crtc_vtotal;
+ desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
+ actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal;
+ actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal) / 1000;
+
+ if (is_cmrr_frac_required(crtc_state)) {
+ muliplierM = 1001;
+ muliplierN = 1000;
+ }
+
+ crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
+ adjusted_mode->crtc_htotal * muliplierN, muliplierM) * muliplierN;
+ vtotal = DIV_ROUND_UP(actual_pixel_rate * muliplierN, crtc_state->cmrr.cmrr_n);
+ crtc_state->cmrr.cmrr_m =
+ (actual_pixel_rate * muliplierM) % crtc_state->cmrr.cmrr_n;
+
+ return vtotal;
+}
+
void
intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
@@ -149,6 +195,27 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
+ /*
+ * When panel is VRR capable and userspace has
+ * not enabled adaptive sync mode then Fixed Average
+ * Vtotal mode should be enabled.
+ */
+ if (crtc_state->uapi.vrr_enabled) {
+ crtc_state->vrr.enable = true;
+ if (HAS_CMRR(i915))
+ crtc_state->cmrr.enable = false;
+ crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+ } else {
+ crtc_state->vrr.enable = false;
+ if (HAS_CMRR(i915)) {
+ crtc_state->cmrr.enable = true;
+ crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
+ crtc_state->vrr.vmin = crtc_state->vrr.vmax;
+ crtc_state->vrr.flipline = crtc_state->vrr.vmin;
+ crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+ }
+ }
+
/*
* For XE_LPD+, we use guardband and pipeline override
* is deprecated.
@@ -161,11 +228,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
crtc_state->framestart_delay - 1);
}
-
- if (crtc_state->uapi.vrr_enabled) {
- crtc_state->vrr.enable = true;
- crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
- }
}
static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
@@ -295,6 +357,15 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
+ if (HAS_CMRR(dev_priv)) {
+ crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE) &&
+ (trans_vrr_ctl & VRR_CTL_VRR_ENABLE);
+ crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
+ !(trans_vrr_ctl & VRR_CTL_CMRR_ENABLE);
+ } else {
+ crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+ }
+
if (crtc_state->cmrr.enable) {
cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 6:30 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
@ 2023-11-15 6:57 ` Ville Syrjälä
2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
2023-11-15 9:14 ` Jani Nikula
1 sibling, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2023-11-15 6:57 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx, ville.syrjala
On Wed, Nov 15, 2023 at 12:00:54PM +0530, Mitul Golani wrote:
> Compute Fixed Average Vtotal/CMRR with resepect to
> userspace VRR enablement. Also calculate required
> parameters in case of CMRR is enabled. During
> intel_vrr_compute_config, CMRR is getting enabled
> based on userspace has enabled Adaptive Sync Vtotal
> mode (Legacy VRR) or not.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> .../drm/i915/display/intel_display_device.h | 1 +
> drivers/gpu/drm/i915/display/intel_vrr.c | 81 +++++++++++++++++--
> 3 files changed, 78 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f5a69309e65a..d61790f8ebb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5479,6 +5479,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_I(vrr.guardband);
> PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> + PIPE_CONF_CHECK_BOOL(cmrr.enable);
> }
>
> #undef PIPE_CONF_CHECK_X
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 4299cc452e05..66cbc3a6bbe8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -68,6 +68,7 @@ struct drm_printer;
> #define HAS_TRANSCODER(i915, trans) ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
> BIT(trans)) != 0)
> #define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
> +#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
> #define INTEL_NUM_PIPES(i915) (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
> #define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)->has_hotplug)
> #define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)->overlay_needs_physical)
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 1e33661062b3..4a056a71b68d 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state)
> return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state);
> }
>
> +static int
> +is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
> +{
> + int target_refresh_k, actual_refresh_k;
> + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> + target_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
That is just your 'actual_refresh' rounded to the nearest
integer. It is *not* any kind of target refresh rate.
> + actual_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> + adjusted_mode->crtc_htotal) * 1000;
> + actual_refresh_k /= adjusted_mode->crtc_vtotal;
> +
> + if (actual_refresh_k == target_refresh_k)
> + return false;
> +
> + return true;
> +}
> +
> +static unsigned int
> +cmrr_get_vtotal(struct intel_crtc_state *crtc_state)
> +{
> + unsigned int muliplierM = 1, muliplierN = 1, vtotal;
> + unsigned int actual_refresh_rate, desired_refresh_rate;
> + unsigned long long actual_pixel_rate;
> + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> + actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> + adjusted_mode->crtc_htotal) * 1000;
> + actual_refresh_rate /= adjusted_mode->crtc_vtotal;
> + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
> + actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal;
> + actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal) / 1000;
> +
> + if (is_cmrr_frac_required(crtc_state)) {
> + muliplierM = 1001;
> + muliplierN = 1000;
> + }
> +
> + crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
> + adjusted_mode->crtc_htotal * muliplierN, muliplierM) * muliplierN;
> + vtotal = DIV_ROUND_UP(actual_pixel_rate * muliplierN, crtc_state->cmrr.cmrr_n);
> + crtc_state->cmrr.cmrr_m =
> + (actual_pixel_rate * muliplierM) % crtc_state->cmrr.cmrr_n;
> +
> + return vtotal;
> +}
> +
> void
> intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> @@ -149,6 +195,27 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>
> crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
>
> + /*
> + * When panel is VRR capable and userspace has
> + * not enabled adaptive sync mode then Fixed Average
> + * Vtotal mode should be enabled.
> + */
> + if (crtc_state->uapi.vrr_enabled) {
> + crtc_state->vrr.enable = true;
> + if (HAS_CMRR(i915))
> + crtc_state->cmrr.enable = false;
> + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> + } else {
> + crtc_state->vrr.enable = false;
> + if (HAS_CMRR(i915)) {
> + crtc_state->cmrr.enable = true;
> + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
> + crtc_state->vrr.vmin = crtc_state->vrr.vmax;
> + crtc_state->vrr.flipline = crtc_state->vrr.vmin;
> + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> + }
> + }
> +
> /*
> * For XE_LPD+, we use guardband and pipeline override
> * is deprecated.
> @@ -161,11 +228,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
> crtc_state->framestart_delay - 1);
> }
> -
> - if (crtc_state->uapi.vrr_enabled) {
> - crtc_state->vrr.enable = true;
> - crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> - }
> }
>
> static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
> @@ -295,6 +357,15 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>
> trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
>
> + if (HAS_CMRR(dev_priv)) {
> + crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE) &&
> + (trans_vrr_ctl & VRR_CTL_VRR_ENABLE);
> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
> + !(trans_vrr_ctl & VRR_CTL_CMRR_ENABLE);
> + } else {
> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> + }
> +
> if (crtc_state->cmrr.enable) {
> cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
> cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
> --
> 2.25.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers
2023-11-15 6:30 ` [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers Mitul Golani
@ 2023-11-15 8:47 ` Jani Nikula
2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2023-11-15 8:47 UTC (permalink / raw)
To: Mitul Golani, intel-gfx; +Cc: ville.syrjala
On Wed, 15 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add register definitions for Transcoder Fixed Average
> Vtotal mode/CMRR function, with the necessary bitfields.
> Compute these registers when CMRR is enabled, extending
> Adaptive refresh rate capabilities.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++-
> .../drm/i915/display/intel_display_types.h | 6 +++++
> drivers/gpu/drm/i915/display/intel_vrr.c | 25 ++++++++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 14 +++++++++++
> 4 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 125903007a29..f99d2de840bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -921,6 +921,13 @@ static bool vrr_params_changed(const struct intel_crtc_state *old_crtc_state,
> old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full;
> }
>
> +static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state,
> + const struct intel_crtc_state *new_crtc_state)
> +{
> + return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m ||
> + old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n;
> +}
> +
> static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> @@ -5067,6 +5074,16 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> } \
> } while (0)
>
> +#define PIPE_CONF_CHECK_LLI(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_mismatch(fastset, crtc, __stringify(name), \
> + "(expected %lli, found %lli)", \
> + current_config->name, \
> + pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> #define PIPE_CONF_CHECK_BOOL(name) do { \
> if (current_config->name != pipe_config->name) { \
> pipe_config_mismatch(fastset, crtc, __stringify(name), \
> @@ -5447,10 +5464,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_I(vrr.flipline);
> PIPE_CONF_CHECK_I(vrr.pipeline_full);
> PIPE_CONF_CHECK_I(vrr.guardband);
> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> }
>
> #undef PIPE_CONF_CHECK_X
> #undef PIPE_CONF_CHECK_I
> +#undef PIPE_CONF_CHECK_LLI
> #undef PIPE_CONF_CHECK_BOOL
> #undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
> #undef PIPE_CONF_CHECK_P
> @@ -6790,7 +6810,8 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
> intel_crtc_needs_fastset(new_crtc_state))
> icl_set_pipe_chicken(new_crtc_state);
>
> - if (vrr_params_changed(old_crtc_state, new_crtc_state))
> + if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
> + cmrr_params_changed(old_crtc_state, new_crtc_state))
> intel_vrr_set_transcoder_timings(new_crtc_state);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9a44350ba05d..e42a0807227b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1406,6 +1406,12 @@ struct intel_crtc_state {
> u16 flipline, vmin, vmax, guardband;
> } vrr;
>
> + /* Content Match Refresh Rate state */
> + struct {
> + bool enable;
> + u64 cmrr_n, cmrr_m;
> + } cmrr;
> +
> /* Stream Splitter for eDP MSO */
> struct {
> bool enable;
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 5d905f932cb4..4aeccbbf1d2a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -199,6 +199,19 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
> return;
> }
>
> + if (crtc_state->cmrr.enable) {
> + intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> + VRR_CTL_CMRR_ENABLE | trans_vrr_ctl(crtc_state));
> + intel_de_write(dev_priv, TRANS_CMRR_M_HI(cpu_transcoder),
> + upper_32_bits(crtc_state->cmrr.cmrr_m));
> + intel_de_write(dev_priv, TRANS_CMRR_M_LO(cpu_transcoder),
> + lower_32_bits(crtc_state->cmrr.cmrr_m));
> + intel_de_write(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder),
> + upper_32_bits(crtc_state->cmrr.cmrr_n));
> + intel_de_write(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder),
> + lower_32_bits(crtc_state->cmrr.cmrr_n));
> + }
> +
> intel_de_write(dev_priv, TRANS_VRR_VMIN(cpu_transcoder), crtc_state->vrr.vmin - 1);
> intel_de_write(dev_priv, TRANS_VRR_VMAX(cpu_transcoder), crtc_state->vrr.vmax - 1);
> intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder), trans_vrr_ctl(crtc_state));
> @@ -263,12 +276,22 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - u32 trans_vrr_ctl;
> + u32 trans_vrr_ctl, cmrr_m_hi, cmrr_m_lo, cmrr_n_hi, cmrr_n_lo;
>
> trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
>
> crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
>
> + if (crtc_state->cmrr.enable) {
> + cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
> + cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
> + cmrr_m_hi = intel_de_read(dev_priv, TRANS_CMRR_M_HI(cpu_transcoder));
> + cmrr_m_lo = intel_de_read(dev_priv, TRANS_CMRR_M_LO(cpu_transcoder));
> +
> + crtc_state->cmrr.cmrr_n = ((u64)cmrr_n_hi << 32) | (u64)cmrr_n_lo;
> + crtc_state->cmrr.cmrr_m = ((u64)cmrr_m_hi << 32) | (u64)cmrr_m_lo;
See intel_de_read64_2x32().
> + }
> +
> if (DISPLAY_VER(dev_priv) >= 13)
> crtc_state->vrr.guardband =
> REG_FIELD_GET(XELPD_VRR_CTL_VRR_GUARDBAND_MASK, trans_vrr_ctl);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 135e8d8dbdf0..8554083ebf98 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2013,6 +2013,7 @@
> #define VRR_CTL_VRR_ENABLE REG_BIT(31)
> #define VRR_CTL_IGN_MAX_SHIFT REG_BIT(30)
> #define VRR_CTL_FLIP_LINE_EN REG_BIT(29)
> +#define VRR_CTL_CMRR_ENABLE REG_BIT(27)
> #define VRR_CTL_PIPELINE_FULL_MASK REG_GENMASK(10, 3)
> #define VRR_CTL_PIPELINE_FULL(x) REG_FIELD_PREP(VRR_CTL_PIPELINE_FULL_MASK, (x))
> #define VRR_CTL_PIPELINE_FULL_OVERRIDE REG_BIT(0)
> @@ -2089,6 +2090,19 @@
> #define TRANS_VRR_STATUS2(trans) _MMIO_TRANS2(trans, _TRANS_VRR_STATUS2_A)
> #define VRR_STATUS2_VERT_LN_CNT_MASK REG_GENMASK(19, 0)
>
> +#define _TRANS_CMRR_M_HI_A 0x604F4
> +#define TRANS_CMRR_M_HI(trans) _MMIO_TRANS2(trans, \
> + _TRANS_CMRR_M_HI_A)
> +#define _TRANS_CMRR_N_HI_A 0x604FC
> +#define TRANS_CMRR_N_HI(trans) _MMIO_TRANS2(trans, \
> + _TRANS_CMRR_N_HI_A)
> +#define _TRANS_CMRR_M_LO_A 0x604F0
> +#define TRANS_CMRR_M_LO(trans) _MMIO_TRANS2(trans, \
> + _TRANS_CMRR_M_LO_A)
> +#define _TRANS_CMRR_N_LO_A 0x604F8
> +#define TRANS_CMRR_N_LO(trans) _MMIO_TRANS2(trans, \
> + _TRANS_CMRR_N_LO_A)
Please fix indent, and order based on register offset. That'll also
group by M/N not by HI/LO.
> +
> #define _TRANS_PUSH_A 0x60A70
> #define _TRANS_PUSH_B 0x61A70
> #define _TRANS_PUSH_C 0x62A70
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state
2023-11-15 6:30 ` [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state Mitul Golani
@ 2023-11-15 8:55 ` Jani Nikula
2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2023-11-15 8:55 UTC (permalink / raw)
To: Mitul Golani, intel-gfx; +Cc: ville.syrjala
On Wed, 15 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add CMRR/Fixed Average Vtotal mode enable and disable
> functions based on change in VRR mode of operation.
> When Adaptive Sync Vtotal is enabled, Fixed Average Vtotal
> mode is disabled and vice versa. With this commit setting
> the stage for subsequent CMRR enablement.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
> .../drm/i915/display/intel_crtc_state_dump.c | 4 +-
> drivers/gpu/drm/i915/display/intel_display.c | 24 ++++++++++--
> drivers/gpu/drm/i915/display/intel_vrr.c | 37 +++++++++++++------
> 3 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 2d15e82c0b3d..908a4c4ccb00 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -299,7 +299,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
> intel_dump_buffer(i915, "ELD: ", pipe_config->eld,
> drm_eld_size(pipe_config->eld));
>
> - drm_dbg_kms(&i915->drm, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> + drm_dbg_kms(&i915->drm,
> + "cmrr: %s, vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d, flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> + str_yes_no(pipe_config->cmrr.enable),
> str_yes_no(pipe_config->vrr.enable),
> pipe_config->vrr.vmin, pipe_config->vrr.vmax,
> pipe_config->vrr.pipeline_full, pipe_config->vrr.guardband,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f99d2de840bc..f5a69309e65a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -937,6 +937,12 @@ static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> vrr_params_changed(old_crtc_state, new_crtc_state)));
> }
>
> +static bool cmrr_enabling(const struct intel_crtc_state *old_crtc_state,
> + const struct intel_crtc_state *new_crtc_state)
> +{
> + return is_enabling(cmrr.enable, old_crtc_state, new_crtc_state);
> +}
> +
> static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> @@ -946,6 +952,12 @@ static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
> vrr_params_changed(old_crtc_state, new_crtc_state)));
> }
>
> +static bool cmrr_disabling(const struct intel_crtc_state *old_crtc_state,
> + const struct intel_crtc_state *new_crtc_state)
> +{
> + return is_disabling(cmrr.enable, old_crtc_state, new_crtc_state);
> +}
> +
See https://patchwork.freedesktop.org/patch/msgid/20231106211915.13406-2-ville.syrjala@linux.intel.com
> #undef is_disabling
> #undef is_enabling
>
> @@ -1064,7 +1076,8 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
> intel_atomic_get_new_crtc_state(state, crtc);
> enum pipe pipe = crtc->pipe;
>
> - if (vrr_disabling(old_crtc_state, new_crtc_state)) {
> + if (vrr_disabling(old_crtc_state, new_crtc_state) ||
> + cmrr_disabling(old_crtc_state, new_crtc_state)) {
> intel_vrr_disable(old_crtc_state);
> intel_crtc_update_active_timings(old_crtc_state, false);
> }
> @@ -6754,7 +6767,8 @@ static void commit_pipe_post_planes(struct intel_atomic_state *state,
> !intel_crtc_needs_modeset(new_crtc_state))
> skl_detach_scalers(new_crtc_state);
>
> - if (vrr_enabling(old_crtc_state, new_crtc_state))
> + if (vrr_enabling(old_crtc_state, new_crtc_state) ||
> + cmrr_enabling(old_crtc_state, new_crtc_state))
> intel_vrr_enable(new_crtc_state);
> }
>
> @@ -6851,9 +6865,11 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> * FIXME Should be synchronized with the start of vblank somehow...
> */
> if (vrr_enabling(old_crtc_state, new_crtc_state) ||
> - new_crtc_state->update_m_n || new_crtc_state->update_lrr)
> + new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
> + cmrr_enabling(old_crtc_state, new_crtc_state))
> intel_crtc_update_active_timings(new_crtc_state,
> - new_crtc_state->vrr.enable);
> + new_crtc_state->vrr.enable |
> + new_crtc_state->cmrr.enable);
Please don't use bitwise OR on booleans.
>
> /*
> * We usually enable FIFO underrun interrupts as part of the
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 4aeccbbf1d2a..1e33661062b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -224,7 +224,7 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> - if (!crtc_state->vrr.enable)
> + if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
Please don't use bitwise OR on booleans.
> return;
>
> intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder),
> @@ -237,7 +237,7 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> - if (!crtc_state->vrr.enable)
> + if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
Please don't use bitwise OR on booleans.
> return false;
>
> return intel_de_read(dev_priv, TRANS_PUSH(cpu_transcoder)) & TRANS_PUSH_SEND;
> @@ -245,15 +245,30 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
>
> void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
Unrelated change.
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> - if (!crtc_state->vrr.enable)
> - return;
>
> - intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
> - intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> - VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
> + if (!crtc_state->cmrr.enable && !crtc_state->vrr.enable) {
> + return;
> + } else if (crtc_state->vrr.enable && crtc_state->cmrr.enable) {
> + drm_WARN_ON(&dev_priv->drm, crtc_state->vrr.enable &&
> + crtc_state->cmrr.enable);
Please don't duplicate the if and the drm_WARN_ON() conditions. You'll
want to do this as the first thing with and early return:
if (drm_WARN_ON(...))
return;
Then you can have two independent blocks:
if (crtc_state->vrr.enable)
// handle vrr
if (crtc_state->cmrr.enable)
// handle cmmr
And you can remove the whole complicated if-ladder.
> + } else {
> + if (!crtc_state->vrr.enable && crtc_state->cmrr.enable) {
> + intel_de_write(dev_priv,
> + TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
> + intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> + VRR_CTL_VRR_ENABLE | VRR_CTL_CMRR_ENABLE |
> + trans_vrr_ctl(crtc_state));
> + } else {
> + intel_de_write(dev_priv,
> + TRANS_PUSH(cpu_transcoder), TRANS_PUSH_EN);
> + intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
> + }
> + }
> }
>
> void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> @@ -262,7 +277,7 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>
> - if (!old_crtc_state->vrr.enable)
> + if (!(old_crtc_state->vrr.enable | old_crtc_state->cmrr.enable))
Please don't use bitwise OR on booleans.
> return;
>
> intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> @@ -280,8 +295,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>
> trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
>
> - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> -
Huh?
> if (crtc_state->cmrr.enable) {
> cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
> cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
> @@ -306,6 +319,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
> crtc_state->vrr.vmin = intel_de_read(dev_priv, TRANS_VRR_VMIN(cpu_transcoder)) + 1;
> }
>
> - if (crtc_state->vrr.enable)
> + if (crtc_state->vrr.enable | crtc_state->cmrr.enable)
Please don't use bitwise OR on booleans.
> crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> }
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement CMRR Support
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
` (2 preceding siblings ...)
2023-11-15 6:30 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
@ 2023-11-15 9:06 ` Patchwork
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-11-15 9:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-11-15 9:06 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx
== Series Details ==
Series: Implement CMRR Support
URL : https://patchwork.freedesktop.org/series/126443/
State : warning
== Summary ==
Error: dim checkpatch failed
235ee232c969 drm/i915: Define and compute Transcoder CMRR registers
-:35: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#35: FILE: drivers/gpu/drm/i915/display/intel_display.c:5077:
+#define PIPE_CONF_CHECK_LLI(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_mismatch(fastset, crtc, __stringify(name), \
+ "(expected %lli, found %lli)", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:35: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#35: FILE: drivers/gpu/drm/i915/display/intel_display.c:5077:
+#define PIPE_CONF_CHECK_LLI(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_mismatch(fastset, crtc, __stringify(name), \
+ "(expected %lli, found %lli)", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
total: 0 errors, 0 warnings, 2 checks, 131 lines checked
6c3d4f923d7a drm/i915: Add Enable/Disable for CMRR based on VRR state
7d5318655d43 drm/i915: Compute CMRR and calculate vtotal
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Implement CMRR Support
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
` (3 preceding siblings ...)
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement CMRR Support Patchwork
@ 2023-11-15 9:06 ` Patchwork
2023-11-15 9:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-11-15 9:06 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx
== Series Details ==
Series: Implement CMRR Support
URL : https://patchwork.freedesktop.org/series/126443/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 6:30 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
2023-11-15 6:57 ` Ville Syrjälä
@ 2023-11-15 9:14 ` Jani Nikula
2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
1 sibling, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2023-11-15 9:14 UTC (permalink / raw)
To: Mitul Golani, intel-gfx; +Cc: ville.syrjala
On Wed, 15 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Compute Fixed Average Vtotal/CMRR with resepect to
> userspace VRR enablement. Also calculate required
> parameters in case of CMRR is enabled. During
> intel_vrr_compute_config, CMRR is getting enabled
> based on userspace has enabled Adaptive Sync Vtotal
> mode (Legacy VRR) or not.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> .../drm/i915/display/intel_display_device.h | 1 +
> drivers/gpu/drm/i915/display/intel_vrr.c | 81 +++++++++++++++++--
> 3 files changed, 78 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f5a69309e65a..d61790f8ebb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5479,6 +5479,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> PIPE_CONF_CHECK_I(vrr.guardband);
> PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> + PIPE_CONF_CHECK_BOOL(cmrr.enable);
> }
>
> #undef PIPE_CONF_CHECK_X
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 4299cc452e05..66cbc3a6bbe8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -68,6 +68,7 @@ struct drm_printer;
> #define HAS_TRANSCODER(i915, trans) ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
> BIT(trans)) != 0)
> #define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
> +#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
> #define INTEL_NUM_PIPES(i915) (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
> #define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)->has_hotplug)
> #define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)->overlay_needs_physical)
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 1e33661062b3..4a056a71b68d 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state)
> return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state);
> }
>
> +static int
> +is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
Predicate functions need to be bool, not int.
> +{
> + int target_refresh_k, actual_refresh_k;
> + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> + target_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
> + actual_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> + adjusted_mode->crtc_htotal) * 1000;
> + actual_refresh_k /= adjusted_mode->crtc_vtotal;
> +
> + if (actual_refresh_k == target_refresh_k)
> + return false;
> +
> + return true;
> +}
> +
> +static unsigned int
> +cmrr_get_vtotal(struct intel_crtc_state *crtc_state)
> +{
> + unsigned int muliplierM = 1, muliplierN = 1, vtotal;
Please no camel case, and please fix the typo in multiplier.
> + unsigned int actual_refresh_rate, desired_refresh_rate;
> + unsigned long long actual_pixel_rate;
All this unsigned math gives me an uneasy feeling. Please consider using
signed math.
> + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> + actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> + adjusted_mode->crtc_htotal) * 1000;
> + actual_refresh_rate /= adjusted_mode->crtc_vtotal;
> + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
> + actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal;
> + actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal) / 1000;
> +
> + if (is_cmrr_frac_required(crtc_state)) {
> + muliplierM = 1001;
> + muliplierN = 1000;
> + }
> +
> + crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
> + adjusted_mode->crtc_htotal * muliplierN, muliplierM) * muliplierN;
> + vtotal = DIV_ROUND_UP(actual_pixel_rate * muliplierN, crtc_state->cmrr.cmrr_n);
> + crtc_state->cmrr.cmrr_m =
> + (actual_pixel_rate * muliplierM) % crtc_state->cmrr.cmrr_n;
You're probably going to need do_div() for the remainder.
> +
> + return vtotal;
> +}
> +
> void
> intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> @@ -149,6 +195,27 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>
> crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
>
> + /*
> + * When panel is VRR capable and userspace has
> + * not enabled adaptive sync mode then Fixed Average
> + * Vtotal mode should be enabled.
> + */
> + if (crtc_state->uapi.vrr_enabled) {
> + crtc_state->vrr.enable = true;
> + if (HAS_CMRR(i915))
> + crtc_state->cmrr.enable = false;
What would have set this to true?
> + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> + } else {
> + crtc_state->vrr.enable = false;
What would have set this to true?
You could simplify this to
if (crtc_state->uapi.vrr_enabled)
...
else if (HAS_CMRR(i915))
...
> + if (HAS_CMRR(i915)) {
> + crtc_state->cmrr.enable = true;
> + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
> + crtc_state->vrr.vmin = crtc_state->vrr.vmax;
> + crtc_state->vrr.flipline = crtc_state->vrr.vmin;
> + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> + }
> + }
> +
> /*
> * For XE_LPD+, we use guardband and pipeline override
> * is deprecated.
> @@ -161,11 +228,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
> crtc_state->framestart_delay - 1);
> }
> -
> - if (crtc_state->uapi.vrr_enabled) {
> - crtc_state->vrr.enable = true;
> - crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> - }
> }
>
> static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
> @@ -295,6 +357,15 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>
> trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
>
> + if (HAS_CMRR(dev_priv)) {
> + crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE) &&
> + (trans_vrr_ctl & VRR_CTL_VRR_ENABLE);
> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
> + !(trans_vrr_ctl & VRR_CTL_CMRR_ENABLE);
> + } else {
> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> + }
> +
> if (crtc_state->cmrr.enable) {
> cmrr_n_hi = intel_de_read(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder));
> cmrr_n_lo = intel_de_read(dev_priv, TRANS_CMRR_N_LO(cpu_transcoder));
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Implement CMRR Support
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
` (4 preceding siblings ...)
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-11-15 9:20 ` Patchwork
5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-11-15 9:20 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 12892 bytes --]
== Series Details ==
Series: Implement CMRR Support
URL : https://patchwork.freedesktop.org/series/126443/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13876 -> Patchwork_126443v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/index.html
Participating hosts (37 -> 39)
------------------------------
Additional (3): fi-kbl-soraka fi-hsw-4770 bat-mtlp-8
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_126443v1 that come from known issues:
### CI changes ###
#### Possible fixes ####
* boot:
- fi-bsw-n3050: [FAIL][1] ([i915#8293]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/fi-bsw-n3050/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-bsw-n3050/boot.html
### IGT changes ###
#### Issues hit ####
* igt@debugfs_test@basic-hwmon:
- bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#9318])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@debugfs_test@basic-hwmon.html
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 other tests skip
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@gem_lmem_swapping@random-engines:
- fi-bsw-n3050: NOTRUN -> [SKIP][6] ([fdo#109271]) +18 other tests skip
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-bsw-n3050/igt@gem_lmem_swapping@random-engines.html
* igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#4613]) +3 other tests skip
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@gem_lmem_swapping@verify-random.html
* igt@gem_mmap@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#4083])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@gem_mmap@basic.html
* igt@gem_mmap_gtt@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4077]) +3 other tests skip
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@gem_mmap_gtt@basic.html
* igt@gem_render_tiled_blits@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4079]) +1 other test skip
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@gem_render_tiled_blits@basic.html
* igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#6621])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][12] ([i915#1886])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: NOTRUN -> [INCOMPLETE][13] ([i915#9527])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
* igt@i915_selftest@live@mman:
- bat-rpls-1: [PASS][14] -> [TIMEOUT][15] ([i915#6794] / [i915#7392])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/bat-rpls-1/igt@i915_selftest@live@mman.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-rpls-1/igt@i915_selftest@live@mman.html
* igt@i915_suspend@basic-s2idle-without-i915:
- bat-rpls-1: [PASS][16] -> [WARN][17] ([i915#8747])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#6645])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- fi-hsw-4770: NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#5190])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html
- bat-mtlp-8: NOTRUN -> [SKIP][20] ([i915#5190])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html
* igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][21] ([i915#4212]) +8 other tests skip
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_addfb_basic@basic-y-tiled-legacy.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][22] ([i915#4213]) +1 other test skip
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_dsc@dsc-basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][23] ([fdo#109271]) +9 other tests skip
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-kbl-soraka/igt@kms_dsc@dsc-basic.html
- bat-mtlp-8: NOTRUN -> [SKIP][24] ([i915#3555] / [i915#3840] / [i915#9159])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_dsc@dsc-basic.html
* igt@kms_force_connector_basic@force-load-detect:
- bat-mtlp-8: NOTRUN -> [SKIP][25] ([fdo#109285])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_force_connector_basic@prune-stale-modes:
- bat-mtlp-8: NOTRUN -> [SKIP][26] ([i915#5274])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_force_connector_basic@prune-stale-modes.html
* igt@kms_hdmi_inject@inject-audio:
- fi-bsw-n3050: NOTRUN -> [FAIL][27] ([IGT#3])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-vga-1:
- fi-hsw-4770: NOTRUN -> [SKIP][28] ([fdo#109271]) +12 other tests skip
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-hsw-4770/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-vga-1.html
* igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
- bat-adlp-9: NOTRUN -> [SKIP][29] ([i915#1845] / [i915#3546]) +2 other tests skip
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-adlp-9/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-rpls-1: NOTRUN -> [SKIP][30] ([i915#1845])
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html
* igt@kms_psr@sprite_plane_onoff:
- fi-hsw-4770: NOTRUN -> [SKIP][31] ([fdo#109271] / [i915#1072]) +3 other tests skip
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-mtlp-8: NOTRUN -> [SKIP][32] ([i915#3555] / [i915#8809])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-fence-mmap:
- bat-mtlp-8: NOTRUN -> [SKIP][33] ([i915#3708] / [i915#4077]) +1 other test skip
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@prime_vgem@basic-fence-mmap.html
* igt@prime_vgem@basic-fence-read:
- bat-mtlp-8: NOTRUN -> [SKIP][34] ([i915#3708]) +2 other tests skip
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-mtlp-8/igt@prime_vgem@basic-fence-read.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [DMESG-FAIL][35] ([i915#5334]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-rpls-1: [ABORT][37] ([i915#7978] / [i915#9631]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_hdmi_inject@inject-audio:
- fi-kbl-guc: [FAIL][39] ([IGT#3]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/fi-kbl-guc/igt@kms_hdmi_inject@inject-audio.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/fi-kbl-guc/igt@kms_hdmi_inject@inject-audio.html
* igt@kms_psr@sprite_plane_onoff:
- bat-jsl-3: [SKIP][41] ([i915#9648]) -> [PASS][42] +3 other tests pass
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13876/bat-jsl-3/igt@kms_psr@sprite_plane_onoff.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/bat-jsl-3/igt@kms_psr@sprite_plane_onoff.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
[i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
[i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
[i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
[i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
[i915#8747]: https://gitlab.freedesktop.org/drm/intel/issues/8747
[i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
[i915#9159]: https://gitlab.freedesktop.org/drm/intel/issues/9159
[i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
[i915#9527]: https://gitlab.freedesktop.org/drm/intel/issues/9527
[i915#9631]: https://gitlab.freedesktop.org/drm/intel/issues/9631
[i915#9648]: https://gitlab.freedesktop.org/drm/intel/issues/9648
Build changes
-------------
* Linux: CI_DRM_13876 -> Patchwork_126443v1
CI-20190529: 20190529
CI_DRM_13876: 46c7c3a2db20f83da3cae4392b36860ebef6623b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7588: 328c5873b8f061267fdf86ed32cb5ecc611ba081 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_126443v1: 46c7c3a2db20f83da3cae4392b36860ebef6623b @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
104a955edb83 drm/i915: Compute CMRR and calculate vtotal
e81edb5fdac7 drm/i915: Add Enable/Disable for CMRR based on VRR state
249401e4722d drm/i915: Define and compute Transcoder CMRR registers
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126443v1/index.html
[-- Attachment #2: Type: text/html, Size: 15258 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 9:14 ` Jani Nikula
@ 2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 17+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-11-15 13:50 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org; +Cc: Syrjala, Ville
Thanks @Jani Nikula
Addressed all review comments.
Regards,
Mitul
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, November 15, 2023 2:44 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate
> vtotal
>
> On Wed, 15 Nov 2023, Mitul Golani
> <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Compute Fixed Average Vtotal/CMRR with resepect to userspace VRR
> > enablement. Also calculate required parameters in case of CMRR is
> > enabled. During intel_vrr_compute_config, CMRR is getting enabled
> > based on userspace has enabled Adaptive Sync Vtotal mode (Legacy VRR)
> > or not.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 1 +
> > .../drm/i915/display/intel_display_device.h | 1 +
> > drivers/gpu/drm/i915/display/intel_vrr.c | 81 +++++++++++++++++--
> > 3 files changed, 78 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index f5a69309e65a..d61790f8ebb4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5479,6 +5479,7 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_I(vrr.guardband);
> > PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> > PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> > + PIPE_CONF_CHECK_BOOL(cmrr.enable);
> > }
> >
> > #undef PIPE_CONF_CHECK_X
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> > b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index 4299cc452e05..66cbc3a6bbe8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -68,6 +68,7 @@ struct drm_printer;
> > #define HAS_TRANSCODER(i915, trans)
> ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
> > BIT(trans)) != 0)
> > #define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
> > +#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
> > #define INTEL_NUM_PIPES(i915)
> (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
> > #define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)-
> >has_hotplug)
> > #define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)-
> >overlay_needs_physical)
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 1e33661062b3..4a056a71b68d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct
> intel_crtc_state *crtc_state)
> > return crtc_state->vrr.vmax -
> > intel_vrr_vblank_exit_length(crtc_state);
> > }
> >
> > +static int
> > +is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
>
> Predicate functions need to be bool, not int.
>
> > +{
> > + int target_refresh_k, actual_refresh_k;
> > + struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > + target_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
> > + actual_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock *
> 1000,
> > + adjusted_mode->crtc_htotal) * 1000;
> > + actual_refresh_k /= adjusted_mode->crtc_vtotal;
> > +
> > + if (actual_refresh_k == target_refresh_k)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static unsigned int
> > +cmrr_get_vtotal(struct intel_crtc_state *crtc_state) {
> > + unsigned int muliplierM = 1, muliplierN = 1, vtotal;
>
> Please no camel case, and please fix the typo in multiplier.
>
> > + unsigned int actual_refresh_rate, desired_refresh_rate;
> > + unsigned long long actual_pixel_rate;
>
> All this unsigned math gives me an uneasy feeling. Please consider using
> signed math.
>
> > + struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > + actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock *
> 1000,
> > + adjusted_mode->crtc_htotal) *
> 1000;
> > + actual_refresh_rate /= adjusted_mode->crtc_vtotal;
> > + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
> > + actual_pixel_rate = actual_refresh_rate * adjusted_mode-
> >crtc_vtotal;
> > + actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal)
> > +/ 1000;
> > +
> > + if (is_cmrr_frac_required(crtc_state)) {
> > + muliplierM = 1001;
> > + muliplierN = 1000;
> > + }
> > +
> > + crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
> > + adjusted_mode->crtc_htotal * muliplierN,
> muliplierM) * muliplierN;
> > + vtotal = DIV_ROUND_UP(actual_pixel_rate * muliplierN, crtc_state-
> >cmrr.cmrr_n);
> > + crtc_state->cmrr.cmrr_m =
> > + (actual_pixel_rate * muliplierM) % crtc_state->cmrr.cmrr_n;
>
> You're probably going to need do_div() for the remainder.
>
> > +
> > + return vtotal;
> > +}
> > +
> > void
> > intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> > struct drm_connector_state *conn_state) @@ -
> 149,6 +195,27 @@
> > intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> >
> > crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
> >
> > + /*
> > + * When panel is VRR capable and userspace has
> > + * not enabled adaptive sync mode then Fixed Average
> > + * Vtotal mode should be enabled.
> > + */
> > + if (crtc_state->uapi.vrr_enabled) {
> > + crtc_state->vrr.enable = true;
> > + if (HAS_CMRR(i915))
> > + crtc_state->cmrr.enable = false;
>
> What would have set this to true?
>
> > + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > + } else {
> > + crtc_state->vrr.enable = false;
>
> What would have set this to true?
>
> You could simplify this to
>
> if (crtc_state->uapi.vrr_enabled)
> ...
> else if (HAS_CMRR(i915))
> ...
>
> > + if (HAS_CMRR(i915)) {
> > + crtc_state->cmrr.enable = true;
> > + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
> > + crtc_state->vrr.vmin = crtc_state->vrr.vmax;
> > + crtc_state->vrr.flipline = crtc_state->vrr.vmin;
> > + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > + }
> > + }
> > +
> > /*
> > * For XE_LPD+, we use guardband and pipeline override
> > * is deprecated.
> > @@ -161,11 +228,6 @@ intel_vrr_compute_config(struct intel_crtc_state
> *crtc_state,
> > min(255, crtc_state->vrr.vmin - adjusted_mode-
> >crtc_vblank_start -
> > crtc_state->framestart_delay - 1);
> > }
> > -
> > - if (crtc_state->uapi.vrr_enabled) {
> > - crtc_state->vrr.enable = true;
> > - crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > - }
> > }
> >
> > static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
> > @@ -295,6 +357,15 @@ void intel_vrr_get_config(struct intel_crtc_state
> > *crtc_state)
> >
> > trans_vrr_ctl = intel_de_read(dev_priv,
> > TRANS_VRR_CTL(cpu_transcoder));
> >
> > + if (HAS_CMRR(dev_priv)) {
> > + crtc_state->cmrr.enable = (trans_vrr_ctl &
> VRR_CTL_CMRR_ENABLE) &&
> > + (trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE);
> > + crtc_state->vrr.enable = trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE &&
> > + !(trans_vrr_ctl &
> VRR_CTL_CMRR_ENABLE);
> > + } else {
> > + crtc_state->vrr.enable = trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE;
> > + }
> > +
> > if (crtc_state->cmrr.enable) {
> > cmrr_n_hi = intel_de_read(dev_priv,
> TRANS_CMRR_N_HI(cpu_transcoder));
> > cmrr_n_lo = intel_de_read(dev_priv,
> > TRANS_CMRR_N_LO(cpu_transcoder));
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers
2023-11-15 8:47 ` Jani Nikula
@ 2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 17+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-11-15 13:50 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org; +Cc: Syrjala, Ville
Thanks @Jani Nikula
Addressed all review comments.
Regards,
Mitul
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, November 15, 2023 2:17 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder
> CMRR registers
>
> On Wed, 15 Nov 2023, Mitul Golani
> <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Add register definitions for Transcoder Fixed Average Vtotal mode/CMRR
> > function, with the necessary bitfields.
> > Compute these registers when CMRR is enabled, extending Adaptive
> > refresh rate capabilities.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++-
> > .../drm/i915/display/intel_display_types.h | 6 +++++
> > drivers/gpu/drm/i915/display/intel_vrr.c | 25 ++++++++++++++++++-
> > drivers/gpu/drm/i915/i915_reg.h | 14 +++++++++++
> > 4 files changed, 66 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 125903007a29..f99d2de840bc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -921,6 +921,13 @@ static bool vrr_params_changed(const struct
> intel_crtc_state *old_crtc_state,
> > old_crtc_state->vrr.pipeline_full !=
> > new_crtc_state->vrr.pipeline_full;
> > }
> >
> > +static bool cmrr_params_changed(const struct intel_crtc_state
> *old_crtc_state,
> > + const struct intel_crtc_state *new_crtc_state)
> {
> > + return old_crtc_state->cmrr.cmrr_m != new_crtc_state-
> >cmrr.cmrr_m ||
> > + old_crtc_state->cmrr.cmrr_n != new_crtc_state-
> >cmrr.cmrr_n; }
> > +
> > static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> > const struct intel_crtc_state *new_crtc_state) { @@
> -5067,6
> > +5074,16 @@ intel_pipe_config_compare(const struct intel_crtc_state
> *current_config,
> > } \
> > } while (0)
> >
> > +#define PIPE_CONF_CHECK_LLI(name) do { \
> > + if (current_config->name != pipe_config->name) { \
> > + pipe_config_mismatch(fastset, crtc, __stringify(name), \
> > + "(expected %lli, found %lli)", \
> > + current_config->name, \
> > + pipe_config->name); \
> > + ret = false; \
> > + } \
> > +} while (0)
> > +
> > #define PIPE_CONF_CHECK_BOOL(name) do { \
> > if (current_config->name != pipe_config->name) { \
> > pipe_config_mismatch(fastset, crtc, __stringify(name), \ @@
> > -5447,10 +5464,13 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_I(vrr.flipline);
> > PIPE_CONF_CHECK_I(vrr.pipeline_full);
> > PIPE_CONF_CHECK_I(vrr.guardband);
> > + PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> > + PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> > }
> >
> > #undef PIPE_CONF_CHECK_X
> > #undef PIPE_CONF_CHECK_I
> > +#undef PIPE_CONF_CHECK_LLI
> > #undef PIPE_CONF_CHECK_BOOL
> > #undef PIPE_CONF_CHECK_BOOL_INCOMPLETE #undef
> PIPE_CONF_CHECK_P @@
> > -6790,7 +6810,8 @@ static void intel_pre_update_crtc(struct
> intel_atomic_state *state,
> > intel_crtc_needs_fastset(new_crtc_state))
> > icl_set_pipe_chicken(new_crtc_state);
> >
> > - if (vrr_params_changed(old_crtc_state, new_crtc_state))
> > + if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
> > + cmrr_params_changed(old_crtc_state, new_crtc_state))
> > intel_vrr_set_transcoder_timings(new_crtc_state);
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 9a44350ba05d..e42a0807227b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1406,6 +1406,12 @@ struct intel_crtc_state {
> > u16 flipline, vmin, vmax, guardband;
> > } vrr;
> >
> > + /* Content Match Refresh Rate state */
> > + struct {
> > + bool enable;
> > + u64 cmrr_n, cmrr_m;
> > + } cmrr;
> > +
> > /* Stream Splitter for eDP MSO */
> > struct {
> > bool enable;
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 5d905f932cb4..4aeccbbf1d2a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -199,6 +199,19 @@ void intel_vrr_set_transcoder_timings(const struct
> intel_crtc_state *crtc_state)
> > return;
> > }
> >
> > + if (crtc_state->cmrr.enable) {
> > + intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> > + VRR_CTL_CMRR_ENABLE |
> trans_vrr_ctl(crtc_state));
> > + intel_de_write(dev_priv,
> TRANS_CMRR_M_HI(cpu_transcoder),
> > + upper_32_bits(crtc_state->cmrr.cmrr_m));
> > + intel_de_write(dev_priv,
> TRANS_CMRR_M_LO(cpu_transcoder),
> > + lower_32_bits(crtc_state->cmrr.cmrr_m));
> > + intel_de_write(dev_priv,
> TRANS_CMRR_N_HI(cpu_transcoder),
> > + upper_32_bits(crtc_state->cmrr.cmrr_n));
> > + intel_de_write(dev_priv,
> TRANS_CMRR_N_LO(cpu_transcoder),
> > + lower_32_bits(crtc_state->cmrr.cmrr_n));
> > + }
> > +
> > intel_de_write(dev_priv, TRANS_VRR_VMIN(cpu_transcoder),
> crtc_state->vrr.vmin - 1);
> > intel_de_write(dev_priv, TRANS_VRR_VMAX(cpu_transcoder),
> crtc_state->vrr.vmax - 1);
> > intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> > trans_vrr_ctl(crtc_state)); @@ -263,12 +276,22 @@ void
> > intel_vrr_get_config(struct intel_crtc_state *crtc_state) {
> > struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc-
> >dev);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > - u32 trans_vrr_ctl;
> > + u32 trans_vrr_ctl, cmrr_m_hi, cmrr_m_lo, cmrr_n_hi, cmrr_n_lo;
> >
> > trans_vrr_ctl = intel_de_read(dev_priv,
> > TRANS_VRR_CTL(cpu_transcoder));
> >
> > crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> >
> > + if (crtc_state->cmrr.enable) {
> > + cmrr_n_hi = intel_de_read(dev_priv,
> TRANS_CMRR_N_HI(cpu_transcoder));
> > + cmrr_n_lo = intel_de_read(dev_priv,
> TRANS_CMRR_N_LO(cpu_transcoder));
> > + cmrr_m_hi = intel_de_read(dev_priv,
> TRANS_CMRR_M_HI(cpu_transcoder));
> > + cmrr_m_lo = intel_de_read(dev_priv,
> > +TRANS_CMRR_M_LO(cpu_transcoder));
> > +
> > + crtc_state->cmrr.cmrr_n = ((u64)cmrr_n_hi << 32) |
> (u64)cmrr_n_lo;
> > + crtc_state->cmrr.cmrr_m = ((u64)cmrr_m_hi << 32) |
> (u64)cmrr_m_lo;
>
> See intel_de_read64_2x32().
>
> > + }
> > +
> > if (DISPLAY_VER(dev_priv) >= 13)
> > crtc_state->vrr.guardband =
> >
> REG_FIELD_GET(XELPD_VRR_CTL_VRR_GUARDBAND_MASK,
> trans_vrr_ctl);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 135e8d8dbdf0..8554083ebf98
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2013,6 +2013,7 @@
> > #define VRR_CTL_VRR_ENABLE REG_BIT(31)
> > #define VRR_CTL_IGN_MAX_SHIFT REG_BIT(30)
> > #define VRR_CTL_FLIP_LINE_EN REG_BIT(29)
> > +#define VRR_CTL_CMRR_ENABLE REG_BIT(27)
> > #define VRR_CTL_PIPELINE_FULL_MASK REG_GENMASK(10, 3)
> > #define VRR_CTL_PIPELINE_FULL(x)
> REG_FIELD_PREP(VRR_CTL_PIPELINE_FULL_MASK, (x))
> > #define VRR_CTL_PIPELINE_FULL_OVERRIDE REG_BIT(0)
> > @@ -2089,6 +2090,19 @@
> > #define TRANS_VRR_STATUS2(trans) _MMIO_TRANS2(trans,
> _TRANS_VRR_STATUS2_A)
> > #define VRR_STATUS2_VERT_LN_CNT_MASK REG_GENMASK(19, 0)
> >
> > +#define _TRANS_CMRR_M_HI_A 0x604F4
> > +#define TRANS_CMRR_M_HI(trans) _MMIO_TRANS2(trans, \
> > + _TRANS_CMRR_M_HI_A)
> > +#define _TRANS_CMRR_N_HI_A 0x604FC
> > +#define TRANS_CMRR_N_HI(trans) _MMIO_TRANS2(trans, \
> > + _TRANS_CMRR_N_HI_A)
> > +#define _TRANS_CMRR_M_LO_A 0x604F0
> > +#define TRANS_CMRR_M_LO(trans) _MMIO_TRANS2(trans, \
> > + _TRANS_CMRR_M_LO_A)
> > +#define _TRANS_CMRR_N_LO_A 0x604F8
> > +#define TRANS_CMRR_N_LO(trans) _MMIO_TRANS2(trans, \
> > + _TRANS_CMRR_N_LO_A)
>
> Please fix indent, and order based on register offset. That'll also group by
> M/N not by HI/LO.
>
> > +
> > #define _TRANS_PUSH_A 0x60A70
> > #define _TRANS_PUSH_B 0x61A70
> > #define _TRANS_PUSH_C 0x62A70
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state
2023-11-15 8:55 ` Jani Nikula
@ 2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 17+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-11-15 13:51 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org; +Cc: Syrjala, Ville
Thanks @Jani Nikula
Addressed all review comments.
Regards,
Mitul
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, November 15, 2023 2:25 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR
> based on VRR state
>
> On Wed, 15 Nov 2023, Mitul Golani
> <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Add CMRR/Fixed Average Vtotal mode enable and disable functions based
> > on change in VRR mode of operation.
> > When Adaptive Sync Vtotal is enabled, Fixed Average Vtotal mode is
> > disabled and vice versa. With this commit setting the stage for
> > subsequent CMRR enablement.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> > .../drm/i915/display/intel_crtc_state_dump.c | 4 +-
> > drivers/gpu/drm/i915/display/intel_display.c | 24 ++++++++++--
> > drivers/gpu/drm/i915/display/intel_vrr.c | 37 +++++++++++++------
> > 3 files changed, 48 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > index 2d15e82c0b3d..908a4c4ccb00 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > @@ -299,7 +299,9 @@ void intel_crtc_state_dump(const struct
> intel_crtc_state *pipe_config,
> > intel_dump_buffer(i915, "ELD: ", pipe_config->eld,
> > drm_eld_size(pipe_config->eld));
> >
> > - drm_dbg_kms(&i915->drm, "vrr: %s, vmin: %d, vmax: %d, pipeline
> full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> > + drm_dbg_kms(&i915->drm,
> > + "cmrr: %s, vrr: %s, vmin: %d, vmax: %d, pipeline full: %d,
> guardband: %d, flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> > + str_yes_no(pipe_config->cmrr.enable),
> > str_yes_no(pipe_config->vrr.enable),
> > pipe_config->vrr.vmin, pipe_config->vrr.vmax,
> > pipe_config->vrr.pipeline_full, pipe_config-
> >vrr.guardband,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index f99d2de840bc..f5a69309e65a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -937,6 +937,12 @@ static bool vrr_enabling(const struct
> intel_crtc_state *old_crtc_state,
> > vrr_params_changed(old_crtc_state, new_crtc_state))); }
> >
> > +static bool cmrr_enabling(const struct intel_crtc_state *old_crtc_state,
> > + const struct intel_crtc_state *new_crtc_state) {
> > + return is_enabling(cmrr.enable, old_crtc_state, new_crtc_state); }
> > +
> > static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
> > const struct intel_crtc_state *new_crtc_state) { @@
> -946,6
> > +952,12 @@ static bool vrr_disabling(const struct intel_crtc_state
> *old_crtc_state,
> > vrr_params_changed(old_crtc_state, new_crtc_state))); }
> >
> > +static bool cmrr_disabling(const struct intel_crtc_state *old_crtc_state,
> > + const struct intel_crtc_state *new_crtc_state) {
> > + return is_disabling(cmrr.enable, old_crtc_state, new_crtc_state); }
> > +
>
> See
> https://patchwork.freedesktop.org/patch/msgid/20231106211915.13406-2-
> ville.syrjala@linux.intel.com
>
> > #undef is_disabling
> > #undef is_enabling
> >
> > @@ -1064,7 +1076,8 @@ static void intel_pre_plane_update(struct
> intel_atomic_state *state,
> > intel_atomic_get_new_crtc_state(state, crtc);
> > enum pipe pipe = crtc->pipe;
> >
> > - if (vrr_disabling(old_crtc_state, new_crtc_state)) {
> > + if (vrr_disabling(old_crtc_state, new_crtc_state) ||
> > + cmrr_disabling(old_crtc_state, new_crtc_state)) {
> > intel_vrr_disable(old_crtc_state);
> > intel_crtc_update_active_timings(old_crtc_state, false);
> > }
> > @@ -6754,7 +6767,8 @@ static void commit_pipe_post_planes(struct
> intel_atomic_state *state,
> > !intel_crtc_needs_modeset(new_crtc_state))
> > skl_detach_scalers(new_crtc_state);
> >
> > - if (vrr_enabling(old_crtc_state, new_crtc_state))
> > + if (vrr_enabling(old_crtc_state, new_crtc_state) ||
> > + cmrr_enabling(old_crtc_state, new_crtc_state))
> > intel_vrr_enable(new_crtc_state);
> > }
> >
> > @@ -6851,9 +6865,11 @@ static void intel_update_crtc(struct
> intel_atomic_state *state,
> > * FIXME Should be synchronized with the start of vblank somehow...
> > */
> > if (vrr_enabling(old_crtc_state, new_crtc_state) ||
> > - new_crtc_state->update_m_n || new_crtc_state->update_lrr)
> > + new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
> > + cmrr_enabling(old_crtc_state, new_crtc_state))
> > intel_crtc_update_active_timings(new_crtc_state,
> > - new_crtc_state->vrr.enable);
> > + new_crtc_state->vrr.enable |
> > + new_crtc_state-
> >cmrr.enable);
>
> Please don't use bitwise OR on booleans.
>
> >
> > /*
> > * We usually enable FIFO underrun interrupts as part of the diff
> > --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 4aeccbbf1d2a..1e33661062b3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -224,7 +224,7 @@ void intel_vrr_send_push(const struct
> intel_crtc_state *crtc_state)
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >
> > - if (!crtc_state->vrr.enable)
> > + if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
>
> Please don't use bitwise OR on booleans.
>
> > return;
> >
> > intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder), @@ -237,7
> > +237,7 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state
> *crtc_state)
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >
> > - if (!crtc_state->vrr.enable)
> > + if (!(crtc_state->vrr.enable | crtc_state->cmrr.enable))
>
> Please don't use bitwise OR on booleans.
>
> > return false;
> >
> > return intel_de_read(dev_priv, TRANS_PUSH(cpu_transcoder)) &
> > TRANS_PUSH_SEND; @@ -245,15 +245,30 @@ bool
> > intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
> >
> > void intel_vrr_enable(const struct intel_crtc_state *crtc_state) {
> > - struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc-
> >dev);
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> Unrelated change.
>
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >
> > - if (!crtc_state->vrr.enable)
> > - return;
> >
> > - intel_de_write(dev_priv, TRANS_PUSH(cpu_transcoder),
> TRANS_PUSH_EN);
> > - intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
> > - VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
> > + if (!crtc_state->cmrr.enable && !crtc_state->vrr.enable) {
> > + return;
> > + } else if (crtc_state->vrr.enable && crtc_state->cmrr.enable) {
> > + drm_WARN_ON(&dev_priv->drm, crtc_state->vrr.enable &&
> > + crtc_state->cmrr.enable);
>
> Please don't duplicate the if and the drm_WARN_ON() conditions. You'll
> want to do this as the first thing with and early return:
>
> if (drm_WARN_ON(...))
> return;
>
> Then you can have two independent blocks:
>
> if (crtc_state->vrr.enable)
> // handle vrr
>
> if (crtc_state->cmrr.enable)
> // handle cmmr
>
> And you can remove the whole complicated if-ladder.
>
> > + } else {
> > + if (!crtc_state->vrr.enable && crtc_state->cmrr.enable) {
> > + intel_de_write(dev_priv,
> > + TRANS_PUSH(cpu_transcoder),
> TRANS_PUSH_EN);
> > + intel_de_write(dev_priv,
> TRANS_VRR_CTL(cpu_transcoder),
> > + VRR_CTL_VRR_ENABLE |
> VRR_CTL_CMRR_ENABLE |
> > + trans_vrr_ctl(crtc_state));
> > + } else {
> > + intel_de_write(dev_priv,
> > + TRANS_PUSH(cpu_transcoder),
> TRANS_PUSH_EN);
> > + intel_de_write(dev_priv,
> TRANS_VRR_CTL(cpu_transcoder),
> > + VRR_CTL_VRR_ENABLE |
> trans_vrr_ctl(crtc_state));
> > + }
> > + }
> > }
> >
> > void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> > @@ -262,7 +277,7 @@ void intel_vrr_disable(const struct intel_crtc_state
> *old_crtc_state)
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
> >
> > - if (!old_crtc_state->vrr.enable)
> > + if (!(old_crtc_state->vrr.enable | old_crtc_state->cmrr.enable))
>
> Please don't use bitwise OR on booleans.
>
> > return;
> >
> > intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder), @@ -
> 280,8
> > +295,6 @@ void intel_vrr_get_config(struct intel_crtc_state
> > *crtc_state)
> >
> > trans_vrr_ctl = intel_de_read(dev_priv,
> > TRANS_VRR_CTL(cpu_transcoder));
> >
> > - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
> > -
>
> Huh?
>
> > if (crtc_state->cmrr.enable) {
> > cmrr_n_hi = intel_de_read(dev_priv,
> TRANS_CMRR_N_HI(cpu_transcoder));
> > cmrr_n_lo = intel_de_read(dev_priv,
> > TRANS_CMRR_N_LO(cpu_transcoder)); @@ -306,6 +319,6 @@ void
> intel_vrr_get_config(struct intel_crtc_state *crtc_state)
> > crtc_state->vrr.vmin = intel_de_read(dev_priv,
> TRANS_VRR_VMIN(cpu_transcoder)) + 1;
> > }
> >
> > - if (crtc_state->vrr.enable)
> > + if (crtc_state->vrr.enable | crtc_state->cmrr.enable)
>
> Please don't use bitwise OR on booleans.
>
> > crtc_state->mode_flags |= I915_MODE_FLAG_VRR; }
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 6:57 ` Ville Syrjälä
@ 2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 17+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-11-15 13:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org
Thanks @Ville Syrjälä
Addressed review comment.
Regards,
Mitul
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, November 15, 2023 12:28 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate
> vtotal
>
> On Wed, Nov 15, 2023 at 12:00:54PM +0530, Mitul Golani wrote:
> > Compute Fixed Average Vtotal/CMRR with resepect to userspace VRR
> > enablement. Also calculate required parameters in case of CMRR is
> > enabled. During intel_vrr_compute_config, CMRR is getting enabled
> > based on userspace has enabled Adaptive Sync Vtotal mode (Legacy VRR)
> > or not.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 1 +
> > .../drm/i915/display/intel_display_device.h | 1 +
> > drivers/gpu/drm/i915/display/intel_vrr.c | 81 +++++++++++++++++--
> > 3 files changed, 78 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index f5a69309e65a..d61790f8ebb4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5479,6 +5479,7 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
> > PIPE_CONF_CHECK_I(vrr.guardband);
> > PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> > PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
> > + PIPE_CONF_CHECK_BOOL(cmrr.enable);
> > }
> >
> > #undef PIPE_CONF_CHECK_X
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> > b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index 4299cc452e05..66cbc3a6bbe8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -68,6 +68,7 @@ struct drm_printer;
> > #define HAS_TRANSCODER(i915, trans)
> ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
> > BIT(trans)) != 0)
> > #define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
> > +#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
> > #define INTEL_NUM_PIPES(i915)
> (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
> > #define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)-
> >has_hotplug)
> > #define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)-
> >overlay_needs_physical)
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 1e33661062b3..4a056a71b68d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct
> intel_crtc_state *crtc_state)
> > return crtc_state->vrr.vmax -
> > intel_vrr_vblank_exit_length(crtc_state);
> > }
> >
> > +static int
> > +is_cmrr_frac_required(struct intel_crtc_state *crtc_state) {
> > + int target_refresh_k, actual_refresh_k;
> > + struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > + target_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
>
> That is just your 'actual_refresh' rounded to the nearest integer. It is *not*
> any kind of target refresh rate.
>
> > + actual_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock *
> 1000,
> > + adjusted_mode->crtc_htotal) * 1000;
> > + actual_refresh_k /= adjusted_mode->crtc_vtotal;
> > +
> > + if (actual_refresh_k == target_refresh_k)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static unsigned int
> > +cmrr_get_vtotal(struct intel_crtc_state *crtc_state) {
> > + unsigned int muliplierM = 1, muliplierN = 1, vtotal;
> > + unsigned int actual_refresh_rate, desired_refresh_rate;
> > + unsigned long long actual_pixel_rate;
> > + struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > + actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock *
> 1000,
> > + adjusted_mode->crtc_htotal) *
> 1000;
> > + actual_refresh_rate /= adjusted_mode->crtc_vtotal;
> > + desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
> > + actual_pixel_rate = actual_refresh_rate * adjusted_mode-
> >crtc_vtotal;
> > + actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal)
> > +/ 1000;
> > +
> > + if (is_cmrr_frac_required(crtc_state)) {
> > + muliplierM = 1001;
> > + muliplierN = 1000;
> > + }
> > +
> > + crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
> > + adjusted_mode->crtc_htotal * muliplierN,
> muliplierM) * muliplierN;
> > + vtotal = DIV_ROUND_UP(actual_pixel_rate * muliplierN, crtc_state-
> >cmrr.cmrr_n);
> > + crtc_state->cmrr.cmrr_m =
> > + (actual_pixel_rate * muliplierM) % crtc_state->cmrr.cmrr_n;
> > +
> > + return vtotal;
> > +}
> > +
> > void
> > intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> > struct drm_connector_state *conn_state) @@ -
> 149,6 +195,27 @@
> > intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> >
> > crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
> >
> > + /*
> > + * When panel is VRR capable and userspace has
> > + * not enabled adaptive sync mode then Fixed Average
> > + * Vtotal mode should be enabled.
> > + */
> > + if (crtc_state->uapi.vrr_enabled) {
> > + crtc_state->vrr.enable = true;
> > + if (HAS_CMRR(i915))
> > + crtc_state->cmrr.enable = false;
> > + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > + } else {
> > + crtc_state->vrr.enable = false;
> > + if (HAS_CMRR(i915)) {
> > + crtc_state->cmrr.enable = true;
> > + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
> > + crtc_state->vrr.vmin = crtc_state->vrr.vmax;
> > + crtc_state->vrr.flipline = crtc_state->vrr.vmin;
> > + crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > + }
> > + }
> > +
> > /*
> > * For XE_LPD+, we use guardband and pipeline override
> > * is deprecated.
> > @@ -161,11 +228,6 @@ intel_vrr_compute_config(struct intel_crtc_state
> *crtc_state,
> > min(255, crtc_state->vrr.vmin - adjusted_mode-
> >crtc_vblank_start -
> > crtc_state->framestart_delay - 1);
> > }
> > -
> > - if (crtc_state->uapi.vrr_enabled) {
> > - crtc_state->vrr.enable = true;
> > - crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> > - }
> > }
> >
> > static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
> > @@ -295,6 +357,15 @@ void intel_vrr_get_config(struct intel_crtc_state
> > *crtc_state)
> >
> > trans_vrr_ctl = intel_de_read(dev_priv,
> > TRANS_VRR_CTL(cpu_transcoder));
> >
> > + if (HAS_CMRR(dev_priv)) {
> > + crtc_state->cmrr.enable = (trans_vrr_ctl &
> VRR_CTL_CMRR_ENABLE) &&
> > + (trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE);
> > + crtc_state->vrr.enable = trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE &&
> > + !(trans_vrr_ctl &
> VRR_CTL_CMRR_ENABLE);
> > + } else {
> > + crtc_state->vrr.enable = trans_vrr_ctl &
> VRR_CTL_VRR_ENABLE;
> > + }
> > +
> > if (crtc_state->cmrr.enable) {
> > cmrr_n_hi = intel_de_read(dev_priv,
> TRANS_CMRR_N_HI(cpu_transcoder));
> > cmrr_n_lo = intel_de_read(dev_priv,
> > TRANS_CMRR_N_LO(cpu_transcoder));
> > --
> > 2.25.1
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 15:49 [Intel-gfx] [RFC 0/3] " Mitul Golani
@ 2023-11-15 15:49 ` Mitul Golani
2023-11-15 20:19 ` kernel test robot
0 siblings, 1 reply; 17+ messages in thread
From: Mitul Golani @ 2023-11-15 15:49 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, ville.syrjala
Compute Fixed Average Vtotal/CMRR with resepect to
userspace VRR enablement. Also calculate required
parameters in case of CMRR is enabled. During
intel_vrr_compute_config, CMRR is getting enabled
based on userspace has enabled Adaptive Sync Vtotal
mode (Legacy VRR) or not.
--v2:
- Update is_cmrr_frac_required function return as bool, not int. [Jani]
- Use signed int math instead of unsigned in cmrr_get_vtotal2. [Jani]
- Fix typo and usage of camel case in cmrr_get_vtotal. [Jani]
- Use do_div in cmrr_get_vtotalwhile calculating cmrr_m. [ Jani]
- Simplify cmrr and vrr compute config in intel_vrr_compute_config. [Jani]
- Correct valiable name usage in is_cmrr_frac_required. [Ville]
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 1 +
.../drm/i915/display/intel_display_device.h | 1 +
drivers/gpu/drm/i915/display/intel_vrr.c | 76 +++++++++++++++++--
3 files changed, 72 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 31ee7984a5fd..84990fffe90a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5485,6 +5485,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(vrr.guardband);
PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
+ PIPE_CONF_CHECK_BOOL(cmrr.enable);
}
#undef PIPE_CONF_CHECK_X
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 4299cc452e05..66cbc3a6bbe8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -68,6 +68,7 @@ struct drm_printer;
#define HAS_TRANSCODER(i915, trans) ((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
BIT(trans)) != 0)
#define HAS_VRR(i915) (DISPLAY_VER(i915) >= 11)
+#define HAS_CMRR(i915) (DISPLAY_VER(i915) >= 20)
#define INTEL_NUM_PIPES(i915) (hweight8(DISPLAY_RUNTIME_INFO(i915)->pipe_mask))
#define I915_HAS_HOTPLUG(i915) (DISPLAY_INFO(i915)->has_hotplug)
#define OVERLAY_NEEDS_PHYSICAL(i915) (DISPLAY_INFO(i915)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 8f1d241e1f79..554e39bb4d94 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -105,6 +105,52 @@ int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state)
return crtc_state->vrr.vmax - intel_vrr_vblank_exit_length(crtc_state);
}
+static bool
+is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
+{
+ int calculated_refresh_k, actual_refresh_k;
+ struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+ actual_refresh_k = drm_mode_vrefresh(adjusted_mode) * 1000;
+ calculated_refresh_k = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+ adjusted_mode->crtc_htotal) * 1000;
+ calculated_refresh_k /= adjusted_mode->crtc_vtotal;
+
+ if (calculated_refresh_k == actual_refresh_k)
+ return false;
+
+ return true;
+}
+
+static unsigned int
+cmrr_get_vtotal(struct intel_crtc_state *crtc_state)
+{
+ int multiplier_m = 1, multiplier_n = 1, vtotal;
+ int actual_refresh_rate, desired_refresh_rate;
+ long long actual_pixel_rate, adjusted_pixel_rate;
+ struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+ actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+ adjusted_mode->crtc_htotal) * 1000;
+ actual_refresh_rate /= adjusted_mode->crtc_vtotal;
+ desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
+ actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal;
+ actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal) / 1000;
+
+ if (is_cmrr_frac_required(crtc_state)) {
+ multiplier_m = 1001;
+ multiplier_n = 1000;
+ }
+
+ crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
+ adjusted_mode->crtc_htotal * multiplier_n, multiplier_m) * multiplier_n;
+ vtotal = DIV_ROUND_UP(actual_pixel_rate * multiplier_n, crtc_state->cmrr.cmrr_n);
+ adjusted_pixel_rate = actual_pixel_rate * multiplier_m;
+ crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, crtc_state->cmrr.cmrr_n);
+
+ return vtotal;
+}
+
void
intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
@@ -149,6 +195,22 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
+ /*
+ * When panel is VRR capable and userspace has
+ * not enabled adaptive sync mode then Fixed Average
+ * Vtotal mode should be enabled.
+ */
+ if (crtc_state->uapi.vrr_enabled) {
+ crtc_state->vrr.enable = true;
+ crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+ } else if (HAS_CMRR(i915)) {
+ crtc_state->cmrr.enable = true;
+ crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state);
+ crtc_state->vrr.vmin = crtc_state->vrr.vmax;
+ crtc_state->vrr.flipline = crtc_state->vrr.vmin;
+ crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+ }
+
/*
* For XE_LPD+, we use guardband and pipeline override
* is deprecated.
@@ -161,11 +223,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start -
crtc_state->framestart_delay - 1);
}
-
- if (crtc_state->uapi.vrr_enabled) {
- crtc_state->vrr.enable = true;
- crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
- }
}
static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
@@ -292,7 +349,14 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(cpu_transcoder));
- crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+ if (HAS_CMRR(dev_priv)) {
+ crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE) &&
+ (trans_vrr_ctl & VRR_CTL_VRR_ENABLE);
+ crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
+ !(trans_vrr_ctl & VRR_CTL_CMRR_ENABLE);
+ } else {
+ crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+ }
if (crtc_state->cmrr.enable) {
crtc_state->cmrr.cmrr_n =
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
2023-11-15 15:49 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
@ 2023-11-15 20:19 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-11-15 20:19 UTC (permalink / raw)
To: Mitul Golani; +Cc: oe-kbuild-all
Hi Mitul,
[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-i915-Define-and-compute-Transcoder-CMRR-registers/20231115-235840
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/20231115154943.3801663-4-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal
config: i386-randconfig-001-20231116 (https://download.01.org/0day-ci/archive/20231116/202311160454.ni1YkCWj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/202311160454.ni1YkCWj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311160454.ni1YkCWj-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/gpu/drm/i915/display/intel_vrr.o: in function `cmrr_get_vtotal':
>> drivers/gpu/drm/i915/display/intel_vrr.c:138: undefined reference to `__divdi3'
ld: drivers/gpu/drm/i915/display/intel_vrr.c:147: undefined reference to `__udivdi3'
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for DRM_I915_DEBUG_GEM
Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=y] && EXPERT [=y] && DRM_I915_WERROR [=n]
Selected by [y]:
- DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && DRM_I915 [=y] && EXPERT [=y] && !COMPILE_TEST [=n]
vim +138 drivers/gpu/drm/i915/display/intel_vrr.c
124
125 static unsigned int
126 cmrr_get_vtotal(struct intel_crtc_state *crtc_state)
127 {
128 int multiplier_m = 1, multiplier_n = 1, vtotal;
129 int actual_refresh_rate, desired_refresh_rate;
130 long long actual_pixel_rate, adjusted_pixel_rate;
131 struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
132
133 actual_refresh_rate = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
134 adjusted_mode->crtc_htotal) * 1000;
135 actual_refresh_rate /= adjusted_mode->crtc_vtotal;
136 desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
137 actual_pixel_rate = actual_refresh_rate * adjusted_mode->crtc_vtotal;
> 138 actual_pixel_rate = (actual_pixel_rate * adjusted_mode->crtc_htotal) / 1000;
139
140 if (is_cmrr_frac_required(crtc_state)) {
141 multiplier_m = 1001;
142 multiplier_n = 1000;
143 }
144
145 crtc_state->cmrr.cmrr_n = DIV_ROUND_UP(desired_refresh_rate *
146 adjusted_mode->crtc_htotal * multiplier_n, multiplier_m) * multiplier_n;
147 vtotal = DIV_ROUND_UP(actual_pixel_rate * multiplier_n, crtc_state->cmrr.cmrr_n);
148 adjusted_pixel_rate = actual_pixel_rate * multiplier_m;
149 crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, crtc_state->cmrr.cmrr_n);
150
151 return vtotal;
152 }
153
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-11-15 20:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-15 6:30 [Intel-gfx] [RFC 0/3] Implement CMRR Support Mitul Golani
2023-11-15 6:30 ` [Intel-gfx] [RFC 1/3] drm/i915: Define and compute Transcoder CMRR registers Mitul Golani
2023-11-15 8:47 ` Jani Nikula
2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
2023-11-15 6:30 ` [Intel-gfx] [RFC 2/3] drm/i915: Add Enable/Disable for CMRR based on VRR state Mitul Golani
2023-11-15 8:55 ` Jani Nikula
2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
2023-11-15 6:30 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
2023-11-15 6:57 ` Ville Syrjälä
2023-11-15 13:51 ` Golani, Mitulkumar Ajitkumar
2023-11-15 9:14 ` Jani Nikula
2023-11-15 13:50 ` Golani, Mitulkumar Ajitkumar
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Implement CMRR Support Patchwork
2023-11-15 9:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-11-15 9:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2023-11-15 15:49 [Intel-gfx] [RFC 0/3] " Mitul Golani
2023-11-15 15:49 ` [Intel-gfx] [RFC 3/3] drm/i915: Compute CMRR and calculate vtotal Mitul Golani
2023-11-15 20:19 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.