Re: [PATCH 06/20] drm/i915/gmbus: use generic poll_timeout*() instead of wait_for*()

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Thu, 2025-08-28 at 15:20 +0300, Jani Nikula wrote:
> Prefer generic poll helpers over i915 custom helpers.
> 
> The "two tier" wait_for_us() + wait_for() combination appeared
> without
> much explanation in commit 4e6c2d58ba86 ("drm/i915: Take forcewake
> once
> for the entire GMBUS transaction"). Try to mimic roughly the same
> with
> the generic helpers.
> 
> wait_for_us() with 10 us or shorter timeouts ends up in
> _wait_for_atomic(). Thus use poll_timeout_us_atomic() for the first
> try,
> with the same 2 us timeout and no sleep.
> 
> For the fallback, the functional change is losing the exponentially
> growing sleep of wait_for(), which used to be 10, 20, 40, ..., 640,
> and
> 1280 us. Use an arbitrary constant 500 us sleep instead. The timeout
> remains at 50 ms.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
> b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 063335053d13..358210adb8f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -30,13 +30,13 @@
>  #include <linux/export.h>
>  #include <linux/i2c-algo-bit.h>
>  #include <linux/i2c.h>
> +#include <linux/iopoll.h>
>  
>  #include <drm/display/drm_hdcp_helper.h>
>  
>  #include "i915_drv.h"
>  #include "i915_irq.h"
>  #include "i915_reg.h"
> -#include "i915_utils.h"
>  #include "intel_de.h"
>  #include "intel_display_regs.h"
>  #include "intel_display_types.h"
> @@ -415,11 +415,14 @@ static int gmbus_wait(struct intel_display
> *display, u32 status, u32 irq_en)
>  	intel_de_write_fw(display, GMBUS4(display), irq_en);
>  
>  	status |= GMBUS_SATOER;
> -	ret = wait_for_us((gmbus2 = intel_de_read_fw(display,
> GMBUS2(display))) & status,
> -			  2);
> +
> +	ret = poll_timeout_us_atomic(gmbus2 =
> intel_de_read_fw(display, GMBUS2(display)),
> +				     gmbus2 & status,
> +				     0, 2, false);
>  	if (ret)
> -		ret = wait_for((gmbus2 = intel_de_read_fw(display,
> GMBUS2(display))) & status,
> -			       50);
> +		ret = poll_timeout_us(gmbus2 =
> intel_de_read_fw(display, GMBUS2(display)),
> +				      gmbus2 & status,
> +				      500, 50 * 1000, false);
>  
>  	intel_de_write_fw(display, GMBUS4(display), 0);
>  	remove_wait_queue(&display->gmbus.wait_queue, &wait);