[PATCH v1 00/13] Add DRM_COLOROP_3D_LUT_DMA for fast-loading 3D LUT

[PATCH v1 00/13] Add DRM_COLOROP_3D_LUT_DMA for fast-loading 3D LUT

[Harry Wentland]

Loading a DRM_COLOROP_3D_LUT on AMD HW takes a long time because the
driver has to load the entire 3D LUT via register writes.

Starting with DCN 4 the HW has the ability to load a 3D LUT via DMA,
which only requires an address update for programming the 3D LUT.

I propose exposing a new DRM_COLOROP_3D_LUT_DMA type to support this DMA
3D LUT loading mechanism. Users of the DRM_COLOROP_3D_LUT_DMA will parse
the supported formats via the LUT_3D_FORMATS blob property, select the
desired format via LUT_3D_FORMAT property, and pass the 3D LUT buffer as
a drm_framebuffer via FB_ID on the colorop.

For details see the patches and/or posted branches.

In addition to the patches posted here the 3d-fl-fixes kernel branch
contains a number of fixes that will be merged via the usual amdgpu
display process. It also contains a patch to enable support on Navi 4
(DCN 4.0.1), which won't be upstreamed since that generation requires a
userspace workaround. The 3d-fl-workaround IGT branch exists solely for
that purpose. Nonetheless, Navi 4 can serve as an enablement vehicle for
anyone who wants to see the new interface working.

The kwin branch makes use of a pipeline with DRM_COLOROP_3D_LUT_DMA if
such a pipeline is discovered before a pipeline with a
DRM_COLOROP_3D_LUT.

### Branches

kernel: https://gitlab.freedesktop.org/hwentland/linux/-/commits/3d-fl
kernel w/ fixes: https://gitlab.freedesktop.org/hwentland/linux/-/commits/3d-fl-fixes
IGT: https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/3d-fl
IGT Navi 4 w/a: https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/3d-fl-workaround
kwin: https://invent.kde.org/hwentlan/kwin/-/commits/3d-fl

Cc: Melissa Wen <mwen@igalia.com>
Cc: Autumn Ashton <misyl@froggi.es>
Cc: Xaver Hugl <xaver.hugl@gmail.com>

Harry Wentland (13):
  drm/colorop: Add DRM_COLOROP_3D_LUT_DMA
  drm/colorop: add DRM_COLOROP_3D_LUT_DMA object and state
  drm/atomic: wire up DMA 3D LUT colorop properties
  drm/atomic: validate DMA 3D LUT colorop framebuffer
  drm/atomic: add colorop prepare_fb and cleanup_fb helper hooks
  drm/tests: test colorop prepare_fb and cleanup_fb helpers
  drm/amd/display: advertise a DMA 3D LUT color pipeline
  drm/amd/display: pin the DMA 3D LUT framebuffer
  drm/amd/display: program the DMA 3D LUT colorop
  drm/amd/display: add 3D LUT DMA pitch alignment cap
  drm/amd/display: derive the DMA 3D LUT pitch from DC caps
  drm/amd/display: reject direct 3D LUT loading mode changes
  drm/amd/display: add KUnit tests for the DMA 3D LUT colorop

 drivers/gpu/drm/amd/display/Kconfig           |  13 +
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  68 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 131 +++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |   8 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  71 +++-
 .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c  | 286 ++++++++++++-
 .../amdgpu_dm/tests/amdgpu_dm_plane_test.c    |  65 +++
 drivers/gpu/drm/amd/display/dc/dc.h           |   2 +
 .../dc/resource/dcn42/dcn42_resource.c        |   2 +
 .../dc/resource/dcn42b/dcn42b_resource.c      |   2 +
 drivers/gpu/drm/drm_atomic.c                  |  66 +++
 drivers/gpu/drm/drm_atomic_helper.c           |  93 ++++
 drivers/gpu/drm/drm_atomic_uapi.c             |  28 ++
 drivers/gpu/drm/drm_colorop.c                 |  96 +++++
 drivers/gpu/drm/drm_framebuffer.c             |  45 +-
 drivers/gpu/drm/drm_gem_atomic_helper.c       |  81 ++++
 drivers/gpu/drm/tests/Makefile                |   1 +
 .../gpu/drm/tests/drm_colorop_helper_test.c   | 397 ++++++++++++++++++
 include/drm/drm_atomic.h                      |  20 +
 include/drm/drm_colorop.h                     |  71 ++++
 include/drm/drm_gem_atomic_helper.h           |  11 +
 include/drm/drm_modeset_helper_vtables.h      |  62 +++
 include/uapi/drm/drm.h                        |   7 +-
 include/uapi/drm/drm_mode.h                   |  79 ++++
 24 files changed, 1672 insertions(+), 33 deletions(-)
 create mode 100644 drivers/gpu/drm/tests/drm_colorop_helper_test.c

--
2.56.0

[PATCH v1 01/13] drm/colorop: Add DRM_COLOROP_3D_LUT_DMA

[Harry Wentland]

Add the uAPI for a fast-loading (DMA) 3D LUT colorop. Unlike the existing
DRM_COLOROP_3D_LUT, which carries its data inline in a DATA blob, the DMA
variant references a framebuffer (FB_ID) whose memory the HW DMAs into
its LUT RAM, replacing a large register-write sequence with an address
update.

The accepted buffer formats are advertised via a read-only LUT_3D_FORMATS
blob - an array of the new struct drm_mode_3dlut_mode describing the grid
size, entry encoding (fourcc), memory layout (modifier), row pitch,
traversal order and interpolation.

Removing the LUT framebuffer with RMFB, or closing the DRM file that
owns it, bypasses the colorop and clears FB_ID, so the plane stays
visible without the LUT. CLOSEFB leaves the colorop unchanged.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
 drivers/gpu/drm/drm_colorop.c     |  2 +
 include/uapi/drm/drm_mode.h       | 79 +++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 1eebcf6f5e05..aca0edb42a5a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1374,6 +1374,10 @@ int drm_atomic_set_property(struct drm_atomic_commit *state,
  *	If the Atomic Commit request has the DRM_MODE_ATOMIC_TEST_ONLY flag
  *	it will only check if the Sync File is a valid one.
  *
+ *	The fence also covers framebuffers referenced by the plane's colorops,
+ *	such as the FB_ID of a DRM_COLOROP_3D_LUT_DMA colorop. Userspace using
+ *	explicit sync for those merges their fences into IN_FENCE_FD.
+ *
  *	On the driver side the fence is stored on the @fence parameter of
  *	&struct drm_plane_state. Drivers which also support implicit fencing
  *	should extract the implicit fence using drm_gem_plane_helper_prepare_fb(),
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 95da03d6c65c..b30413ad797b 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -69,6 +69,7 @@ static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
 	{ DRM_COLOROP_MULTIPLIER, "Multiplier"},
 	{ DRM_COLOROP_3D_LUT, "3D LUT"},
 	{ DRM_COLOROP_FIXED_MATRIX, "Fixed Matrix"},
+	{ DRM_COLOROP_3D_LUT_DMA, "3D LUT DMA"},
 };
 
 static const char * const colorop_curve_1d_type_names[] = {
@@ -688,6 +689,7 @@ static const char * const colorop_type_name[] = {
 	[DRM_COLOROP_MULTIPLIER] = "Multiplier",
 	[DRM_COLOROP_3D_LUT] = "3D LUT",
 	[DRM_COLOROP_FIXED_MATRIX] = "Fixed Matrix",
+	[DRM_COLOROP_3D_LUT_DMA] = "3D LUT DMA",
 };
 
 static const char * const colorop_lu3d_interpolation_name[] = {
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ed1a660a3dfd..f788ece18d1f 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -967,6 +967,44 @@ enum drm_colorop_type {
 	 */
 	DRM_COLOROP_FIXED_MATRIX,
 
+	/**
+	 * @DRM_COLOROP_3D_LUT_DMA:
+	 *
+	 * enum string "3D LUT DMA"
+	 *
+	 * A 3D LUT whose data is DMA'd from a framebuffer (via the FB_ID
+	 * property) instead of being carried inline in a DATA blob. This lets
+	 * the driver load the LUT with an address update instead of a large
+	 * sequence of register writes.
+	 *
+	 * The accepted buffer formats are advertised via the read-only
+	 * LUT_3D_FORMATS blob property, an array of struct drm_mode_3dlut_mode.
+	 * Userspace selects one entry by index via the LUT_3D_FORMAT property
+	 * and points FB_ID at a matching framebuffer.
+	 *
+	 * Rendering to the LUT framebuffer is synchronized like rendering to a
+	 * plane framebuffer. With implicit sync the commit waits for the write
+	 * fences on the framebuffer's buffer objects. With explicit sync
+	 * userspace merges the fence for the LUT write into the IN_FENCE_FD of
+	 * the colorop's plane. If IN_FENCE_FD is set, implicit write fences on
+	 * the LUT framebuffer are ignored, as they are for the plane framebuffer.
+	 *
+	 * The hardware may read the LUT framebuffer at any time while a colorop
+	 * state referencing it is current, not only for the first frame.
+	 * Userspace must not write to a LUT framebuffer while it is attached. To
+	 * change the LUT, write a different framebuffer and point FB_ID at it.
+	 * The old framebuffer can be reused once the commit that replaced it has
+	 * completed, e.g. after its flip event or OUT_FENCE_PTR fence.
+	 *
+	 * Removing the LUT framebuffer with RMFB, or closing the DRM file that
+	 * owns it, clears FB_ID and sets BYPASS on every colorop using it, so
+	 * the plane is shown without the LUT. Re-enabling the colorop requires
+	 * a new FB_ID. CLOSEFB leaves the colorop unchanged. Closing the GEM
+	 * handle or a DMA-BUF file descriptor of the LUT buffer does not affect
+	 * the framebuffer.
+	 */
+	DRM_COLOROP_3D_LUT_DMA,
+
 };
 
 /**
@@ -981,6 +1019,47 @@ enum drm_colorop_lut3d_interpolation_type {
 	DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 };
 
+/**
+ * enum drm_colorop_lut3d_traversal_order - cube axis enumeration order
+ *
+ * The order in which a 3D LUT's entries are to be traversed.
+ */
+enum drm_colorop_lut3d_traversal_order {
+	/**
+	 * @DRM_COLOROP_LUT3D_TRAVERSAL_RGB:
+	 *
+	 * R varies fastest, then G, then B (B slowest).
+	 */
+	DRM_COLOROP_LUT3D_TRAVERSAL_RGB,
+};
+
+/**
+ * struct drm_mode_3dlut_mode - fast-loadable 3D LUT format
+ * @lut_size: number of entries along one cube edge (e.g. 17 for a 17^3 LUT)
+ * @format: DRM fourcc describing the encoding of a single entry
+ * @modifier: DRM format modifier describing how the entries are laid
+ *	out in memory (e.g. DRM_FORMAT_MOD_LINEAR)
+ * @pitch: row stride in bytes
+ * @traversal_order: enum drm_colorop_lut3d_traversal_order describing how
+ *	entries are traversed
+ * @interpolation: enum drm_colorop_lut3d_interpolation_type specifying how
+ *	the HW interpolates between entries
+ * @flags: reserved, must be zero
+ *
+ * One entry of a &DRM_COLOROP_3D_LUT_DMA colorop's read-only LUT_3D_FORMATS
+ * blob. Userspace selects an entry by index via the LUT_3D_FORMAT property
+ * and points FB_ID at a matching framebuffer.
+ */
+struct drm_mode_3dlut_mode {
+	__u32 lut_size;
+	__u32 format;
+	__u64 modifier;
+	__u32 pitch;
+	__u16 traversal_order;
+	__u16 interpolation;
+	__u32 flags;
+};
+
 /**
  * enum drm_colorop_lut1d_interpolation_type - type of interpolation for 1D LUTs
  */
-- 
2.56.0

[PATCH v1 02/13] drm/colorop: add DRM_COLOROP_3D_LUT_DMA object and state

[Harry Wentland]

Introduce the core object and state for the fast-loading (DMA) 3D LUT
colorop. Add drm_plane_colorop_3dlut_dma_init(), which creates the
read-only LUT_3D_FORMATS blob (advertising the driver's accepted struct
drm_mode_3dlut_mode entries), the LUT_3D_FORMAT selection range, and the
FB_ID object property. The colorop must allow bypass, since removing
the LUT framebuffer bypasses it.

The colorop state gains a selected-format index and a reference-counted
framebuffer pointer; the state duplicate/destroy helpers take and drop
the framebuffer reference so an in-flight LUT buffer stays pinned for the
lifetime of the state.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_colorop.c | 86 +++++++++++++++++++++++++++++++++++
 include/drm/drm_colorop.h     | 56 +++++++++++++++++++++++
 2 files changed, 142 insertions(+)

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index b30413ad797b..3fc7884befbf 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -28,6 +28,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_plane.h>
+#include <drm/drm_framebuffer.h>
 
 #include "drm_crtc_internal.h"
 
@@ -464,6 +465,87 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
 }
 EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
 
+/**
+ * drm_plane_colorop_3dlut_dma_init - Initialize a DRM_COLOROP_3D_LUT_DMA
+ *
+ * @dev: DRM device
+ * @colorop: The drm_colorop object to initialize
+ * @plane: The associated drm_plane
+ * @funcs: control functions for the new colorop
+ * @formats: array of accepted buffer formats advertised to userspace
+ * @num_formats: number of entries in @formats
+ * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines. Must include
+ *         DRM_COLOROP_FLAG_ALLOW_BYPASS, since removing the LUT framebuffer
+ *         bypasses the colorop.
+ *
+ * Initializes a fast-loading (DMA) 3D LUT colorop. The colorop advertises
+ * the accepted buffer formats via a read-only LUT_3D_FORMATS blob, lets
+ * userspace select one via LUT_3D_FORMAT, and points FB_ID at the
+ * framebuffer holding the LUT data.
+ *
+ * @return zero on success, -E value on failure
+ */
+int drm_plane_colorop_3dlut_dma_init(struct drm_device *dev, struct drm_colorop *colorop,
+				     struct drm_plane *plane,
+				     const struct drm_colorop_funcs *funcs,
+				     const struct drm_mode_3dlut_mode *formats,
+				     size_t num_formats, uint32_t flags)
+{
+	struct drm_property_blob *blob;
+	struct drm_property *prop;
+	int ret;
+
+	if (!formats || !num_formats)
+		return -EINVAL;
+
+	/* Removing the LUT framebuffer bypasses the colorop. */
+	if (drm_WARN_ON(dev, !(flags & DRM_COLOROP_FLAG_ALLOW_BYPASS)))
+		return -EINVAL;
+
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
+				     DRM_COLOROP_3D_LUT_DMA, flags);
+	if (ret)
+		return ret;
+
+	/* LUT_3D_FORMATS: immutable blob listing the accepted buffer formats */
+	blob = drm_property_create_blob(dev,
+					num_formats * sizeof(*formats),
+					formats);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+				   "LUT_3D_FORMATS", 0);
+	if (!prop) {
+		drm_property_blob_put(blob);
+		return -ENOMEM;
+	}
+	colorop->lut_3d_formats_property = prop;
+	colorop->lut_3d_formats_blob = blob;
+	drm_object_attach_property(&colorop->base, prop, blob->base.id);
+
+	/* LUT_3D_FORMAT: selected index into the LUT_3D_FORMATS array */
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC, "LUT_3D_FORMAT",
+					 0, num_formats - 1);
+	if (!prop)
+		return -ENOMEM;
+	colorop->lut_3d_format_property = prop;
+	drm_object_attach_property(&colorop->base, prop, 0);
+
+	/* FB_ID: framebuffer holding the LUT data */
+	prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, "FB_ID",
+					  DRM_MODE_OBJECT_FB);
+	if (!prop)
+		return -ENOMEM;
+	colorop->fb_property = prop;
+	drm_object_attach_property(&colorop->base, prop, 0);
+
+	drm_colorop_reset(colorop);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_colorop_3dlut_dma_init);
+
 /**
  * drm_plane_colorop_fixed_matrix_init - Initialize a DRM_COLOROP_FIXED_MATRIX
  *
@@ -545,6 +627,8 @@ static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colo
 
 	if (state->data)
 		drm_property_blob_get(state->data);
+	if (state->fb)
+		drm_framebuffer_get(state->fb);
 }
 
 struct drm_colorop_state *
@@ -573,6 +657,8 @@ drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop)
 static void __drm_atomic_helper_colorop_destroy_state(struct drm_colorop_state *state)
 {
 	drm_property_blob_put(state->data);
+	if (state->fb)
+		drm_framebuffer_put(state->fb);
 }
 
 void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop,
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 26ef67c835e2..95922eb1044c 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -31,6 +31,8 @@
 #include <drm/drm_mode.h>
 #include <drm/drm_property.h>
 
+struct drm_framebuffer;
+
 /* DRM colorop flags */
 #define DRM_COLOROP_FLAG_ALLOW_BYPASS	(1<<0)	/* Allow bypass on the drm_colorop */
 
@@ -312,6 +314,22 @@ struct drm_colorop_state {
 	 */
 	enum drm_colorop_fixed_matrix_type fixed_matrix_type;
 
+	/**
+	 * @lut_3d_format:
+	 *
+	 * Index into the LUT_3D_FORMATS blob selecting the buffer format for
+	 * a DRM_COLOROP_3D_LUT_DMA colorop.
+	 */
+	uint32_t lut_3d_format;
+
+	/**
+	 * @fb:
+	 *
+	 * Reference-counted framebuffer holding the LUT data for a
+	 * DRM_COLOROP_3D_LUT_DMA colorop.
+	 */
+	struct drm_framebuffer *fb;
+
 	/** @state: backpointer to global drm_atomic_commit */
 	struct drm_atomic_commit *state;
 };
@@ -488,6 +506,39 @@ struct drm_colorop {
 	 */
 	struct drm_property *fixed_matrix_type_property;
 
+	/**
+	 * @lut_3d_formats_property:
+	 *
+	 * Immutable blob property listing the accepted buffer formats (an
+	 * array of struct drm_mode_3dlut_mode) for a DRM_COLOROP_3D_LUT_DMA
+	 * colorop.
+	 */
+	struct drm_property *lut_3d_formats_property;
+
+	/**
+	 * @lut_3d_format_property:
+	 *
+	 * Range property selecting an entry, by index, of the
+	 * @lut_3d_formats_blob array.
+	 */
+	struct drm_property *lut_3d_format_property;
+
+	/**
+	 * @fb_property:
+	 *
+	 * Object property (DRM_MODE_OBJECT_FB) pointing at the framebuffer
+	 * that holds the LUT data for a DRM_COLOROP_3D_LUT_DMA colorop.
+	 */
+	struct drm_property *fb_property;
+
+	/**
+	 * @lut_3d_formats_blob:
+	 *
+	 * The advertised list backing @lut_3d_formats_property, retained for
+	 * validation during atomic check.
+	 */
+	struct drm_property_blob *lut_3d_formats_blob;
+
 	/**
 	 * @next_property:
 	 *
@@ -544,6 +595,11 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
 				 uint32_t lut_size,
 				 enum drm_colorop_lut3d_interpolation_type interpolation,
 				 uint32_t flags);
+int drm_plane_colorop_3dlut_dma_init(struct drm_device *dev, struct drm_colorop *colorop,
+				     struct drm_plane *plane,
+				     const struct drm_colorop_funcs *funcs,
+				     const struct drm_mode_3dlut_mode *formats,
+				     size_t num_formats, uint32_t flags);
 int drm_plane_colorop_fixed_matrix_init(struct drm_device *dev, struct drm_colorop *colorop,
 					struct drm_plane *plane,
 					const struct drm_colorop_funcs *funcs,
-- 
2.56.0

[PATCH v1 03/13] drm/atomic: wire up DMA 3D LUT colorop properties

[Harry Wentland]

Handle the FB_ID and LUT_3D_FORMAT properties in the colorop atomic
get/set paths, and report LUT_3D_FORMATS (the immutable formats blob) on
get. Setting FB_ID looks up the framebuffer and swaps the reference into
the colorop state (taking the new reference, dropping the old), mirroring
the plane FB_ID handling.

RMFB, and closing the DRM file that owns a framebuffer, disable the
planes that scan it out. Without further handling a colorop using the
framebuffer would keep it alive and the hardware would keep fetching
from it. Make atomic_remove_fb() also bypass every colorop using the
framebuffer and clear its FB_ID, flagging the plane's color management
as changed. Bypassing keeps the plane visible without the LUT, which is
less disruptive than disabling it. Document this in the RMFB and
CLOSEFB uAPI.

Also print the selected format index and framebuffer id for a
DRM_COLOROP_3D_LUT_DMA colorop in the atomic state dump.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_atomic.c      |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c | 24 +++++++++++++++++
 drivers/gpu/drm/drm_framebuffer.c | 45 ++++++++++++++++++++++++++++++-
 include/uapi/drm/drm.h            |  7 ++---
 4 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9bb761eda59f..158f41eae76f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -939,6 +939,10 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
 		drm_printf_indent(p, 1, "fixed_matrix_type=%s\n",
 				  drm_get_colorop_fixed_matrix_type_name(state->fixed_matrix_type));
 		break;
+	case DRM_COLOROP_3D_LUT_DMA:
+		drm_printf_indent(p, 1, "lut_3d_format=%u\n", state->lut_3d_format);
+		drm_printf_indent(p, 1, "fb=%u\n", state->fb ? state->fb->base.id : 0);
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index aca0edb42a5a..05a2a3b46351 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -791,6 +791,24 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
 			state->fixed_matrix_type = val;
 			*replaced = true;
 		}
+	} else if (property == colorop->lut_3d_format_property) {
+		if (state->lut_3d_format != val) {
+			state->lut_3d_format = val;
+			*replaced = true;
+		}
+	} else if (property == colorop->fb_property) {
+		struct drm_framebuffer *fb = NULL;
+
+		if (val) {
+			fb = drm_framebuffer_lookup(colorop->dev, file_priv, val);
+			if (!fb)
+				return -EINVAL;
+		}
+		if (state->fb != fb)
+			*replaced = true;
+		drm_framebuffer_assign(&state->fb, fb);
+		if (fb)
+			drm_framebuffer_put(fb);
 	} else {
 		drm_dbg_atomic(colorop->dev,
 			       "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
@@ -825,6 +843,12 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
 		*val = (state->data) ? state->data->base.id : 0;
 	else if (property == colorop->fixed_matrix_type_property)
 		*val = state->fixed_matrix_type;
+	else if (property == colorop->lut_3d_formats_property)
+		*val = colorop->lut_3d_formats_blob ? colorop->lut_3d_formats_blob->base.id : 0;
+	else if (property == colorop->lut_3d_format_property)
+		*val = state->lut_3d_format;
+	else if (property == colorop->fb_property)
+		*val = (state->fb) ? state->fb->base.id : 0;
 	else
 		return -EINVAL;
 
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index d32aceb6ca9b..05cce02238bc 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -26,6 +26,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_uapi.h>
 #include <drm/drm_auth.h>
+#include <drm/drm_colorop.h>
 #include <drm/drm_debugfs.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
@@ -1002,10 +1003,12 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
 	struct drm_device *dev = fb->dev;
 	struct drm_atomic_commit *state;
 	struct drm_plane *plane;
+	struct drm_colorop *colorop;
 	struct drm_connector *conn __maybe_unused;
 	struct drm_connector_state *conn_state;
 	int i, ret;
 	unsigned plane_mask;
+	bool colorops_changed;
 	bool disable_crtcs = false;
 
 retry_disable:
@@ -1020,6 +1023,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
 
 retry:
 	plane_mask = 0;
+	colorops_changed = false;
 	ret = drm_modeset_lock_all_ctx(dev, &ctx);
 	if (ret)
 		goto unlock;
@@ -1068,6 +1072,45 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
 		plane_mask |= drm_plane_mask(plane);
 	}
 
+	/*
+	 * Colorops that fetch data from a framebuffer, such as a DMA 3D LUT,
+	 * are bypassed rather than disabling their plane. Such colorops are
+	 * required to support bypass.
+	 */
+	drm_for_each_colorop(colorop, dev) {
+		struct drm_colorop_state *colorop_state;
+		struct drm_plane_state *plane_state;
+
+		if (!colorop->state || colorop->state->fb != fb)
+			continue;
+
+		if (drm_WARN_ON(dev, !colorop->bypass_property)) {
+			ret = -EINVAL;
+			goto unlock;
+		}
+
+		drm_dbg_kms(dev,
+			    "Bypassing [COLOROP:%d] because [FB:%d] is removed\n",
+			    colorop->base.id, fb->base.id);
+
+		colorop_state = drm_atomic_get_colorop_state(state, colorop);
+		if (IS_ERR(colorop_state)) {
+			ret = PTR_ERR(colorop_state);
+			goto unlock;
+		}
+
+		plane_state = drm_atomic_get_plane_state(state, colorop->plane);
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto unlock;
+		}
+
+		drm_framebuffer_assign(&colorop_state->fb, NULL);
+		colorop_state->bypass = true;
+		plane_state->color_mgmt_changed = true;
+		colorops_changed = true;
+	}
+
 	/* This list is only filled when disable_crtcs is set. */
 	for_each_new_connector_in_state(state, conn, conn_state, i) {
 		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
@@ -1076,7 +1119,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
 			goto unlock;
 	}
 
-	if (plane_mask)
+	if (plane_mask || colorops_changed)
 		ret = drm_atomic_commit(state);
 
 unlock:
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index bc7ef7684099..2a06e8e08dbe 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -1217,7 +1217,8 @@ extern "C" {
  *
  * Warning: removing a framebuffer currently in-use on an enabled plane will
  * disable that plane. The CRTC the plane is linked to may also be disabled
- * (depending on driver capabilities).
+ * (depending on driver capabilities). Colorops using the framebuffer, such
+ * as a DMA 3D LUT, are bypassed instead.
  */
 #define DRM_IOCTL_MODE_RMFB		DRM_IOWR(0xAF, unsigned int)
 #define DRM_IOCTL_MODE_PAGE_FLIP	DRM_IOWR(0xB0, struct drm_mode_crtc_page_flip)
@@ -1321,8 +1322,8 @@ extern "C" {
  * argument is a framebuffer object ID.
  *
  * This IOCTL is similar to &DRM_IOCTL_MODE_RMFB, except it doesn't disable
- * planes and CRTCs. As long as the framebuffer is used by a plane, it's kept
- * alive. When the plane no longer uses the framebuffer (because the
+ * planes and CRTCs, or bypass colorops. As long as the framebuffer is used by
+ * a plane or colorop, it's kept alive. When it is no longer used (because the
  * framebuffer is replaced with another one, or the plane is disabled), the
  * framebuffer is cleaned up.
  *
-- 
2.56.0

[PATCH v1 04/13] drm/atomic: validate DMA 3D LUT colorop framebuffer

[Harry Wentland]

Add a core atomic check for DRM_COLOROP_3D_LUT_DMA colorops. When the
colorop is enabled (not bypassed), validate the selected LUT_3D_FORMAT
index and require a framebuffer whose fourcc, modifier, pitch and
dimensions match the advertised drm_mode_3dlut_mode entry: width equal to
the cube edge and height equal to lut_size^2 (due to the 2D flattening
of the cube). Driver-specific constraints are left to the driver's own
atomic_check.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_atomic.c | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 158f41eae76f..a6f1be7ce76c 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1745,6 +1745,57 @@ drm_atomic_add_affected_colorops(struct drm_atomic_commit *state,
 }
 EXPORT_SYMBOL(drm_atomic_add_affected_colorops);
 
+static int drm_atomic_colorop_check(struct drm_colorop *colorop,
+				    struct drm_colorop_state *new_colorop_state)
+{
+	const struct drm_mode_3dlut_mode *modes, *mode;
+	struct drm_framebuffer *fb = new_colorop_state->fb;
+	u32 num_modes;
+
+	if (colorop->type != DRM_COLOROP_3D_LUT_DMA)
+		return 0;
+
+	/* Nothing to validate while the colorop is bypassed. */
+	if (colorop->bypass_property && new_colorop_state->bypass)
+		return 0;
+
+	if (!colorop->lut_3d_formats_blob)
+		return -EINVAL;
+
+	num_modes = colorop->lut_3d_formats_blob->length /
+		    sizeof(struct drm_mode_3dlut_mode);
+	if (new_colorop_state->lut_3d_format >= num_modes) {
+		drm_dbg_atomic(colorop->dev,
+			       "[COLOROP:%d] LUT_3D_FORMAT %u out of range (have %u)\n",
+			       colorop->base.id, new_colorop_state->lut_3d_format,
+			       num_modes);
+		return -EINVAL;
+	}
+
+	modes = colorop->lut_3d_formats_blob->data;
+	mode = &modes[new_colorop_state->lut_3d_format];
+
+	if (!fb) {
+		drm_dbg_atomic(colorop->dev,
+			       "[COLOROP:%d] enabled DMA 3D LUT without a framebuffer\n",
+			       colorop->base.id);
+		return -EINVAL;
+	}
+
+	if (fb->format->format != mode->format ||
+	    fb->modifier != mode->modifier ||
+	    fb->pitches[0] != mode->pitch ||
+	    fb->width != mode->lut_size ||
+	    fb->height != (u64)mode->lut_size * mode->lut_size) {
+		drm_dbg_atomic(colorop->dev,
+			       "[COLOROP:%d] framebuffer does not match LUT_3D_FORMAT %u\n",
+			       colorop->base.id, new_colorop_state->lut_3d_format);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * drm_atomic_check_only - check whether a given config would work
  * @state: atomic configuration to check
@@ -1768,6 +1819,8 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
 	struct drm_crtc_state *new_crtc_state;
 	struct drm_connector *conn;
 	struct drm_connector_state *conn_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	unsigned int requested_crtc = 0;
 	unsigned int affected_crtc = 0;
 	int i, ret = 0;
@@ -1806,6 +1859,15 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
 		}
 	}
 
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+		ret = drm_atomic_colorop_check(colorop, new_colorop_state);
+		if (ret) {
+			drm_dbg_atomic(dev, "[COLOROP:%d] atomic core check failed\n",
+				       colorop->base.id);
+			return ret;
+		}
+	}
+
 	if (config->funcs->atomic_check) {
 		ret = config->funcs->atomic_check(state->dev, state);
 
-- 
2.56.0

[PATCH v1 05/13] drm/atomic: add colorop prepare_fb and cleanup_fb helper hooks

[Harry Wentland]

A DRM_COLOROP_3D_LUT_DMA colorop references a framebuffer through its
FB_ID property. Drivers need to prepare that framebuffer (pin it, wait
for rendering) the same way they prepare plane framebuffers, but the
plane hooks only see the plane state and can't reliably find the
colorop state that was prepared once the commit is swapped or aborted.

Add struct drm_colorop_helper_funcs with prepare_fb and cleanup_fb,
set with drm_colorop_helper_add(). The atomic helpers call them for
each colorop in the commit:

- prepare_fb on the new colorop states from
  drm_atomic_helper_prepare_planes(), before the plane hooks
- cleanup_fb on the old colorop states from
  drm_atomic_helper_cleanup_planes()
- cleanup_fb on the new colorop states from
  drm_atomic_helper_unprepare_planes() and the prepare_planes()
  failure path

Add a fence to drm_colorop_state and drm_gem_colorop_helper_prepare_fb(),
which extracts the implicit write fence from the colorop framebuffer.
If the colorop's plane has an explicit IN_FENCE_FD fence, only kernel
fences are extracted, as in drm_gem_plane_helper_prepare_fb(). It is
the default prepare_fb for GEM drivers. drm_atomic_helper_wait_for_fences()
waits on colorop fences after the plane fences.

Add for_each_old_colorop_in_state() for the cleanup path.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_atomic_helper.c      | 93 ++++++++++++++++++++++++
 drivers/gpu/drm/drm_colorop.c            |  8 ++
 drivers/gpu/drm/drm_gem_atomic_helper.c  | 81 +++++++++++++++++++++
 include/drm/drm_atomic.h                 | 20 +++++
 include/drm/drm_colorop.h                | 15 ++++
 include/drm/drm_gem_atomic_helper.h      | 11 +++
 include/drm/drm_modeset_helper_vtables.h | 62 ++++++++++++++++
 7 files changed, 290 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9d006f98413a..ccd182d58875 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1786,6 +1786,8 @@ static void set_fence_deadline(struct drm_device *dev,
 	struct drm_crtc_state *new_crtc_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	ktime_t vbltime = 0;
 	int i;
 
@@ -1814,6 +1816,12 @@ static void set_fence_deadline(struct drm_device *dev,
 			continue;
 		dma_fence_set_deadline(new_plane_state->fence, vbltime);
 	}
+
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+		if (!new_colorop_state->fence)
+			continue;
+		dma_fence_set_deadline(new_colorop_state->fence, vbltime);
+	}
 }
 
 /**
@@ -1828,6 +1836,8 @@ static void set_fence_deadline(struct drm_device *dev,
  * drm_atomic_helper_swap_state() so it uses the current plane state (and
  * just uses the atomic state to find the changed planes)
  *
+ * Fences stashed in &drm_colorop_state.fence are waited on as well.
+ *
  * Note that @pre_swap is needed since the point where we block for fences moves
  * around depending upon whether an atomic commit is blocking or
  * non-blocking. For non-blocking commit all waiting needs to happen after
@@ -1843,6 +1853,8 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 {
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	int i, ret;
 
 	set_fence_deadline(dev, state);
@@ -1866,6 +1878,18 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 		new_plane_state->fence = NULL;
 	}
 
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+		if (!new_colorop_state->fence)
+			continue;
+
+		ret = dma_fence_wait(new_colorop_state->fence, pre_swap);
+		if (ret)
+			return ret;
+
+		dma_fence_put(new_colorop_state->fence);
+		new_colorop_state->fence = NULL;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_helper_wait_for_fences);
@@ -2804,6 +2828,49 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_commit *state)
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
 
+static void colorop_cleanup_fb(struct drm_colorop *colorop,
+			       struct drm_colorop_state *colorop_state)
+{
+	const struct drm_colorop_helper_funcs *funcs = colorop->helper_private;
+
+	if (funcs && funcs->cleanup_fb)
+		funcs->cleanup_fb(colorop, colorop_state);
+}
+
+static int prepare_colorops(struct drm_device *dev,
+			    struct drm_atomic_commit *state)
+{
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
+	int ret, i, j;
+
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+		const struct drm_colorop_helper_funcs *funcs = colorop->helper_private;
+
+		if (funcs && funcs->prepare_fb)
+			ret = funcs->prepare_fb(colorop, new_colorop_state);
+		else if (drm_core_check_feature(dev, DRIVER_GEM))
+			ret = drm_gem_colorop_helper_prepare_fb(colorop, new_colorop_state);
+		else
+			ret = 0;
+
+		if (ret)
+			goto fail;
+	}
+
+	return 0;
+
+fail:
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, j) {
+		if (j >= i)
+			continue;
+
+		colorop_cleanup_fb(colorop, new_colorop_state);
+	}
+
+	return ret;
+}
+
 /**
  * drm_atomic_helper_prepare_planes - prepare plane resources before commit
  * @dev: DRM device
@@ -2814,6 +2881,9 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
  * is encountered this function will call &drm_plane_helper_funcs.cleanup_fb on
  * any already successfully prepared framebuffer.
  *
+ * Framebuffers referenced by colorops are prepared first, through
+ * &drm_colorop_helper_funcs.prepare_fb, and cleaned up the same way on failure.
+ *
  * Returns:
  * 0 on success, negative error code on failure.
  */
@@ -2822,6 +2892,8 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *new_conn_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
 	int ret, i, j;
@@ -2835,6 +2907,10 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 			return ret;
 	}
 
