Re: [PATCH 08/20] drm/i915/display: Enable MSA Ignore Timing PAR only when in not fixed_rr mode

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Feb 24, 2025 at 11:47:05AM +0530, Ankit Nautiyal wrote:
> MSA Ignore Timing PAR enable is set in the DP sink when we enable variable
> refresh rate.
> 
> Currently for link training we depend on flipline to decide whether we
> want to ignore the msa timings. With fixed refresh rate we will still
> fill the flipline in all cases whether panel supports VRR or not.
> 
> Change the condition for link training to ignore the msa timings if
> vrr.in_range.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 3cc06c916017..549e4ebd9404 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -725,7 +725,7 @@ static void intel_dp_update_downspread_ctrl(struct intel_dp *intel_dp,
>  					    const struct intel_crtc_state *crtc_state)
>  {
>  	intel_dp_link_training_set_mode(intel_dp,
> -					crtc_state->port_clock, crtc_state->vrr.flipline);
> +					crtc_state->port_clock, crtc_state->vrr.in_range);

I think one thing we are perhaps missing is the readout of
vrr.in_range. But we can't really read that out during driver
load since we don't have the connector information read in yet.
So if we do end up doing a modeset during initial_commit() we'll
clear the MSA ignore bit. But I guess the GOP probably wouldn't
even set that bit.

Where things can go wrong is when we do enable actual VRR mode
and there hasn't been any modesets since the initial_commit().
We might not have to perform a full modeset to enable VRR, and
thus we would never set the MSA ignore bit.

So I think we will need some kind of fastset support for this
DPCD write after all. Though we'd only have to do it on the
first real commit when we clear the inherited flag.

But this is a pre-existing problem so not something we have
to deal with this in this patch. Just something to put on the
TODO list...

This patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  void intel_dp_link_training_set_bw(struct intel_dp *intel_dp,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel