Re: [Intel-gfx] [PATCH v3 10/18] drm/i915/pps: Reinit PPS delays after VBT has been fully parsed

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

On Tue, 26 Apr 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> During the eDP probe we may not yet know the panel_type used
> to index the VBT panel tables. So the initial eDP probe will have
> to be done without that, and thus we won't yet have the PPS delays
> from the VBT. Once the VBT has been fully parse we should reinit
> the PPS delays to make sure it's fully accounted for.
>
> TODO: I wonder if we should do the eDP probe with some super safe
> PPS delayes (eg. max of all VBT PPS delays) just to make sure we
> don't violate the timings. Though typically the VBIOS/GOP do leave
> VDD enabled after boot in which case we don't actually have to care
> about the delays at all.

The trouble here is that the first init writes the registers, and the
second init reads them back as "bios set values", and never goes lower
than them. The late init can only increase the values based on VBT data.

So I'm holding off on the r-b on both the PPS changes for now because I
think this will need some kind of a redesign. Maybe we'll need to keep
track of the origins of the values at each step, and choose the right
ones at late init and skip the register writes if there are no
changes. I don't know.

BR,
Jani.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 0ae2be5c5318..15cbdc465a86 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -1396,6 +1396,11 @@ void intel_pps_init_late(struct intel_dp *intel_dp)
>  	intel_wakeref_t wakeref;
>  
>  	with_intel_pps_lock(intel_dp, wakeref) {
> +		/* Reinit delays after per-panel info has been parsed from VBT */
> +		memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp->pps.pps_delays));
> +		pps_init_delays(intel_dp);
> +		pps_init_registers(intel_dp, false);
> +
>  		if (edp_have_panel_vdd(intel_dp))
>  			edp_panel_vdd_schedule_off(intel_dp);
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center