[PATCH v4 0/3] Expose modifiers/formats supported by async flips

[PATCH v4 0/3] Expose modifiers/formats supported by async flips

[Arun R Murthy]

All of the formats/modifiers supported by the plane during synchronous
flips are nor supported by asynchronous flips. The formats/modifiers
exposed to user by IN_FORMATS exposes all formats/modifiers supported by
plane and this list varies for async flips. If the async flip supported
formats/modifiers are exposed to the user, user based on this list can
take decision to proceed or not and avoid flip failures during async
flips.
Discussion around this can be located @
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29618#note_2487123
Userspace implementation for IN_FORMARTS_ASYNC under review @
https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4063

TODO: Upon merge of the patch related to async flip
https://patchwork.freedesktop.org/patch/626849/?series=139807&rev=6
the patch 5 in this series will have to make use of the new function
pointer can_async_flip().

v3: Add new plane->funcs format_mod_supported_async (Ville)
v4: reuse the existing can_async_flip in i915 (Ville)
    pass format_mod_supported/async as parameter to create_in_formats
    function & leave the modifiers/formats in drm_format_modifer_blob
    untouched (Ville)

Arun R Murthy (3):
  drm/plane: Add new plane property IN_FORMATS_ASYNC
  drm/plane: Expose function to create format/modifier blob
  drm/i915/display: Populate list of async supported formats/modifiers

 drivers/gpu/drm/drm_mode_config.c             |  7 +++
 drivers/gpu/drm/drm_plane.c                   | 50 ++++++++++++------
 .../drm/i915/display/skl_universal_plane.c    | 51 +++++++++++++++++++
 include/drm/drm_mode_config.h                 |  6 +++
 include/drm/drm_plane.h                       |  4 ++
 5 files changed, 103 insertions(+), 15 deletions(-)

--
2.25.1

---
Arun R Murthy (5):
      drm/plane: Add new plane property IN_FORMATS_ASYNC
      drm/plane: Expose function to create format/modifier blob
      drm/plane: Function to check async supported modifier/format
      drm/i915/display: Populate list of async supported formats/modifiers
      drm/i915/display: Add function for format_mod_supported_async

 drivers/gpu/drm/drm_mode_config.c                  |   7 ++
 drivers/gpu/drm/drm_plane.c                        |  72 +++++++++----
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 113 ++++++++++++++++++---
 include/drm/drm_mode_config.h                      |   6 ++
 include/drm/drm_plane.h                            |  24 +++++
 5 files changed, 188 insertions(+), 34 deletions(-)
---
base-commit: 08bd590935a5258ffd79355c59adffd72fb2c642
change-id: 20250102-asyn-bf76730501cc

Best regards,
--
Arun R Murthy <arun.r.murthy@intel.com>

---
Arun R Murthy (3):
      drm/plane: Add new plane property IN_FORMATS_ASYNC
      drm/plane: modify create_in_formats to accommodate async
      drm/i915/display: Add i915 hook for format_mod_supported_async

 drivers/gpu/drm/drm_mode_config.c                  |  7 +++
 drivers/gpu/drm/drm_plane.c                        | 46 +++++++++++++-----
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 56 ++++++++++++++++------
 include/drm/drm_mode_config.h                      |  6 +++
 include/drm/drm_plane.h                            | 20 ++++++++
 5 files changed, 109 insertions(+), 26 deletions(-)
---
base-commit: a19d8731db07e41101ed00b9d86ac8868df2a763
change-id: 20250102-asyn-bf76730501cc

Best regards,
-- 
Arun R Murthy <arun.r.murthy@intel.com>

[PATCH v4 1/3] drm/plane: Add new plane property IN_FORMATS_ASYNC

[Arun R Murthy]

There exists a property IN_FORMATS which exposes the plane supported
modifiers/formats to the user. In some platforms when asynchronous flips
are used all of modifiers/formats mentioned in IN_FORMATS are not
supported. This patch adds a new plane property IN_FORMATS_ASYNC to
expose the async flips supported modifiers/formats so that user can use
this information ahead and done flips with unsupported
formats/modifiers. This will save flips failures.
Add a new function pointer similar to format_mod_supported specifically
for asynchronous flips.

v2: Remove async variable from drm_plane (Ville)
v3: Add new function pointer for async (Ville)

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/drm_mode_config.c |  7 +++++++
 drivers/gpu/drm/drm_plane.c       |  6 ++++++
 include/drm/drm_mode_config.h     |  6 ++++++
 include/drm/drm_plane.h           | 20 ++++++++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 8642a2fb25a90116dab975aa0ab6b51deafb4b96..dbbef20753f834a85ae9ded748cff2b3f0e85043 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -388,6 +388,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.size_hints_property = prop;
 
