Re: [PATCH] drm/i915: Fix assert on pending async-put power domain work when it requeues itself

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Thu, 2024-05-30 at 13:53 +0300, Imre Deak wrote:
> Commit dd839aa857eb ("drm/i915: Fix incorrect assert about pending
> power domain async-put work")
> 
> fixed the assert about a pending work dropping a display power
> reference
> asynchronously, leading to the
> 
> drm_WARN_ON(!queue_delayed_work(&power_domains->async_put_work));
> 
> warn next time around a power reference was put asynchronously, due
> to a
> stale instance of the work still being pending. However the fix
> didn't
> consider the case where multiple power reference was acquired and
> put,
> requiring the work to requeue itself. Extend the fix for this case as
> well canceling the pending instance of the work before it requeues
> itself.

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10915
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 34b6d843bc9ef..8c49ac053dfb0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -675,6 +675,12 @@ intel_display_power_put_async_work(struct
> work_struct *work)
>         release_async_put_domains(power_domains,
>                                   &power_domains-
> >async_put_domains[0]);
>  
> +       /*
> +        * Cancel the work that got queued after this one got
> dequeued,
> +        * since here we released the corresponding async-put
> reference.
> +        */
> +       cancel_async_put_work(power_domains, false);
> +
>         /* Requeue the work if more domains were async put meanwhile.
> */
>         if (!bitmap_empty(power_domains->async_put_domains[1].bits,
> POWER_DOMAIN_NUM)) {
>                 bitmap_copy(power_domains->async_put_domains[0].bits,
> @@ -686,12 +692,6 @@ intel_display_power_put_async_work(struct
> work_struct *work)
>                                             
> fetch_and_zero(&new_work_wakeref),
>                                              power_domains-
> >async_put_next_delay);
>                 power_domains->async_put_next_delay = 0;
> -       } else {
> -               /*
> -                * Cancel the work that got queued after this one got
> dequeued,
> -                * since here we released the corresponding async-put
> reference.
> -                */
> -               cancel_async_put_work(power_domains, false);
>         }
>  
>  out_verify: