dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/doc: Document that modifiers are always required for fb
@ 2020-09-02 10:24 Daniel Vetter
  2020-09-02 11:02 ` Pekka Paalanen
  2020-09-02 12:40 ` Simon Ser
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-09-02 10:24 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Stone, Marek Olšák, Daniel Vetter, Juston Li,
	Daniel Vetter

Even for legacy userspace, since otherwise GETFB2 is broken and if you
switch between modifier-less and modifier-aware compositors, smooth
transitions break.

Also it's just best practice to make sure modifiers are invariant for
a given drm_fb, and that a modifier-aware kms drivers only has one
place to store them, ignoring any old implicit bo flags or whatever
else might float around.

Motivated by some irc discussion with Bas about amdgpu modifier
support.

Fixes: 455e00f1412f ("drm: Add getfb2 ioctl")
Cc: Daniel Stone <daniels@collabora.com>
Cc: Juston Li <juston.li@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Cc: Marek Olšák <maraeo@gmail.com>
Cc: "Wentland, Harry" <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 include/drm/drm_mode_config.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index a18f73eb3cf6..5ffbb4ed5b35 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -58,6 +58,12 @@ struct drm_mode_config_funcs {
 	 * actual modifier used if the request doesn't have it specified,
 	 * ie. when (@mode_cmd->flags & DRM_MODE_FB_MODIFIERS) == 0.
 	 *
+	 * IMPORTANT: These implied modifiers for legacy userspace must be
+	 * stored in struct &drm_framebuffer, including all relevant metadata
+	 * like &drm_framebuffer.pitches and &drm_framebuffer.offsets if the
+	 * modifier enables additional planes beyond the fourcc pixel format
+	 * code. This is required by the GETFB2 ioctl.
+	 *
 	 * If the parameters are deemed valid and the backing storage objects in
 	 * the underlying memory manager all exist, then the driver allocates
 	 * a new &drm_framebuffer structure, subclassed to contain
@@ -915,6 +921,13 @@ struct drm_mode_config {
 	 * @allow_fb_modifiers:
 	 *
 	 * Whether the driver supports fb modifiers in the ADDFB2.1 ioctl call.
+	 *
+	 * IMPORTANT:
+	 *
+	 * If this is set the driver must fill out the full implicit modifier
+	 * information in their &drm_mode_config_funcs.fb_create hook for legacy
+	 * userspace which does not set modifiers. Otherwise the GETFB2 ioctl is
+	 * broken for modifier aware userspace.
 	 */
 	bool allow_fb_modifiers;
 
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH] drm/doc: Document that modifiers are always required for fb
@ 2020-09-17 16:47 Daniel Vetter
  2020-09-17 17:24 ` Bas Nieuwenhuizen
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2020-09-17 16:47 UTC (permalink / raw)
  To: DRI Development
  Cc: Pekka Paalanen, Daniel Stone, Marek Olšák,
	Daniel Vetter, Juston Li, Daniel Vetter

Even for legacy userspace, since otherwise GETFB2 is broken and if you
switch between modifier-less and modifier-aware compositors, smooth
transitions break.

Also it's just best practice to make sure modifiers are invariant for
a given drm_fb, and that a modifier-aware kms drivers only has one
place to store them, ignoring any old implicit bo flags or whatever
else might float around.

Motivated by some irc discussion with Bas about amdgpu modifier
support.

Acked-by: Simon Ser <contact@emersion.fr>
Acked-by: Daniel Stone <daniels@collabora.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Fixes: 455e00f1412f ("drm: Add getfb2 ioctl")
Cc: Daniel Stone <daniels@collabora.com>
Cc: Juston Li <juston.li@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Cc: Marek Olšák <maraeo@gmail.com>
Cc: "Wentland, Harry" <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
Sending this out again to double-check there's no objections or concerns.
From what I understand looking at the code the latest modifier patches for
amdgpu follow this now.

Cheers, Daniel
---
 include/drm/drm_mode_config.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index a18f73eb3cf6..5ffbb4ed5b35 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -58,6 +58,12 @@ struct drm_mode_config_funcs {
 	 * actual modifier used if the request doesn't have it specified,
 	 * ie. when (@mode_cmd->flags & DRM_MODE_FB_MODIFIERS) == 0.
 	 *
+	 * IMPORTANT: These implied modifiers for legacy userspace must be
+	 * stored in struct &drm_framebuffer, including all relevant metadata
+	 * like &drm_framebuffer.pitches and &drm_framebuffer.offsets if the
+	 * modifier enables additional planes beyond the fourcc pixel format
+	 * code. This is required by the GETFB2 ioctl.
+	 *
 	 * If the parameters are deemed valid and the backing storage objects in
 	 * the underlying memory manager all exist, then the driver allocates
 	 * a new &drm_framebuffer structure, subclassed to contain
@@ -915,6 +921,13 @@ struct drm_mode_config {
 	 * @allow_fb_modifiers:
 	 *
 	 * Whether the driver supports fb modifiers in the ADDFB2.1 ioctl call.
+	 *
+	 * IMPORTANT:
+	 *
+	 * If this is set the driver must fill out the full implicit modifier
+	 * information in their &drm_mode_config_funcs.fb_create hook for legacy
+	 * userspace which does not set modifiers. Otherwise the GETFB2 ioctl is
+	 * broken for modifier aware userspace.
 	 */
 	bool allow_fb_modifiers;
 
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-09-23 15:54 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 10:24 [PATCH] drm/doc: Document that modifiers are always required for fb Daniel Vetter
2020-09-02 11:02 ` Pekka Paalanen
2020-09-02 11:15   ` Daniel Vetter
2020-09-02 11:34     ` Pekka Paalanen
2020-09-02 12:40 ` Simon Ser
2020-09-02 12:44   ` Daniel Vetter
2020-09-02 12:49     ` Simon Ser
2020-09-02 12:56       ` Daniel Stone
2020-09-02 14:29       ` Bas Nieuwenhuizen
2020-09-02 14:29       ` Daniel Vetter
2020-09-02 14:59         ` Simon Ser
2020-09-07  8:31           ` Daniel Vetter
2020-09-07  8:37             ` Simon Ser
2020-09-07  8:41               ` Daniel Vetter
2020-09-07  9:07                 ` Pekka Paalanen
2020-09-07 13:51                   ` Daniel Vetter
2020-09-17 16:47 Daniel Vetter
2020-09-17 17:24 ` Bas Nieuwenhuizen
2020-09-23 15:53   ` Daniel Vetter

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox