[Intel-gfx] [PATCH] drm/i915/edp: wait power off delay at driver remove to optimize probe

[Jani Nikula]

On Wed, 16 Nov 2022, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Wed, Nov 16, 2022 at 05:06:57PM +0200, Jani Nikula wrote:
>> Panel power off delay is the time the panel power needs to remain off
>> after being switched off, before it can be switched on again.
>> 
>> For the purpose of respecting panel power off delay at driver probe,
>> assuming the panel was last switched off at driver probe is overly
>> pessimistic. If the panel was never on, we'd end up waiting for no
>> reason.
>> 
>> We don't know what has happened before kernel boot, but we can make some
>> assumptions:
>> 
>> - The panel may have been switched off right before kernel boot by some
>>   pre-os environment.
>> 
>> - After kernel boot, the panel may only be switched off by i915.
>> 
>> - At i915 driver probe, only a previously loaded and removed i915 may
>>   have switched the panel power off.
>> 
>> With these assumptions, we can initialize the last power off time to
>> kernel boot time, if we also ensure i915 driver remove waits for the
>> panel power off delay after switching panel power off.
>> 
>> This shaves off the time it takes from kernel boot to i915 probe from
>> the first panel enable, if (and only if) the panel was not already
>> enabled at boot.
>> 
>> The encoder destroy hook is pretty much the last place where we can
>> wait, right after we've ensured the panel power has been switched off,
>> and before the whole encode is destroyed.
>
> Yeah, the fact that we have the vdd_off_sync() in there at least
> theoretically means the vdd might be on at any point before that.
>
> I was also pondering about doing this for all encoder types.
> Though the benefits are slightly less pronounced I guess:
> a) we don't need to power the panel for the output probe on those,
>    so a bit more time will have elapsed anyway before we have to
>    power the panel on during the first modeset
> b) for LVDS we rely 100% on the pps state machine so the initial
>    boot case is already as optimal as possible. Adding the explicit
>    wait into the unload path could thus only help the speed of
>    of the first modeset during module reload
>
> But maybe we'd stil want to do that, for consistency if nothing else?
>
> Either way, this patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thanks for the review and testing, pushed to drm-intel-next.

BR,
Jani.

>
>> 
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7417
>> Cc: Lee Shawn C <shawn.c.lee at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c  | 6 ++++++
>>  drivers/gpu/drm/i915/display/intel_pps.c | 8 +++++++-
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 914161d7d122..67089711d9e2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4877,6 +4877,12 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
>>  
>>  	intel_pps_vdd_off_sync(intel_dp);
>>  
>> +	/*
>> +	 * Ensure power off delay is respected on module remove, so that we can
>> +	 * reduce delays at driver probe. See pps_init_timestamps().
>> +	 */
>> +	intel_pps_wait_power_cycle(intel_dp);
>> +
>>  	intel_dp_aux_fini(intel_dp);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> index 81ee7f3aadf6..9bbf41a076f7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -1100,7 +1100,13 @@ bool intel_pps_have_panel_power_or_vdd(struct intel_dp *intel_dp)
>>  
>>  static void pps_init_timestamps(struct intel_dp *intel_dp)
>>  {
>> -	intel_dp->pps.panel_power_off_time = ktime_get_boottime();
>> +	/*
>> +	 * Initialize panel power off time to 0, assuming panel power could have
>> +	 * been toggled between kernel boot and now only by a previously loaded
>> +	 * and removed i915, which has already ensured sufficient power off
>> +	 * delay at module remove.
>> +	 */
>> +	intel_dp->pps.panel_power_off_time = 0;
>>  	intel_dp->pps.last_power_on = jiffies;
>>  	intel_dp->pps.last_backlight_off = jiffies;
>>  }
>> -- 
>> 2.34.1

-- 
Jani Nikula, Intel Open Source Graphics Center