+	ret = prepare_colorops(dev, state);
+	if (ret)
+		return ret;
+
 	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
@@ -2892,6 +2968,9 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 			funcs->cleanup_fb(plane, new_plane_state);
 	}
 
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, j)
+		colorop_cleanup_fb(colorop, new_colorop_state);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
@@ -2905,12 +2984,16 @@ EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
  * atomic state. It undoes the effects of drm_atomic_helper_prepare_planes()
  * when aborting an atomic commit. For cleaning up after a successful commit
  * use drm_atomic_helper_cleanup_planes().
+ *
+ * &drm_colorop_helper_funcs.cleanup_fb is called for the new colorop states.
  */
 void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
 					struct drm_atomic_commit *state)
 {
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	int i;
 
 	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
@@ -2926,6 +3009,9 @@ void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
 		if (funcs->cleanup_fb)
 			funcs->cleanup_fb(plane, new_plane_state);
 	}
+
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i)
+		colorop_cleanup_fb(colorop, new_colorop_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_unprepare_planes);
 
@@ -3204,12 +3290,16 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc);
  * This function may not be called on the new state when the atomic update
  * fails at any point after calling drm_atomic_helper_prepare_planes(). Use
  * drm_atomic_helper_unprepare_planes() in this case.
+ *
+ * &drm_colorop_helper_funcs.cleanup_fb is called for the old colorop states.
  */
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 				      struct drm_atomic_commit *state)
 {
 	struct drm_plane *plane;
 	struct drm_plane_state *old_plane_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *old_colorop_state;
 	int i;
 
 	for_each_old_plane_in_state(state, plane, old_plane_state, i) {
@@ -3218,6 +3308,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 		if (funcs->cleanup_fb)
 			funcs->cleanup_fb(plane, old_plane_state);
 	}
+
+	for_each_old_colorop_in_state(state, colorop, old_colorop_state, i)
+		colorop_cleanup_fb(colorop, old_colorop_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
 
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 3fc7884befbf..4fe2bd98fb94 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -24,6 +24,8 @@
  *
  */
 
+#include <linux/dma-fence.h>
+
 #include <drm/drm_colorop.h>
 #include <drm/drm_print.h>
 #include <drm/drm_drv.h>
@@ -483,6 +485,9 @@ EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
  * userspace select one via LUT_3D_FORMAT, and points FB_ID at the
  * framebuffer holding the LUT data.
  *
+ * Drivers that need to pin the framebuffer set &drm_colorop_helper_funcs with
+ * drm_colorop_helper_add().
+ *
  * @return zero on success, -E value on failure
  */
 int drm_plane_colorop_3dlut_dma_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -624,6 +629,7 @@ static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colo
 							struct drm_colorop_state *state)
 {
 	memcpy(state, colorop->state, sizeof(*state));
+	state->fence = NULL;
 
 	if (state->data)
 		drm_property_blob_get(state->data);
@@ -659,6 +665,8 @@ static void __drm_atomic_helper_colorop_destroy_state(struct drm_colorop_state *
 	drm_property_blob_put(state->data);
 	if (state->fb)
 		drm_framebuffer_put(state->fb);
+	if (state->fence)
+		dma_fence_put(state->fence);
 }
 
 void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop,
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index c952cfed84c1..7497c2725e22 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -4,8 +4,10 @@
 #include <linux/dma-fence-chain.h>
 #include <linux/export.h>
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_atomic_uapi.h>
+#include <drm/drm_colorop.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_atomic_helper.h>
@@ -175,6 +177,85 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane,
 }
 EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
 
+/*
+ * Colorop Helpers
+ */
+
+/**
+ * drm_gem_colorop_helper_prepare_fb() - Prepare a GEM backed colorop framebuffer
+ * @colorop: Colorop
+ * @state: Colorop state the fence will be attached to
+ *
+ * This is the colorop counterpart of drm_gem_plane_helper_prepare_fb(). It
+ * extracts the write fence from &drm_colorop_state.fb and stores it in
+ * &drm_colorop_state.fence for drm_atomic_helper_wait_for_fences() to wait on.
+ *
+ * If the colorop's plane has an explicit fence set through IN_FENCE_FD, only
+ * kernel fences are extracted, matching drm_gem_plane_helper_prepare_fb().
+ * This relies on being called before the plane's prepare_fb, which adds
+ * implicit fences to &drm_plane_state.fence.
+ *
+ * This function is the default implementation for GEM drivers of
+ * &drm_colorop_helper_funcs.prepare_fb if no callback is provided.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_gem_colorop_helper_prepare_fb(struct drm_colorop *colorop,
+				      struct drm_colorop_state *state)
+{
+	enum dma_resv_usage usage = DMA_RESV_USAGE_WRITE;
+	struct drm_plane_state *plane_state;
+	struct dma_fence *fence = NULL;
+	size_t i;
+	int ret;
+
+	if (!state->fb)
+		return 0;
+
+	plane_state = drm_atomic_get_new_plane_state(state->state, colorop->plane);
+	if (plane_state && plane_state->fence)
+		usage = DMA_RESV_USAGE_KERNEL;
+
+	for (i = 0; i < state->fb->format->num_planes; ++i) {
+		struct drm_gem_object *obj = drm_gem_fb_get_obj(state->fb, i);
+		struct dma_fence *new;
+
+		if (!obj) {
+			ret = -EINVAL;
+			goto error;
+		}
+
+		ret = dma_resv_get_singleton(obj->resv, usage, &new);
+		if (ret)
+			goto error;
+
+		if (new && fence) {
+			struct dma_fence_chain *chain = dma_fence_chain_alloc();
+
+			if (!chain) {
+				dma_fence_put(new);
+				ret = -ENOMEM;
+				goto error;
+			}
+
+			dma_fence_chain_init(chain, fence, new, 1);
+			fence = &chain->base;
+		} else if (new) {
+			fence = new;
+		}
+	}
+
+	dma_fence_put(state->fence);
+	state->fence = fence;
+	return 0;
+
+error:
+	dma_fence_put(fence);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(drm_gem_colorop_helper_prepare_fb);
+
 /*
  * Shadow-buffered Planes
  */
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 3ae35b09c0cf..80259ded0efc 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -1127,6 +1127,26 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 			      (new_colorop_state) = (__state)->colorops[__i].new_state,\
 			      (void)(new_colorop_state) /* Only to avoid unused-but-set-variable warning */, 1))
 
+/**
+ * for_each_old_colorop_in_state - iterate over all colorops in an atomic update
+ * @__state: &struct drm_atomic_commit pointer
+ * @colorop: &struct drm_colorop iteration cursor
+ * @old_colorop_state: &struct drm_colorop_state iteration cursor for the old state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all colorops in an atomic update, tracking only the old
+ * state. This is useful in disable functions, where we need the old state the
+ * hardware is still in.
+ */
+#define for_each_old_colorop_in_state(__state, colorop, old_colorop_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_colorop;		\
+	     (__i)++)							\
+		for_each_if ((__state)->colorops[__i].ptr &&		\
+			     ((colorop) = (__state)->colorops[__i].ptr,	\
+			      (void)(colorop) /* Only to avoid unused-but-set-variable warning */, \
+			      (old_colorop_state) = (__state)->colorops[__i].old_state, 1))
+
 /**
  * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update
  * @__state: &struct drm_atomic_commit pointer
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 95922eb1044c..36afe9c7c298 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -31,6 +31,8 @@
 #include <drm/drm_mode.h>
 #include <drm/drm_property.h>
 
+struct dma_fence;
+struct drm_colorop_helper_funcs;
 struct drm_framebuffer;
 
 /* DRM colorop flags */
@@ -330,6 +332,16 @@ struct drm_colorop_state {
 	 */
 	struct drm_framebuffer *fb;
 
+	/**
+	 * @fence:
+	 *
+	 * Optional fence to wait for before @fb is used by the hardware. Set
+	 * by &drm_colorop_helper_funcs.prepare_fb, typically through
+	 * drm_gem_colorop_helper_prepare_fb(), and consumed by
+	 * drm_atomic_helper_wait_for_fences(). Not copied on duplicate.
+	 */
+	struct dma_fence *fence;
+
 	/** @state: backpointer to global drm_atomic_commit */
 	struct drm_atomic_commit *state;
 };
@@ -548,6 +560,9 @@ struct drm_colorop {
 
 	/** @funcs: colorop control functions */
 	const struct drm_colorop_funcs *funcs;
+
+	/** @helper_private: mid-layer private data */
+	const struct drm_colorop_helper_funcs *helper_private;
 };
 
 #define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
index f605a8a7b017..f2141226535d 100644
--- a/include/drm/drm_gem_atomic_helper.h
+++ b/include/drm/drm_gem_atomic_helper.h
@@ -9,12 +9,23 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_plane.h>
 
+struct drm_colorop;
+struct drm_colorop_state;
+struct drm_simple_display_pipe;
+
 /*
  * Plane Helpers
  */
 
 int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state);
 
+/*
+ * Colorop Helpers
+ */
+
+int drm_gem_colorop_helper_prepare_fb(struct drm_colorop *colorop,
+				      struct drm_colorop_state *state);
+
 /*
  * Helpers for planes with shadow buffers
  */
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 60b30cc26940..821b051f5632 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -29,6 +29,7 @@
 #ifndef __DRM_MODESET_HELPER_VTABLES_H__
 #define __DRM_MODESET_HELPER_VTABLES_H__
 
+#include <drm/drm_colorop.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_encoder.h>
 
@@ -1534,6 +1535,67 @@ static inline void drm_plane_helper_add(struct drm_plane *plane,
 	plane->helper_private = funcs;
 }
 
