Re: [PATCH] drm/i915/dmc_wl: Do not check for DMC payload

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Gustavo Sousa (2025-01-24 16:12:34-03:00)
>Enabling and disabling of DMC wakelock is already coupled with enabling
>and disabling of dynamic DC states, which already depend on the DMC
>being properly loaded. As such, we do not need to check if we already
>have a DMC payload parsed in __intel_dmc_wl_supported().
>
>Furthermore, the presence of such a check causes inconsistencies in the
>refcount if the following sequence of events happen:
>
>  1. A call to one of the register accessors from intel_de.h is done
>     before the DMC payload is parsed. That causes intel_dmc_wl_get() to
>     be called. Suppose the register offset qualifies as needing the
>     wakelock.
>
>     In normal circumstances, the refcount would be incremented, but,
>     because __intel_dmc_wl_supported() returns false, the refcount is
>     untouched.
>
>  2. In a separate worker thread, the DMC firmware is parsed. Parsing of
>     the DMC payload is finished before the corresponding
>     intel_dmc_wl_put() from (1) is called.
>
>  3. When in the context of (1), intel_dmc_wl_put() gets called, now we
>     have __intel_dmc_wl_supported() returning true and we hit the
>     warning, because the code doesn't expect a zero refcount.
>
>Let's remove that check, since it is unnecessary and causes the
>inconsistency illustrated above.
>
>Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Pushed to drm-intel-next. Thank you all for the reviews/feedback!

--
Gustavo Sousa

>---
> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>index 43884740f8ea..9be6ad11ff5d 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>@@ -10,7 +10,6 @@
> #include "i915_drv.h"
> #include "i915_reg.h"
> #include "intel_de.h"
>-#include "intel_dmc.h"
> #include "intel_dmc_regs.h"
> #include "intel_dmc_wl.h"
> 
>@@ -282,7 +281,7 @@ static bool intel_dmc_wl_check_range(struct intel_display *display,
> 
> static bool __intel_dmc_wl_supported(struct intel_display *display)
> {
>-        return display->params.enable_dmc_wl && intel_dmc_has_payload(display);
>+        return display->params.enable_dmc_wl;
> }
> 
> static void intel_dmc_wl_sanitize_param(struct intel_display *display)
>-- 
>2.48.1
>