On 7/2/2025 7:42 PM, Jani Nikula wrote:
On Wed, 02 Jul 2025, Gustavo Sousa <gustavo.so...@intel.com> wrote:
Quoting Ankit Nautiyal (2025-07-02 05:46:18-03:00)
Introduce a generic helper to check display workarounds using an enum.
Convert Wa_16023588340 to use the new interface, simplifying WA checks
and making future additions easier.
v2: Use drm_WARN instead of MISSING_CASE and simplify intel_display_wa
macro. (Jani)
Suggested-by: Jani Nikula <jani.nik...@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
---
drivers/gpu/drm/i915/display/intel_display_wa.c | 15 +++++++++++++++
drivers/gpu/drm/i915/display/intel_display_wa.h | 9 +++++++++
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c
b/drivers/gpu/drm/i915/display/intel_display_wa.c
index f57280e9d041..f5e8d58d9a68 100644
--- a/drivers/gpu/drm/i915/display/intel_display_wa.c
+++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
@@ -3,6 +3,8 @@
* Copyright © 2023 Intel Corporation
*/
+#include "drm/drm_print.h"
+
#include "i915_reg.h"
#include "intel_de.h"
#include "intel_display_core.h"
@@ -39,3 +41,16 @@ void intel_display_wa_apply(struct intel_display *display)
else if (DISPLAY_VER(display) == 11)
gen11_display_wa_apply(display);
}
+
+bool __intel_display_wa(struct intel_display *display, enum intel_display_wa
wa)
+{
+ switch (wa) {
+ case INTEL_DISPLAY_WA_16023588340:
+ return intel_display_needs_wa_16023588340(display);
+ default:
+ drm_WARN(display->drm, 1, "Missing Wa number: %d\n", wa);
Hm... I wonder how useful the message would be if we just show the enum
value. For example, if the next workaround that we added was
INTEL_DISPLAY_WA_99999999999 and we had it missing here, I think we
would get the following warning message:
"Missing Wa number: 1"
Perhaps the enum identifier could be found in the callstack that is
presented with the warning, but I'm wondering if we could do better
here.
Not sure there is a good solution without requiring extra memory to map
each enum member to its corresponding the workaround number.
The solution would be to make the function:
bool __intel_display_wa(struct intel_display *display, enum intel_display_wa
wa, const char *name);
and the macro:
#define intel_display_wa(__display, __wa) \
__intel_display_wa((__display), INTEL_DISPLAY_WA_##__wa,
__stringify(__wa))
and then you could debug log the name.
Worth it? Not sure.
Hmm... there might be some value to have some mechanism to print the WA
name.
While it might not add much value in this particular case, but it could
be helpful in other failure scenario,
where identifying the exact WA name might give clue while debugging.
Regards,
Ankit
BR,
Jani.
--
Gustavo Sousa
+ break;
+ }
+
+ return false;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h
b/drivers/gpu/drm/i915/display/intel_display_wa.h
index babd9d16603d..146ee70d66f7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_wa.h
+++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
@@ -21,4 +21,13 @@ static inline bool intel_display_needs_wa_16023588340(struct
intel_display *disp
bool intel_display_needs_wa_16023588340(struct intel_display *display);
#endif
+enum intel_display_wa {
+ INTEL_DISPLAY_WA_16023588340,
+};
+
+bool __intel_display_wa(struct intel_display *display, enum intel_display_wa
wa);
+
+#define intel_display_wa(__display, __wa) \
+ __intel_display_wa((__display), INTEL_DISPLAY_WA_##__wa)
+
#endif
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
b/drivers/gpu/drm/i915/display/intel_fbc.c
index 6e26cb4c5724..e2e03af520b2 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1464,7 +1464,7 @@ static int intel_fbc_check_plane(struct
intel_atomic_state *state,
return 0;
}
- if (intel_display_needs_wa_16023588340(display)) {
+ if (intel_display_wa(display, 16023588340)) {
plane_state->no_fbc_reason = "Wa_16023588340";
return 0;
}
--
2.45.2