+/**
+ * struct drm_colorop_helper_funcs - helper operations for colorops
+ *
+ * These functions are used by the atomic helpers for colorops that reference
+ * a framebuffer, such as &DRM_COLOROP_3D_LUT_DMA.
+ */
+struct drm_colorop_helper_funcs {
+	/**
+	 * @prepare_fb:
+	 *
+	 * This hook prepares &drm_colorop_state.fb for use by the hardware,
+	 * e.g. by pinning it in memory. It can also extract an implicit fence
+	 * into &drm_colorop_state.fence, see drm_gem_colorop_helper_prepare_fb().
+	 *
+	 * It is called from drm_atomic_helper_prepare_planes() for each colorop
+	 * in the commit, before the &drm_plane_helper_funcs.prepare_fb hooks.
+	 * Whatever it sets up must depend only on @new_state, since
+	 * @cleanup_fb only gets the colorop state to undo it.
+	 * If this hook returns an error, it must undo any resources it acquired
+	 * before returning; @cleanup_fb is called only when preparation
+	 * succeeded.
+	 *
+	 * If not set, GEM drivers get drm_gem_colorop_helper_prepare_fb().
+	 *
+	 * This function must not block for outstanding rendering, since it is
+	 * called in the context of the atomic IOCTL even for nonblocking
+	 * commits. Set &drm_colorop_state.fence instead.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or one of the negative error codes allowed by the
+	 * &drm_mode_config_funcs.atomic_commit vfunc.
+	 */
+	int (*prepare_fb)(struct drm_colorop *colorop,
+			  struct drm_colorop_state *new_state);
+
+	/**
+	 * @cleanup_fb:
+	 *
+	 * This hook undoes @prepare_fb for a colorop state. It is called for
+	 * the old colorop states after a commit, and for the new colorop states
+	 * when a commit is aborted after @prepare_fb. It is called only for
+	 * colorop states whose preparation succeeded.
+	 *
+	 * This callback is optional.
+	 */
+	void (*cleanup_fb)(struct drm_colorop *colorop,
+			   struct drm_colorop_state *old_state);
+};
+
+/**
+ * drm_colorop_helper_add - sets the helper vtable for a colorop
+ * @colorop: DRM colorop
+ * @funcs: helper vtable to set for @colorop
+ */
+static inline void drm_colorop_helper_add(struct drm_colorop *colorop,
+					  const struct drm_colorop_helper_funcs *funcs)
+{
+	colorop->helper_private = funcs;
+}
+
 /**
  * struct drm_mode_config_helper_funcs - global modeset helper operations
  *
-- 
2.56.0

[PATCH v1 06/13] drm/tests: test colorop prepare_fb and cleanup_fb helpers

[Harry Wentland]

drm_atomic_helper_prepare_planes() prepares the framebuffers of the
colorops in a commit before those of the planes, and has to unwind
them on every failure path. Each prepared colorop state has to be
cleaned up exactly once, on the abort path through
drm_atomic_helper_unprepare_planes() or, after the state swap, through
drm_atomic_helper_cleanup_planes() on the old state.

Add KUnit tests with mock colorop and plane helpers that record their
calls and can fail on request. Check that:

- colorops are prepared once, before the planes;
- a failing colorop unwinds only the colorops prepared before it;
- a failing plane prepare_fb or begin_fb_access cleans up all
  colorops;
- unprepare_planes() cleans up the new colorop states;
- cleanup_planes() after the swap cleans up the old colorop states.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/tests/Makefile                |   1 +
 .../gpu/drm/tests/drm_colorop_helper_test.c   | 397 ++++++++++++++++++
 2 files changed, 398 insertions(+)
 create mode 100644 drivers/gpu/drm/tests/drm_colorop_helper_test.c

diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index 42207df1d1c8..4bf8696fa49c 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
 	drm_atomic_commit_test.o \
 	drm_bridge_test.o \
 	drm_cmdline_parser_test.o \
+	drm_colorop_helper_test.o \
 	drm_connector_test.o \
 	drm_damage_helper_test.o \
 	drm_dp_mst_helper_test.o \
diff --git a/drivers/gpu/drm/tests/drm_colorop_helper_test.c b/drivers/gpu/drm/tests/drm_colorop_helper_test.c
new file mode 100644
index 000000000000..2a650500dd60
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_colorop_helper_test.c
@@ -0,0 +1,397 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the colorop prepare_fb and cleanup_fb atomic helper hooks
+ *
+ * Copyright 2026 Advanced Micro Devices, Inc.
+ */
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_colorop.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_kunit_helpers.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_modeset_lock.h>
+
+#include <kunit/test.h>
+#include <kunit/test-bug.h>
+
+#define TEST_NUM_COLOROPS	3
+
+struct colorop_hook_calls {
+	int prepared;
+	int cleaned;
+	int prepare_seq;
+	struct drm_colorop_state *prepared_state;
+	struct drm_colorop_state *cleaned_state;
+};
+
+struct colorop_test_priv {
+	struct drm_device *drm;
+	struct drm_plane *plane;
+	struct drm_colorop *colorops[TEST_NUM_COLOROPS];
+	struct drm_atomic_commit *state;
+
+	/* Fault injection */
+	int colorop_fail_at;
+	int plane_prepare_ret;
+	int plane_begin_access_ret;
+
+	/* Recorded calls */
+	int seq;
+	struct colorop_hook_calls colorop[TEST_NUM_COLOROPS];
+	int plane_prepared;
+	int plane_prepare_seq;
+	int plane_cleaned;
+	int plane_end_access;
+};
+
+static struct colorop_test_priv *colorop_test_priv(void)
+{
+	return kunit_get_current_test()->priv;
+}
+
+static int colorop_test_prepare_fb(struct drm_colorop *colorop,
+				   struct drm_colorop_state *new_state)
+{
+	struct colorop_test_priv *priv = colorop_test_priv();
+	struct colorop_hook_calls *calls = &priv->colorop[colorop->index];
+
+	calls->prepared++;
+	calls->prepare_seq = priv->seq++;
+	calls->prepared_state = new_state;
+
+	return (int)colorop->index == priv->colorop_fail_at ? -EIO : 0;
+}
+
+static void colorop_test_cleanup_fb(struct drm_colorop *colorop,
+				    struct drm_colorop_state *state)
+{
+	struct colorop_test_priv *priv = colorop_test_priv();
+	struct colorop_hook_calls *calls = &priv->colorop[colorop->index];
+
+	calls->cleaned++;
+	calls->cleaned_state = state;
+}
+
+static const struct drm_colorop_helper_funcs colorop_test_helper_funcs = {
+	.prepare_fb = colorop_test_prepare_fb,
+	.cleanup_fb = colorop_test_cleanup_fb,
+};
+
+static const struct drm_colorop_funcs colorop_test_funcs = {
+	.destroy = drm_colorop_destroy,
+};
+
+static int plane_test_prepare_fb(struct drm_plane *plane,
+				 struct drm_plane_state *new_state)
+{
+	struct colorop_test_priv *priv = colorop_test_priv();
+
+	priv->plane_prepared++;
+	priv->plane_prepare_seq = priv->seq++;
+
+	return priv->plane_prepare_ret;
+}
+
+static void plane_test_cleanup_fb(struct drm_plane *plane,
+				  struct drm_plane_state *state)
+{
+	colorop_test_priv()->plane_cleaned++;
+}
+
+static int plane_test_begin_fb_access(struct drm_plane *plane,
+				      struct drm_plane_state *new_state)
+{
+	return colorop_test_priv()->plane_begin_access_ret;
+}
+
+static void plane_test_end_fb_access(struct drm_plane *plane,
+				     struct drm_plane_state *new_state)
+{
+	colorop_test_priv()->plane_end_access++;
+}
+
+static const struct drm_plane_helper_funcs plane_test_helper_funcs = {
+	.prepare_fb = plane_test_prepare_fb,
+	.cleanup_fb = plane_test_cleanup_fb,
+	.begin_fb_access = plane_test_begin_fb_access,
+	.end_fb_access = plane_test_end_fb_access,
+};
+
+static void colorop_test_pipeline_destroy(void *drm)
+{
+	drm_colorop_pipeline_destroy(drm);
+}
+
+static void colorop_test_acquire_fini(void *ptr)
+{
+	struct drm_modeset_acquire_ctx *ctx = ptr;
+
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+}
+
+/*
+ * Create a plane with a chain of colorops, and an atomic state containing the
+ * plane and all colorops.
+ */
+static int colorop_test_init(struct kunit *test)
+{
+	struct drm_modeset_acquire_ctx *ctx;
+	struct colorop_test_priv *priv;
+	struct drm_plane_state *plane_state;
+	struct device *dev;
+	int i, ret;
+
+	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+	priv->colorop_fail_at = -1;
+	test->priv = priv;
+
+	dev = drm_kunit_helper_alloc_device(test);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+	priv->drm = __drm_kunit_helper_alloc_drm_device(test, dev,
+							sizeof(*priv->drm), 0,
+							DRIVER_MODESET | DRIVER_ATOMIC);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);
+
+	priv->plane = drm_kunit_helper_create_primary_plane(test, priv->drm, NULL,
+							    &plane_test_helper_funcs,
+							    NULL, 0, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->plane);
+
+	/* Destroy the colorops after the atomic state that references them. */
+	ret = kunit_add_action_or_reset(test, colorop_test_pipeline_destroy,
+					priv->drm);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct drm_colorop *colorop;
+
+		colorop = kzalloc_obj(*colorop);
+		KUNIT_ASSERT_NOT_NULL(test, colorop);
+
+		ret = drm_plane_colorop_mult_init(priv->drm, colorop, priv->plane,
+						  &colorop_test_funcs,
+						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		if (ret)
+			kfree(colorop);
+		KUNIT_ASSERT_EQ(test, ret, 0);
+
+		drm_colorop_helper_add(colorop, &colorop_test_helper_funcs);
+		if (i)
+			drm_colorop_set_next_property(priv->colorops[i - 1], colorop);
+		priv->colorops[i] = colorop;
+	}
+
+	drm_mode_config_reset(priv->drm);
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+	drm_modeset_acquire_init(ctx, 0);
+	ret = kunit_add_action_or_reset(test, colorop_test_acquire_fini, ctx);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	priv->state = drm_kunit_helper_atomic_state_alloc(test, priv->drm, ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->state);
+
+	plane_state = drm_atomic_get_plane_state(priv->state, priv->plane);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_state);
+
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct drm_colorop_state *colorop_state;
+
+		colorop_state = drm_atomic_get_colorop_state(priv->state,
+							     priv->colorops[i]);
+		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, colorop_state);
+	}
+
+	return 0;
+}
+
+static struct drm_colorop_state *
+colorop_test_new_state(struct colorop_test_priv *priv, int i)
+{
+	return drm_atomic_get_new_colorop_state(priv->state, priv->colorops[i]);
+}
+
+/*
+ * A successful prepare_planes() prepares every colorop in the commit once,
+ * before the planes, without cleaning anything up. Aborting the commit with
+ * unprepare_planes() then cleans up every new colorop state once.
+ */
+static void drm_test_colorop_prepare_then_unprepare(struct kunit *test)
+{
+	struct colorop_test_priv *priv = test->priv;
+	int i, ret;
+
+	ret = drm_atomic_helper_prepare_planes(priv->drm, priv->state);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_prepared, 1);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_EQ(test, calls->prepared, 1);
+		KUNIT_EXPECT_PTR_EQ(test, calls->prepared_state,
+				    colorop_test_new_state(priv, i));
+		KUNIT_EXPECT_LT(test, calls->prepare_seq, priv->plane_prepare_seq);
+		KUNIT_EXPECT_EQ(test, calls->cleaned, 0);
+	}
+
+	drm_atomic_helper_unprepare_planes(priv->drm, priv->state);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_cleaned, 1);
+	KUNIT_EXPECT_EQ(test, priv->plane_end_access, 1);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_EQ(test, calls->cleaned, 1);
+		KUNIT_EXPECT_PTR_EQ(test, calls->cleaned_state,
+				    colorop_test_new_state(priv, i));
+	}
+}
+
+/*
+ * After the state swap, cleanup_planes() cleans up the old colorop states,
+ * which were current before the commit, and leaves the new ones prepared.
+ */
+static void drm_test_colorop_cleanup_after_swap(struct kunit *test)
+{
+	struct colorop_test_priv *priv = test->priv;
+	struct drm_colorop_state *old_state[TEST_NUM_COLOROPS];
+	int i, ret;
+
+	for (i = 0; i < TEST_NUM_COLOROPS; i++)
+		old_state[i] = priv->colorops[i]->state;
+
+	ret = drm_atomic_helper_prepare_planes(priv->drm, priv->state);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	ret = drm_atomic_helper_swap_state(priv->state, false);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_atomic_helper_cleanup_planes(priv->drm, priv->state);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_cleaned, 1);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_PTR_EQ(test, priv->colorops[i]->state,
+				    calls->prepared_state);
+		KUNIT_EXPECT_EQ(test, calls->cleaned, 1);
+		KUNIT_EXPECT_PTR_EQ(test, calls->cleaned_state, old_state[i]);
+	}
+}
+
+static const int colorop_fail_at_params[] = { 0, 1, TEST_NUM_COLOROPS - 1 };
+
+static void colorop_fail_at_desc(const int *fail_at, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "colorop %d", *fail_at);
+}
+
+KUNIT_ARRAY_PARAM(colorop_fail_at, colorop_fail_at_params, colorop_fail_at_desc);
+
+/*
+ * When a colorop fails to prepare, only the colorops prepared before it are
+ * cleaned up, and the planes are not prepared.
+ */
+static void drm_test_colorop_prepare_fail(struct kunit *test)
+{
+	struct colorop_test_priv *priv = test->priv;
+	int fail_at = *(const int *)test->param_value;
+	int i, ret;
+
+	priv->colorop_fail_at = fail_at;
+
+	ret = drm_atomic_helper_prepare_planes(priv->drm, priv->state);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_prepared, 0);
+	KUNIT_EXPECT_EQ(test, priv->plane_cleaned, 0);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_EQ(test, calls->prepared, i <= fail_at);
+		KUNIT_EXPECT_EQ(test, calls->cleaned, i < fail_at);
+		if (i < fail_at)
+			KUNIT_EXPECT_PTR_EQ(test, calls->cleaned_state,
+					    colorop_test_new_state(priv, i));
+	}
+}
+
+/*
+ * When a plane fails to prepare after the colorops, every colorop is cleaned
+ * up once. The failing plane is not.
+ */
+static void drm_test_colorop_plane_prepare_fail(struct kunit *test)
+{
+	struct colorop_test_priv *priv = test->priv;
+	int i, ret;
+
+	priv->plane_prepare_ret = -EIO;
+
+	ret = drm_atomic_helper_prepare_planes(priv->drm, priv->state);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_prepared, 1);
+	KUNIT_EXPECT_EQ(test, priv->plane_cleaned, 0);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_EQ(test, calls->prepared, 1);
+		KUNIT_EXPECT_EQ(test, calls->cleaned, 1);
+		KUNIT_EXPECT_PTR_EQ(test, calls->cleaned_state,
+				    colorop_test_new_state(priv, i));
+	}
+}
+
+/*
+ * When a plane fails begin_fb_access, the prepared plane and every colorop
+ * are cleaned up once.
+ */
+static void drm_test_colorop_plane_begin_access_fail(struct kunit *test)
+{
+	struct colorop_test_priv *priv = test->priv;
+	int i, ret;
+
+	priv->plane_begin_access_ret = -EIO;
+
+	ret = drm_atomic_helper_prepare_planes(priv->drm, priv->state);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+
+	KUNIT_EXPECT_EQ(test, priv->plane_prepared, 1);
+	KUNIT_EXPECT_EQ(test, priv->plane_cleaned, 1);
+	KUNIT_EXPECT_EQ(test, priv->plane_end_access, 0);
+	for (i = 0; i < TEST_NUM_COLOROPS; i++) {
+		struct colorop_hook_calls *calls = &priv->colorop[i];
+
+		KUNIT_EXPECT_EQ(test, calls->prepared, 1);
+		KUNIT_EXPECT_EQ(test, calls->cleaned, 1);
+		KUNIT_EXPECT_PTR_EQ(test, calls->cleaned_state,
+				    colorop_test_new_state(priv, i));
+	}
+}
+
+static struct kunit_case drm_colorop_helper_tests[] = {
+	KUNIT_CASE(drm_test_colorop_prepare_then_unprepare),
+	KUNIT_CASE(drm_test_colorop_cleanup_after_swap),
+	KUNIT_CASE_PARAM(drm_test_colorop_prepare_fail, colorop_fail_at_gen_params),
+	KUNIT_CASE(drm_test_colorop_plane_prepare_fail),
+	KUNIT_CASE(drm_test_colorop_plane_begin_access_fail),
+	{ }
+};
+
+static struct kunit_suite drm_colorop_helper_test_suite = {
+	.name = "drm_colorop_helper",
+	.init = colorop_test_init,
+	.test_cases = drm_colorop_helper_tests,
+};
+
+kunit_test_suite(drm_colorop_helper_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the colorop atomic helper hooks");
+MODULE_LICENSE("GPL");
-- 
2.56.0

[PATCH v1 07/13] drm/amd/display: advertise a DMA 3D LUT color pipeline

[Harry Wentland]

The DMA (fast-load) 3D LUT and the legacy inline 3D LUT map to the same
MCM 3D LUT HW block, so rather than adding a second 3D LUT op to the
existing pipeline, advertise a second color pipeline that is identical to
the default one except the inline DRM_COLOROP_3D_LUT is replaced by a
DRM_COLOROP_3D_LUT_DMA.

amdgpu_dm_build_default_pipeline() gains a dma_3d_lut argument that selects
the 3D LUT variant; amdgpu_dm_initialize_dma_3dlut_pipeline() builds the
DMA variant, and dm_plane_init_colorops() advertises it as a second
pipeline when mcm_3d_lut_caps.dma_3d_lut is set. The advertised format is
a 17^3 XBGR16161616 LINEAR cube with the DCN-derived 256-byte pitch.

Keep it behind an experimental KConfig for now.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/Kconfig           | 13 ++++++
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 43 ++++++++++++++++---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  4 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 17 ++++++++
 .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c  |  4 +-
 5 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index a319680c329b..e62efe422aeb 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -65,6 +65,19 @@ config DRM_AMD_SECURE_DISPLAY
 	  This option enables the calculation of crc of specific region via
 	  debugfs. Cooperate with specific DMCU FW.
 
+config DRM_AMD_DC_3DLUT_DMA
+	bool "Expose the DMA 3D LUT color pipeline (EXPERIMENTAL)"
+	depends on DRM_AMD_DC
+	default n
+	help
+	  Advertise a color pipeline with a DRM_COLOROP_3D_LUT_DMA colorop
+	  when the display hardware supports fast-loading the 3D LUT from a
+	  framebuffer.
+
+	  This is experimental.
+
+	  If unsure, say N.
+
 config DRM_AMD_DC_KUNIT_TEST
 	tristate "KUnit tests for the AMD DC display driver" if !KUNIT_ALL_TESTS
 	depends on DRM_AMD_DC && KUNIT && DEBUG_FS
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index 47d90b67120e..9d7720a9bd2e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -28,6 +28,7 @@
 #include <drm/drm_plane.h>
 #include <drm/drm_property.h>
 #include <drm/drm_colorop.h>
+#include <drm/drm_fourcc.h>
 
 #include "amdgpu.h"
 #include "amdgpu_dm_colorop.h"
@@ -67,6 +68,17 @@ const u64 amdgpu_dm_supported_fm =
 
 #define LUT3D_SIZE		17
 
+static const struct drm_mode_3dlut_mode amdgpu_dm_3dlut_dma_modes[] = {
+	{
+		.lut_size = LUT3D_SIZE,
+		.format = DRM_FORMAT_XBGR16161616,
+		.modifier = DRM_FORMAT_MOD_LINEAR,
+		.pitch = 256,
+		.traversal_order = DRM_COLOROP_LUT3D_TRAVERSAL_RGB,
+		.interpolation = DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
+	},
+};
+
 static const struct drm_colorop_funcs dm_colorop_funcs = {
 	.destroy = drm_colorop_destroy,
 };
@@ -115,7 +127,8 @@ EXPORT_IF_KUNIT(amdgpu_dm_colorop_kunit_set_ops);
 
 STATIC_IFN_KUNIT int
 amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane,
-				  bool hw_3d_lut, struct drm_prop_enum_list *list)
+				  bool hw_3d_lut, bool dma_3d_lut,
+				  struct drm_prop_enum_list *list)
 {
 	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
 	int ret;
@@ -225,17 +238,23 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane
 
 		i++;
 
-		/* 3D LUT */
+		/* 3D LUT - host loading, or DMA (fast-loading) */
 		ops[i] = colorop_kzalloc_obj();
 		if (!ops[i]) {
 			ret = -ENOMEM;
 			goto cleanup;
 		}
 
-		ret = colorop_3dlut_init(dev, ops[i], plane, &dm_colorop_funcs,
-					 LUT3D_SIZE,
-					 DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
-					 DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		if (dma_3d_lut)
+			ret = drm_plane_colorop_3dlut_dma_init(dev, ops[i], plane,
+					&dm_colorop_funcs, amdgpu_dm_3dlut_dma_modes,
+					ARRAY_SIZE(amdgpu_dm_3dlut_dma_modes),
+					DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		else
+			ret = colorop_3dlut_init(dev, ops[i], plane,
+					&dm_colorop_funcs, LUT3D_SIZE,
+					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
+					DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		if (ret)
 			goto cleanup;
 
@@ -298,6 +317,16 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 	bool hw_3d_lut = adev->dm.dc->caps.color.dpp.hw_3d_lut ||
 			 adev->dm.dc->caps.color.mpc.preblend;
 
-	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, list);
+	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, false, list);
 }
 EXPORT_IF_KUNIT(amdgpu_dm_initialize_default_pipeline);
+
+int amdgpu_dm_initialize_dma_3dlut_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list)
+{
+	struct drm_device *dev = plane->dev;
+	struct amdgpu_device *adev = drm_to_adev(dev);
+	bool hw_3d_lut = adev->dm.dc->caps.color.dpp.hw_3d_lut ||
+			 adev->dm.dc->caps.color.mpc.preblend;
+
+	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, true, list);
+}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
index edf7df7c19e9..510c367f8ca5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
@@ -33,6 +33,7 @@ extern const u64 amdgpu_dm_supported_blnd_tfs;
 extern const u64 amdgpu_dm_supported_fm;
 
 int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list);
