Re: [PATCH v10 17/17] drm/i915/vrr: Enable DC Balance

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

On 12/2/2025 1:06 PM, Mitul Golani wrote:
> Enable DC Balance from vrr compute config and related hw flag.
> Also to add pipe restrictions along with this.
>
> --v2:
> - Use dc balance check instead of source restriction.
> --v3:
> - Club pipe restriction check with dc balance enablement. (Ankit)
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_vrr.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index ba8b3c664e70..db74744ddb31 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -399,6 +399,7 @@ intel_vrr_dc_balance_compute_config(struct intel_crtc_state *crtc_state)
>   	crtc_state->vrr.dc_balance.vblank_target =
>   		DIV_ROUND_UP((crtc_state->vrr.vmax - crtc_state->vrr.vmin) *
>   			     DCB_BLANK_TARGET, 100);
> +	crtc_state->vrr.dc_balance.enable = true;
>   }
>   
>   void
> @@ -789,6 +790,7 @@ intel_vrr_enable_dc_balancing(const struct intel_crtc_state *crtc_state)
>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>   	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>   	enum pipe pipe = crtc->pipe;
> +	u32 vrr_ctl = intel_de_read(display, TRANS_VRR_CTL(display, cpu_transcoder));

As Uma pointed out, it would be better to use trans_vrr_ctl() here.


>   
>   	if (!crtc_state->vrr.dc_balance.enable)
>   		return;
> @@ -827,6 +829,9 @@ intel_vrr_enable_dc_balancing(const struct intel_crtc_state *crtc_state)
>   	intel_de_write(display, TRANS_ADAPTIVE_SYNC_DCB_CTL(cpu_transcoder),
>   		       ADAPTIVE_SYNC_COUNTER_EN);
>   	intel_pipedmc_dcb_enable(NULL, crtc);
> +
> +	vrr_ctl |= VRR_CTL_DCB_ADJ_ENABLE;
> +	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), vrr_ctl);
>   }
>   
>   static void
> @@ -836,6 +841,7 @@ intel_vrr_disable_dc_balancing(const struct intel_crtc_state *old_crtc_state)
>   	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>   	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
>   	enum pipe pipe = crtc->pipe;
> +	u32 vrr_ctl = intel_de_read(display, TRANS_VRR_CTL(display, cpu_transcoder));
>   
>   	if (!old_crtc_state->vrr.dc_balance.enable)
>   		return;
> @@ -858,6 +864,9 @@ intel_vrr_disable_dc_balancing(const struct intel_crtc_state *old_crtc_state)
>   	intel_de_write(display, TRANS_VRR_DCB_ADJ_FLIPLINE_CFG(cpu_transcoder), 0);
>   	intel_de_write(display, TRANS_VRR_DCB_VMAX(cpu_transcoder), 0);
>   	intel_de_write(display, TRANS_VRR_DCB_FLIPLINE(cpu_transcoder), 0);
> +
> +	vrr_ctl &= ~VRR_CTL_DCB_ADJ_ENABLE;
> +	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), vrr_ctl);
>   }
>   
>   static void intel_vrr_tg_enable(const struct intel_crtc_state *crtc_state,
> @@ -963,7 +972,7 @@ void intel_vrr_get_dc_balance_config(struct intel_crtc_state *crtc_state)
>   	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>   	enum pipe pipe = crtc->pipe;
>   
> -	if (!HAS_VRR_DC_BALANCE(display))
> +	if (!intel_vrr_dc_balance_possible(crtc_state))
>   		return;
This change looks out of place.
Hmm .. In Patch#5, intel_vrr_get_dc_balance_config() is added, but at 
that point intel_vrr_dc_balance_possible() is not yet defined.
I think the above update should either:

Move to Patch#7 (where the helper is introduced), or
Introduce the helper in a separate patch placed right after Patch#7 but 
before the enablement patch.

PS:
My earlier suggestion in [1] was to introduce the helper sooner to avoid 
having it as the last patch.
This was to make sure we have pipe restriction in place before the 
enablement logic lands, to avoid any risk enabling DC balancing on 
unsupported pipes.
Given the current series structure, moving this change to Patch#7 or 
introducing a separate patch right after Patch#7 (before enablement) 
seems a better idea.

[1] https://patchwork.freedesktop.org/patch/690789/?series=158156&rev=1


Regards,
Ankit
>   
>   	reg_val = intel_de_read(display, PIPEDMC_DCB_VMIN(pipe));