[PATCH v3 17/18] drm/i915/dmc_wl: Do nothing until initialized

Gustavo Sousa gustavo.sousa at intel.com
Thu Nov 7 18:27:22 UTC 2024


There is a bit of a chicken and egg situation where we depend on runtime
info to know that DMC and wakelock are supported by the hardware, and
such information is grabbed via display MMIO functions, which in turns
call intel_dmc_wl_get() and intel_dmc_wl_put() as part of their regular
flow.

Since we do not expect DC states (and consequently the wakelock
mechanism) to be enabled until DMC and DMC wakelock software structures
are initialized, a simple and safe solution to this is to turn
intel_dmc_wl_get() and intel_dmc_wl_put() into no-op until we have
properly initialized.

Let's implement that via a new field "initialized". Not that, since we
expect __intel_dmc_wl_supported() to be used for most non-static DMC
wakelock functions, let's add a drm_WARN_ONCE() there for when it is
called prior to initialization.

The only exception of functions that can be called before initialization
are intel_dmc_wl_get() and intel_dmc_wl_put(), so we bail before
calling __intel_dmc_wl_supported() if not initialized.

An alternative solution would be to revise MMIO-related stuff in the
whole driver initialization sequence, but that would possibly come with
the cost of some added ordering dependencies and complexity to the
source code.

Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 13 +++++++++++++
 drivers/gpu/drm/i915/display/intel_dmc_wl.h | 11 +++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index c164ac6e1ada..aae5ea0c72ff 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -264,6 +264,11 @@ static bool intel_dmc_wl_check_range(i915_reg_t reg, u32 dc_state)
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
 {
+	struct intel_dmc_wl *wl = &display->wl;
+
+	if (drm_WARN_ON(display->drm, !wl->initialized))
+		return false;
+
 	return display->params.enable_dmc_wl && intel_dmc_has_payload(display);
 }
 
@@ -282,6 +287,8 @@ void intel_dmc_wl_init(struct intel_display *display)
 
 	intel_dmc_wl_sanitize_param(display);
 
+	wl->initialized = true;
+
 	if (!display->params.enable_dmc_wl)
 		return;
 
@@ -378,6 +385,9 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 	struct intel_dmc_wl *wl = &display->wl;
 	unsigned long flags;
 
+	if (!wl->initialized)
+		return;
+
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
@@ -410,6 +420,9 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 	struct intel_dmc_wl *wl = &display->wl;
 	unsigned long flags;
 
+	if (!wl->initialized)
+		return;
+
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.h b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
index 147eeb4d8432..06c8b61d7e87 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
@@ -15,6 +15,17 @@
 struct intel_display;
 
 struct intel_dmc_wl {
+	/*
+	 * There is a bit of a chicken and egg situation where we depend
+	 * on runtime info to know that DMC and wakelock are supported
+	 * by the hardware, and such information is grabbed via display
+	 * MMIO functions, which in turns call intel_dmc_wl_get() and
+	 * intel_dmc_wl_put() as part of their regular flow.
+	 *
+	 * So we need the initialized field to ensure that we turn the
+	 * get/put routines into a no-op until we have initialized.
+	 */
+	bool initialized;
 	spinlock_t lock; /* protects enabled, taken, dc_state and refcount */
 	bool enabled;
 	bool taken;
-- 
2.47.0



More information about the Intel-gfx mailing list