+int amdgpu_dm_initialize_dma_3dlut_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list);
 
 #if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST)
 #include <drm/drm_colorop.h>
@@ -63,7 +64,8 @@ struct amdgpu_dm_colorop_kunit_ops {
 };
 
 int amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane,
-				      bool hw_3d_lut, struct drm_prop_enum_list *list);
+				      bool hw_3d_lut, bool dma_3d_lut,
+				      struct drm_prop_enum_list *list);
 void amdgpu_dm_colorop_kunit_set_ops(const struct amdgpu_dm_colorop_kunit_ops *ops);
 #endif
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 91398230d408..21fe005fe583 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -2172,6 +2172,23 @@ dm_plane_init_colorops(struct drm_plane *plane)
 
 	/* initialize pipeline */
 	if (dc->ctx->dce_version >= DCN_VERSION_3_0) {
+		/*
+		 * Advertise the DMA (fast-load) 3D LUT pipeline first
+		 * so that userspace which understands it prefers it
+		 * over the host-loading 3D LUT pipeline.
+		 */
+		if (IS_ENABLED(CONFIG_DRM_AMD_DC_3DLUT_DMA) &&
+		    dc->caps.color.mpc.mcm_3d_lut_caps.dma_3d_lut &&
+		    (dc->caps.color.dpp.hw_3d_lut || dc->caps.color.mpc.preblend)) {
+			ret = amdgpu_dm_initialize_dma_3dlut_pipeline(plane, &pipelines[len]);
+			if (ret) {
+				drm_err(plane->dev, "Failed to create DMA 3D LUT pipeline for plane %d: %d\n",
+					plane->base.id, ret);
+				goto out;
+			}
+			len++;
+		}
+
 		ret = amdgpu_dm_initialize_default_pipeline(plane, &pipelines[len]);
 		if (ret) {
 			drm_err(plane->dev, "Failed to create color pipeline for plane %d: %pe\n",
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
index 90cc5592ef5d..49d1d252a67d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
@@ -208,7 +208,7 @@ static void dm_test_initialize_default_pipeline(struct kunit *test)
 
 	plane = dm_colorop_alloc_plane(test, &drm);
 
-	ret = amdgpu_dm_build_default_pipeline(drm, plane, true, &list);
+	ret = amdgpu_dm_build_default_pipeline(drm, plane, true, false, &list);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 	kfree(list.name);
 
@@ -430,7 +430,7 @@ static int dm_colorop_build_with_fault(struct kunit *test, struct drm_device **d
 	amdgpu_dm_colorop_kunit_set_ops(&dm_colorop_test_ops);
 	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dm_colorop_test_reset_ops, NULL), 0);
 
-	ret = amdgpu_dm_build_default_pipeline(*drm, plane, true, &list);
+	ret = amdgpu_dm_build_default_pipeline(*drm, plane, true, false, &list);
 	kfree(list.name);
 
 	return ret;
-- 
2.56.0

[PATCH v1 08/13] drm/amd/display: pin the DMA 3D LUT framebuffer

[Harry Wentland]

Add prepare_fb and cleanup_fb colorop helper hooks for the DMA 3D LUT
colorop to pin and unpin its framebuffer, tying the pin to the colorop
state. prepare_fb calls drm_gem_colorop_helper_prepare_fb() to handle
implicit fencing.

A plane-only update does not necessarily carry the colorop states, so
call drm_atomic_add_affected_colorops() in atomic_check. The plane's
prepare_fb can then take the pinned framebuffer from the new colorop
state and write the LUT address into a newly created dc_plane_state.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 91 ++++++++++++++++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  3 +
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 39 +++++++-
 3 files changed, 129 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index 9d7720a9bd2e..5e6e40b5fd7d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -29,8 +29,12 @@
 #include <drm/drm_property.h>
 #include <drm/drm_colorop.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modeset_helper_vtables.h>
 
 #include "amdgpu.h"
+#include "amdgpu_display.h"
 #include "amdgpu_dm_colorop.h"
 #include "dm_helpers.h"
 #include "dc.h"
@@ -122,9 +126,88 @@ EXPORT_IF_KUNIT(amdgpu_dm_colorop_kunit_set_ops);
 #define colorop_ctm_3x4_init		drm_plane_colorop_ctm_3x4_init
 #define colorop_mult_init		drm_plane_colorop_mult_init
 #define colorop_3dlut_init		drm_plane_colorop_3dlut_init
-
 #endif
 
+bool amdgpu_dm_colorop_dma_3dlut_active(const struct drm_colorop *colorop,
+					const struct drm_colorop_state *colorop_state)
+{
+	return colorop->type == DRM_COLOROP_3D_LUT_DMA && colorop_state &&
+	       !colorop_state->bypass && colorop_state->fb;
+}
+
+/* Pinned while its colorop state is current, since DC re-fetches the LUT */
+static int dm_colorop_prepare_fb(struct drm_colorop *colorop,
+				 struct drm_colorop_state *new_state)
+{
+	struct drm_framebuffer *fb = new_state->fb;
+	struct amdgpu_framebuffer *afb;
+	struct amdgpu_device *adev;
+	struct amdgpu_bo *rbo;
+	uint32_t domain;
+	int r;
+
+	if (!amdgpu_dm_colorop_dma_3dlut_active(colorop, new_state))
+		return 0;
+
+	r = drm_gem_colorop_helper_prepare_fb(colorop, new_state);
+	if (r)
+		return r;
+
+	afb = to_amdgpu_framebuffer(fb);
+	rbo = gem_to_amdgpu_bo(drm_gem_fb_get_obj(fb, 0));
+	adev = amdgpu_ttm_adev(rbo->tbo.bdev);
+
+	r = amdgpu_bo_reserve(rbo, true);
+	if (r) {
+		drm_err(adev_to_drm(adev), "fail to reserve 3D LUT bo (%d)\n", r);
+		return r;
+	}
+
+	domain = amdgpu_display_supported_domains(adev, rbo->flags);
+	rbo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+	r = amdgpu_bo_pin(rbo, domain);
+	if (r) {
+		if (r != -ERESTARTSYS)
+			drm_err(adev_to_drm(adev), "Failed to pin 3D LUT bo (%d)\n", r);
+		amdgpu_bo_unreserve(rbo);
+		return r;
+	}
+
+	r = amdgpu_ttm_alloc_gart(&rbo->tbo);
+	if (r) {
+		amdgpu_bo_unpin(rbo);
+		amdgpu_bo_unreserve(rbo);
+		return r;
+	}
+
+	amdgpu_bo_unreserve(rbo);
+	afb->address = amdgpu_bo_gpu_offset(rbo);
+	amdgpu_bo_ref(rbo);
+
+	return 0;
+}
+
+static void dm_colorop_cleanup_fb(struct drm_colorop *colorop,
+				  struct drm_colorop_state *old_state)
+{
+	struct amdgpu_bo *rbo;
+
+	if (!amdgpu_dm_colorop_dma_3dlut_active(colorop, old_state))
+		return;
+
+	rbo = gem_to_amdgpu_bo(drm_gem_fb_get_obj(old_state->fb, 0));
+	if (amdgpu_bo_reserve(rbo, false))
+		return;
+	amdgpu_bo_unpin(rbo);
+	amdgpu_bo_unreserve(rbo);
+	amdgpu_bo_unref(&rbo);
+}
+
+static const struct drm_colorop_helper_funcs dm_colorop_helper_funcs = {
+	.prepare_fb = dm_colorop_prepare_fb,
+	.cleanup_fb = dm_colorop_cleanup_fb,
+};
+
 STATIC_IFN_KUNIT int
 amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane,
 				  bool hw_3d_lut, bool dma_3d_lut,
@@ -245,16 +328,18 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane
 			goto cleanup;
 		}
 
-		if (dma_3d_lut)
+		if (dma_3d_lut) {
 			ret = drm_plane_colorop_3dlut_dma_init(dev, ops[i], plane,
 					&dm_colorop_funcs, amdgpu_dm_3dlut_dma_modes,
 					ARRAY_SIZE(amdgpu_dm_3dlut_dma_modes),
 					DRM_COLOROP_FLAG_ALLOW_BYPASS);
-		else
+			drm_colorop_helper_add(ops[i], &dm_colorop_helper_funcs);
+		} else {
 			ret = colorop_3dlut_init(dev, ops[i], plane,
 					&dm_colorop_funcs, LUT3D_SIZE,
 					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
 					DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		}
 		if (ret)
 			goto cleanup;
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
index 510c367f8ca5..58604773db75 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
@@ -35,6 +35,9 @@ extern const u64 amdgpu_dm_supported_fm;
 int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list);
 int amdgpu_dm_initialize_dma_3dlut_pipeline(struct drm_plane *plane, struct drm_prop_enum_list *list);
 
+bool amdgpu_dm_colorop_dma_3dlut_active(const struct drm_colorop *colorop,
+					const struct drm_colorop_state *colorop_state);
+
 #if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST)
 #include <drm/drm_colorop.h>
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 21fe005fe583..469041145196 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -32,6 +32,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_colorop.h>
 
 #include "amdgpu.h"
 #include "dc.h"
@@ -1208,6 +1209,23 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
 }
 EXPORT_IF_KUNIT(amdgpu_dm_plane_fill_plane_buffer_attributes);
 
