Re: [PATCH v2 11/13] drm/i915/vrr: Add state checker for dc balance params

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

On 4/23/2025 3:51 PM, Jani Nikula wrote:
> On Wed, 23 Apr 2025, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>> On 4/21/2025 9:18 PM, Mitul Golani wrote:
>>> Add state checker for dc balance params. Also add macro to
>>> check source support.
>>>
>>> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_display.c |  7 +++++++
>>>    drivers/gpu/drm/i915/display/intel_vrr.c     | 20 +++++++++++++++++++-
>>>    2 files changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 274d01552ccf..4a21acb88aa7 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -5402,6 +5402,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>>>    		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
>>>    		PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
>>>    		PIPE_CONF_CHECK_BOOL(cmrr.enable);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.vmin);
>> Add check for vrr.dc_balance.enable?
> They should match regardless, no? Something's wrong if they differ.


Hmm... as I understand when we are in fixed refresh rate mode this will 
not be enabled.

This gets enabled only when VRR is enabled from the user-space, so they 
can be different.

Regards,

Ankit



>
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.vmax);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.guardband);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.slope);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.max_increase);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.max_decrease);
>>> +		PIPE_CONF_CHECK_I(vrr.dc_balance.vblank_target);
>>>    	}
>>>    
>>>    	if (!fastset || intel_vrr_always_use_vrr_tg(display)) {
>>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>>> index fb96d03bbf03..e8802348e5fa 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>>> @@ -740,7 +740,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>>    {
>>>    	struct intel_display *display = to_intel_display(crtc_state);
>>>    	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>> -	u32 trans_vrr_ctl, trans_vrr_vsync;
>>> +	u32 trans_vrr_ctl, trans_vrr_vsync, dcb_ctl;
>>>    	bool vrr_enable;
>>>    
>>>    	trans_vrr_ctl = intel_de_read(display,
>>> @@ -802,6 +802,24 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>>    	else
>>>    		crtc_state->vrr.enable = vrr_enable;
>>>    
>>> +	if (HAS_DC_BALANCE(display)) {
>>> +		dcb_ctl = intel_de_read(display, PIPEDMC_DCB_CTL(display, cpu_transcoder));
>> dsb_ctl is not used. Need to use it for vrr.dc_balance.enable.
>>
>>
>> IMO we can introduce the new members (which is done in Patch#4), readout
>> and state checker in one patch.
>>
>> Regards,
>>
>> Ankit
>>
>>
>>> +		crtc_state->vrr.dc_balance.vmin =
>>> +			intel_de_read(display, PIPEDMC_DCB_VMIN(display, cpu_transcoder)) + 1;
>>> +		crtc_state->vrr.dc_balance.vmax =
>>> +			intel_de_read(display, PIPEDMC_DCB_VMAX(display, cpu_transcoder)) + 1;
>>> +		crtc_state->vrr.dc_balance.guardband =
>>> +			intel_de_read(display, PIPEDMC_DCB_GUARDBAND(display, cpu_transcoder));
>>> +		crtc_state->vrr.dc_balance.max_increase =
>>> +			intel_de_read(display, PIPEDMC_DCB_MAX_INCREASE(display, cpu_transcoder));
>>> +		crtc_state->vrr.dc_balance.max_decrease =
>>> +			intel_de_read(display, PIPEDMC_DCB_MAX_DECREASE(display, cpu_transcoder));
>>> +		crtc_state->vrr.dc_balance.slope =
>>> +			intel_de_read(display, PIPEDMC_DCB_SLOPE(display, cpu_transcoder));
>>> +		crtc_state->vrr.dc_balance.vblank_target =
>>> +			intel_de_read(display, PIPEDMC_DCB_VBLANK(display, cpu_transcoder));
>>> +	}
>>> +
>>>    	/*
>>>    	 * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags.
>>>    	 * Since CMRR is currently disabled, set this flag for VRR for now.