Re: [PATCH 3/3] drm/i915: Eliminate one more frequent drm_format_info()

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

On Thu, 2025-11-13 at 01:30 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Another (somewhat expensive) drm_format_info() call has
> appeared in intel_plane_can_async_flip(). That one may get
> called several times per commit so we need to get rid of
> it.
> 
> Fortunately most callers already have the framebuffer at
> hand, so we can just grab the format info from there.
> The one exception is intel_plane_format_mod_supported_async()
> where we have to do the lookup. But that only gets called
> (a bunch of times) during driver init to build the
> IN_FORMATS_ASYNC blob, and afterwards there is no runtime
> cost.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/i9xx_plane.c     |  4 ++--
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/display/intel_plane.c    | 22 +++++++++++------
> --
>  drivers/gpu/drm/i915/display/intel_plane.h    |  4 +++-
>  .../drm/i915/display/skl_universal_plane.c    |  2 +-
>  5 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 51ccc6bd5f21..b93c86197b4a 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -819,7 +819,7 @@ unsigned int vlv_plane_min_alignment(struct
> intel_plane *plane,
>  {
>  	struct intel_display *display = to_intel_display(plane);
>  
> -	if (intel_plane_can_async_flip(plane, fb->format->format,
> fb->modifier))
> +	if (intel_plane_can_async_flip(plane, fb->format, fb-
> >modifier))
>  		return 256 * 1024;
>  
>  	/* FIXME undocumented so not sure what's actually needed */
> @@ -843,7 +843,7 @@ static unsigned int
> g4x_primary_min_alignment(struct intel_plane *plane,
>  {
>  	struct intel_display *display = to_intel_display(plane);
>  
> -	if (intel_plane_can_async_flip(plane, fb->format->format,
> fb->modifier))
> +	if (intel_plane_can_async_flip(plane, fb->format, fb-
> >modifier))
>  		return 256 * 1024;
>  
>  	if (intel_scanout_needs_vtd_wa(display))
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 069967114bd9..71bd8484885b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6122,7 +6122,7 @@ static int intel_async_flip_check_hw(struct
> intel_atomic_state *state, struct in
>  		if (!plane->async_flip)
>  			continue;
>  
> -		if (!intel_plane_can_async_flip(plane,
> new_plane_state->hw.fb->format->format,
> +		if (!intel_plane_can_async_flip(plane,
> new_plane_state->hw.fb->format,
>  						new_plane_state-
> >hw.fb->modifier)) {
>  			drm_dbg_kms(display->drm,
>  				    "[PLANE:%d:%s] pixel format
> %p4cc / modifier 0x%llx does not support async flip\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c
> b/drivers/gpu/drm/i915/display/intel_plane.c
> index 5105e3278bc4..ee9cda94a7c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -178,25 +178,29 @@ bool intel_plane_needs_physical(struct
> intel_plane *plane)
>  		DISPLAY_INFO(display)->cursor_needs_physical;
>  }
>  
> -bool intel_plane_can_async_flip(struct intel_plane *plane, u32
> format,
> +bool intel_plane_can_async_flip(struct intel_plane *plane,
> +				const struct drm_format_info *info,
>  				u64 modifier)
>  {
> -	if
> (intel_format_info_is_yuv_semiplanar(drm_format_info(format),
> modifier) ||
> -	    format == DRM_FORMAT_C8)
> +	if (intel_format_info_is_yuv_semiplanar(info, modifier) ||
> +	    info->format == DRM_FORMAT_C8)
>  		return false;
>  
>  	return plane->can_async_flip && plane-
> >can_async_flip(modifier);
>  }
>  
> -bool intel_plane_format_mod_supported_async(struct drm_plane *plane,
> -					    u32 format,
> -					    u64 modifier)
> +bool intel_plane_format_mod_supported_async(struct drm_plane
> *_plane,
> +					    u32 format, u64
> modifier)
>  {
> -	if (!plane->funcs->format_mod_supported(plane, format,
> modifier))
> +	struct intel_plane *plane = to_intel_plane(_plane);
> +	const struct drm_format_info *info;
> +
> +	if (!plane->base.funcs->format_mod_supported(&plane->base,
> format, modifier))
>  		return false;
>  
> -	return intel_plane_can_async_flip(to_intel_plane(plane),
> -					format, modifier);
> +	info = drm_get_format_info(plane->base.dev, format,
> modifier);
> +
> +	return intel_plane_can_async_flip(plane, info, modifier);
>  }
>  
>  unsigned int intel_adjusted_rate(const struct drm_rect *src,
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.h
> b/drivers/gpu/drm/i915/display/intel_plane.h
> index 4e99df9de3e8..5a8f2f3baab5 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_plane.h
> @@ -8,6 +8,7 @@
>  
>  #include <linux/types.h>
>  
> +struct drm_format_info;
>  struct drm_plane;
>  struct drm_property;
>  struct drm_rect;
> @@ -21,7 +22,8 @@ enum plane_id;
>  
>  struct intel_plane *
>  intel_crtc_get_plane(struct intel_crtc *crtc, enum plane_id
> plane_id);
> -bool intel_plane_can_async_flip(struct intel_plane *plane, u32
> format,
> +bool intel_plane_can_async_flip(struct intel_plane *plane,
> +				const struct drm_format_info *info,
>  				u64 modifier);
>  unsigned int intel_adjusted_rate(const struct drm_rect *src,
>  				 const struct drm_rect *dst,
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 89c8003ccfe7..921b2f73d27a 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -595,7 +595,7 @@ static u32 tgl_plane_min_alignment(struct
> intel_plane *plane,
>  	 * Figure out what's going on here...
>  	 */
>  	if (display->platform.alderlake_p &&
> -	    intel_plane_can_async_flip(plane, fb->format->format,
> fb->modifier))
> +	    intel_plane_can_async_flip(plane, fb->format, fb-
> >modifier))
>  		return mult * 16 * 1024;
>  
>  	switch (fb->modifier) {