+	prop = drm_property_create(dev,
+				   DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+				   "IN_FORMATS_ASYNC", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.async_modifiers_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index a28b22fdd7a41aca82d097d42237851da9a0a79b..416818e54ccffcf3d3aada2723e96ce8ccf1dd97 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -141,6 +141,12 @@
  *     various bugs in this area with inconsistencies between the capability
  *     flag and per-plane properties.
  *
+ * IN_FORMATS_ASYNC:
+ *     Blob property which contains the set of buffer format and modifier
+ *     pairs supported by this plane for asynchronous flips. The blob is a struct
+ *     drm_format_modifier_blob. Without this property the plane doesn't
+ *     support buffers with modifiers. Userspace cannot change this property.
+ *
  * SIZE_HINTS:
  *     Blob property which contains the set of recommended plane size
  *     which can used for simple "cursor like" use cases (eg. no scaling).
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 271765e2e9f2da62aaf0d258828ef4196e14822e..0c116d6dfd277262b1a4c0f097fce2d719f43844 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -936,6 +936,12 @@ struct drm_mode_config {
 	 */
 	struct drm_property *modifiers_property;
 
+	/**
+	 * @async_modifiers_property: Plane property to list support modifier/format
+	 * combination for asynchronous flips.
+	 */
+	struct drm_property *async_modifiers_property;
+
 	/**
 	 * @size_hints_property: Plane SIZE_HINTS property.
 	 */
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index dd718c62ac31bf16606f3ee9f025a5b171cd1e67..179da19d602ac94cb6eeea3e6522ae5c7738bb46 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -549,6 +549,26 @@ struct drm_plane_funcs {
 	 */
 	bool (*format_mod_supported)(struct drm_plane *plane, uint32_t format,
 				     uint64_t modifier);
+	/**
+	 * @format_mod_supported_async:
+	 *
+	 * This optional hook is used for the DRM to determine if for
+	 * asynchronous flip the given format/modifier combination is valid for
+	 * the plane. This allows the DRM to generate the correct format
+	 * bitmask (which formats apply to which modifier), and to validate
+	 * modifiers at atomic_check time.
+	 *
+	 * If not present, then any modifier in the plane's modifier
+	 * list is allowed with any of the plane's formats.
+	 *
+	 * Returns:
+	 *
+	 * True if the given modifier is valid for that format on the plane.
+	 * False otherwise.
+	 */
+	bool (*format_mod_supported_async)(struct drm_plane *plane,
+					   uint32_t format, uint64_t modifier);
+
 };
 
 /**

-- 
2.25.1

[PATCH v4 2/3] drm/plane: modify create_in_formats to accommodate async

[Arun R Murthy]

create_in_formats creates the list of supported format/modifiers for
synchronous flips, modify the same function so as to take the
format_mod_supported as argument and create list of format/modifier for
async as well.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ceb64613bd5e0a663 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -191,7 +191,11 @@ modifiers_ptr(struct drm_format_modifier_blob *blob)
 	return (struct drm_format_modifier *)(((char *)blob) + blob->modifiers_offset);
 }
 
-static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane)
+static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane,
+				 bool (*format_mod_supported)
+						(struct drm_plane *plane,
+						 uint32_t format,
+						 uint64_t modifier))
 {
 	const struct drm_mode_config *config = &dev->mode_config;
 	struct drm_property_blob *blob;
@@ -235,10 +239,10 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 	mod = modifiers_ptr(blob_data);
 	for (i = 0; i < plane->modifier_count; i++) {
 		for (j = 0; j < plane->format_count; j++) {
-			if (!plane->funcs->format_mod_supported ||
-			    plane->funcs->format_mod_supported(plane,
-							       plane->format_types[j],
-							       plane->modifiers[i])) {
+			if (!format_mod_supported || format_mod_supported
+							(plane,
+							 plane->format_types[j],
+							 plane->modifiers[i])) {
 				mod->formats |= 1ULL << j;
 			}
 		}
@@ -249,10 +253,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 		mod++;
 	}
 
-	drm_object_attach_property(&plane->base, config->modifiers_property,
-				   blob->base.id);
-
-	return 0;
+	return blob->base.id;
 }
 
 /**
@@ -369,6 +370,7 @@ static int __drm_universal_plane_init(struct drm_device *dev,
 	};
 	unsigned int format_modifier_count = 0;
 	int ret;
+	int blob_id;
 
 	/* plane index is used with 32bit bitmasks */
 	if (WARN_ON(config->num_total_plane >= 32))
@@ -475,8 +477,24 @@ static int __drm_universal_plane_init(struct drm_device *dev,
 		drm_plane_create_hotspot_properties(plane);
 	}
 
-	if (format_modifier_count)
-		create_in_format_blob(dev, plane);
+	if (format_modifier_count) {
+		blob_id = create_in_format_blob(dev, plane,
+						plane->funcs->format_mod_supported);
+		if (blob_id)
+			drm_object_attach_property(&plane->base,
+						   config->modifiers_property,
+						   blob_id);
+	}
+
+	if (plane->funcs->format_mod_supported_async) {
+		blob_id = create_in_format_blob(dev, plane,
+						plane->funcs->format_mod_supported_async);
+		if (blob_id)
+			drm_object_attach_property(&plane->base,
+						   config->async_modifiers_property,
+						   blob_id);
+	}
+
 
 	return 0;
 }

-- 
2.25.1

[PATCH v4 3/3] drm/i915/display: Add i915 hook for format_mod_supported_async

[Arun R Murthy]

Hook up the newly added plane function pointer
format_mod_supported_async to populate the modifiers/formats supported
by asynchronous flips.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 56 ++++++++++++++++------
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index ba5db553c374259f8f3246c1408b55d32c8794e5..96d53b22715cf397483acbdb23b4dd60d6cbb604 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -511,6 +511,33 @@ skl_plane_max_stride(struct intel_plane *plane,
 				modifier, rotation,
 				max_pixels, max_bytes);
 }
+static bool intel_plane_async_formats(struct intel_plane *plane, uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_ABGR8888:
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_XBGR16161616F:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool intel_plane_format_mod_supported_async(struct drm_plane *plane,
+						   uint32_t format,
+						   uint64_t modifier)
+{
+	if (intel_plane_can_async_flip(to_intel_plane(plane), modifier))
+		return false;
+
+	return intel_plane_async_formats(to_intel_plane(plane), format);
+}
 
 static bool tgl_plane_can_async_flip(u64 modifier)
 {
@@ -2559,30 +2586,29 @@ static bool tgl_plane_format_mod_supported(struct drm_plane *_plane,
 	}
 }
 
+#define INTEL_PLANE_FUNCS \
+	.update_plane = drm_atomic_helper_update_plane, \
+	.disable_plane = drm_atomic_helper_disable_plane, \
+	.destroy = intel_plane_destroy, \
+	.atomic_duplicate_state = intel_plane_duplicate_state, \
+	.atomic_destroy_state = intel_plane_destroy_state, \
+	.format_mod_supported_async = intel_plane_format_mod_supported_async
+
 static const struct drm_plane_funcs skl_plane_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = intel_plane_destroy,
-	.atomic_duplicate_state = intel_plane_duplicate_state,
-	.atomic_destroy_state = intel_plane_destroy_state,
+	INTEL_PLANE_FUNCS,
+
 	.format_mod_supported = skl_plane_format_mod_supported,
 };
 
 static const struct drm_plane_funcs icl_plane_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = intel_plane_destroy,
-	.atomic_duplicate_state = intel_plane_duplicate_state,
-	.atomic_destroy_state = intel_plane_destroy_state,
+	INTEL_PLANE_FUNCS,
+
 	.format_mod_supported = icl_plane_format_mod_supported,
 };
 
 static const struct drm_plane_funcs tgl_plane_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = intel_plane_destroy,
-	.atomic_duplicate_state = intel_plane_duplicate_state,
-	.atomic_destroy_state = intel_plane_destroy_state,
+	INTEL_PLANE_FUNCS,
+
 	.format_mod_supported = tgl_plane_format_mod_supported,
 };
 

-- 
2.25.1

✗ Fi.CI.CHECKPATCH: warning for Expose modifiers/formats supported by async flips (rev3)

[Patchwork]

== Series Details ==

Series: Expose modifiers/formats supported by async flips (rev3)
URL   : https://patchwork.freedesktop.org/series/140935/
State : warning

== Summary ==

Error: dim checkpatch failed
4b314a7da773 drm/plane: Add new plane property IN_FORMATS_ASYNC
-:99: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t'
#99: FILE: include/drm/drm_plane.h:570:
+					   uint32_t format, uint64_t modifier);

total: 0 errors, 0 warnings, 1 checks, 63 lines checked
a17169f68d28 drm/plane: modify create_in_formats to accommodate async
-:25: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t'
#25: FILE: drivers/gpu/drm/drm_plane.c:197:
+						 uint32_t format,

total: 0 errors, 0 warnings, 1 checks, 70 lines checked
0db53f5b101c drm/i915/display: Add i915 hook for format_mod_supported_async
-:21: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#21: FILE: drivers/gpu/drm/i915/display/skl_universal_plane.c:514:
 }
