> -----Original Message-----
> From: Murthy, Arun R <arun.r.mur...@intel.com>
> Sent: Tuesday, February 25, 2025 1:04 PM
> To: dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> x...@lists.freedesktop.org; ville.syrj...@linux.intel.com
> Cc: Murthy, Arun R <arun.r.mur...@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.bo...@intel.com>; Brzezinka, Sebastian
> <sebastian.brzezi...@intel.com>; Kumar, Naveen1
> <naveen1.ku...@intel.com>
> Subject: [PATCH v7 3/3] drm/i915/display: Add i915 hook for
> format_mod_supported_async
>
> Hook up the newly added plane function pointer
> format_mod_supported_async to populate the modifiers/formats supported
> by asynchronous flips.
>
> v5: Correct the if condition for modifier support check (Chaitanya)
> v6: Replace uint32_t/uint64_t with u32/u64 (Jani)
> v7: Move plannar check from intel_async_flip_check_hw() to
> intel_plane_format_mod_supported_async() (Ville)
>
> Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
> Reviewed-by: Sebastian Brzezinka <sebastian.brzezi...@intel.com>
> Tested-by: Naveen Kumar <naveen1.ku...@intel.com>
> ---
> drivers/gpu/drm/i915/display/i9xx_plane.c | 6 +++--
> drivers/gpu/drm/i915/display/intel_atomic_plane.c | 30
> +++++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_atomic_plane.h | 6 ++++-
> drivers/gpu/drm/i915/display/intel_display.c | 11 ++------
> drivers/gpu/drm/i915/display/skl_universal_plane.c | 5 +++-
> 5 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index
> 013295f66d56ec5e919b3a0c904034bf7985986a..6bd09adb8a30ba002ef3342
> 61d7638f398587a3e 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -820,7 +820,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->modifier))
> + if (intel_plane_can_async_flip(plane, fb->format->format,
> +fb->modifier))
> return 256 * 1024;
>
> /* FIXME undocumented so not sure what's actually needed */ @@ -
> 844,7 +844,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->modifier))
> + if (intel_plane_can_async_flip(plane, fb->format->format,
> +fb->modifier))
> return 256 * 1024;
>
> if (intel_scanout_needs_vtd_wa(display))
> @@ -889,6 +889,7 @@ static const struct drm_plane_funcs i965_plane_funcs
> = {
> .atomic_duplicate_state = intel_plane_duplicate_state,
> .atomic_destroy_state = intel_plane_destroy_state,
> .format_mod_supported = i965_plane_format_mod_supported,
> + .format_mod_supported_async =
> intel_plane_format_mod_supported_async,
> };
>
> static const struct drm_plane_funcs i8xx_plane_funcs = { @@ -898,6 +899,7
> @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
> .atomic_duplicate_state = intel_plane_duplicate_state,
> .atomic_destroy_state = intel_plane_destroy_state,
> .format_mod_supported = i8xx_plane_format_mod_supported,
> + .format_mod_supported_async =
> intel_plane_format_mod_supported_async,
> };
>
> struct intel_plane *
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index
> 124cd9ddba0b96657a8166e613b93003e77e133c..4ebc40c914b72b8fe3118ed
> b74d9bf95ab661b13 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -170,11 +170,39 @@ bool intel_plane_needs_physical(struct intel_plane
> *plane)
> DISPLAY_INFO(i915)->cursor_needs_physical;
> }
>
> -bool intel_plane_can_async_flip(struct intel_plane *plane, u64 modifier)
> +bool intel_plane_can_async_flip(struct intel_plane *plane, u32 format,
> + u64 modifier)
> {
> + struct intel_display *display = to_intel_display(plane);
> +
> + if (DISPLAY_VER(display) <= 14 ?
> + drm_format_info(format)->is_yuv :
> + intel_format_info_is_yuv_semiplanar(drm_format_info(format),
> + modifier)) {
> + drm_dbg_kms(plane->base.dev,
> + "[PLANE:%d:%s] Planar formats do not support
> async flips\n",
> + plane->base.base.id, plane->base.name);
> + 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)
> +{
> + if (!plane->funcs->format_mod_supported(plane, format, modifier)) {
In theory this hook is still optional, we should check for its availability.
> + drm_dbg_kms(plane->dev,
> + "[PLANE:%d:%s] Planar format/modifier not in
> universal list\n",
The debug message is incorrect. We are not checking only for planar formats
here.
We can perhaps print the format and modifier rejected.
> + plane->base.id, plane->name);
> + return false;
> + }
> +
> + return intel_plane_can_async_flip(to_intel_plane(plane),
> + format, modifier);
> +}
> +
> unsigned int intel_adjusted_rate(const struct drm_rect *src,
> const struct drm_rect *dst,
> unsigned int rate)
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index
> 65edd88d28a9c532d6347fbd13b0f45698e9e5bb..0d1700bd5c6d7b1b3e4e7e
> 6be760b548493d6846 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -21,7 +21,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, u64 modifier);
> +bool intel_plane_can_async_flip(struct intel_plane *plane, u32 format,
> + u64 modifier);
> unsigned int intel_adjusted_rate(const struct drm_rect *src,
> const struct drm_rect *dst,
> unsigned int rate);
> @@ -87,5 +88,8 @@ void intel_plane_init_cursor_vblank_work(struct
> intel_plane_state *old_plane_sta int
> intel_atomic_add_affected_planes(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> int intel_atomic_check_planes(struct intel_atomic_state *state);
> +bool intel_plane_format_mod_supported_async(struct drm_plane *plane,
> + u32 format,
> + u64 modifier);
>
> #endif /* __INTEL_ATOMIC_PLANE_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index
> 065fdf6dbb88e3c4ac990b38e7f1575e0c9ca413..c9abe1412aef12d0eda17110
> 431a57a06f3f5d20 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6188,7 +6188,8 @@ 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->modifier)) {
> + if (!intel_plane_can_async_flip(plane, new_plane_state-
> >hw.fb->format->format,
> + new_plane_state->hw.fb->modifier)) {
> drm_dbg_kms(&i915->drm,
> "[PLANE:%d:%s] Modifier 0x%llx does not
> support async flip\n",
Now that we are checking for formats too. Good to print it.
Regards
Chaitanya
> plane->base.base.id, plane->base.name,
> @@ -6196,14 +6197,6 @@ static int intel_async_flip_check_hw(struct
> intel_atomic_state *state, struct in
> return -EINVAL;
> }
>
> - if (intel_format_info_is_yuv_semiplanar(new_plane_state-
> >hw.fb->format,
> - new_plane_state-
> >hw.fb->modifier)) {
> - drm_dbg_kms(&i915->drm,
> - "[PLANE:%d:%s] Planar formats do not
> support async flips\n",
> - plane->base.base.id, plane->base.name);
> - return -EINVAL;
> - }
> -
> /*
> * We turn the first async flip request into a sync flip
> * so that we can reconfigure the plane (eg. change modifier).
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index
> cd9762947f1de227a3abbcd61b7c7b0c9848e439..1b0cafe9b158c24e74bf4622
> 2ac23d2c933555a5 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -601,7 +601,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->modifier))
> + intel_plane_can_async_flip(plane, fb->format->format,
> +fb->modifier))
> return mult * 16 * 1024;
>
> switch (fb->modifier) {
> @@ -2623,6 +2623,7 @@ static const struct drm_plane_funcs
> skl_plane_funcs = {
> .atomic_duplicate_state = intel_plane_duplicate_state,
> .atomic_destroy_state = intel_plane_destroy_state,
> .format_mod_supported = skl_plane_format_mod_supported,
> + .format_mod_supported_async =
> intel_plane_format_mod_supported_async,
> };
>
> static const struct drm_plane_funcs icl_plane_funcs = { @@ -2632,6 +2633,7
> @@ static const struct drm_plane_funcs icl_plane_funcs = {
> .atomic_duplicate_state = intel_plane_duplicate_state,
> .atomic_destroy_state = intel_plane_destroy_state,
> .format_mod_supported = icl_plane_format_mod_supported,
> + .format_mod_supported_async =
> intel_plane_format_mod_supported_async,
> };
>
> static const struct drm_plane_funcs tgl_plane_funcs = { @@ -2641,6 +2643,7
> @@ static const struct drm_plane_funcs tgl_plane_funcs = {
> .atomic_duplicate_state = intel_plane_duplicate_state,
> .atomic_destroy_state = intel_plane_destroy_state,
> .format_mod_supported = tgl_plane_format_mod_supported,
> + .format_mod_supported_async =
> intel_plane_format_mod_supported_async,
> };
>
> static void
>
> --
> 2.25.1