+/* Pinned by the colorop's prepare_fb, which runs before the plane's */
+static struct drm_framebuffer *
+dm_plane_lut3d_dma_fb(struct drm_plane_state *plane_state)
+{
+	struct drm_colorop *colorop;
+
+	for (colorop = plane_state->color_pipeline; colorop; colorop = colorop->next) {
+		struct drm_colorop_state *cs =
+			drm_atomic_get_new_colorop_state(plane_state->state, colorop);
+
+		if (amdgpu_dm_colorop_dma_3dlut_active(colorop, cs))
+			return cs->fb;
+	}
+
+	return NULL;
+}
+
 STATIC_IFN_KUNIT int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
 						       struct drm_plane_state *new_state)
 {
@@ -1216,6 +1234,7 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
 	struct amdgpu_device *adev;
 	struct amdgpu_bo *rbo;
 	struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
+	struct drm_framebuffer *lut_fb;
 	uint32_t domain;
 	int r;
 
@@ -1295,6 +1314,15 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
 			&plane_state->tiling_info, &plane_state->plane_size,
 			&plane_state->dcc, &plane_state->address,
 			afb->tmz_surface);
+
+		lut_fb = dm_plane_lut3d_dma_fb(new_state);
+		if (lut_fb) {
+			uint64_t addr = to_amdgpu_framebuffer(lut_fb)->address +
+					lut_fb->offsets[0];
+
+			plane_state->cm.lut3d_dma.addr.grph.addr.low_part = lower_32_bits(addr);
+			plane_state->cm.lut3d_dma.addr.grph.addr.high_part = upper_32_bits(addr);
+		}
 	}
 
 	return 0;
@@ -1533,6 +1561,16 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_atomic_check(struct drm_plane *plane,
 
 	dm_plane_state = to_dm_plane_state(new_plane_state);
 
+	/*
+	 * Pull the plane's colorops into the atomic state so the checks below
+	 * see them, even on a plane-only update that does not touch color.
+	 */
+	if (new_plane_state->color_pipeline) {
+		ret = drm_atomic_add_affected_colorops(state, plane);
+		if (ret)
+			return ret;
+	}
+
 	if (!dm_plane_state->dc_state)
 		return 0;
 
@@ -2375,4 +2413,3 @@ bool amdgpu_dm_plane_is_video_format(uint32_t format)
 	return false;
 }
 EXPORT_IF_KUNIT(amdgpu_dm_plane_is_video_format);
-
-- 
2.56.0

[PATCH v1 09/13] drm/amd/display: program the DMA 3D LUT colorop

[Harry Wentland]

Translate an enabled DRM_COLOROP_3D_LUT_DMA colorop into the DC fast-load
3D LUT config. __set_dm_plane_colorop_3dlut() now handles both the inline
3D LUT (DATA blob -> lut3d_func) and the DMA variant, which fills
dc_plane_state->cm.lut3d_dma from the pinned framebuffer's GPU address
(LINEAR_RGB swizzle, RGBA16161616 UNORM 12MSB, 17^3, neutral bias/scale)
and sets lut3d_dma_enable. Both 3D LUT variants map to the same MCM HW
block, so the rest of the per-colorop walk is shared; only this step
branches on the colorop type.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 68 ++++++++++++++-----
 1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index ebb05a20aa0c..8936b30e9eae 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -1868,25 +1868,60 @@ __set_dm_plane_colorop_3dlut(struct drm_plane_state *plane_state,
 	bool has_3dlut = adev->dm.dc->caps.color.dpp.hw_3d_lut || adev->dm.dc->caps.color.mpc.preblend;
 	const struct drm_device *dev = colorop->dev;
 	const struct drm_color_lut32 *lut3d;
+	struct amdgpu_framebuffer *afb;
 	uint32_t lut3d_size;
+	uint64_t addr;
 	int i = 0, ret = 0;
 
-	/* 3D LUT */
+	/* 3D LUT - inline or DMA (fast-load); both map to the same HW block */
 	old_colorop = colorop;
 	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
 		if (new_colorop_state->colorop == old_colorop &&
-		    new_colorop_state->colorop->type == DRM_COLOROP_3D_LUT) {
+		    (new_colorop_state->colorop->type == DRM_COLOROP_3D_LUT ||
+		     new_colorop_state->colorop->type == DRM_COLOROP_3D_LUT_DMA)) {
 			colorop_state = new_colorop_state;
 			break;
 		}
 	}
 
-	if (colorop_state && !colorop_state->bypass && colorop->type == DRM_COLOROP_3D_LUT) {
-		if (!has_3dlut) {
-			drm_dbg(dev, "3D LUT is not supported by hardware\n");
+	if (!colorop_state || colorop_state->bypass) {
+		dc_plane_state->cm.flags.bits.lut3d_enable = 0;
+		dc_plane_state->cm.flags.bits.lut3d_dma_enable = 0;
+		return 0;
+	}
+
+	if (!has_3dlut) {
+		drm_dbg(dev, "3D LUT is not supported by hardware\n");
+		return -EINVAL;
+	}
+
+	if (colorop->type == DRM_COLOROP_3D_LUT_DMA) {
+		if (!colorop_state->fb) {
+			drm_dbg(dev, "DMA 3D LUT colorop %d has no framebuffer\n",
+				colorop->base.id);
 			return -EINVAL;
 		}
 
+		drm_dbg(dev, "DMA 3D LUT colorop with ID: %d\n", colorop->base.id);
+
+		/* Not pinned yet for a new FB; the plane's prepare_fb fixes up addr */
+		afb = to_amdgpu_framebuffer(colorop_state->fb);
+		addr = afb->address + colorop_state->fb->offsets[0];
+
+		memset(&dc_plane_state->cm.lut3d_dma, 0, sizeof(dc_plane_state->cm.lut3d_dma));
+		dc_plane_state->cm.lut3d_dma.addr.type = PLN_ADDR_TYPE_GRAPHICS;
+		dc_plane_state->cm.lut3d_dma.addr.grph.addr.low_part = lower_32_bits(addr);
+		dc_plane_state->cm.lut3d_dma.addr.grph.addr.high_part = upper_32_bits(addr);
+		dc_plane_state->cm.lut3d_dma.swizzle = CM_LUT_3D_SWIZZLE_LINEAR_RGB;
+		dc_plane_state->cm.lut3d_dma.format = CM_LUT_PIXEL_FORMAT_RGBA16161616_UNORM_12MSB;
+		dc_plane_state->cm.lut3d_dma.size = CM_LUT_SIZE_171717;
+		/* neutral bias/scale for UNORM input */
+		dc_plane_state->cm.lut3d_dma.bias = 0;
+		dc_plane_state->cm.lut3d_dma.scale = 0x3C00; /* 1.0 in FP1.5.10 */
+		dc_plane_state->cm.flags.bits.lut3d_dma_enable = 1;
+		/* lut3d_enable gates the fast-load trigger and MCM 3D LUT enable */
+		dc_plane_state->cm.flags.bits.lut3d_enable = 1;
+	} else {
 		drm_dbg(dev, "3D LUT colorop with ID: %d\n", colorop->base.id);
 		lut3d = __extract_blob_lut32(colorop_state->data, &lut3d_size);
 		lut3d_size = lut3d != NULL ? lut3d_size : 0;
@@ -1898,19 +1933,18 @@ __set_dm_plane_colorop_3dlut(struct drm_plane_state *plane_state,
 		}
 
 		dc_plane_state->cm.flags.bits.lut3d_enable = 1;
+		dc_plane_state->cm.flags.bits.lut3d_dma_enable = 0;
+	}
 
-		/* 3D LUT requires shaper. If shaper colorop is bypassed, enable shaper curve
-		 * with TRANSFER_FUNCTION_LINEAR
-		 */
-		if (tf->type == TF_TYPE_BYPASS) {
-			tf->type = TF_TYPE_DISTRIBUTED_POINTS;
-			tf->tf = TRANSFER_FUNCTION_LINEAR;
-			tf->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
-			ret = __set_output_tf_32(tf, NULL, 0, false);
-			dc_plane_state->cm.flags.bits.shaper_enable = 1;
-		}
-	} else {
-		dc_plane_state->cm.flags.bits.lut3d_enable = 0;
+	/* 3D LUT requires shaper. If shaper colorop is bypassed, enable shaper curve
+	 * with TRANSFER_FUNCTION_LINEAR
+	 */
+	if (tf->type == TF_TYPE_BYPASS) {
+		tf->type = TF_TYPE_DISTRIBUTED_POINTS;
+		tf->tf = TRANSFER_FUNCTION_LINEAR;
+		tf->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
+		ret = __set_output_tf_32(tf, NULL, 0, false);
+		dc_plane_state->cm.flags.bits.shaper_enable = 1;
 	}
 
 	return ret;
-- 
2.56.0

[PATCH v1 10/13] drm/amd/display: add 3D LUT DMA pitch alignment cap

[Harry Wentland]

Rows of a DMA-fetched 3D LUT in the 3D swizzle layouts must be aligned
to 256 bytes when paired with gfx11 and to 128 bytes when paired with
gfx12, as modeled by the DML 3D LUT pitch calculation. DM needs this to
advertise the framebuffer pitch it expects for a DMA 3D LUT.

Add a pitch_alignment field to struct lut3d_caps and populate it for the
ASICs that support DMA 3D LUTs: 128 bytes for DCN401, which is paired
with gfx12, and 256 bytes for DCN42 and DCN42B, which are paired with
gfx11.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dc.h                             | 2 ++
 drivers/gpu/drm/amd/display/dc/resource/dcn42/dcn42_resource.c  | 2 ++
 .../gpu/drm/amd/display/dc/resource/dcn42b/dcn42b_resource.c    | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index d3c484f45333..e82db13687bd 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -297,6 +297,8 @@ struct lut3d_caps {
 		uint32_t dim_45 : 1; /* 3D LUT support for 45x45x45 */
 		uint32_t dim_65 : 1; /* 3D LUT support for 65x65x65 */
 	} lut_dim_caps;
+	/*< DMA row pitch alignment in bytes for the 3D swizzle layouts */
+	uint32_t pitch_alignment;
 };
 
 /**
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn42/dcn42_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn42/dcn42_resource.c
index e5e94e955157..a1fbe89a32bb 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn42/dcn42_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn42/dcn42_resource.c
@@ -2129,6 +2129,7 @@ static bool dcn42_resource_construct(
 	dc->caps.color.mpc.ocsc = 1;
 	dc->caps.color.mpc.preblend = true;
 	dc->caps.color.mpc.mcm_3d_lut_caps.dma_3d_lut = 1;
+	dc->caps.color.mpc.mcm_3d_lut_caps.pitch_alignment = 256;
 	dc->caps.color.mpc.mcm_3d_lut_caps.lut_dim_caps.dim_9 = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.lut_dim_caps.dim_17 = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_layout_support.linear_1d = 1;
@@ -2140,6 +2141,7 @@ static bool dcn42_resource_construct(
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_pixel_order_support.order_rgba = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_pixel_order_support.order_bgra = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.dma_3d_lut = 1;
+	dc->caps.color.mpc.rmcm_3d_lut_caps.pitch_alignment = 256;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.lut_dim_caps.dim_17 = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.lut_dim_caps.dim_33 = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.mem_layout_support.linear_1d = 1;
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn42b/dcn42b_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn42b/dcn42b_resource.c
index e111e7532383..4e91c908a948 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn42b/dcn42b_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn42b/dcn42b_resource.c
@@ -2095,6 +2095,7 @@ static bool dcn42b_resource_construct(
 	dc->caps.color.mpc.ocsc = 1;
 	dc->caps.color.mpc.preblend = true;
 	dc->caps.color.mpc.mcm_3d_lut_caps.dma_3d_lut = 1;
+	dc->caps.color.mpc.mcm_3d_lut_caps.pitch_alignment = 256;
 	dc->caps.color.mpc.mcm_3d_lut_caps.lut_dim_caps.dim_9 = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.lut_dim_caps.dim_17 = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_layout_support.linear_1d = 1;
@@ -2106,6 +2107,7 @@ static bool dcn42b_resource_construct(
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_pixel_order_support.order_rgba = 1;
 	dc->caps.color.mpc.mcm_3d_lut_caps.mem_pixel_order_support.order_bgra = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.dma_3d_lut = 1;
+	dc->caps.color.mpc.rmcm_3d_lut_caps.pitch_alignment = 256;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.lut_dim_caps.dim_17 = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.lut_dim_caps.dim_33 = 1;
 	dc->caps.color.mpc.rmcm_3d_lut_caps.mem_layout_support.linear_1d = 1;
-- 
2.56.0

[PATCH v1 11/13] drm/amd/display: derive the DMA 3D LUT pitch from DC caps

[Harry Wentland]

The LUT_3D_FORMATS entry of the DMA 3D LUT colorop hard-codes a 256-byte
row pitch. The required pitch depends on the row alignment of the DCN
generation, which DC now reports through the 3D LUT pitch_alignment cap.

Build the advertised mode when the DMA 3D LUT pipeline is created and
compute its pitch by aligning one LUT row to the reported alignment.
Only expose the DMA 3D LUT pipeline when DC reports a pitch alignment.

For the 17x17x17 XBGR16161616 LUT the pitch remains 256 bytes for both
128- and 256-byte alignment, so this has no functional effect on
current hardware.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 33 +++++++++----------
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  3 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  1 +
 .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c  |  4 +--
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index 5e6e40b5fd7d..b737b45fb2f4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -72,17 +72,6 @@ const u64 amdgpu_dm_supported_fm =
 
 #define LUT3D_SIZE		17
 
-static const struct drm_mode_3dlut_mode amdgpu_dm_3dlut_dma_modes[] = {
-	{
-		.lut_size = LUT3D_SIZE,
-		.format = DRM_FORMAT_XBGR16161616,
-		.modifier = DRM_FORMAT_MOD_LINEAR,
-		.pitch = 256,
-		.traversal_order = DRM_COLOROP_LUT3D_TRAVERSAL_RGB,
-		.interpolation = DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
-	},
-};
-
 static const struct drm_colorop_funcs dm_colorop_funcs = {
 	.destroy = drm_colorop_destroy,
 };
@@ -210,7 +199,8 @@ static const struct drm_colorop_helper_funcs dm_colorop_helper_funcs = {
 
 STATIC_IFN_KUNIT int
 amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane,
-				  bool hw_3d_lut, bool dma_3d_lut,
+				  bool hw_3d_lut,
+				  const struct drm_mode_3dlut_mode *dma_3dlut_mode,
 				  struct drm_prop_enum_list *list)
 {
 	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
@@ -328,10 +318,9 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane
 			goto cleanup;
 		}
 
-		if (dma_3d_lut) {
+		if (dma_3dlut_mode) {
 			ret = drm_plane_colorop_3dlut_dma_init(dev, ops[i], plane,
-					&dm_colorop_funcs, amdgpu_dm_3dlut_dma_modes,
-					ARRAY_SIZE(amdgpu_dm_3dlut_dma_modes),
+					&dm_colorop_funcs, dma_3dlut_mode, 1,
 					DRM_COLOROP_FLAG_ALLOW_BYPASS);
 			drm_colorop_helper_add(ops[i], &dm_colorop_helper_funcs);
 		} else {
@@ -402,7 +391,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
 	bool hw_3d_lut = adev->dm.dc->caps.color.dpp.hw_3d_lut ||
 			 adev->dm.dc->caps.color.mpc.preblend;
 
-	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, false, list);
+	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, NULL, list);
 }
 EXPORT_IF_KUNIT(amdgpu_dm_initialize_default_pipeline);
 
@@ -412,6 +401,16 @@ int amdgpu_dm_initialize_dma_3dlut_pipeline(struct drm_plane *plane, struct drm_
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	bool hw_3d_lut = adev->dm.dc->caps.color.dpp.hw_3d_lut ||
 			 adev->dm.dc->caps.color.mpc.preblend;
+	struct drm_mode_3dlut_mode mode = {
+		.lut_size = LUT3D_SIZE,
+		.format = DRM_FORMAT_XBGR16161616,
+		.modifier = DRM_FORMAT_MOD_LINEAR,
+		.traversal_order = DRM_COLOROP_LUT3D_TRAVERSAL_RGB,
+		.interpolation = DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
+	};
 
-	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, true, list);
+	mode.pitch = ALIGN(mode.lut_size * drm_format_info(mode.format)->cpp[0],
+			   adev->dm.dc->caps.color.mpc.mcm_3d_lut_caps.pitch_alignment);
+
+	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, &mode, list);
 }
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
index 58604773db75..ac0f26ffef32 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
@@ -67,7 +67,8 @@ struct amdgpu_dm_colorop_kunit_ops {
 };
 
 int amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane,
-				      bool hw_3d_lut, bool dma_3d_lut,
+				      bool hw_3d_lut,
+				      const struct drm_mode_3dlut_mode *dma_3dlut_mode,
 				      struct drm_prop_enum_list *list);
 void amdgpu_dm_colorop_kunit_set_ops(const struct amdgpu_dm_colorop_kunit_ops *ops);
 #endif
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 469041145196..aad146d9c6a7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -2217,6 +2217,7 @@ dm_plane_init_colorops(struct drm_plane *plane)
 		 */
 		if (IS_ENABLED(CONFIG_DRM_AMD_DC_3DLUT_DMA) &&
 		    dc->caps.color.mpc.mcm_3d_lut_caps.dma_3d_lut &&
+		    dc->caps.color.mpc.mcm_3d_lut_caps.pitch_alignment &&
 		    (dc->caps.color.dpp.hw_3d_lut || dc->caps.color.mpc.preblend)) {
 			ret = amdgpu_dm_initialize_dma_3dlut_pipeline(plane, &pipelines[len]);
 			if (ret) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
index 49d1d252a67d..fda30c29a81f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
@@ -208,7 +208,7 @@ static void dm_test_initialize_default_pipeline(struct kunit *test)
 
 	plane = dm_colorop_alloc_plane(test, &drm);
 
-	ret = amdgpu_dm_build_default_pipeline(drm, plane, true, false, &list);
+	ret = amdgpu_dm_build_default_pipeline(drm, plane, true, NULL, &list);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 	kfree(list.name);
 
@@ -430,7 +430,7 @@ static int dm_colorop_build_with_fault(struct kunit *test, struct drm_device **d
 	amdgpu_dm_colorop_kunit_set_ops(&dm_colorop_test_ops);
 	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dm_colorop_test_reset_ops, NULL), 0);
 
-	ret = amdgpu_dm_build_default_pipeline(*drm, plane, true, false, &list);
+	ret = amdgpu_dm_build_default_pipeline(*drm, plane, true, NULL, &list);
 	kfree(list.name);
 
 	return ret;
-- 
2.56.0

[PATCH v1 12/13] drm/amd/display: reject direct 3D LUT loading mode changes

[Harry Wentland]

Hardware cannot switch directly between a fast-loaded and a host-loaded
3D LUT in a single commit. Require userspace to disable or bypass the
LUT in an intermediate commit before enabling the other loading mode.

Compare the old and new DC plane color state in the atomic plane check
and reject a loading mode change when both states have the LUT enabled.
Using the derived DC flags covers both colorop pipelines and the legacy
host-loaded LUT properties, without rejecting pipeline changes that
leave the LUT disabled or keep the same loading mode.

Add parameterized KUnit coverage for disabled, host-loaded and DMA LUT
transitions, including disabled LUTs with the DMA flag still set.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 14 ++++
 .../amdgpu_dm/tests/amdgpu_dm_plane_test.c    | 65 +++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index aad146d9c6a7..e04e9a0cef73 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1550,9 +1550,11 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
 										 plane);
