Re: [PATCH 2/2] drm/i915/display: Force full modeset for eDP

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 06 Feb 2024, Mika Kahola <mika.kahola@intel.com> wrote:
> Force full modeset for eDP when booting up. GOP programs
> PLL parameters and hence, we would be able to use fastset
> for eDP. However, with fastset we are not setting PLL values
> from the driver and rely that GOP and driver PLL values match.
> We have discovered that with some of the panels this is not
> true and hence we would need to program PLL values by the
> driver. The patch suggests a workaround as enabling full
> modeset when booting up. This way we force the driver to
> write the PLL values to the hw.

No. We want to avoid full modesets if possible, both in general and at
probe.

And when we do end up with modesets, the decision needs to be based on
changes in the state that we can't write to the hardware without a
modeset.

We can't unconditionally force a modeset on eDP panels at probe.


BR,
Jani.

>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ab415f41924d..9699ded1eb5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3319,6 +3319,7 @@ bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
>  	 * of crtc_state->dsc, we have no way to ensure reliable fastset.
>  	 * Remove once we have readout for DSC.
>  	 */
> +
>  	if (crtc_state->dsc.compression_enable) {
>  		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] Forcing full modeset due to DSC being enabled\n",
>  			    encoder->base.base.id, encoder->base.name);
> @@ -3326,6 +3327,18 @@ bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
>  		fastset = false;
>  	}
>  
> +	/*
> +	 * FIXME hack to force full modeset for eDP as not always BIOS written PLL
> +	 * values does not match with the ones defined in the driver code
> +	 */
> +	if (!crtc_state->uapi.mode_changed &&
> +	    intel_dp_is_edp(intel_dp)) {
> +		drm_dbg_kms(&i915->drm, "Forcing full modeset for eDP\n");
> +		crtc_state->uapi.mode_changed = true;
> +		fastset = false;
> +	}
> +
> +
>  	return fastset;
>  }

-- 
Jani Nikula, Intel