Re: [PATCH v2 3/3] drm/xe/display: Flush DMC wakelock release work on runtime suspend

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Nov 29, 2024 at 01:37:56PM -0300, Gustavo Sousa wrote:
> We currently are not calling display runtime suspend functions when
> D3cold is not allowed. Because of that, we end up not disabling dynamic
> DC states (and do not go to DC9). With dynamic DC states enabled, we
> will also have DMC wakelock enabled. Since we use a delayed work to
> release the DMC wakelock, the work might get executed a little too late
> (after the PCI device has been put to D3hot) and we get a timeout
> warning ("DMC wakelock release timed out") because the MMIO for
> releasing the wakelock will be invalid after that point.
> 
> To fix that, make sure we flush the release work at the end of
> xe_display_pm_runtime_suspend_late(). We can do that unconditionally
> because, if there is no pending work, that turns into a no-op.
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(answering patch by patch to make your life easier now when applying
with dim b4-shazam ;) )

> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 739db5b7b01f..eec91ab1b311 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -22,6 +22,7 @@
>  #include "intel_display_irq.h"
>  #include "intel_display_types.h"
>  #include "intel_dmc.h"
> +#include "intel_dmc_wl.h"
>  #include "intel_dp.h"
>  #include "intel_encoder.h"
>  #include "intel_fbdev.h"
> @@ -409,11 +410,20 @@ void xe_display_pm_suspend_late(struct xe_device *xe)
>  
>  void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
>  {
> +	struct intel_display *display = &xe->display;
> +
>  	if (!xe->info.probe_display)
>  		return;
>  
>  	if (xe->d3cold.allowed)
>  		xe_display_pm_suspend_late(xe);
> +
> +	/*
> +	 * If xe_display_pm_suspend_late() is not called, it is likely
> +	 * that we will be on dynamic DC states with DMC wakelock enabled. We
> +	 * need to flush the release work in that case.
> +	 */
> +	intel_dmc_wl_flush_release_work(display);
>  }
>  
>  void xe_display_pm_shutdown_late(struct xe_device *xe)
> -- 
> 2.47.0
>