+static bool intel_plane_async_formats(struct intel_plane *plane, uint32_t format)

-:40: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t'
#40: FILE: drivers/gpu/drm/i915/display/skl_universal_plane.c:533:
+						   uint32_t format,

total: 0 errors, 0 warnings, 2 checks, 77 lines checked

✗ Fi.CI.SPARSE: warning for Expose modifiers/formats supported by async flips (rev3)

[Patchwork]

== Series Details ==

Series: Expose modifiers/formats supported by async flips (rev3)
URL   : https://patchwork.freedesktop.org/series/140935/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✓ i915.CI.BAT: success for Expose modifiers/formats supported by async flips (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5035 bytes --]

== Series Details ==

Series: Expose modifiers/formats supported by async flips (rev3)
URL   : https://patchwork.freedesktop.org/series/140935/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_16069 -> Patchwork_140935v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/index.html

Participating hosts (40 -> 39)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (2): bat-atsm-1 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_140935v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests:
    - fi-pnv-d510:        NOTRUN -> [INCOMPLETE][1] ([i915#12904]) +1 other test incomplete
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/fi-pnv-d510/igt@dmabuf@all-tests.html

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-7:          [PASS][2] -> [FAIL][3] ([i915#13633])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-dg1-7/igt@i915_pm_rpm@module-reload.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         [PASS][4] -> [SKIP][5] ([i915#9197]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_psr@psr-primary-mmap-gtt:
    - fi-pnv-d510:        NOTRUN -> [SKIP][6] +33 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/fi-pnv-d510/igt@kms_psr@psr-primary-mmap-gtt.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-twl-1:          [ABORT][7] ([i915#12919] / [i915#13503]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-twl-1/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-twl-1/igt@i915_selftest@live.html
    - bat-adlp-11:        [DMESG-WARN][9] ([i915#13570]) -> [PASS][10] +1 other test pass
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-adlp-11/igt@i915_selftest@live.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-adlp-11/igt@i915_selftest@live.html

  * igt@i915_selftest@live@gt_heartbeat:
    - bat-twl-1:          [ABORT][11] ([i915#12919]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-twl-1/igt@i915_selftest@live@gt_heartbeat.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-twl-1/igt@i915_selftest@live@gt_heartbeat.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-kbl-7567u:       [DMESG-WARN][13] ([i915#11621] / [i915#180]) -> [DMESG-WARN][14] ([i915#11621] / [i915#180] / [i915#1982])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/fi-kbl-7567u/igt@i915_module_load@reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/fi-kbl-7567u/igt@i915_module_load@reload.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [DMESG-FAIL][15] ([i915#12061]) -> [DMESG-FAIL][16] ([i915#12061] / [i915#12435])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/bat-arlh-3/igt@i915_selftest@live.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/bat-arlh-3/igt@i915_selftest@live.html

  
  [i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12435]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12435
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904
  [i915#12919]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12919
  [i915#13503]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13503
  [i915#13570]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13570
  [i915#13633]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13633
  [i915#180]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1982
  [i915#9197]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9197


Build changes
-------------

  * Linux: CI_DRM_16069 -> Patchwork_140935v3

  CI-20190529: 20190529
  CI_DRM_16069: 5f025a38e4ddb453bfbf89d4ab37facf07427721 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8223: ccfe042787b082c06402ff9af257f8338b8edd5e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_140935v3: 5f025a38e4ddb453bfbf89d4ab37facf07427721 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/index.html

[-- Attachment #2: Type: text/html, Size: 6121 bytes --]

✓ i915.CI.Full: success for Expose modifiers/formats supported by async flips (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6856 bytes --]

== Series Details ==

Series: Expose modifiers/formats supported by async flips (rev3)
URL   : https://patchwork.freedesktop.org/series/140935/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_16069_full -> Patchwork_140935v3_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (12 -> 12)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in Patchwork_140935v3_full that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@i915_module_load@load:
    - shard-dg1:          ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [DMESG-WARN][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22]) ([i915#4423]) -> ([PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-13/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-14/igt@i915_module_load@load.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-14/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-14/igt@i915_module_load@load.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-14/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-14/igt@i915_module_load@load.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-17/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-17/igt@i915_module_load@load.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-17/igt@i915_module_load@load.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-17/igt@i915_module_load@load.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-18/igt@i915_module_load@load.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-12/igt@i915_module_load@load.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-12/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-12/igt@i915_module_load@load.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-12/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-13/igt@i915_module_load@load.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-13/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-13/igt@i915_module_load@load.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-13/igt@i915_module_load@load.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-18/igt@i915_module_load@load.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-18/igt@i915_module_load@load.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16069/shard-dg1-18/igt@i915_module_load@load.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-12/igt@i915_module_load@load.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-12/igt@i915_module_load@load.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-12/igt@i915_module_load@load.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-14/igt@i915_module_load@load.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-13/igt@i915_module_load@load.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-13/igt@i915_module_load@load.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-13/igt@i915_module_load@load.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-13/igt@i915_module_load@load.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-13/igt@i915_module_load@load.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-14/igt@i915_module_load@load.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-14/igt@i915_module_load@load.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-14/igt@i915_module_load@load.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-17/igt@i915_module_load@load.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-17/igt@i915_module_load@load.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-17/igt@i915_module_load@load.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-17/igt@i915_module_load@load.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-18/igt@i915_module_load@load.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-18/igt@i915_module_load@load.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-18/igt@i915_module_load@load.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-18/igt@i915_module_load@load.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-18/igt@i915_module_load@load.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-12/igt@i915_module_load@load.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/shard-dg1-12/igt@i915_module_load@load.html

  
  [i915#4423]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4423


Build changes
-------------

  * Linux: CI_DRM_16069 -> Patchwork_140935v3

  CI_DRM_16069: 5f025a38e4ddb453bfbf89d4ab37facf07427721 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8223: ccfe042787b082c06402ff9af257f8338b8edd5e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_140935v3: 5f025a38e4ddb453bfbf89d4ab37facf07427721 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140935v3/index.html

[-- Attachment #2: Type: text/html, Size: 7456 bytes --]

RE: [PATCH v4 2/3] drm/plane: modify create_in_formats to accommodate async

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Wednesday, February 5, 2025 3:57 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> Subject: [PATCH v4 2/3] drm/plane: modify create_in_formats to
> accommodate async
> 
> create_in_formats creates the list of supported format/modifiers for
> synchronous flips, modify the same function so as to take the
> format_mod_supported as argument and create list of format/modifier for
> async as well.
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++--------
> ---
>  1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index
> 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce
> b64613bd5e0a663 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -191,7 +191,11 @@ modifiers_ptr(struct drm_format_modifier_blob
> *blob)
>  	return (struct drm_format_modifier *)(((char *)blob) + blob-
> >modifiers_offset);  }
> 
> -static int create_in_format_blob(struct drm_device *dev, struct drm_plane
> *plane)
> +static int create_in_format_blob(struct drm_device *dev, struct drm_plane
> *plane,
> +				 bool (*format_mod_supported)
> +						(struct drm_plane *plane,
> +						 uint32_t format,
> +						 uint64_t modifier))
>  {
>  	const struct drm_mode_config *config = &dev->mode_config;
>  	struct drm_property_blob *blob;
> @@ -235,10 +239,10 @@ static int create_in_format_blob(struct drm_device
> *dev, struct drm_plane *plane
>  	mod = modifiers_ptr(blob_data);
>  	for (i = 0; i < plane->modifier_count; i++) {
>  		for (j = 0; j < plane->format_count; j++) {
> -			if (!plane->funcs->format_mod_supported ||
> -			    plane->funcs->format_mod_supported(plane,
> -							       plane-
> >format_types[j],
> -							       plane-
> >modifiers[i])) {
> +			if (!format_mod_supported ||
> format_mod_supported
> +							(plane,
> +							 plane-
> >format_types[j],
> +							 plane->modifiers[i]))
> {
>  				mod->formats |= 1ULL << j;
>  			}
>  		}
> @@ -249,10 +253,7 @@ static int create_in_format_blob(struct drm_device
> *dev, struct drm_plane *plane
>  		mod++;
>  	}
> 
> -	drm_object_attach_property(&plane->base, config-
> >modifiers_property,
> -				   blob->base.id);
> -
> -	return 0;
> +	return blob->base.id;

I think we can return the blob instead of the id, more on this later.

>  }
> 
>  /**
> @@ -369,6 +370,7 @@ static int __drm_universal_plane_init(struct
> drm_device *dev,
>  	};
>  	unsigned int format_modifier_count = 0;
>  	int ret;
> +	int blob_id;
> 
>  	/* plane index is used with 32bit bitmasks */
>  	if (WARN_ON(config->num_total_plane >= 32)) @@ -475,8 +477,24
> @@ static int __drm_universal_plane_init(struct drm_device *dev,
>  		drm_plane_create_hotspot_properties(plane);
>  	}
> 
> -	if (format_modifier_count)
> -		create_in_format_blob(dev, plane);
> +	if (format_modifier_count) {
> +		blob_id = create_in_format_blob(dev, plane,
> +						plane->funcs-
> >format_mod_supported);
> +		if (blob_id)

The function create_in_format_blob() can return negative value. This check does not work if that is the case.

> 0 can be used as the blob id's start range is from 1 to INT_MAX

Regards

Chaitanya

> +			drm_object_attach_property(&plane->base,
> +						   config->modifiers_property,
> +						   blob_id);
> +	}
> +
> +	if (plane->funcs->format_mod_supported_async) {
> +		blob_id = create_in_format_blob(dev, plane,
> +						plane->funcs-
> >format_mod_supported_async);
> +		if (blob_id)
> +			drm_object_attach_property(&plane->base,
> +						   config-
> >async_modifiers_property,
> +						   blob_id);
> +	}
> +
> 
>  	return 0;
>  }
> 
> --
> 2.25.1

RE: [PATCH v4 1/3] drm/plane: Add new plane property IN_FORMATS_ASYNC

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Wednesday, February 5, 2025 3:57 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> Subject: [PATCH v4 1/3] drm/plane: Add new plane property
> IN_FORMATS_ASYNC
> 
> There exists a property IN_FORMATS which exposes the plane supported
> modifiers/formats to the user. In some platforms when asynchronous flips are
> used all of modifiers/formats mentioned in IN_FORMATS are not supported.
> This patch adds a new plane property IN_FORMATS_ASYNC to expose the
> async flips supported modifiers/formats so that user can use this information
> ahead and done flips with unsupported formats/modifiers. This will save flips

s/done/do
s/flips/flip

> failures.
> Add a new function pointer similar to format_mod_supported specifically for
> asynchronous flips.
> 
> v2: Remove async variable from drm_plane (Ville)
> v3: Add new function pointer for async (Ville)
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c |  7 +++++++
>  drivers/gpu/drm/drm_plane.c       |  6 ++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  include/drm/drm_plane.h           | 20 ++++++++++++++++++++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index
> 8642a2fb25a90116dab975aa0ab6b51deafb4b96..dbbef20753f834a85ae9ded
> 748cff2b3f0e85043 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -388,6 +388,13 @@ static int
> drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.size_hints_property = prop;
> 
> +	prop = drm_property_create(dev,
> +				   DRM_MODE_PROP_IMMUTABLE |
> DRM_MODE_PROP_BLOB,
> +				   "IN_FORMATS_ASYNC", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.async_modifiers_property = prop;
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index
> a28b22fdd7a41aca82d097d42237851da9a0a79b..416818e54ccffcf3d3aada27
> 23e96ce8ccf1dd97 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -141,6 +141,12 @@
>   *     various bugs in this area with inconsistencies between the capability
>   *     flag and per-plane properties.
>   *
> + * IN_FORMATS_ASYNC:
> + *     Blob property which contains the set of buffer format and modifier
> + *     pairs supported by this plane for asynchronous flips. The blob is a struct
> + *     drm_format_modifier_blob. Without this property the plane doesn't
> + *     support buffers with modifiers.

As pointed out in my previous comment we should remove this line or replace it with something like this

Without this property the plane supports all the format modifier pair listed by IN_FORMAT for asynchronous flips.

> Userspace cannot change this property.
> + *
>   * SIZE_HINTS:
>   *     Blob property which contains the set of recommended plane size
>   *     which can used for simple "cursor like" use cases (eg. no scaling).
> diff --git a/include/drm/drm_mode_config.h
> b/include/drm/drm_mode_config.h index
> 271765e2e9f2da62aaf0d258828ef4196e14822e..0c116d6dfd277262b1a4c0f0
> 97fce2d719f43844 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -936,6 +936,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *modifiers_property;
> 
> +	/**
> +	 * @async_modifiers_property: Plane property to list support
> modifier/format
> +	 * combination for asynchronous flips.
> +	 */
> +	struct drm_property *async_modifiers_property;
> +
>  	/**
>  	 * @size_hints_property: Plane SIZE_HINTS property.
>  	 */
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> dd718c62ac31bf16606f3ee9f025a5b171cd1e67..179da19d602ac94cb6eeea3e
> 6522ae5c7738bb46 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -549,6 +549,26 @@ struct drm_plane_funcs {
>  	 */
>  	bool (*format_mod_supported)(struct drm_plane *plane, uint32_t
> format,
>  				     uint64_t modifier);
> +	/**
> +	 * @format_mod_supported_async:
> +	 *
> +	 * This optional hook is used for the DRM to determine if for
> +	 * asynchronous flip the given format/modifier combination is valid
> for
> +	 * the plane. This allows the DRM to generate the correct format
> +	 * bitmask (which formats apply to which modifier), and to validate
> +	 * modifiers at atomic_check time.
> +	 *
> +	 * If not present, then any modifier in the plane's modifier
> +	 * list is allowed with any of the plane's formats.
> +	 *

This is a bit misleading because userspace still needs to consider IN_FORMATS.
It cannot randomly pair any format with any modifier.

Regards

Chaitanya

> +	 * Returns:
> +	 *
> +	 * True if the given modifier is valid for that format on the plane.
> +	 * False otherwise.
> +	 */
> +	bool (*format_mod_supported_async)(struct drm_plane *plane,
> +					   uint32_t format, uint64_t modifier);
> +
>  };
> 
>  /**
> 
> --
> 2.25.1

Re: [PATCH v4 1/3] drm/plane: Add new plane property IN_FORMATS_ASYNC

[Murthy, Arun R]

On 12-02-2025 10:23, Borah, Chaitanya Kumar wrote:
>> -----Original Message-----
>> From: Murthy, Arun R <arun.r.murthy@intel.com>
>> Sent: Wednesday, February 5, 2025 3:57 PM
>> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
>> xe@lists.freedesktop.org
>> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
>> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
>> Subject: [PATCH v4 1/3] drm/plane: Add new plane property
>> IN_FORMATS_ASYNC
>>
>> There exists a property IN_FORMATS which exposes the plane supported
>> modifiers/formats to the user. In some platforms when asynchronous flips are
>> used all of modifiers/formats mentioned in IN_FORMATS are not supported.
>> This patch adds a new plane property IN_FORMATS_ASYNC to expose the
>> async flips supported modifiers/formats so that user can use this information
>> ahead and done flips with unsupported formats/modifiers. This will save flips
> s/done/do
> s/flips/flip
Done!
>> failures.
>> Add a new function pointer similar to format_mod_supported specifically for
>> asynchronous flips.
>>
>> v2: Remove async variable from drm_plane (Ville)
>> v3: Add new function pointer for async (Ville)
>>
>> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> ---
>>   drivers/gpu/drm/drm_mode_config.c |  7 +++++++
>>   drivers/gpu/drm/drm_plane.c       |  6 ++++++
>>   include/drm/drm_mode_config.h     |  6 ++++++
>>   include/drm/drm_plane.h           | 20 ++++++++++++++++++++
>>   4 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>> b/drivers/gpu/drm/drm_mode_config.c
>> index
>> 8642a2fb25a90116dab975aa0ab6b51deafb4b96..dbbef20753f834a85ae9ded
>> 748cff2b3f0e85043 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -388,6 +388,13 @@ static int
>> drm_mode_create_standard_properties(struct drm_device *dev)
>>   		return -ENOMEM;
>>   	dev->mode_config.size_hints_property = prop;
>>
>> +	prop = drm_property_create(dev,
>> +				   DRM_MODE_PROP_IMMUTABLE |
>> DRM_MODE_PROP_BLOB,
>> +				   "IN_FORMATS_ASYNC", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.async_modifiers_property = prop;
>> +
>>   	return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index
>> a28b22fdd7a41aca82d097d42237851da9a0a79b..416818e54ccffcf3d3aada27
>> 23e96ce8ccf1dd97 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -141,6 +141,12 @@
>>    *     various bugs in this area with inconsistencies between the capability
>>    *     flag and per-plane properties.
>>    *
>> + * IN_FORMATS_ASYNC:
>> + *     Blob property which contains the set of buffer format and modifier
>> + *     pairs supported by this plane for asynchronous flips. The blob is a struct
>> + *     drm_format_modifier_blob. Without this property the plane doesn't
>> + *     support buffers with modifiers.
> As pointed out in my previous comment we should remove this line or replace it with something like this
>
> Without this property the plane supports all the format modifier pair listed by IN_FORMAT for asynchronous flips.

Done!

>> Userspace cannot change this property.
>> + *
>>    * SIZE_HINTS:
>>    *     Blob property which contains the set of recommended plane size
>>    *     which can used for simple "cursor like" use cases (eg. no scaling).
>> diff --git a/include/drm/drm_mode_config.h
>> b/include/drm/drm_mode_config.h index
>> 271765e2e9f2da62aaf0d258828ef4196e14822e..0c116d6dfd277262b1a4c0f0
>> 97fce2d719f43844 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -936,6 +936,12 @@ struct drm_mode_config {
>>   	 */
>>   	struct drm_property *modifiers_property;
>>
>> +	/**
>> +	 * @async_modifiers_property: Plane property to list support
>> modifier/format
>> +	 * combination for asynchronous flips.
>> +	 */
>> +	struct drm_property *async_modifiers_property;
>> +
>>   	/**
>>   	 * @size_hints_property: Plane SIZE_HINTS property.
>>   	 */
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
>> dd718c62ac31bf16606f3ee9f025a5b171cd1e67..179da19d602ac94cb6eeea3e
>> 6522ae5c7738bb46 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -549,6 +549,26 @@ struct drm_plane_funcs {
>>   	 */
>>   	bool (*format_mod_supported)(struct drm_plane *plane, uint32_t
>> format,
>>   				     uint64_t modifier);
>> +	/**
>> +	 * @format_mod_supported_async:
>> +	 *
>> +	 * This optional hook is used for the DRM to determine if for
>> +	 * asynchronous flip the given format/modifier combination is valid
>> for
>> +	 * the plane. This allows the DRM to generate the correct format
>> +	 * bitmask (which formats apply to which modifier), and to validate
>> +	 * modifiers at atomic_check time.
>> +	 *
>> +	 * If not present, then any modifier in the plane's modifier
>> +	 * list is allowed with any of the plane's formats.
>> +	 *
> This is a bit misleading because userspace still needs to consider IN_FORMATS.
> It cannot randomly pair any format with any modifier.

Done

Thanks and Regards,
Arun R Murthy
--------------------

> Regards
>
> Chaitanya
>
>> +	 * Returns:
>> +	 *
>> +	 * True if the given modifier is valid for that format on the plane.
>> +	 * False otherwise.
>> +	 */
>> +	bool (*format_mod_supported_async)(struct drm_plane *plane,
>> +					   uint32_t format, uint64_t modifier);
>> +
>>   };
>>
>>   /**
>>
>> --
>> 2.25.1

Re: [PATCH v4 2/3] drm/plane: modify create_in_formats to accommodate async

[Murthy, Arun R]

On 12-02-2025 10:23, Borah, Chaitanya Kumar wrote:
>> -----Original Message-----
>> From: Murthy, Arun R <arun.r.murthy@intel.com>
>> Sent: Wednesday, February 5, 2025 3:57 PM
>> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
>> xe@lists.freedesktop.org
>> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
>> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
>> Subject: [PATCH v4 2/3] drm/plane: modify create_in_formats to
>> accommodate async
>>
>> create_in_formats creates the list of supported format/modifiers for
>> synchronous flips, modify the same function so as to take the
>> format_mod_supported as argument and create list of format/modifier for
>> async as well.
>>
>> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> ---
>>   drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++--------
>> ---
>>   1 file changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index
>> 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce
>> b64613bd5e0a663 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -191,7 +191,11 @@ modifiers_ptr(struct drm_format_modifier_blob
>> *blob)
>>   	return (struct drm_format_modifier *)(((char *)blob) + blob-
>>> modifiers_offset);  }
>> -static int create_in_format_blob(struct drm_device *dev, struct drm_plane
>> *plane)
>> +static int create_in_format_blob(struct drm_device *dev, struct drm_plane
>> *plane,
>> +				 bool (*format_mod_supported)
>> +						(struct drm_plane *plane,
>> +						 uint32_t format,
>> +						 uint64_t modifier))
>>   {
>>   	const struct drm_mode_config *config = &dev->mode_config;
>>   	struct drm_property_blob *blob;
>> @@ -235,10 +239,10 @@ static int create_in_format_blob(struct drm_device
>> *dev, struct drm_plane *plane
>>   	mod = modifiers_ptr(blob_data);
>>   	for (i = 0; i < plane->modifier_count; i++) {
>>   		for (j = 0; j < plane->format_count; j++) {
>> -			if (!plane->funcs->format_mod_supported ||
>> -			    plane->funcs->format_mod_supported(plane,
>> -							       plane-
>>> format_types[j],
>> -							       plane-
>>> modifiers[i])) {
>> +			if (!format_mod_supported ||
>> format_mod_supported
>> +							(plane,
>> +							 plane-
>>> format_types[j],
>> +							 plane->modifiers[i]))
>> {
>>   				mod->formats |= 1ULL << j;
>>   			}
>>   		}
>> @@ -249,10 +253,7 @@ static int create_in_format_blob(struct drm_device
>> *dev, struct drm_plane *plane
>>   		mod++;
>>   	}
>>
>> -	drm_object_attach_property(&plane->base, config-
>>> modifiers_property,
>> -				   blob->base.id);
>> -
>> -	return 0;
>> +	return blob->base.id;
> I think we can return the blob instead of the id, more on this later.
Do you mean pointer to the blob. That can also be done.
But any advantage of doing so, because from blob_id we can get the blob.
>
>>   }
>>
>>   /**
>> @@ -369,6 +370,7 @@ static int __drm_universal_plane_init(struct
>> drm_device *dev,
>>   	};
>>   	unsigned int format_modifier_count = 0;
>>   	int ret;
>> +	int blob_id;
>>
>>   	/* plane index is used with 32bit bitmasks */
>>   	if (WARN_ON(config->num_total_plane >= 32)) @@ -475,8 +477,24
>> @@ static int __drm_universal_plane_init(struct drm_device *dev,
>>   		drm_plane_create_hotspot_properties(plane);
>>   	}
>>
>> -	if (format_modifier_count)
>> -		create_in_format_blob(dev, plane);
>> +	if (format_modifier_count) {
>> +		blob_id = create_in_format_blob(dev, plane,
>> +						plane->funcs-
>>> format_mod_supported);
>> +		if (blob_id)
> The function create_in_format_blob() can return negative value. This check does not work if that is the case.

corrected!

Thanks and Regards,
Arun R Murthy
-------------------

>> 0 can be used as the blob id's start range is from 1 to INT_MAX
> Regards
>
> Chaitanya
>
>> +			drm_object_attach_property(&plane->base,
>> +						   config->modifiers_property,
>> +						   blob_id);
>> +	}
>> +
>> +	if (plane->funcs->format_mod_supported_async) {
>> +		blob_id = create_in_format_blob(dev, plane,
>> +						plane->funcs-
>>> format_mod_supported_async);
>> +		if (blob_id)
>> +			drm_object_attach_property(&plane->base,
>> +						   config-
>>> async_modifiers_property,
>> +						   blob_id);
>> +	}
>> +
>>
>>   	return 0;
>>   }
>>
>> --
>> 2.25.1

RE: [PATCH v4 2/3] drm/plane: modify create_in_formats to accommodate async

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Wednesday, February 12, 2025 10:55 AM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; dri-
> devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [PATCH v4 2/3] drm/plane: modify create_in_formats to
> accommodate async
> 
> On 12-02-2025 10:23, Borah, Chaitanya Kumar wrote:
> >> -----Original Message-----
> >> From: Murthy, Arun R <arun.r.murthy@intel.com>
> >> Sent: Wednesday, February 5, 2025 3:57 PM
> >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> >> intel- xe@lists.freedesktop.org
> >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> >> Syrjala, Ville <ville.syrjala@intel.com>; Murthy, Arun R
> >> <arun.r.murthy@intel.com>
> >> Subject: [PATCH v4 2/3] drm/plane: modify create_in_formats to
> >> accommodate async
> >>
> >> create_in_formats creates the list of supported format/modifiers for
> >> synchronous flips, modify the same function so as to take the
> >> format_mod_supported as argument and create list of format/modifier
> >> for async as well.
> >>
> >> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_plane.c | 40
> >> +++++++++++++++++++++++++++++--------
> >> ---
> >>   1 file changed, 29 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_plane.c
> >> b/drivers/gpu/drm/drm_plane.c index
> >>
> 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce
> >> b64613bd5e0a663 100644
> >> --- a/drivers/gpu/drm/drm_plane.c
> >> +++ b/drivers/gpu/drm/drm_plane.c
> >> @@ -191,7 +191,11 @@ modifiers_ptr(struct drm_format_modifier_blob
> >> *blob)
> >>   	return (struct drm_format_modifier *)(((char *)blob) + blob-
> >>> modifiers_offset);  }
> >> -static int create_in_format_blob(struct drm_device *dev, struct
> >> drm_plane
> >> *plane)
> >> +static int create_in_format_blob(struct drm_device *dev, struct
> >> +drm_plane
> >> *plane,
> >> +				 bool (*format_mod_supported)
> >> +						(struct drm_plane *plane,
> >> +						 uint32_t format,
> >> +						 uint64_t modifier))
> >>   {
> >>   	const struct drm_mode_config *config = &dev->mode_config;
> >>   	struct drm_property_blob *blob;
> >> @@ -235,10 +239,10 @@ static int create_in_format_blob(struct
> >> drm_device *dev, struct drm_plane *plane
> >>   	mod = modifiers_ptr(blob_data);
> >>   	for (i = 0; i < plane->modifier_count; i++) {
> >>   		for (j = 0; j < plane->format_count; j++) {
> >> -			if (!plane->funcs->format_mod_supported ||
> >> -			    plane->funcs->format_mod_supported(plane,
> >> -							       plane-
> >>> format_types[j],
> >> -							       plane-
> >>> modifiers[i])) {
> >> +			if (!format_mod_supported ||
> >> format_mod_supported
> >> +							(plane,
> >> +							 plane-
> >>> format_types[j],
> >> +							 plane->modifiers[i]))
> >> {
> >>   				mod->formats |= 1ULL << j;
> >>   			}
> >>   		}
> >> @@ -249,10 +253,7 @@ static int create_in_format_blob(struct
> >> drm_device *dev, struct drm_plane *plane
> >>   		mod++;
> >>   	}
> >>
> >> -	drm_object_attach_property(&plane->base, config-
> >>> modifiers_property,
> >> -				   blob->base.id);
> >> -
> >> -	return 0;
> >> +	return blob->base.id;
> > I think we can return the blob instead of the id, more on this later.
> Do you mean pointer to the blob. That can also be done.
> But any advantage of doing so, because from blob_id we can get the blob.

Forgot to modify this comment. This was before I found out a valid  blob id is always greater than 0.

So you can go ahead with using the check of >0 as discussed below.

Regards

Chaitanya


> >
> >>   }
> >>
> >>   /**
> >> @@ -369,6 +370,7 @@ static int __drm_universal_plane_init(struct
> >> drm_device *dev,
> >>   	};
> >>   	unsigned int format_modifier_count = 0;
> >>   	int ret;
> >> +	int blob_id;
> >>
> >>   	/* plane index is used with 32bit bitmasks */
> >>   	if (WARN_ON(config->num_total_plane >= 32)) @@ -475,8 +477,24
> @@
> >> static int __drm_universal_plane_init(struct drm_device *dev,
> >>   		drm_plane_create_hotspot_properties(plane);
> >>   	}
> >>
> >> -	if (format_modifier_count)
> >> -		create_in_format_blob(dev, plane);
> >> +	if (format_modifier_count) {
> >> +		blob_id = create_in_format_blob(dev, plane,
> >> +						plane->funcs-
> >>> format_mod_supported);
> >> +		if (blob_id)
> > The function create_in_format_blob() can return negative value. This check
> does not work if that is the case.
> 
> corrected!
> 
> Thanks and Regards,
> Arun R Murthy
> -------------------
> 
> >> 0 can be used as the blob id's start range is from 1 to INT_MAX
> > Regards
> >
> > Chaitanya
> >
> >> +			drm_object_attach_property(&plane->base,
> >> +						   config->modifiers_property,
> >> +						   blob_id);
> >> +	}
> >> +
> >> +	if (plane->funcs->format_mod_supported_async) {
> >> +		blob_id = create_in_format_blob(dev, plane,
> >> +						plane->funcs-
> >>> format_mod_supported_async);
> >> +		if (blob_id)
> >> +			drm_object_attach_property(&plane->base,
> >> +						   config-
> >>> async_modifiers_property,
> >> +						   blob_id);
> >> +	}
> >> +
> >>
> >>   	return 0;
> >>   }
> >>
> >> --
> >> 2.25.1

RE: [PATCH v4 3/3] drm/i915/display: Add i915 hook for format_mod_supported_async

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Wednesday, February 5, 2025 3:57 PM
> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> Subject: [PATCH v4 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.
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 56 ++++++++++++++++-
> -----
>  1 file changed, 41 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index
> ba5db553c374259f8f3246c1408b55d32c8794e5..96d53b22715cf397483acbdb
> 23b4dd60d6cbb604 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -511,6 +511,33 @@ skl_plane_max_stride(struct intel_plane *plane,
>  				modifier, rotation,
>  				max_pixels, max_bytes);
>  }
> +static bool intel_plane_async_formats(struct intel_plane *plane,
> +uint32_t format) {
> +	switch (format) {
> +	case DRM_FORMAT_RGB565:
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_ABGR8888:
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_XBGR2101010:
> +	case DRM_FORMAT_XRGB16161616F:
> +	case DRM_FORMAT_XBGR16161616F:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool intel_plane_format_mod_supported_async(struct drm_plane
> *plane,
> +						   uint32_t format,
> +						   uint64_t modifier)
> +{
> +	if (intel_plane_can_async_flip(to_intel_plane(plane), modifier))
> +		return false;

This should be

if (!intel_plane_can_async_flip(to_intel_plane(plane), modifier))


I think we should eventually move to a switch case ladder that actually checks for compatibility among format-modifier pair
rather than checking them individually. Though we don't have a good use of it today, we might end up having scenarios where some formats
are supported only for certain modifiers and vice versa. But it can be taken up later.

I see some comments made on rev 2 [1] regarding re-using the format_mod_sync() hook for this.
I have not yet formed a strong opinion about it and with the above suggested change, it at least seems to work.


[1] https://patchwork.freedesktop.org/patch/631264/?series=140935&rev=2

> +
> +	return intel_plane_async_formats(to_intel_plane(plane), format); }
> 
>  static bool tgl_plane_can_async_flip(u64 modifier)  { @@ -2559,30 +2586,29
> @@ static bool tgl_plane_format_mod_supported(struct drm_plane *_plane,
>  	}
>  }
> 
> +#define INTEL_PLANE_FUNCS \
> +	.update_plane = drm_atomic_helper_update_plane, \
> +	.disable_plane = drm_atomic_helper_disable_plane, \
> +	.destroy = intel_plane_destroy, \
> +	.atomic_duplicate_state = intel_plane_duplicate_state, \
> +	.atomic_destroy_state = intel_plane_destroy_state, \
> +	.format_mod_supported_async =
> intel_plane_format_mod_supported_async
> +
>  static const struct drm_plane_funcs skl_plane_funcs = {
> -	.update_plane = drm_atomic_helper_update_plane,
> -	.disable_plane = drm_atomic_helper_disable_plane,
> -	.destroy = intel_plane_destroy,
> -	.atomic_duplicate_state = intel_plane_duplicate_state,
> -	.atomic_destroy_state = intel_plane_destroy_state,
> +	INTEL_PLANE_FUNCS,
> +
>  	.format_mod_supported = skl_plane_format_mod_supported,  };
> 
>  static const struct drm_plane_funcs icl_plane_funcs = {
> -	.update_plane = drm_atomic_helper_update_plane,
> -	.disable_plane = drm_atomic_helper_disable_plane,
> -	.destroy = intel_plane_destroy,
> -	.atomic_duplicate_state = intel_plane_duplicate_state,
> -	.atomic_destroy_state = intel_plane_destroy_state,
> +	INTEL_PLANE_FUNCS,
> +
>  	.format_mod_supported = icl_plane_format_mod_supported,  };
> 
>  static const struct drm_plane_funcs tgl_plane_funcs = {
> -	.update_plane = drm_atomic_helper_update_plane,
> -	.disable_plane = drm_atomic_helper_disable_plane,
> -	.destroy = intel_plane_destroy,
> -	.atomic_duplicate_state = intel_plane_duplicate_state,
> -	.atomic_destroy_state = intel_plane_destroy_state,
> +	INTEL_PLANE_FUNCS,
> +
>  	.format_mod_supported = tgl_plane_format_mod_supported,  };
> 

This leads to exposing the property even on planes that do not support async flip.
Functionally it should not be a problem as can_async is exposed only for primary plane.

May be we can live with it for now. Ville any thoughts?

Regards

Chaitanya

> 
> --
> 2.25.1

Re: [PATCH v4 3/3] drm/i915/display: Add i915 hook for format_mod_supported_async

[Murthy, Arun R]

[-- Attachment #1: Type: text/plain, Size: 5943 bytes --]

On 12-02-2025 13:02, Borah, Chaitanya Kumar wrote:
>> -----Original Message-----
>> From: Murthy, Arun R<arun.r.murthy@intel.com>
>> Sent: Wednesday, February 5, 2025 3:57 PM
>> To:dri-devel@lists.freedesktop.org;intel-gfx@lists.freedesktop.org; intel-
>> xe@lists.freedesktop.org
>> Cc: Borah, Chaitanya Kumar<chaitanya.kumar.borah@intel.com>; Syrjala,
>> Ville<ville.syrjala@intel.com>; Murthy, Arun R<arun.r.murthy@intel.com>
>> Subject: [PATCH v4 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.
>>
>> Signed-off-by: Arun R Murthy<arun.r.murthy@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 56 ++++++++++++++++-
>> -----
>>   1 file changed, 41 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index
>> ba5db553c374259f8f3246c1408b55d32c8794e5..96d53b22715cf397483acbdb
>> 23b4dd60d6cbb604 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -511,6 +511,33 @@ skl_plane_max_stride(struct intel_plane *plane,
>>   				modifier, rotation,
>>   				max_pixels, max_bytes);
>>   }
>> +static bool intel_plane_async_formats(struct intel_plane *plane,
>> +uint32_t format) {
>> +	switch (format) {
>> +	case DRM_FORMAT_RGB565:
>> +	case DRM_FORMAT_XRGB8888:
>> +	case DRM_FORMAT_XBGR8888:
>> +	case DRM_FORMAT_ARGB8888:
>> +	case DRM_FORMAT_ABGR8888:
>> +	case DRM_FORMAT_XRGB2101010:
>> +	case DRM_FORMAT_XBGR2101010:
>> +	case DRM_FORMAT_XRGB16161616F:
>> +	case DRM_FORMAT_XBGR16161616F:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static bool intel_plane_format_mod_supported_async(struct drm_plane
>> *plane,
>> +						   uint32_t format,
>> +						   uint64_t modifier)
>> +{
>> +	if (intel_plane_can_async_flip(to_intel_plane(plane), modifier))
>> +		return false;
> This should be
>
> if (!intel_plane_can_async_flip(to_intel_plane(plane), modifier))
Typo from my side, corrected!
> I think we should eventually move to a switch case ladder that actually checks for compatibility among format-modifier pair
> rather than checking them individually. Though we don't have a good use of it today, we might end up having scenarios where some formats
> are supported only for certain modifiers and vice versa. But it can be taken up later.
I am fine! I can switch back to my previous patchset from which 
achieving this switch case latter is much easier.
> I see some comments made on rev 2 [1] regarding re-using the format_mod_sync() hook for this.
> I have not yet formed a strong opinion about it and with the above suggested change, it at least seems to work.

The comment from rev 2 [1] is

> > intel_plane_format_mod_supported_async()
> > {
> > 	// some generic checks here (eg. reject planar formats)
> > 
> > 	return plane->format_mod_supported() &&
> > 		plane->can_async_flip();
> > }

Here some generic checks to reject plannar formats is being done from 
intel_plane_async_formats() After this we have the result as to if the 
format is supported for not and with plane->can_async_flip we have a 
result of modifier being supported or not. That itself should be 
sufficient. If you still prefer to make use of the call 
plane->format_mod_supported() with some hacks can be done. Let me know!

> [1]https://patchwork.freedesktop.org/patch/631264/?series=140935&rev=2
>
>> +
>> +	return intel_plane_async_formats(to_intel_plane(plane), format); }
>>
>>   static bool tgl_plane_can_async_flip(u64 modifier)  { @@ -2559,30 +2586,29
>> @@ static bool tgl_plane_format_mod_supported(struct drm_plane *_plane,
>>   	}
>>   }
>>
>> +#define INTEL_PLANE_FUNCS \
>> +	.update_plane = drm_atomic_helper_update_plane, \
>> +	.disable_plane = drm_atomic_helper_disable_plane, \
>> +	.destroy = intel_plane_destroy, \
>> +	.atomic_duplicate_state = intel_plane_duplicate_state, \
>> +	.atomic_destroy_state = intel_plane_destroy_state, \
>> +	.format_mod_supported_async =
>> intel_plane_format_mod_supported_async
>> +
>>   static const struct drm_plane_funcs skl_plane_funcs = {
>> -	.update_plane = drm_atomic_helper_update_plane,
>> -	.disable_plane = drm_atomic_helper_disable_plane,
>> -	.destroy = intel_plane_destroy,
>> -	.atomic_duplicate_state = intel_plane_duplicate_state,
>> -	.atomic_destroy_state = intel_plane_destroy_state,
>> +	INTEL_PLANE_FUNCS,
>> +
>>   	.format_mod_supported = skl_plane_format_mod_supported,  };
>>
>>   static const struct drm_plane_funcs icl_plane_funcs = {
>> -	.update_plane = drm_atomic_helper_update_plane,
>> -	.disable_plane = drm_atomic_helper_disable_plane,
>> -	.destroy = intel_plane_destroy,
>> -	.atomic_duplicate_state = intel_plane_duplicate_state,
>> -	.atomic_destroy_state = intel_plane_destroy_state,
>> +	INTEL_PLANE_FUNCS,
>> +
>>   	.format_mod_supported = icl_plane_format_mod_supported,  };
>>
>>   static const struct drm_plane_funcs tgl_plane_funcs = {
>> -	.update_plane = drm_atomic_helper_update_plane,
>> -	.disable_plane = drm_atomic_helper_disable_plane,
>> -	.destroy = intel_plane_destroy,
>> -	.atomic_duplicate_state = intel_plane_duplicate_state,
>> -	.atomic_destroy_state = intel_plane_destroy_state,
>> +	INTEL_PLANE_FUNCS,
>> +
>>   	.format_mod_supported = tgl_plane_format_mod_supported,  };
>>
> This leads to exposing the property even on planes that do not support async flip.
> Functionally it should not be a problem as can_async is exposed only for primary plane.

Yes property will be created for all the planes but data will be 
populated only for primary plane.

Thanks and Regards,
Arun R Murthy
-------------------

>
> May be we can live with it for now. Ville any thoughts?
>
> Regards
>
> Chaitanya
>
>> --
>> 2.25.1

[-- Attachment #2: Type: text/html, Size: 9396 bytes --]