> -----Original Message----- > From: Nautiyal, Ankit K <[email protected]> > Sent: 16 December 2025 18:01 > To: Golani, Mitulkumar Ajitkumar <[email protected]>; > [email protected] > Cc: [email protected]; [email protected]; Shankar, > Uma > <[email protected]>; Nikula, Jani <[email protected]> > Subject: Re: [PATCH v10 17/17] drm/i915/vrr: Enable DC Balance > > > 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 <[email protected]> > > --- > > 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
Sure Ankit, I will a separate patch after patch 7. Thanks for the reviews.. Regards, Mitul > > > Regards, > Ankit > > > > reg_val = intel_de_read(display, PIPEDMC_DCB_VMIN(pipe));
