[PATCH] drm/i915/display: Change ret value type from int to long

[Jani Nikula]

On Fri, 04 Jul 2025, Aakash Deep Sarkar <aakash.deep.sarkar at intel.com> wrote:
> dma_fence_wait_timeout returns a long type but the driver is
> only using the lower 32 bits of the retval and discarding the
> upper 32 bits.
>
> This is particularly problematic if there are already signalled
> or stub fences on some of the hw planes. In this case the
> dma_fence_wait_timeout function will immediately return with
> timeout value MAX_SCHEDULE_TIMEOUT (0x7fffffffffffffff) since
> the fence is already signalled. If the driver only uses the lower
> 32 bits of this return value then it'll interpret it as an error
> code (0xFFFFFFFF or (-1)) and skip the wait on the remaining fences.
>
> This issue was first observed with the Android compositor where
> the GPU composited layer was not properly waited on when there
> were stub fences in other overlay planes resulting in significant
> visual artifacts.

Thanks for the patch, good catch!

> Test: No graphical artifacts with shadertoy

We've never used this commit trailer before, please let's not start now.

The subject should plainly state the "what", and the commit message
should answer the "why". You do have that here, but I think the subject
is still a bit too much nuts and bolts.

For example,

	drm/i915/display: Fix dma_fence_wait_timeout() return value handling

would state the "what" in *much* more helpful terms for anyone looking
at git logs.

Presumably this has been an error for some time. You should do a little
bit of git blame on the offending lines. It'll find commit d59cf7bb73f3
("drm/i915/display: Use dma_fence interfaces instead of i915_sw_fence").

Based on that, we should add:

Fixes: d59cf7bb73f3 ("drm/i915/display: Use dma_fence interfaces instead of i915_sw_fence")
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Cc: Jouni Högander <jouni.hogander at intel.com>
Cc: <stable at vger.kernel.org> # v6.8+

Then it occurs to me this looks like a common mistake to make. A little
bit of git grep on dma_fence_wait_timeout() quickly finds multiple
similar mistakes in drm, at least amdgpu, etnaviv, msm, and tegra. Cc
some maintainers FYI. This class of bugs could cause issues elsewhere.

Let's also Cc Julia and Dan in case they have ideas to improve static
analysis to catch this class of bugs. Or maybe one exists already, but
we're just not running them!

Finally, for the actual change,

Reviewed-by: Jani Nikula <jani.nikula at intel.com>


> Signed-off-by: Aakash Deep Sarkar <aakash.deep.sarkar at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 456fc4b04cda..7035c1fc9033 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7092,7 +7092,8 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat
>  	struct drm_i915_private *i915 = to_i915(intel_state->base.dev);
>  	struct drm_plane *plane;
>  	struct drm_plane_state *new_plane_state;
> -	int ret, i;
> +	long ret;
> +	int i;
>  
>  	for_each_new_plane_in_state(&intel_state->base, plane, new_plane_state, i) {
>  		if (new_plane_state->fence) {

-- 
Jani Nikula, Intel