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@xxxxxxxxx>
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
>
