Re: [PATCH 2/2] drm/i915/gmbus: Add Wa_16025573575 for PTL/WCL for bit-bashing

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

Quoting Ankit Nautiyal (2025-07-15 11:22:11-03:00)
>As per Wa_16025573575 for PTL/WCL, set the GPIO masks bit before starting
>bit-bashing and maintain value through the bit-bashing sequence. After the
>bit-bashing sequence is done, clear the GPIO masks bits.
>
>v2:
>-Use new helper for display workarounds. (Jani)
>-Use a separate if-block for the workaround. (Gustavo)
>
>v3:
>-Document the workaround details in intel_display_wa.c
>-Extend the workaround to WCL too. (Gustavo)
>
>v4:
>-Fix the platform check. (Gustavo)
>-Avoid read when preserve bits are 0. (Gustavo)
>
>Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> .../gpu/drm/i915/display/intel_display_wa.c   | 12 ++++++
> .../gpu/drm/i915/display/intel_display_wa.h   |  1 +
> drivers/gpu/drm/i915/display/intel_gmbus.c    | 39 ++++++++++++++++---
> 3 files changed, 47 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c b/drivers/gpu/drm/i915/display/intel_display_wa.c
>index 32719e2c6025..399c08902413 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_wa.c
>+++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
>@@ -42,11 +42,23 @@ void intel_display_wa_apply(struct intel_display *display)
>                 gen11_display_wa_apply(display);
> }
> 
>+/*
>+ * Wa_16025573575:
>+ * Fixes: Issue with bitbashing on Xe3 based platforms.
>+ * Workaround: Set masks bits in GPIO CTL and preserve it during bitbashing sequence.
>+ */
>+static bool intel_display_needs_wa_16025573575(struct intel_display *display)
>+{
>+        return DISPLAY_VERx100(display) == 3000 || DISPLAY_VERx100(display) == 3002;
>+}
>+
> bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa, const char *name)
> {
>         switch (wa) {
>         case INTEL_DISPLAY_WA_16023588340:
>                 return intel_display_needs_wa_16023588340(display);
>+        case INTEL_DISPLAY_WA_16025573575:
>+                return intel_display_needs_wa_16025573575(display);
>         default:
>                 drm_WARN(display->drm, 1, "Missing Wa number: %s\n", name);
>                 break;
>diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h b/drivers/gpu/drm/i915/display/intel_display_wa.h
>index 8319e16eb460..aedea4cfa3ce 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_wa.h
>+++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
>@@ -23,6 +23,7 @@ bool intel_display_needs_wa_16023588340(struct intel_display *display);
> 
> enum intel_display_wa {
>         INTEL_DISPLAY_WA_16023588340,
>+        INTEL_DISPLAY_WA_16025573575,
> };
> 
> bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa, const char *name);
>diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
>index 0d73f32fe7f1..637f0f23f163 100644
>--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>@@ -39,6 +39,7 @@
> #include "intel_de.h"
> #include "intel_display_regs.h"
> #include "intel_display_types.h"
>+#include "intel_display_wa.h"
> #include "intel_gmbus.h"
> #include "intel_gmbus_regs.h"
> 
>@@ -240,14 +241,20 @@ static void bxt_gmbus_clock_gating(struct intel_display *display,
> static u32 get_reserved(struct intel_gmbus *bus)
> {
>         struct intel_display *display = bus->display;
>-        u32 reserved = 0;
>+        u32 preserve_bits = 0;
>+
>+        if (display->platform.i830 || display->platform.i845g)
>+                return 0;
> 
>         /* On most chips, these bits must be preserved in software. */
>-        if (!display->platform.i830 && !display->platform.i845g)
>-                reserved = intel_de_read_notrace(display, bus->gpio_reg) &
>-                        (GPIO_DATA_PULLUP_DISABLE | GPIO_CLOCK_PULLUP_DISABLE);
>+        preserve_bits |= GPIO_DATA_PULLUP_DISABLE | GPIO_CLOCK_PULLUP_DISABLE;
>+
>+        /* Wa_16025573575: the masks bits need to be preserved through out */
>+        if (intel_display_wa(display, 16025573575))
>+                preserve_bits |= GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK |
>+                                 GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK;
> 
>-        return reserved;
>+        return intel_de_read_notrace(display, bus->gpio_reg) & preserve_bits;
> }
> 
> static int get_clock(void *data)
>@@ -308,6 +315,22 @@ static void set_data(void *data, int state_high)
>         intel_de_posting_read(display, bus->gpio_reg);
> }
> 
>+static void
>+ptl_handle_mask_bits(struct intel_gmbus *bus, bool set)
>+{
>+        struct intel_display *display = bus->display;
>+        u32 reg_val = intel_de_read_notrace(display, bus->gpio_reg);
>+        u32 mask_bits = GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK |
>+                        GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK;
>+        if (set)
>+                reg_val |= mask_bits;
>+        else
>+                reg_val &= ~mask_bits;
>+
>+        intel_de_write_notrace(display, bus->gpio_reg, reg_val);
>+        intel_de_posting_read(display, bus->gpio_reg);
>+}
>+
> static int
> intel_gpio_pre_xfer(struct i2c_adapter *adapter)
> {
>@@ -319,6 +342,9 @@ intel_gpio_pre_xfer(struct i2c_adapter *adapter)
>         if (display->platform.pineview)
>                 pnv_gmbus_clock_gating(display, false);
> 
>+        if (intel_display_wa(display, 16025573575))
>+                ptl_handle_mask_bits(bus, true);
>+
>         set_data(bus, 1);
>         set_clock(bus, 1);
>         udelay(I2C_RISEFALL_TIME);
>@@ -336,6 +362,9 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
> 
>         if (display->platform.pineview)
>                 pnv_gmbus_clock_gating(display, true);
>+
>+        if (intel_display_wa(display, 16025573575))
>+                ptl_handle_mask_bits(bus, false);
> }
> 
> static void
>-- 
>2.45.2
>