Re: [PATCH 1/2] drm/xe/display: Match i915 driver suspend/resume sequences better

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

On Mon, 05 Aug 2024, Imre Deak <imre.deak@intel.com> wrote:
> On Mon, Aug 05, 2024 at 05:13:08PM +0200, Maarten Lankhorst wrote:
>> Suspend fbdev sooner, and disable user access before suspending to
>> prevent some races. I've noticed this when comparing xe suspend to
>> i915's.
>
> This is only part of
>
> 24b412b1bfeb ("drm/i915: Disable intel HPD poll after DRM poll init/enable")
> f4ed123ae295 ("drm/i915: Suspend the framebuffer console during driver shutdown")
> 1ef28d86bea9 ("drm/i915: Suspend the framebuffer console earlier during system suspend")
> bd738d859e71 ("drm/i915: Prevent modesets during driver init/shutdown")
>
> done for i915, imo all the above changes should be applied to xe.

Really, all of should happen inside drivers/gpu/drm/i915/display/, not
duplicated in i915 and xe drivers.

BR,
Jani.


>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/xe/display/xe_display.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>> index ca4468c820788..44405f17c1d28 100644
>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>> @@ -294,8 +294,12 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>>  	 * properly.
>>  	 */
>>  	intel_power_domains_disable(xe);
>> -	if (has_display(xe))
>> +	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
>> +
>> +	if (has_display(xe)) {
>>  		drm_kms_helper_poll_disable(&xe->drm);
>> +		intel_display_driver_disable_user_access(xe);
>> +	}
>>  
>>  	if (!runtime)
>>  		intel_display_driver_suspend(xe);
>> @@ -308,8 +312,6 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>>  
>>  	intel_opregion_suspend(xe, s2idle ? PCI_D1 : PCI_D3cold);
>>  
>> -	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
>> -
>>  	intel_dmc_suspend(xe);
>>  }
>>  
>> @@ -353,8 +355,10 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime)
>>  		intel_display_driver_resume(xe);
>>  
>>  	intel_hpd_poll_disable(xe);
>> -	if (has_display(xe))
>> +	if (has_display(xe)) {
>>  		drm_kms_helper_poll_enable(&xe->drm);
>> +		intel_display_driver_enable_user_access(xe);
>> +	}
>>  
>>  	intel_opregion_resume(xe);
>>  
>> -- 
>> 2.45.2
>> 

-- 
Jani Nikula, Intel