Re: [PATCH v2 2/7] drm/i915/ddi: use intel_de_wait_custom() instead of wait_for_us()

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

Quoting Jani Nikula (2025-07-31 07:05:09-03:00)
>Prefer the register read specific wait function over i915 wait_for_us().
>
>v2: Wait for bits to clear in mtl_ddi_disable_d2d()
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 0405396c7750..866ed3e46664 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -2561,6 +2561,7 @@ mtl_ddi_enable_d2d(struct intel_encoder *encoder)
>         enum port port = encoder->port;
>         i915_reg_t reg;
>         u32 set_bits, wait_bits;
>+        int ret;
> 
>         if (DISPLAY_VER(display) < 14)
>                 return;
>@@ -2576,7 +2577,11 @@ mtl_ddi_enable_d2d(struct intel_encoder *encoder)
>         }
> 
>         intel_de_rmw(display, reg, 0, set_bits);
>-        if (wait_for_us(intel_de_read(display, reg) & wait_bits, 100)) {
>+
>+        ret = intel_de_wait_custom(display, reg,
>+                                   wait_bits, wait_bits,
>+                                   100, 0, NULL);

Hm... Strictly speaking, in the previous version, we are waiting for the
register value with wait_bits mask applied to be any non-zero value.
But, since wait_bits is just a single bit, I think using
intel_de_wait_custom() works fine here.

Perhaps using plural in wait_bits was not precise?

Anyways,

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


>+        if (ret) {
>                 drm_err(display->drm, "Timeout waiting for D2D Link enable for DDI/PORT_BUF_CTL %c\n",
>                         port_name(port));
>         }
>@@ -3058,6 +3063,7 @@ mtl_ddi_disable_d2d(struct intel_encoder *encoder)
>         enum port port = encoder->port;
>         i915_reg_t reg;
>         u32 clr_bits, wait_bits;
>+        int ret;
> 
>         if (DISPLAY_VER(display) < 14)
>                 return;
>@@ -3073,7 +3079,11 @@ mtl_ddi_disable_d2d(struct intel_encoder *encoder)
>         }
> 
>         intel_de_rmw(display, reg, clr_bits, 0);
>-        if (wait_for_us(!(intel_de_read(display, reg) & wait_bits), 100))
>+
>+        ret = intel_de_wait_custom(display, reg,
>+                                   wait_bits, 0,
>+                                   100, 0, NULL);
>+        if (ret)
>                 drm_err(display->drm, "Timeout waiting for D2D Link disable for DDI/PORT_BUF_CTL %c\n",
>                         port_name(port));
> }
>-- 
>2.39.5
>