Re: [PATCH v8 07/18] drm/i915/vrr: Add compute config for DC Balance params

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 11/17/2025 11:14 AM, 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.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 650077eb280f..7cb484dd96df 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


As mentioned in comment in last version, we can just have this value as 
1000 * 10 instead of multiplying 10 where we are using this macro.

This is a bit unclear to me. More about this below.


> +#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,33 @@ 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 val;
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> +	if (!crtc_state->vrr.dc_balance.enable)
> +		return;

Here check should be for crtc_state->vrr.enable, and 
HAS_VRR_DC_BALANCE(display) and return early if these are not true.
I think this vrr.dc_balance.enable should be set in this function, 
perhaps not in this patch, but in the last patch.
Currently its set in intel_vrr_compute_vrr_timings() in #patch17.



> +
> +	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);
> +	val = DIV_ROUND_UP(DCB_CORRECTION_AGGRESSIVENESS * 10,
> +			   crtc_state->vrr.dc_balance.guardband);
> +	crtc_state->vrr.dc_balance.slope =
> +		intel_usecs_to_scanlines(adjusted_mode, val);


This needs to be written with more clarity.

Perhaps need some comments to explain what is happening.

DCB_CORRECTION_AGGRESSIVENESS is the number of millisecs to adjust when balance is twice the guardband, as per our settings we perhaps want 10msec.

Slope is the ratio between Agressiveness msecs : Guardband msecs.
We can use ratio of agressiveness usecs : guardband usecs.

Currently guardband is in lines, (30% of vmax lines to be precise)

guardband_usecs = intel_usecs_to_scanlines(adjusted_mode, crtc_state->vrr.dc_balance.guardband);
agressiveness usecs = (10 msec) * 1000 = 10000 usecs;

slope = DIV_ROUND_UP(agressiveness_usecs, guardband_usecs)

So IMO name the macro 	10 * 1000
Use slope as ratio DCB_CORRECTION_AGGRESSIVENESS_USECS : guardband_usecs

Regards,
Ankit


> +	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 +435,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