On 11/28/2025 6:40 PM, Nautiyal, Ankit K wrote:
On 11/27/2025 2:46 PM, Mitul Golani wrote:
Compute DC Balance parameters and tunable params based on
experiments.
--v2:
- Document tunable params. (Ankit)
--v3:
- Add line spaces to compute config. (Ankit)
- Remove redundancy checks.
--v4:
- Separate out conpute config to separate function.
- As all the valuse are being computed in scanlines, and slope
is still in usec, convert and store it to scanlines.
--v5:
- Update and add comments for slope calculation. (Ankit)
- Update early return conditions for dc balance compute. (Ankit)
Signed-off-by: Mitul Golani <[email protected]>
---
drivers/gpu/drm/i915/display/intel_vrr.c | 46 ++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 650077eb280f..45e632e8a981 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -6,6 +6,7 @@
#include <drm/drm_print.h>
+#include "intel_crtc.h"
#include "intel_de.h"
#include "intel_display_regs.h"
#include "intel_display_types.h"
@@ -20,6 +21,14 @@
#define FIXED_POINT_PRECISION 100
#define CMRR_PRECISION_TOLERANCE 10
+/*
+ * Tunable parameters for DC Balance correction.
+ * These are captured based on experimentations.
+ */
+#define DCB_CORRECTION_SENSITIVITY 30
+#define DCB_CORRECTION_AGGRESSIVENESS 1000
+#define DCB_BLANK_TARGET 50
+
bool intel_vrr_is_capable(struct intel_connector *connector)
{
struct intel_display *display = to_intel_display(connector);
@@ -342,6 +351,41 @@ int intel_vrr_compute_vmax(struct
intel_connector *connector,
return vmax;
}
+static void
+intel_vrr_dc_balance_compute_config(struct intel_crtc_state
*crtc_state)
+{
+ int guardband_usec, adjustment_usec;
+ struct intel_display *display = to_intel_display(crtc_state);
+ struct drm_display_mode *adjusted_mode =
&crtc_state->hw.adjusted_mode;
+
+ if (!(HAS_VRR_DC_BALANCE(display) && crtc_state->vrr.enable))
+ return;
Can simplify to:
if (!HAS_VRR_DC_BALANCE(display) || !crtc_state->vrr.enable)
return;
I think we can introduce intel_vrr_dc_balance_possible() here itself,
rather than later.
So:
if (!intel_vrr_dc_balance_possible() || !crtc_state->vrr.enable)
return;
Regards,
Ankit
IMO, if (notA or not B) is more readable than: if not (A and B)
+
+ crtc_state->vrr.dc_balance.vmax = crtc_state->vrr.vmax;
+ crtc_state->vrr.dc_balance.vmin = crtc_state->vrr.vmin;
+ crtc_state->vrr.dc_balance.max_increase =
+ crtc_state->vrr.vmax - crtc_state->vrr.vmin;
+ crtc_state->vrr.dc_balance.max_decrease =
+ crtc_state->vrr.vmax - crtc_state->vrr.vmin;
+ crtc_state->vrr.dc_balance.guardband =
+ DIV_ROUND_UP(crtc_state->vrr.dc_balance.vmax *
+ DCB_CORRECTION_SENSITIVITY, 100);
+ guardband_usec =
+ intel_scanlines_to_usecs(adjusted_mode,
+ crtc_state->vrr.dc_balance.guardband);
+ /*
+ * The correction_aggressiveness/100 is the number of
milliseconds to
+ * adjust by when the balance is at twice the guardband.
+ * guardband_slope = correction_aggressiveness / (guardband * 100)
+ */
+ adjustment_usec = DCB_CORRECTION_AGGRESSIVENESS * 10;
The current value represents milliseconds x 100, so 10 msecs becomes
1000.
This scaling can be confusing compared to working directly with
microseconds or milliseconds.
IMO ideally we could define the correction aggressiveness in either
usecs or msecs for clarity, but that might make it harder to match
values from the spec.
If this factor changes in the future (e.g., to 400 or 1400 based on
experimentation), we might need to recalculate if we switch to pure
msecs or usecs.
However, I think it would still be clearer if we rename the macro to:
#define DCB_CORRECTION_AGGRESSIVENESS_msecs_X100 1000
Then, when we use:
adjustment_usec = DCB_CORRECTION_AGGRESSIVENESS_msecs_X100 * 10;
it becomes more intuitive because we can see the conversion: msecs x
100 x 10 -> usecs
Regards,
Ankit
+ crtc_state->vrr.dc_balance.slope =
+ DIV_ROUND_UP(adjustment_usec, guardband_usec);
+ crtc_state->vrr.dc_balance.vblank_target =
+ DIV_ROUND_UP((crtc_state->vrr.vmax - crtc_state->vrr.vmin) *
+ DCB_BLANK_TARGET, 100);
+}
+
void
intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
@@ -399,6 +443,8 @@ intel_vrr_compute_config(struct intel_crtc_state
*crtc_state,
(crtc_state->hw.adjusted_mode.crtc_vtotal -
crtc_state->hw.adjusted_mode.crtc_vsync_end);
}
+
+ intel_vrr_dc_balance_compute_config(crtc_state);
}
static int