+	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
 	struct amdgpu_device *adev = drm_to_adev(plane->dev);
 	struct dc *dc = adev->dm.dc;
 	struct dm_plane_state *dm_plane_state;
+	const struct dc_plane_state *old_dc_state;
 	struct dc_scaling_info scaling_info;
 	struct drm_crtc_state *new_crtc_state;
 	int ret;
@@ -1574,6 +1576,18 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_atomic_check(struct drm_plane *plane,
 	if (!dm_plane_state->dc_state)
 		return 0;
 
+	old_dc_state = old_plane_state ? to_dm_plane_state(old_plane_state)->dc_state : NULL;
+	/* Switching loading modes requires an intermediate commit with the LUT disabled. */
+	if (old_dc_state && old_dc_state->cm.flags.bits.lut3d_enable &&
+	    dm_plane_state->dc_state->cm.flags.bits.lut3d_enable &&
+	    old_dc_state->cm.flags.bits.lut3d_dma_enable !=
+	    dm_plane_state->dc_state->cm.flags.bits.lut3d_dma_enable) {
+		drm_dbg_atomic(plane->dev,
+			       "[PLANE:%d:%s] Cannot switch between host-loaded and DMA 3D LUTs in one commit\n",
+			       plane->base.id, plane->name);
+		return -EINVAL;
+	}
+
 	new_crtc_state =
 		drm_atomic_get_new_crtc_state(state,
 					      new_plane_state->crtc);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
index 7422e8ba9564..2925b2803a38 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c
@@ -3019,6 +3019,69 @@ static void dm_test_atomic_check_success(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, amdgpu_dm_plane_atomic_check(plane, state), 0);
 }
 
