Re: [PATCH 01/23] drm/i915/vrr: Refactor VRR Timing Computation

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

On 11/11/2024 11:18 PM, Garg, Nemesa wrote:
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ankit
>> Nautiyal
>> Sent: Monday, November 11, 2024 2:42 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: intel-xe@lists.freedesktop.org; jani.nikula@linux.intel.com;
>> ville.syrjala@linux.intel.com; Golani, Mitulkumar Ajitkumar
>> <mitulkumar.ajitkumar.golani@intel.com>
>> Subject: [PATCH 01/23] drm/i915/vrr: Refactor VRR Timing Computation
>>
>> Introduce helper functions to compute timins gfor different mode of operation of
>> VRR timing generator.
>>
> Typo : timing

Thanks for catching this, will fix this in next version.

Regards,

Ankit

>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 115 +++++++++++++++--------
>>   1 file changed, 75 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
>> b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 19a5d0076bb8..defe346b0261 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -161,6 +161,73 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state,
>> bool video_mode_required)
>>   	return vtotal;
>>   }
>>
>> +static
>> +void intel_vrr_compute_cmrr_timings(struct intel_crtc_state
>> +*crtc_state) {
>> +	crtc_state->vrr.enable = true;
>> +	crtc_state->cmrr.enable = true;
>> +	/*
>> +	 * TODO: Compute precise target refresh rate to determine
>> +	 * if video_mode_required should be true. Currently set to
>> +	 * false due to uncertainty about the precise target
>> +	 * refresh Rate.
>> +	 */
>> +	crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state, false);
>> +	crtc_state->vrr.vmin = crtc_state->vrr.vmax;
>> +	crtc_state->vrr.flipline = crtc_state->vrr.vmin;
>> +	crtc_state->mode_flags |= I915_MODE_FLAG_VRR; }
>> +
>> +static
>> +int intel_vrr_compute_vmin(struct intel_connector *connector,
>> +			   struct drm_display_mode *adjusted_mode) {
>> +	int vmin;
>> +	const struct drm_display_info *info = &connector->base.display_info;
>> +
>> +	vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
>> +			    adjusted_mode->crtc_htotal * info-
>>> monitor_range.max_vfreq);
>> +	vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
>> +
>> +	return vmin;
>> +}
>> +
>> +static
>> +int intel_vrr_compute_vmax(struct intel_connector *connector,
>> +			   struct drm_display_mode *adjusted_mode) {
>> +	int vmax;
>> +	const struct drm_display_info *info = &connector->base.display_info;
>> +
>> +	vmax = adjusted_mode->crtc_clock * 1000 /
>> +		(adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
>> +
>> +	vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
>> +
>> +	return vmax;
>> +}
>> +
>> +static
>> +void intel_vrr_prepare_vrr_timings(struct intel_crtc_state *crtc_state,
>> +int vmin, int vmax) {
>> +	/*
>> +	 * flipline determines the min vblank length the hardware will
>> +	 * generate, and flipline>=vmin+1, hence we reduce vmin by one
>> +	 * to make sure we can get the actual min vblank length.
>> +	 */
>> +	crtc_state->vrr.vmin = vmin - 1;
>> +	crtc_state->vrr.vmax = vmax;
>> +	crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1; }
>> +
>> +static
>> +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state,
>> +int vmin, int vmax) {
>> +	intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
>> +	crtc_state->vrr.enable = true;
>> +	crtc_state->mode_flags |= I915_MODE_FLAG_VRR; }
>> +
>>   void
>>   intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   			 struct drm_connector_state *conn_state) @@ -171,7
>> +238,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>   	bool is_edp = intel_dp_is_edp(intel_dp);
>>   	struct drm_display_mode *adjusted_mode = &crtc_state-
>>> hw.adjusted_mode;
>> -	const struct drm_display_info *info = &connector->base.display_info;
>>   	int vmin, vmax;
>>
>>   	/*
>> @@ -192,49 +258,18 @@ intel_vrr_compute_config(struct intel_crtc_state
>> *crtc_state,
>>   	if (HAS_LRR(display))
>>   		crtc_state->update_lrr = true;
>>
>> -	vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
>> -			    adjusted_mode->crtc_htotal * info-
>>> monitor_range.max_vfreq);
>> -	vmax = adjusted_mode->crtc_clock * 1000 /
>> -		(adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
>> -
>> -	vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
>> -	vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
>> +	vmin = intel_vrr_compute_vmin(connector, adjusted_mode);
>> +	vmax = intel_vrr_compute_vmax(connector, adjusted_mode);
>>
>>   	if (vmin >= vmax)
>>   		return;
>>
>> -	/*
>> -	 * flipline determines the min vblank length the hardware will
>> -	 * generate, and flipline>=vmin+1, hence we reduce vmin by one
>> -	 * to make sure we can get the actual min vblank length.
>> -	 */
>> -	crtc_state->vrr.vmin = vmin - 1;
>> -	crtc_state->vrr.vmax = vmax;
>> -
>> -	crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
>> -
>> -	/*
>> -	 * When panel is VRR capable and userspace has
>> -	 * not enabled adaptive sync mode then Fixed Average
>> -	 * Vtotal mode should be enabled.
>> -	 */
>> -	if (crtc_state->uapi.vrr_enabled) {
>> -		crtc_state->vrr.enable = true;
>> -		crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
>> -	} else if (is_cmrr_frac_required(crtc_state) && is_edp) {
>> -		crtc_state->vrr.enable = true;
>> -		crtc_state->cmrr.enable = true;
>> -		/*
>> -		 * TODO: Compute precise target refresh rate to determine
>> -		 * if video_mode_required should be true. Currently set to
>> -		 * false due to uncertainty about the precise target
>> -		 * refresh Rate.
>> -		 */
>> -		crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state, false);
>> -		crtc_state->vrr.vmin = crtc_state->vrr.vmax;
>> -		crtc_state->vrr.flipline = crtc_state->vrr.vmin;
>> -		crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
>> -	}
>> +	if (crtc_state->uapi.vrr_enabled)
>> +		intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax);
>> +	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>> +		intel_vrr_compute_cmrr_timings(crtc_state);
>> +	else
>> +		intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
>>
>>   	if (intel_dp->as_sdp_supported && crtc_state->vrr.enable) {
>>   		crtc_state->vrr.vsync_start =
>> --
>> 2.45.2