+struct dm_test_lut3d_transition {
+	const char *name;
+	bool old_enable;
+	bool old_dma;
+	bool new_enable;
+	bool new_dma;
+	int expected;
+};
+
+static const struct dm_test_lut3d_transition dm_test_lut3d_transitions[] = {
+	{ "disabled_to_disabled", false, false, false, false, 0 },
+	{ "disabled_to_host", false, false, true, false, 0 },
+	{ "disabled_to_dma", false, false, true, true, 0 },
+	{ "host_to_disabled", true, false, false, false, 0 },
+	{ "host_to_host", true, false, true, false, 0 },
+	{ "host_to_dma", true, false, true, true, -EINVAL },
+	{ "dma_to_disabled", true, true, false, false, 0 },
+	{ "dma_to_host", true, true, true, false, -EINVAL },
+	{ "dma_to_dma", true, true, true, true, 0 },
+	{ "disabled_dma_to_host", false, true, true, false, 0 },
+	{ "host_to_disabled_dma", true, false, false, true, 0 },
+};
+
+KUNIT_ARRAY_PARAM_DESC(dm_test_lut3d_transition, dm_test_lut3d_transitions, name);
+
+static void dm_test_atomic_check_lut3d_transition(struct kunit *test)
+{
+	const struct dm_test_lut3d_transition *param = test->param_value;
+	struct dm_plane_state *dm_plane_state, *old_plane_state;
+	struct drm_crtc_state *new_crtc_state;
+	struct drm_atomic_commit *state;
+	struct resource_pool *res_pool;
+	struct amdgpu_device *adev;
+	struct drm_framebuffer *fb;
+	struct drm_plane *plane;
+
+	adev = dm_test_init_atomic_check_state(test, &state, &plane, &dm_plane_state,
+					       &new_crtc_state, &fb);
+	res_pool = kunit_kzalloc(test, sizeof(*res_pool), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, res_pool);
+	res_pool->funcs = kunit_kzalloc(test, sizeof(*res_pool->funcs), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, res_pool->funcs);
+	adev->dm.dc->res_pool = res_pool;
+
+	old_plane_state = kunit_kzalloc(test, sizeof(*old_plane_state), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, old_plane_state);
+	old_plane_state->dc_state = kunit_kzalloc(test, sizeof(*old_plane_state->dc_state),
+						 GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, old_plane_state->dc_state);
+	state->planes[0].old_state = &old_plane_state->base;
+
+	old_plane_state->dc_state->cm.flags.bits.lut3d_enable = param->old_enable;
+	old_plane_state->dc_state->cm.flags.bits.lut3d_dma_enable = param->old_dma;
+	dm_plane_state->dc_state->cm.flags.bits.lut3d_enable = param->new_enable;
+	dm_plane_state->dc_state->cm.flags.bits.lut3d_dma_enable = param->new_dma;
+	dm_plane_state->dc_state->src_rect.width = 100;
+	dm_plane_state->dc_state->src_rect.height = 100;
+	dm_plane_state->dc_state->dst_rect.width = 100;
+	dm_plane_state->dc_state->dst_rect.height = 100;
+
+	KUNIT_EXPECT_EQ(test, amdgpu_dm_plane_atomic_check(plane, state), param->expected);
+}
+
 struct dm_test_cursor_ctx {
 	struct amdgpu_device *adev;
 	struct amdgpu_crtc *acrtc;
@@ -3304,6 +3367,8 @@ static struct kunit_case amdgpu_dm_plane_test_cases[] = {
 	KUNIT_CASE(dm_test_atomic_check_missing_crtc_state),
 	KUNIT_CASE(dm_test_atomic_check_rejects),
 	KUNIT_CASE(dm_test_atomic_check_success),
+	KUNIT_CASE_PARAM(dm_test_atomic_check_lut3d_transition,
+			 dm_test_lut3d_transition_gen_params),
 	/* amdgpu_dm_plane_panic_flush() */
 	KUNIT_CASE(dm_test_panic_flush_no_dc_state),
 	KUNIT_CASE(dm_test_panic_flush_disables_dcc),
-- 
2.56.0

[PATCH v1 13/13] drm/amd/display: add KUnit tests for the DMA 3D LUT colorop

[Harry Wentland]

The DMA 3D LUT pipeline is not covered by the colorop tests, which
always build the pipeline with a host-loaded 3D LUT.

Export amdgpu_dm_colorop_dma_3dlut_active() and
amdgpu_dm_initialize_dma_3dlut_pipeline() for KUnit and test that:

- the 3D LUT slot becomes a bypassable DMA 3D LUT colorop advertising
  the given mode, and only it has prepare_fb and cleanup_fb hooks;
- no DMA 3D LUT colorop is created without 3D LUT hardware;
- the advertised mode is a 17-point XBGR16161616 linear LUT, and its
  pitch follows the DC pitch alignment;
- the LUT is only active when not bypassed and a framebuffer is set;
- prepare_fb and cleanup_fb do not touch the framebuffer of an
  inactive LUT.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c |   2 +
 .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c  | 282 ++++++++++++++++++
 2 files changed, 284 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index b737b45fb2f4..f42339a98549 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -123,6 +123,7 @@ bool amdgpu_dm_colorop_dma_3dlut_active(const struct drm_colorop *colorop,
 	return colorop->type == DRM_COLOROP_3D_LUT_DMA && colorop_state &&
 	       !colorop_state->bypass && colorop_state->fb;
 }
+EXPORT_IF_KUNIT(amdgpu_dm_colorop_dma_3dlut_active);
 
 /* Pinned while its colorop state is current, since DC re-fetches the LUT */
 static int dm_colorop_prepare_fb(struct drm_colorop *colorop,
@@ -414,3 +415,4 @@ int amdgpu_dm_initialize_dma_3dlut_pipeline(struct drm_plane *plane, struct drm_
 
 	return amdgpu_dm_build_default_pipeline(dev, plane, hw_3d_lut, &mode, list);
 }
+EXPORT_IF_KUNIT(amdgpu_dm_initialize_dma_3dlut_pipeline);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
index fda30c29a81f..fadeb690b8f0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_colorop_test.c
@@ -7,7 +7,11 @@
 
 #include <kunit/test.h>
 #include <drm/drm_colorop.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_kunit_helpers.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_property.h>
 
 #include "dc.h"
 #include "amdgpu.h"
@@ -310,6 +314,277 @@ static void dm_test_initialize_default_pipeline_no_3d_lut(struct kunit *test)
 						 expected, ARRAY_SIZE(expected));
 }
 
+/* Tests for the DMA (fast-loading) 3D LUT pipeline */
+
+static const enum drm_colorop_type dm_dma_3dlut_pipeline[] = {
+	DRM_COLOROP_1D_CURVE,	/* degam TF */
+	DRM_COLOROP_MULTIPLIER,
+	DRM_COLOROP_CTM_3X4,
+	DRM_COLOROP_1D_CURVE,	/* shaper TF */
+	DRM_COLOROP_1D_LUT,	/* shaper LUT */
+	DRM_COLOROP_3D_LUT_DMA,
+	DRM_COLOROP_1D_CURVE,	/* blnd TF */
+	DRM_COLOROP_1D_LUT,	/* blnd LUT */
+};
+
+static const struct drm_mode_3dlut_mode dm_dma_3dlut_test_mode = {
+	.lut_size = 17,
+	.format = DRM_FORMAT_XBGR16161616,
+	.modifier = DRM_FORMAT_MOD_LINEAR,
+	.pitch = 256,
+	.traversal_order = DRM_COLOROP_LUT3D_TRAVERSAL_RGB,
+	.interpolation = DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
+};
+
+static struct drm_colorop *dm_find_dma_3dlut_colorop(struct kunit *test,
+						     struct drm_device *drm)
+{
+	struct drm_colorop *op;
+
+	drm_for_each_colorop(op, drm)
+		if (op->type == DRM_COLOROP_3D_LUT_DMA)
+			return op;
+
+	KUNIT_FAIL_AND_ABORT(test, "no DMA 3D LUT colorop\n");
+	return NULL;
+}
+
+static const struct drm_mode_3dlut_mode *
+dm_expect_dma_3dlut_formats(struct kunit *test, struct drm_colorop *op)
+{
+	struct drm_property_blob *blob = op->lut_3d_formats_blob;
+
+	KUNIT_ASSERT_NOT_NULL(test, blob);
+	KUNIT_ASSERT_EQ(test, blob->length, sizeof(struct drm_mode_3dlut_mode));
+
+	return blob->data;
+}
+
+/**
+ * dm_test_build_dma_3dlut_pipeline() - Verify the 3D LUT slot becomes a DMA
+ *   3D LUT colorop advertising the given mode, and only it pins framebuffers.
+ * @test: KUnit test context.
+ */
+static void dm_test_build_dma_3dlut_pipeline(struct kunit *test)
+{
+	const struct drm_colorop_helper_funcs *funcs;
+	const struct drm_mode_3dlut_mode *mode;
+	struct drm_prop_enum_list list = {};
+	struct drm_colorop *op;
+	struct drm_device *drm;
+	struct drm_plane *plane;
+	int ret;
+
+	plane = dm_colorop_alloc_plane(test, &drm);
+
+	ret = amdgpu_dm_build_default_pipeline(drm, plane, true,
+					       &dm_dma_3dlut_test_mode, &list);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	kfree(list.name);
+
+	dm_expect_colorop_pipeline(test, drm, &list, dm_dma_3dlut_pipeline,
+				   ARRAY_SIZE(dm_dma_3dlut_pipeline));
+
+	drm_for_each_colorop(op, drm) {
+		if (op->type == DRM_COLOROP_3D_LUT_DMA)
+			continue;
+		KUNIT_EXPECT_NULL(test, op->helper_private);
+	}
+
+	op = dm_find_dma_3dlut_colorop(test, drm);
+	KUNIT_EXPECT_NOT_NULL(test, op->fb_property);
+	KUNIT_EXPECT_NOT_NULL(test, op->lut_3d_format_property);
+	KUNIT_EXPECT_NOT_NULL(test, op->lut_3d_formats_property);
+
+	funcs = op->helper_private;
+	KUNIT_ASSERT_NOT_NULL(test, funcs);
+	KUNIT_EXPECT_NOT_NULL(test, funcs->prepare_fb);
+	KUNIT_EXPECT_NOT_NULL(test, funcs->cleanup_fb);
+
+	mode = dm_expect_dma_3dlut_formats(test, op);
+	KUNIT_EXPECT_MEMEQ(test, mode, &dm_dma_3dlut_test_mode, sizeof(*mode));
+}
+
+/**
+ * dm_test_build_dma_3dlut_pipeline_no_3d_lut() - Verify no DMA 3D LUT colorop
+ *   is created without 3D LUT hardware.
+ * @test: KUnit test context.
+ */
+static void dm_test_build_dma_3dlut_pipeline_no_3d_lut(struct kunit *test)
+{
+	static const enum drm_colorop_type expected[] = {
+		DRM_COLOROP_1D_CURVE,
+		DRM_COLOROP_MULTIPLIER,
+		DRM_COLOROP_CTM_3X4,
+		DRM_COLOROP_1D_CURVE,
+		DRM_COLOROP_1D_LUT,
+	};
+	struct drm_prop_enum_list list = {};
+	struct drm_device *drm;
+	struct drm_plane *plane;
+	int ret;
+
+	plane = dm_colorop_alloc_plane(test, &drm);
+
+	ret = amdgpu_dm_build_default_pipeline(drm, plane, false,
+					       &dm_dma_3dlut_test_mode, &list);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	kfree(list.name);
+
+	dm_expect_colorop_pipeline(test, drm, &list, expected, ARRAY_SIZE(expected));
+}
+
+struct dm_dma_3dlut_pitch_param {
+	const char *name;
+	u32 pitch_alignment;
+	u32 pitch;
+};
+
+/* A 17-entry row of XBGR16161616 is 136 bytes. */
+static const struct dm_dma_3dlut_pitch_param dm_dma_3dlut_pitch_params[] = {
+	{ "align_1",	1,	136 },
+	{ "align_64",	64,	192 },
+	{ "align_256",	256,	256 },
+};
+
+KUNIT_ARRAY_PARAM_DESC(dm_dma_3dlut_pitch, dm_dma_3dlut_pitch_params, name);
+
+/**
+ * dm_test_initialize_dma_3dlut_pipeline() - Verify the advertised DMA 3D LUT
+ *   mode, and that its pitch follows the DC pitch alignment.
+ * @test: KUnit test context.
+ */
+static void dm_test_initialize_dma_3dlut_pipeline(struct kunit *test)
+{
+	const struct dm_dma_3dlut_pitch_param *param = test->param_value;
+	const struct drm_mode_3dlut_mode *mode;
+	struct drm_prop_enum_list list = {};
+	struct amdgpu_device *adev;
+	struct drm_device *drm;
+	struct drm_plane *plane;
+	struct dc *dc;
+	int ret;
+
+	adev = dm_kunit_alloc_adev(test);
+	drm = &adev->ddev;
+
+	dc = kunit_kzalloc(test, sizeof(*dc), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dc);
+	adev->dm.dc = dc;
+	dc->caps.color.mpc.preblend = true;
+	dc->caps.color.mpc.mcm_3d_lut_caps.pitch_alignment = param->pitch_alignment;
+
+	plane = drm_kunit_helper_create_primary_plane(test, drm, NULL, NULL,
+						      NULL, 0, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane);
+
+	kunit_add_action(test, kunit_colorop_pipeline_destroy, drm);
+
+	ret = amdgpu_dm_initialize_dma_3dlut_pipeline(plane, &list);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	kfree(list.name);
+
+	dm_expect_colorop_pipeline(test, drm, &list, dm_dma_3dlut_pipeline,
+				   ARRAY_SIZE(dm_dma_3dlut_pipeline));
+
+	mode = dm_expect_dma_3dlut_formats(test, dm_find_dma_3dlut_colorop(test, drm));
+	KUNIT_EXPECT_EQ(test, mode->lut_size, 17);
+	KUNIT_EXPECT_EQ(test, mode->format, DRM_FORMAT_XBGR16161616);
+	KUNIT_EXPECT_EQ(test, mode->modifier, DRM_FORMAT_MOD_LINEAR);
+	KUNIT_EXPECT_EQ(test, mode->pitch, param->pitch);
+	KUNIT_EXPECT_EQ(test, mode->traversal_order, DRM_COLOROP_LUT3D_TRAVERSAL_RGB);
+	KUNIT_EXPECT_EQ(test, mode->interpolation,
+			DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL);
+	KUNIT_EXPECT_EQ(test, mode->flags, 0);
+}
+
+/**
+ * dm_test_colorop_dma_3dlut_active() - Verify the DMA 3D LUT is only active
+ *   when not bypassed and a framebuffer is set.
+ * @test: KUnit test context.
+ */
+static void dm_test_colorop_dma_3dlut_active(struct kunit *test)
+{
+	struct drm_colorop_state *state;
+	struct drm_framebuffer *fb;
+	struct drm_colorop *op;
+
+	op = kunit_kzalloc(test, sizeof(*op), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, op);
+	state = kunit_kzalloc(test, sizeof(*state), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, state);
+	fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, fb);
+
+	op->type = DRM_COLOROP_3D_LUT_DMA;
+	state->colorop = op;
+	state->bypass = false;
+	state->fb = fb;
+	KUNIT_EXPECT_TRUE(test, amdgpu_dm_colorop_dma_3dlut_active(op, state));
+
+	KUNIT_EXPECT_FALSE(test, amdgpu_dm_colorop_dma_3dlut_active(op, NULL));
+
+	state->bypass = true;
+	KUNIT_EXPECT_FALSE(test, amdgpu_dm_colorop_dma_3dlut_active(op, state));
+
+	state->bypass = false;
+	state->fb = NULL;
+	KUNIT_EXPECT_FALSE(test, amdgpu_dm_colorop_dma_3dlut_active(op, state));
+
+	state->fb = fb;
+	op->type = DRM_COLOROP_3D_LUT;
+	KUNIT_EXPECT_FALSE(test, amdgpu_dm_colorop_dma_3dlut_active(op, state));
+}
+
+/**
+ * dm_test_dma_3dlut_inactive_prepare_cleanup() - Verify prepare_fb and
+ *   cleanup_fb leave a bypassed or framebuffer-less DMA 3D LUT alone.
+ * @test: KUnit test context.
+ *
+ * The framebuffer has no GEM object, so any attempt to pin it would fail or
+ * crash.
+ */
+static void dm_test_dma_3dlut_inactive_prepare_cleanup(struct kunit *test)
+{
+	const struct drm_colorop_helper_funcs *funcs;
+	struct drm_prop_enum_list list = {};
+	struct drm_colorop_state *state;
+	struct drm_framebuffer *fb;
+	struct drm_colorop *op;
+	struct drm_device *drm;
+	struct drm_plane *plane;
+	int ret;
+
+	plane = dm_colorop_alloc_plane(test, &drm);
+
+	ret = amdgpu_dm_build_default_pipeline(drm, plane, true,
+					       &dm_dma_3dlut_test_mode, &list);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	kfree(list.name);
+
+	op = dm_find_dma_3dlut_colorop(test, drm);
+	funcs = op->helper_private;
+	KUNIT_ASSERT_NOT_NULL(test, funcs);
+
+	state = kunit_kzalloc(test, sizeof(*state), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, state);
+	fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, fb);
+	state->colorop = op;
+
+	/* Bypassed, but still pointing at a framebuffer */
+	state->bypass = true;
+	state->fb = fb;
+	KUNIT_EXPECT_EQ(test, funcs->prepare_fb(op, state), 0);
+	funcs->cleanup_fb(op, state);
+
+	/* Enabled without a framebuffer */
+	state->bypass = false;
+	state->fb = NULL;
+	KUNIT_EXPECT_EQ(test, funcs->prepare_fb(op, state), 0);
+	funcs->cleanup_fb(op, state);
+}
+
 /* Failure paths of amdgpu_dm_build_default_pipeline() */
 
 /*
@@ -517,6 +792,13 @@ static struct kunit_case dm_colorop_test_cases[] = {
 	KUNIT_CASE(dm_test_initialize_default_pipeline_dpp_3d_lut),
 	KUNIT_CASE(dm_test_initialize_default_pipeline_mpc_preblend),
 	KUNIT_CASE(dm_test_initialize_default_pipeline_no_3d_lut),
+	/* DMA 3D LUT */
+	KUNIT_CASE(dm_test_build_dma_3dlut_pipeline),
+	KUNIT_CASE(dm_test_build_dma_3dlut_pipeline_no_3d_lut),
+	KUNIT_CASE_PARAM(dm_test_initialize_dma_3dlut_pipeline,
+			 dm_dma_3dlut_pitch_gen_params),
+	KUNIT_CASE(dm_test_colorop_dma_3dlut_active),
+	KUNIT_CASE(dm_test_dma_3dlut_inactive_prepare_cleanup),
 	/* amdgpu_dm_build_default_pipeline failure paths */
 	KUNIT_CASE_PARAM(dm_test_build_pipeline_alloc_fail, dm_colorop_step_gen_params),
 	KUNIT_CASE_PARAM(dm_test_build_pipeline_init_fail, dm_colorop_step_gen_params),
-- 
2.56.0

Re: [PATCH v1 00/13] Add DRM_COLOROP_3D_LUT_DMA for fast-loading 3D LUT

[Ville Syrjälä]

On Tue, Nov 03, 2026 at 04:37:43PM -0500, Harry Wentland wrote:
> Loading a DRM_COLOROP_3D_LUT on AMD HW takes a long time because the
> driver has to load the entire 3D LUT via register writes.
> 
> Starting with DCN 4 the HW has the ability to load a 3D LUT via DMA,
> which only requires an address update for programming the 3D LUT.
> 
> I propose exposing a new DRM_COLOROP_3D_LUT_DMA type to support this DMA
> 3D LUT loading mechanism. Users of the DRM_COLOROP_3D_LUT_DMA will parse
> the supported formats via the LUT_3D_FORMATS blob property, select the
> desired format via LUT_3D_FORMAT property, and pass the 3D LUT buffer as
> a drm_framebuffer via FB_ID on the colorop.

A framebuffer? That sounds like a very weird thing to use
for non-pixel data.

Why don't you just copy the LUT from the blob into a BO in the kernel
and DMA it from there? That's kinda how everything now works on Intel
hardware. Sure, it's not 100% optimal because of the extra copy, but
at least it avoids userspace having to care about it so much.

> 
> For details see the patches and/or posted branches.
> 
> In addition to the patches posted here the 3d-fl-fixes kernel branch
> contains a number of fixes that will be merged via the usual amdgpu
> display process. It also contains a patch to enable support on Navi 4
> (DCN 4.0.1), which won't be upstreamed since that generation requires a
> userspace workaround. The 3d-fl-workaround IGT branch exists solely for
> that purpose. Nonetheless, Navi 4 can serve as an enablement vehicle for
> anyone who wants to see the new interface working.
> 
> The kwin branch makes use of a pipeline with DRM_COLOROP_3D_LUT_DMA if
> such a pipeline is discovered before a pipeline with a
> DRM_COLOROP_3D_LUT.
> 
> ### Branches
> 
> kernel: https://gitlab.freedesktop.org/hwentland/linux/-/commits/3d-fl
> kernel w/ fixes: https://gitlab.freedesktop.org/hwentland/linux/-/commits/3d-fl-fixes
> IGT: https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/3d-fl
> IGT Navi 4 w/a: https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/3d-fl-workaround
> kwin: https://invent.kde.org/hwentlan/kwin/-/commits/3d-fl
> 
> Cc: Melissa Wen <mwen@igalia.com>
> Cc: Autumn Ashton <misyl@froggi.es>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> 
> Harry Wentland (13):
>   drm/colorop: Add DRM_COLOROP_3D_LUT_DMA
>   drm/colorop: add DRM_COLOROP_3D_LUT_DMA object and state
>   drm/atomic: wire up DMA 3D LUT colorop properties
>   drm/atomic: validate DMA 3D LUT colorop framebuffer
>   drm/atomic: add colorop prepare_fb and cleanup_fb helper hooks
>   drm/tests: test colorop prepare_fb and cleanup_fb helpers
>   drm/amd/display: advertise a DMA 3D LUT color pipeline
>   drm/amd/display: pin the DMA 3D LUT framebuffer
>   drm/amd/display: program the DMA 3D LUT colorop
>   drm/amd/display: add 3D LUT DMA pitch alignment cap
>   drm/amd/display: derive the DMA 3D LUT pitch from DC caps
>   drm/amd/display: reject direct 3D LUT loading mode changes
>   drm/amd/display: add KUnit tests for the DMA 3D LUT colorop
> 
>  drivers/gpu/drm/amd/display/Kconfig           |  13 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  68 ++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 131 +++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |   8 +-
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  71 +++-
>  .../amdgpu_dm/tests/amdgpu_dm_colorop_test.c  | 286 ++++++++++++-
>  .../amdgpu_dm/tests/amdgpu_dm_plane_test.c    |  65 +++
>  drivers/gpu/drm/amd/display/dc/dc.h           |   2 +
>  .../dc/resource/dcn42/dcn42_resource.c        |   2 +
>  .../dc/resource/dcn42b/dcn42b_resource.c      |   2 +
>  drivers/gpu/drm/drm_atomic.c                  |  66 +++
>  drivers/gpu/drm/drm_atomic_helper.c           |  93 ++++
>  drivers/gpu/drm/drm_atomic_uapi.c             |  28 ++
>  drivers/gpu/drm/drm_colorop.c                 |  96 +++++
>  drivers/gpu/drm/drm_framebuffer.c             |  45 +-
>  drivers/gpu/drm/drm_gem_atomic_helper.c       |  81 ++++
>  drivers/gpu/drm/tests/Makefile                |   1 +
>  .../gpu/drm/tests/drm_colorop_helper_test.c   | 397 ++++++++++++++++++
>  include/drm/drm_atomic.h                      |  20 +
>  include/drm/drm_colorop.h                     |  71 ++++
>  include/drm/drm_gem_atomic_helper.h           |  11 +
>  include/drm/drm_modeset_helper_vtables.h      |  62 +++
>  include/uapi/drm/drm.h                        |   7 +-
>  include/uapi/drm/drm_mode.h                   |  79 ++++
>  24 files changed, 1672 insertions(+), 33 deletions(-)
>  create mode 100644 drivers/gpu/drm/tests/drm_colorop_helper_test.c
> 
> --
> 2.56.0

-- 
Ville Syrjälä
Intel