Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane Color Pipeline
On Thu, 7 Sep 2023 12:31:47 + "Shankar, Uma" wrote: > > -Original Message- > > From: Pekka Paalanen > > Sent: Tuesday, September 5, 2023 5:03 PM > > To: Shankar, Uma > > Cc: intel-...@lists.freedesktop.org; Borah, Chaitanya Kumar > > ; dri-de...@lists.freedesktop.org; wayland- > > de...@lists.freedesktop.org; Harry Wentland ; > > Sebastian Wick ; ville.syrj...@linux.intel.com; > > Jonas Adahl > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane > > Color Pipeline > > > > On Mon, 4 Sep 2023 13:44:49 + > > "Shankar, Uma" wrote: > > > > > > -Original Message- > > > > From: dri-devel On Behalf > > > > Of Pekka Paalanen > > > > Sent: Wednesday, August 30, 2023 5:59 PM > > > > To: Shankar, Uma > > > > Cc: intel-...@lists.freedesktop.org; Borah, Chaitanya Kumar > > > > ; dri-de...@lists.freedesktop.org; > > > > wayland- de...@lists.freedesktop.org > > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed > > > > Plane Color Pipeline > > > > > > > > On Wed, 30 Aug 2023 08:59:36 + > > > > "Shankar, Uma" wrote: > > > > > > > > > > -Original Message- > > > > > > From: Harry Wentland > > > > > > Sent: Wednesday, August 30, 2023 1:10 AM > > > > > > To: Shankar, Uma ; > > > > > > intel-...@lists.freedesktop.org; dri- > > > > > > de...@lists.freedesktop.org > > > > > > Cc: Borah, Chaitanya Kumar ; > > > > > > wayland- de...@lists.freedesktop.org > > > > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for > > > > > > proposed Plane Color Pipeline > > > > > > > > > > > > > > > > > > > > > > > > On 2023-08-29 12:03, Uma Shankar wrote: > > > > > > > Add the documentation for the new proposed Plane Color Pipeline. > > > > > > > > > > > > > > Co-developed-by: Chaitanya Kumar Borah > > > > > > > > > > > > > > Signed-off-by: Chaitanya Kumar Borah > > > > > > > > > > > > > > Signed-off-by: Uma Shankar > > > > > > > --- > > > > > > > .../gpu/rfc/plane_color_pipeline.rst | 394 > > > > > > > ++ > > > > > > > 1 file changed, 394 insertions(+) > > > > > > > create mode 100644 > > > > > > > Documentation/gpu/rfc/plane_color_pipeline.rst > > > > > > > > > > > > > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst > > > > > > > b/Documentation/gpu/rfc/plane_color_pipeline.rst > > > > > > > new file mode 100644 > > > > > > > index ..60ce515b6ea7 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst > > > > > > > > ... > > > > > > > > Hi Uma! > > > > > > Thanks Pekka for the feedback and useful inputs. > > > > Hi Uma, > > > > sorry to say, but the overall feeling I get from this proposal is that it > > is just the > > current color related KMS properties wrapped in a pipeline blob. That is > > not the > > re-design I believe we are looking for, and the feeling is based on several > > details > > that are just copied from the current property design. Also the "private" > > stuff has > > to go. > > Hi Pekka, > Ok, got the concerns in general. We will try to evaluate in deeper detail the > property based design and come back if there are some issues or inputs. > > At Intel we don't need private as of now, but we thought of having an option > to > enable any custom hardware or vendor. But we can drop the same for now if > community doesn't feel the need for it. > > > All the varying LUT entries, varying LUT precision, 1D/3D LUTs, varying LUT > > tap > > distribution, and parametrized curves are good development, but right now we > > are looking at things one step higher level: the overall color pipeline > > design and > > how to represent any operation. Most of this series is considering details > > below > > the current attention level, hence I'm paying attention only to the first > > few > > patches. > > We will need to precisely describe the hardware in userspace. Number of luts, > precision, > segments etc.., we can't just pass EOTF's as enum from userspace and let > driver put > hardcoded values to LUT. This will be nothing but an extension of descriptive > behaviour. > This will be needed as there are multiple colorspaces possible and even LUTS > can be > used to perform tone mapping. So, we need userspace to be able to program > luts directly. Hi Uma, yes, we do need to expose freely programmable LUTs when hardware has them. That's why I say it is good development. However, this is not an either-or situation. We must also be able to expose fixed-function curve blocks when hardware has them. Please, do not confuse this with a descriptive design. This is not about saying "this FB is using PQ encoding, convert it to NNN for me". This is about defining an operation, that is mathematically defined as "the PQ EOTF with normalized domain and range", for example. This is prescriptive, because the exact mathematical formula of the operation is defined, and it does not depend on any properties of the block'
[RFC PATCH 00/10] Color Pipeline API w/ VKMS
This is an early RFC set for a color pipeline API, along with a sample implementation in VKMS. All the key API bits are here, but I would like to show a larger variety of colorop types, as well as examples of different possible color pipelines for a given plane. The first patch is a doc patch that will explain the motivation and reasoning behind this approach and give an overview over the API. IGT tests can be found at https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/merge_requests/1 IGT patches are also being sent to the igt-dev mailing list. libdrm changes to support the new IOCTLs are at https://gitlab.freedesktop.org/hwentland/drm/-/merge_requests/1 If you prefer a gitlab MR for review you can find it at https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 A slightly different approach for a Color Pipeline API was sent by Uma Shankar and can be found at https://patchwork.freedesktop.org/series/123024/ The main difference is that his approach is not introducing a new DRM core object but instead exposes color pipelines via blob properties. There are pros and cons to both approaches. Cc: Ville Syrjala Cc: Pekka Paalanen Cc: Simon Ser Cc: Harry Wentland Cc: Melissa Wen Cc: Jonas Ådahl Cc: Sebastian Wick Cc: Shashank Sharma Cc: Alexander Goins Cc: Joshua Ashton Cc: Michel Dänzer Cc: Aleix Pol Cc: Xaver Hugl Cc: Victoria Brekenfeld Cc: Daniel Vetter Cc: Uma Shankar Cc: Naseer Ahmed Cc: Christopher Braga Harry Wentland (10): drm/doc/rfc: Describe why prescriptive color pipeline is needed drm/colorop: Introduce new drm_colorop mode object drm/colorop: Add TYPE property drm/color: Add 1D Curve subtype drm/colorop: Add BYPASS property drm/colorop: Add NEXT property drm/colorop: Add atomic state print for drm_colorop drm/colorop: Add new IOCTLs to retrieve drm_colorop objects drm/plane: Add COLOR PIPELINE property drm/vkms: Add enumerated 1D curve colorop Documentation/gpu/rfc/color_pipeline.rst | 278 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_atomic.c | 154 ++ drivers/gpu/drm/drm_atomic_helper.c | 12 + drivers/gpu/drm/drm_atomic_state_helper.c | 5 + drivers/gpu/drm/drm_atomic_uapi.c | 110 +++ drivers/gpu/drm/drm_colorop.c | 343 ++ drivers/gpu/drm/drm_crtc_internal.h | 4 + drivers/gpu/drm/drm_ioctl.c | 5 + drivers/gpu/drm/drm_mode_config.c | 7 + drivers/gpu/drm/drm_plane_helper.c| 2 +- drivers/gpu/drm/vkms/Makefile | 3 +- drivers/gpu/drm/vkms/vkms_colorop.c | 108 +++ drivers/gpu/drm/vkms/vkms_composer.c | 316 drivers/gpu/drm/vkms/vkms_drv.h | 4 + drivers/gpu/drm/vkms/vkms_plane.c | 2 + include/drm/drm_atomic.h | 82 ++ include/drm/drm_atomic_uapi.h | 3 + include/drm/drm_colorop.h | 233 +++ include/drm/drm_mode_config.h | 18 ++ include/drm/drm_plane.h | 10 + include/uapi/drm/drm.h| 3 + include/uapi/drm/drm_mode.h | 22 ++ 23 files changed, 1723 insertions(+), 2 deletions(-) create mode 100644 Documentation/gpu/rfc/color_pipeline.rst create mode 100644 drivers/gpu/drm/drm_colorop.c create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c create mode 100644 include/drm/drm_colorop.h -- 2.42.0
[RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
Signed-off-by: Harry Wentland Cc: Ville Syrjala Cc: Pekka Paalanen Cc: Simon Ser Cc: Harry Wentland Cc: Melissa Wen Cc: Jonas Ådahl Cc: Sebastian Wick Cc: Shashank Sharma Cc: Alexander Goins Cc: Joshua Ashton Cc: Michel Dänzer Cc: Aleix Pol Cc: Xaver Hugl Cc: Victoria Brekenfeld Cc: Daniel Vetter Cc: Uma Shankar Cc: Naseer Ahmed Cc: Christopher Braga --- Documentation/gpu/rfc/color_pipeline.rst | 278 +++ 1 file changed, 278 insertions(+) create mode 100644 Documentation/gpu/rfc/color_pipeline.rst diff --git a/Documentation/gpu/rfc/color_pipeline.rst b/Documentation/gpu/rfc/color_pipeline.rst new file mode 100644 index ..bfa4a8f12087 --- /dev/null +++ b/Documentation/gpu/rfc/color_pipeline.rst @@ -0,0 +1,278 @@ + +Linux Color Pipeline API + + +What problem are we solving? + + +We would like to support pre-, and post-blending complex color transformations +in order to allow for HW-supported HDR use-cases, as well as to provide support +to color-managed applications, such as video or image editors. + +While it is possible to support an HDR output on HW supporting the Colorspace +and HDR Metadata drm_connector properties that requires the compositor or +application to render and compose the content into one final buffer intended for +display. Doing so is costly. + +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and other +operations to support color transformations. These operations are often +implemented in fixed-function HW and therefore much more power efficient than +performing similar operations via shaders or CPU. + +We would like to make use of this HW functionality to support complex color +transformations with no, or minimal CPU or shader load. + + +How are other OSes solving this problem? + + +The most widely supported use-cases regard HDR content, whether video or +gaming. + +Most OSes will specify the source content format (color gamut, encoding transfer +function, and other metadata, such as max and average light levels) to a driver. +Drivers will then program their fixed-function HW accordingly to map from a +source content buffer's space to a display's space. + +When fixed-function HW is not available the compositor will assemble a shader to +ask the GPU to perform the transformation from the source content format to the +display's format. + +A compositor's mapping function and a driver's mapping function are usually +entirely separate concepts. On OSes where a HW vendor has no insight into +closed-source compositor code such a vendor will tune their color management +code to visually match the compositor's. On other OSes, where both mapping +functions are open to an implementer they will ensure both mappings match. + + +Why is Linux different? +=== + +Unlike other OSes, where there is one compositor for one or more drivers, on +Linux we have a many-to-many relationship. Many compositors; many drivers. +In addition each compositor vendor or community has their own view of how +color management should be done. This is what makes Linux so beautiful. + +This means that a HW vendor can now no longer tune their driver to one +compositor, as tuning it to one will almost inevitably make it look very +different from another compositor's color mapping. + +We need a better solution. + + +Descriptive API +=== + +An API that describes the source and destination colorspaces is a descriptive +API. It describes the input and output color spaces but does not describe +how precisely they should be mapped. Such a mapping includes many minute +design decision that can greatly affect the look of the final result. + +It is not feasible to describe such mapping with enough detail to ensure the +same result from each implementation. In fact, these mappings are a very active +research area. + + +Prescriptive API + + +A prescriptive API describes not the source and destination colorspaces. It +instead prescribes a recipe for how to manipulate pixel values to arrive at the +desired outcome. + +This recipe is generally an order straight-forward operations, with clear +mathematical definitions, such as 1D LUTs, 3D LUTs, matrices, or other +operations that can be described in a precise manner. + + +The Color Pipeline API +== + +HW color management pipelines can significantly differ between HW +vendors in terms of availability, ordering, and capabilities of HW +blocks. This makes a common definition of color management blocks and +their ordering nigh impossible. Instead we are defining an API that +allows user space to discover the HW capabilities. + + +drm_colorop Object & IOCTLs +=== + +To support the definition of color pipelines we introduce a new DRM core +object, a drm_colorop. Individual drm_colorop objects will be chained +via the NEXT
[RFC PATCH 02/10] drm/colorop: Introduce new drm_colorop mode object
This patches introduces a new drm_colorop mode object. This
object represents color transformations and can be used to
define color pipelines.
We also introduce the drm_colorop_state here, as well as
various helpers and state tracking bits.
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_atomic.c| 79 +
drivers/gpu/drm/drm_atomic_helper.c | 12 ++
drivers/gpu/drm/drm_atomic_uapi.c | 48
drivers/gpu/drm/drm_colorop.c | 169
drivers/gpu/drm/drm_mode_config.c | 7 ++
drivers/gpu/drm/drm_plane_helper.c | 2 +-
include/drm/drm_atomic.h| 82 ++
include/drm/drm_atomic_uapi.h | 1 +
include/drm/drm_colorop.h | 157 ++
include/drm/drm_mode_config.h | 18 +++
include/drm/drm_plane.h | 2 +
include/uapi/drm/drm.h | 3 +
include/uapi/drm/drm_mode.h | 1 +
14 files changed, 581 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_colorop.c
create mode 100644 include/drm/drm_colorop.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1855863b4d7a..941de0269709 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,6 +16,7 @@ drm-y := \
drm_client.o \
drm_client_modeset.o \
drm_color_mgmt.o \
+ drm_colorop.o \
drm_connector.o \
drm_crtc.o \
drm_displayid.o \
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 11f3a130f6f4..d734e9d5bfed 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -108,6 +109,7 @@ void drm_atomic_state_default_release(struct
drm_atomic_state *state)
kfree(state->connectors);
kfree(state->crtcs);
kfree(state->planes);
+ kfree(state->colorops);
kfree(state->private_objs);
}
EXPORT_SYMBOL(drm_atomic_state_default_release);
@@ -139,6 +141,10 @@ drm_atomic_state_init(struct drm_device *dev, struct
drm_atomic_state *state)
sizeof(*state->planes), GFP_KERNEL);
if (!state->planes)
goto fail;
+ state->colorops = kcalloc(dev->mode_config.num_colorop,
+ sizeof(*state->colorops), GFP_KERNEL);
+ if (!state->colorops)
+ goto fail;
state->dev = dev;
@@ -244,6 +250,20 @@ void drm_atomic_state_default_clear(struct
drm_atomic_state *state)
state->planes[i].new_state = NULL;
}
+ for (i = 0; i < config->num_colorop; i++) {
+ struct drm_colorop *colorop = state->colorops[i].ptr;
+
+ if (!colorop)
+ continue;
+
+ drm_colorop_atomic_destroy_state(colorop,
+state->colorops[i].state);
+ state->colorops[i].ptr = NULL;
+ state->colorops[i].state = NULL;
+ state->colorops[i].old_state = NULL;
+ state->colorops[i].new_state = NULL;
+ }
+
for (i = 0; i < state->num_private_objs; i++) {
struct drm_private_obj *obj = state->private_objs[i].ptr;
@@ -562,6 +582,65 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_plane_state);
+
+/**
+ * drm_atomic_get_colorop_state - get colorop state
+ * @state: global atomic state object
+ * @colorop: colorop to get state object for
+ *
+ * This function returns the colorop state for the given colorop, allocating it
+ * if needed. It will also grab the relevant plane lock to make sure that the
+ * state is consistent.
+ *
+ * Returns:
+ *
+ * Either the allocated state or the error code encoded into the pointer. When
+ * the error is EDEADLK then the w/w mutex code has detected a deadlock and the
+ * entire atomic sequence must be restarted. All other errors are fatal.
+ */
+struct drm_colorop_state *
+drm_atomic_get_colorop_state(struct drm_atomic_state *state,
+struct drm_colorop *colorop)
+{
+ int ret, index = drm_colorop_index(colorop);
+ struct drm_colorop_state *colorop_state;
+ struct drm_plane_state *plane_state;
+
+ WARN_ON(!state->acquire_ctx);
+
+ colorop_state = drm_atomic_get_existing_colorop_state(state, colorop);
+ if (colorop_state)
+ return colorop_state;
+
+ /* TODO where is the unlock? */
+
[RFC PATCH 03/10] drm/colorop: Add TYPE property
Add a read-only TYPE property. The TYPE specifies the colorop
type, such as enumerated curve, 1D LUT, CTM, 3D LUT, PWL LUT,
etc.
For now we're only introducing an enumerated 1D LUT type to
illustrate the concept.
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_atomic.c | 4 +--
drivers/gpu/drm/drm_atomic_uapi.c | 8 +-
drivers/gpu/drm/drm_colorop.c | 44 ++-
include/drm/drm_colorop.h | 21 ++-
4 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d734e9d5bfed..8a5f8cd22c8d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -627,8 +627,8 @@ drm_atomic_get_colorop_state(struct drm_atomic_state *state,
state->colorops[index].new_state = colorop_state;
colorop_state->state = state;
- drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d] %p state to %p\n",
- colorop->base.id, colorop_state, state);
+ drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d:%d] %p state to %p\n",
+ colorop->base.id, colorop->type, colorop_state, state);
/* TODO is this necessary? */
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index b1aa752c1848..51072fe2b548 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -660,7 +660,13 @@ drm_atomic_colorop_get_property(struct drm_colorop
*colorop,
const struct drm_colorop_state *state,
struct drm_property *property, uint64_t *val)
{
- return -EINVAL;
+ if (property == colorop->type_property) {
+ *val = colorop->type;
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
}
static int drm_atomic_set_writeback_fb_for_connector(
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 78d6a0067f5b..c028d5426d42 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -32,12 +32,17 @@
/* TODO big colorop doc, including properties, etc. */
+static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
+ { DRM_COLOROP_1D_CURVE, "1D Curve" },
+};
+
/* Init Helpers */
int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
-struct drm_plane *plane)
+struct drm_plane *plane, enum drm_colorop_type type)
{
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property *prop;
int ret = 0;
ret = drm_mode_object_add(dev, &colorop->base, DRM_MODE_OBJECT_COLOROP);
@@ -46,12 +51,28 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->base.properties = &colorop->properties;
colorop->dev = dev;
+ colorop->type = type;
colorop->plane = plane;
list_add_tail(&colorop->head, &config->colorop_list);
colorop->index = config->num_colorop++;
/* add properties */
+
+ /* type */
+ prop = drm_property_create_enum(dev,
+ DRM_MODE_PROP_IMMUTABLE |
DRM_MODE_PROP_ATOMIC,
+ "TYPE", drm_colorop_type_enum_list,
+ ARRAY_SIZE(drm_colorop_type_enum_list));
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->type_property = prop;
+
+ drm_object_attach_property(&colorop->base,
+ colorop->type_property,
+ colorop->type);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -167,3 +188,24 @@ void drm_colorop_reset(struct drm_colorop *colorop)
__drm_colorop_reset(colorop, colorop->state);
}
EXPORT_SYMBOL(drm_colorop_reset);
+
+
+static const char * const colorop_type_name[] = {
+ [DRM_COLOROP_1D_CURVE] = "1D Curve",
+};
+
+/**
+ * drm_get_colorop_type_name - return a string for colorop type
+ * @range: colorop type to compute name of
+ *
+ * In contrast to the other drm_get_*_name functions this one here returns a
+ * const pointer and hence is threadsafe.
+ */
+const char *drm_get_colorop_type_name(enum drm_colorop_type type)
+{
+ if (WARN_ON(type >= ARRAY_SIZE(colorop_type_name)))
+ return "unknown";
+
+ return colorop_type_name[type];
+}
+
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 3dd169b0317d..22a217372428 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -30,6 +30,10 @@
#include
#include
+enum
[RFC PATCH 04/10] drm/color: Add 1D Curve subtype
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_atomic_uapi.c | 18 ++
drivers/gpu/drm/drm_colorop.c | 39 +++
include/drm/drm_colorop.h | 20
3 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 51072fe2b548..9b01f234b04e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -648,11 +648,17 @@ static int drm_atomic_colorop_set_property(struct
drm_colorop *colorop,
struct drm_colorop_state *state, struct drm_file *file_priv,
struct drm_property *property, uint64_t val)
{
- drm_dbg_atomic(colorop->dev,
- "[COLOROP:%d] unknown property [PROP:%d:%s]]\n",
- colorop->base.id,
- property->base.id, property->name);
- return -EINVAL;
+ if (property == colorop->curve_1d_type_property) {
+ state->curve_1d_type = val;
+ } else {
+ drm_dbg_atomic(colorop->dev,
+ "[COLOROP:%d:%d] unknown property
[PROP:%d:%s]]\n",
+ colorop->base.id, colorop->type,
+ property->base.id, property->name);
+ return -EINVAL;
+ }
+
+ return 0;
}
static int
@@ -662,6 +668,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
{
if (property == colorop->type_property) {
*val = colorop->type;
+ } else if (property == colorop->curve_1d_type_property) {
+ *val = state->curve_1d_type;
} else {
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index c028d5426d42..f665a12a214e 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -36,6 +36,11 @@ static const struct drm_prop_enum_list
drm_colorop_type_enum_list[] = {
{ DRM_COLOROP_1D_CURVE, "1D Curve" },
};
+static const struct drm_prop_enum_list drm_colorop_curve_1d_type_enum_list[] =
{
+ { DRM_COLOROP_1D_CURVE_SRGB_EOTF, "sRGB EOTF" },
+ { DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF, "sRGB Inverse EOTF" },
+};
+
/* Init Helpers */
int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -73,6 +78,20 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->type_property,
colorop->type);
+ /* curve_1d_type */
+ /* TODO move to mode_config? */
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
+ "CURVE_1D_TYPE",
+ drm_colorop_curve_1d_type_enum_list,
+
ARRAY_SIZE(drm_colorop_curve_1d_type_enum_list));
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->curve_1d_type_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->curve_1d_type_property,
+ 0);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -194,6 +213,11 @@ static const char * const colorop_type_name[] = {
[DRM_COLOROP_1D_CURVE] = "1D Curve",
};
+static const char * const colorop_curve_1d_type_name[] = {
+ [DRM_COLOROP_1D_CURVE_SRGB_EOTF] = "sRGB EOTF",
+ [DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF] = "sRGB Inverse EOTF",
+};
+
/**
* drm_get_colorop_type_name - return a string for colorop type
* @range: colorop type to compute name of
@@ -209,3 +233,18 @@ const char *drm_get_colorop_type_name(enum
drm_colorop_type type)
return colorop_type_name[type];
}
+/**
+ * drm_get_colorop_curve_1d_type_name - return a string for 1D curve type
+ * @range: 1d curve type to compute name of
+ *
+ * In contrast to the other drm_get_*_name functions this one here returns a
+ * const pointer and hence is threadsafe.
+ */
+const char *drm_get_colorop_curve_1d_type_name(enum drm_colorop_curve_1d_type
type)
+{
+ if (WARN_ON(type >= ARRAY_SIZE(colorop_curve_1d_type_name)))
+ return "unknown";
+
+ return colorop_curve_1d_type_name[type];
+}
+
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 22a217372428..7701b61ff7e9 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -34,6 +34,11 @@ enum drm_colorop_type {
DRM_COLOROP_1D_CURVE
};
+enum drm_colorop_curve_1d_type {
+ DRM_COLOROP_1D_CURVE_SRGB_EOTF
[RFC PATCH 05/10] drm/colorop: Add BYPASS property
We want to be able to bypass each colorop at all times.
Introduce a new BYPASS boolean property for this.
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_atomic_uapi.c | 6 +-
drivers/gpu/drm/drm_colorop.c | 15 +++
include/drm/drm_colorop.h | 20
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 9b01f234b04e..ca3512038d4c 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -648,7 +648,9 @@ static int drm_atomic_colorop_set_property(struct
drm_colorop *colorop,
struct drm_colorop_state *state, struct drm_file *file_priv,
struct drm_property *property, uint64_t val)
{
- if (property == colorop->curve_1d_type_property) {
+ if (property == colorop->bypass_property) {
+ state->bypass = val;
+ } else if (property == colorop->curve_1d_type_property) {
state->curve_1d_type = val;
} else {
drm_dbg_atomic(colorop->dev,
@@ -668,6 +670,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
{
if (property == colorop->type_property) {
*val = colorop->type;
+ } else if (property == colorop->bypass_property) {
+ *val = state->bypass;
} else if (property == colorop->curve_1d_type_property) {
*val = state->curve_1d_type;
} else {
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index f665a12a214e..409df022b256 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -78,6 +78,18 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->type_property,
colorop->type);
+ /* bypass */
+ /* TODO can we reuse the mode_config->active_prop? */
+ prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
+ "BYPASS");
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->bypass_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->bypass_property,
+ 1);
+
/* curve_1d_type */
/* TODO move to mode_config? */
prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
@@ -100,6 +112,8 @@ void __drm_atomic_helper_colorop_duplicate_state(struct
drm_colorop *colorop,
struct drm_colorop_state
*state)
{
memcpy(state, colorop->state, sizeof(*state));
+
+ state->bypass = true;
}
struct drm_colorop_state *
@@ -164,6 +178,7 @@ void __drm_colorop_state_reset(struct drm_colorop_state
*colorop_state,
struct drm_colorop *colorop)
{
colorop_state->colorop = colorop;
+ colorop_state->bypass = true;
}
EXPORT_SYMBOL(__drm_colorop_state_reset);
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 7701b61ff7e9..69636f6752a0 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -48,6 +48,14 @@ struct drm_colorop_state {
/* colorop properties */
+ /**
+* @bypass:
+*
+* True if colorop shall be bypassed. False if colorop is
+* enabled.
+*/
+ bool bypass;
+
/**
* @curve_1d_type:
*
@@ -135,6 +143,18 @@ struct drm_colorop {
*/
struct drm_property *type_property;
+ /**
+* @bypass_property:
+*
+* Boolean property to control enablement of the color
+* operation. Setting bypass to "true" shall always be supported
+* in order to allow compositors to quickly fall back to
+* alternate methods of color processing. This is important
+* since setting color operations can fail due to unique
+* HW constraints.
+*/
+ struct drm_property *bypass_property;
+
/**
* @curve_1d_type:
*
--
2.42.0
[RFC PATCH 06/10] drm/colorop: Add NEXT property
We'll construct color pipelines out of drm_colorop by
chaining them via the NEXT pointer. NEXT will point to
the next drm_colorop in the pipeline, or by 0 if we're
at the end of the pipeline.
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_colorop.c | 27 +++
include/drm/drm_colorop.h | 12
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 409df022b256..a92e170aed87 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -104,6 +104,15 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->curve_1d_type_property,
0);
+ prop = drm_property_create_object(dev, DRM_MODE_PROP_IMMUTABLE |
DRM_MODE_PROP_ATOMIC,
+ "NEXT", DRM_MODE_OBJECT_COLOROP);
+ if (!prop)
+ return -ENOMEM;
+ colorop->next_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->next_property,
+ 0);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -263,3 +272,21 @@ const char *drm_get_colorop_curve_1d_type_name(enum
drm_colorop_curve_1d_type ty
return colorop_curve_1d_type_name[type];
}
+
+/**
+ * drm_colorop_set_next_property - sets the next pointer
+ * @colorop: drm colorop
+ * @next: next colorop
+ *
+ * Should be used when constructing the color pipeline
+ */
+void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next)
+{
+ if (!colorop->next_property)
+ return;
+
+ drm_object_property_set_value(&colorop->base,
+ colorop->next_property,
+ next->base.id);
+}
+EXPORT_SYMBOL(drm_colorop_set_next_property);
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 69636f6752a0..1ddd0e65fe36 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -162,10 +162,20 @@ struct drm_colorop {
*/
struct drm_property *curve_1d_type_property;
+ /**
+* @next_property
+*
+* Read-only property to next colorop in the pipeline
+*/
+ struct drm_property *next_property;
+
};
#define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
+
+
+
/**
* drm_crtc_find - look up a Colorop object from its ID
* @dev: DRM device
@@ -212,5 +222,7 @@ static inline unsigned int drm_colorop_index(const struct
drm_colorop *colorop)
#define drm_for_each_colorop(colorop, dev) \
list_for_each_entry(colorop, &(dev)->mode_config.colorop_list, head)
+void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next);
+
#endif /* __DRM_COLOROP_H__ */
--
2.42.0
[RFC PATCH 08/10] drm/colorop: Add new IOCTLs to retrieve drm_colorop objects
Since we created a new DRM object we need new IOCTLs (and
new libdrm functions) to retrieve those objects.
TODO: Can we make these IOCTLs and libdrm functions generic
to allow for new DRM objects in the future without the need
for new IOCTLs and libdrm functions?
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_colorop.c | 51 +
drivers/gpu/drm/drm_crtc_internal.h | 4 +++
drivers/gpu/drm/drm_ioctl.c | 5 +++
include/uapi/drm/drm_mode.h | 21
4 files changed, 81 insertions(+)
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index a92e170aed87..fb85b5c41cc4 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -32,6 +32,57 @@
/* TODO big colorop doc, including properties, etc. */
+/* IOCTLs */
+
+int drm_mode_getcolorop_res(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_colorop_res *colorop_resp = data;
+ struct drm_colorop *colorop;
+ uint32_t __user *colorop_ptr;
+ int count = 0;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EOPNOTSUPP;
+
+ colorop_ptr = u64_to_user_ptr(colorop_resp->colorop_id_ptr);
+
+ /*
+* This ioctl is called twice, once to determine how much space is
+* needed, and the 2nd time to fill it.
+*/
+ drm_for_each_colorop(colorop, dev) {
+ if (drm_lease_held(file_priv, colorop->base.id)) {
+ if (count < colorop_resp->count_colorops &&
+ put_user(colorop->base.id, colorop_ptr + count))
+ return -EFAULT;
+ count++;
+ }
+ }
+ colorop_resp->count_colorops = count;
+
+ return 0;
+}
+
+int drm_mode_getcolorop(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_colorop *colorop_resp = data;
+ struct drm_colorop *colorop;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EOPNOTSUPP;
+
+ colorop = drm_colorop_find(dev, file_priv, colorop_resp->colorop_id);
+ if (!colorop)
+ return -ENOENT;
+
+ colorop_resp->colorop_id = colorop->base.id;
+ colorop_resp->plane_id = colorop->plane ? colorop->plane->base.id : 0;
+
+ return 0;
+}
+
static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
{ DRM_COLOROP_1D_CURVE, "1D Curve" },
};
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 501a10edd0e1..b68e05c2cf57 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -278,6 +278,10 @@ int drm_mode_getplane(struct drm_device *dev,
void *data, struct drm_file *file_priv);
int drm_mode_setplane(struct drm_device *dev,
void *data, struct drm_file *file_priv);
+int drm_mode_getcolorop_res(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+int drm_mode_getcolorop(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
int drm_mode_cursor_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
int drm_mode_cursor2_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7c9d66ee917d..a3c137ac88c6 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -716,6 +716,11 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl,
DRM_MASTER),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl,
DRM_MASTER),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl,
DRM_MASTER),
+
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCOLOROPRESOURCES,
drm_mode_getcolorop_res, 0),
+ /* TODO do we need GETCOLOROP? */
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCOLOROP, drm_mode_getcolorop, 0),
+
};
#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE(drm_ioctls)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 6dcf628def56..9e37eec55291 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -357,6 +357,27 @@ struct drm_mode_get_plane {
__u64 format_type_ptr;
};
+struct drm_mode_get_colorop_res {
+ __u64 colorop_id_ptr;
+ __u32 count_colorops;
+};
+
+
+/**
+ * struct drm_mode_get_colorop -
[RFC PATCH 07/10] drm/colorop: Add atomic state print for drm_colorop
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_atomic.c | 29 +
include/drm/drm_colorop.h| 5 +
2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8a5f8cd22c8d..30308b8dec53 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -783,6 +783,19 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
return 0;
}
+
+
+static void drm_atomic_colorop_print_state(struct drm_printer *p,
+ const struct drm_colorop_state *state)
+{
+ struct drm_colorop *colorop = state->colorop;
+
+ drm_printf(p, "colorop[%u]:\n", colorop->base.id);
+ drm_printf(p, "\ttype=%s\n", drm_get_colorop_type_name(colorop->type));
+ drm_printf(p, "\tbypass=%u\n", state->bypass);
+ drm_printf(p, "\tcurve_1d_type=%s\n",
drm_get_colorop_curve_1d_type_name(state->curve_1d_type));
+}
+
static void drm_atomic_plane_print_state(struct drm_printer *p,
const struct drm_plane_state *state)
{
@@ -803,6 +816,13 @@ static void drm_atomic_plane_print_state(struct
drm_printer *p,
drm_get_color_encoding_name(state->color_encoding));
drm_printf(p, "\tcolor-range=%s\n",
drm_get_color_range_name(state->color_range));
+#if 0
+ drm_printf(p, "\tcolor-pipeline=%s\n",
+ drm_get_color_pipeline_name(state->color_pipeline));
+#else
+ drm_printf(p, "\tcolor-pipeline=%d\n",
+ state->color_pipeline ? state->color_pipeline->base.id : 0);
+#endif
if (plane->funcs->atomic_print_state)
plane->funcs->atomic_print_state(p, state);
@@ -1779,6 +1799,7 @@ static void __drm_state_dump(struct drm_device *dev,
struct drm_printer *p,
bool take_locks)
{
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_colorop *colorop;
struct drm_plane *plane;
struct drm_crtc *crtc;
struct drm_connector *connector;
@@ -1787,6 +1808,14 @@ static void __drm_state_dump(struct drm_device *dev,
struct drm_printer *p,
if (!drm_drv_uses_atomic_modeset(dev))
return;
+ list_for_each_entry(colorop, &config->colorop_list, head) {
+ if (take_locks)
+ drm_modeset_lock(&colorop->plane->mutex, NULL);
+ drm_atomic_colorop_print_state(p, colorop->state);
+ if (take_locks)
+ drm_modeset_unlock(&colorop->plane->mutex);
+ }
+
list_for_each_entry(plane, &config->plane_list, head) {
if (take_locks)
drm_modeset_lock(&plane->mutex, NULL);
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 1ddd0e65fe36..622a671d2458 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -222,6 +222,11 @@ static inline unsigned int drm_colorop_index(const struct
drm_colorop *colorop)
#define drm_for_each_colorop(colorop, dev) \
list_for_each_entry(colorop, &(dev)->mode_config.colorop_list, head)
+const char *drm_get_color_pipeline_name(struct drm_colorop *colorop);
+
+const char *drm_get_colorop_type_name(enum drm_colorop_type type);
+const char *drm_get_colorop_curve_1d_type_name(enum drm_colorop_curve_1d_type
type);
+
void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next);
--
2.42.0
[RFC PATCH 09/10] drm/plane: Add COLOR PIPELINE property
We're adding a new enum COLOR PIPELINE property. This
property will have entries for each COLOR PIPELINE by
referencing the DRM object ID of the first drm_colorop
of the pipeline. 0 disables the entire COLOR PIPELINE.
Userspace can use this to discover the available color
pipelines, as well as set the desired one. The color
pipelines are programmed via properties on the actual
drm_colorop objects.
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/drm_atomic.c | 46 +++
drivers/gpu/drm/drm_atomic_state_helper.c | 5 +++
drivers/gpu/drm/drm_atomic_uapi.c | 44 ++
include/drm/drm_atomic_uapi.h | 2 +
include/drm/drm_plane.h | 8
5 files changed, 105 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30308b8dec53..a8b978e8f3eb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1403,6 +1403,52 @@ drm_atomic_add_affected_planes(struct drm_atomic_state
*state,
}
EXPORT_SYMBOL(drm_atomic_add_affected_planes);
+/**
+ * drm_atomic_add_affected_colorops - add colorops for plane
+ * @state: atomic state
+ * @plane: DRM plane
+ *
+ * This function walks the current configuration and adds all colorops
+ * currently used by @plane to the atomic configuration @state. This is useful
+ * when an atomic commit also needs to check all currently enabled colorop on
+ * @plane, e.g. when changing the mode. It's also useful when re-enabling a
plane
+ * to avoid special code to force-enable all colorops.
+ *
+ * Since acquiring a colorop state will always also acquire the w/w mutex of
the
+ * current plane for that colorop (if there is any) adding all the colorop
states for
+ * a plane will not reduce parallelism of atomic updates.
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
+ * then the w/w mutex code has detected a deadlock and the entire atomic
+ * sequence must be restarted. All other errors are fatal.
+ */
+int
+drm_atomic_add_affected_colorops(struct drm_atomic_state *state,
+struct drm_plane *plane)
+{
+ struct drm_colorop *colorop;
+ struct drm_colorop_state *colorop_state;
+
+ WARN_ON(!drm_atomic_get_new_plane_state(state, plane));
+
+ drm_dbg_atomic(plane->dev,
+ "Adding all current colorops for [plane:%d:%s] to %p\n",
+ plane->base.id, plane->name, state);
+
+ drm_for_each_colorop(colorop, plane->dev) {
+ if (colorop->plane != plane)
+ continue;
+
+ colorop_state = drm_atomic_get_colorop_state(state, colorop);
+ if (IS_ERR(colorop_state))
+ return PTR_ERR(colorop_state);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_add_affected_colorops);
+
/**
* drm_atomic_check_only - check whether a given config would work
* @state: atomic configuration to check
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..3c5f2c8e33d0 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -267,6 +267,11 @@ void __drm_atomic_helper_plane_state_reset(struct
drm_plane_state *plane_state,
plane_state->color_range = val;
}
+ if (plane->color_pipeline_property) {
+ /* default is always NULL, i.e., bypass */
+ plane_state->color_pipeline = NULL;
+ }
+
if (plane->zpos_property) {
if (!drm_object_property_get_default_value(&plane->base,
plane->zpos_property,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index ca3512038d4c..44ceb10acb6f 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -256,6 +256,38 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state
*plane_state,
}
EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
+
+/**
+ * drm_atomic_set_colorop_for_plane - set colorop for plane
+ * @plane_state: atomic state object for the plane
+ * @colorop: colorop to use for the plane
+ *
+ * Changing the assigned framebuffer for a plane requires us to grab a
reference
+ * to the new fb and drop the reference to the old fb, if there is one. This
+ * function takes care of all these details besides updating the pointer in the
+ * state object itself.
+ */
+void
+drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
+
[RFC PATCH 10/10] drm/vkms: Add enumerated 1D curve colorop
Signed-off-by: Harry Wentland
Cc: Ville Syrjala
Cc: Pekka Paalanen
Cc: Simon Ser
Cc: Harry Wentland
Cc: Melissa Wen
Cc: Jonas Ådahl
Cc: Sebastian Wick
Cc: Shashank Sharma
Cc: Alexander Goins
Cc: Joshua Ashton
Cc: Michel Dänzer
Cc: Aleix Pol
Cc: Xaver Hugl
Cc: Victoria Brekenfeld
Cc: Daniel Vetter
Cc: Uma Shankar
Cc: Naseer Ahmed
Cc: Christopher Braga
---
drivers/gpu/drm/vkms/Makefile| 3 +-
drivers/gpu/drm/vkms/vkms_colorop.c | 108 +
drivers/gpu/drm/vkms/vkms_composer.c | 316 +++
drivers/gpu/drm/vkms/vkms_drv.h | 4 +
drivers/gpu/drm/vkms/vkms_plane.c| 2 +
5 files changed, 432 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 1b28a6a32948..bcf508873622 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -6,6 +6,7 @@ vkms-y := \
vkms_formats.o \
vkms_crtc.o \
vkms_composer.o \
- vkms_writeback.o
+ vkms_writeback.o \
+ vkms_colorop.o
obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
b/drivers/gpu/drm/vkms/vkms_colorop.c
new file mode 100644
index ..b3da0705bca7
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: AMD
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_COLOR_PIPELINES 5
+
+const int vkms_initialize_tf_pipeline(struct drm_plane *plane, struct
drm_prop_enum_list *list)
+{
+
+ struct drm_colorop *op, *prev_op;
+ struct drm_device *dev = plane->dev;
+ int ret;
+
+ /* 1st op: 1d curve */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_1D_CURVE);
+ if (ret)
+ return ret;
+
+ list->type = op->base.id;
+ list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", op->base.id);
+
+ prev_op = op;
+
+ /* 2nd op: 1d curve */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_1D_CURVE);
+ if (ret)
+ return ret;
+
+ drm_colorop_set_next_property(prev_op, op);
+
+ return 0;
+}
+
+int vkms_initialize_colorops(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_property *prop;
+ struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
+ int len = 0;
+ int ret;
+
+ /* Add "Bypass" (i.e. NULL) pipeline */
+ pipelines[len].type = 0;
+ pipelines[len].name = "Bypass";
+ len++;
+
+ /* Add pipeline consisting of transfer functions */
+ ret = vkms_initialize_tf_pipeline(plane, &(pipelines[len]));
+ if (ret)
+ return ret;
+ len++;
+
+ /* Create COLOR_PIPELINE property and attach */
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
+ "COLOR_PIPELINE",
+ pipelines, len);
+ if (!prop)
+ return -ENOMEM;
+
+ plane->color_pipeline_property = prop;
+
+ drm_object_attach_property(&plane->base, prop, 0);
+
+ /* TODO do we even need this? */
+ if (plane->state)
+ plane->state->color_pipeline = NULL;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index f6c311e8a87c..92ab9c62a554
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
Hey Harry, Thank you and Simon for this great document. Really happy about it, but obviously I've got a few notes and questions inline. On Fri, Sep 08, 2023 at 11:02:26AM -0400, Harry Wentland wrote: > Signed-off-by: Harry Wentland > Cc: Ville Syrjala > Cc: Pekka Paalanen > Cc: Simon Ser > Cc: Harry Wentland > Cc: Melissa Wen > Cc: Jonas Ådahl > Cc: Sebastian Wick > Cc: Shashank Sharma > Cc: Alexander Goins > Cc: Joshua Ashton > Cc: Michel Dänzer > Cc: Aleix Pol > Cc: Xaver Hugl > Cc: Victoria Brekenfeld > Cc: Daniel Vetter > Cc: Uma Shankar > Cc: Naseer Ahmed > Cc: Christopher Braga > --- > Documentation/gpu/rfc/color_pipeline.rst | 278 +++ > 1 file changed, 278 insertions(+) > create mode 100644 Documentation/gpu/rfc/color_pipeline.rst > > diff --git a/Documentation/gpu/rfc/color_pipeline.rst > b/Documentation/gpu/rfc/color_pipeline.rst > new file mode 100644 > index ..bfa4a8f12087 > --- /dev/null > +++ b/Documentation/gpu/rfc/color_pipeline.rst > @@ -0,0 +1,278 @@ > + > +Linux Color Pipeline API > + > + > +What problem are we solving? > + > + > +We would like to support pre-, and post-blending complex color > transformations > +in order to allow for HW-supported HDR use-cases, as well as to provide > support > +to color-managed applications, such as video or image editors. > + > +While it is possible to support an HDR output on HW supporting the Colorspace > +and HDR Metadata drm_connector properties that requires the compositor or > +application to render and compose the content into one final buffer intended > for > +display. Doing so is costly. > + > +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and other > +operations to support color transformations. These operations are often > +implemented in fixed-function HW and therefore much more power efficient than > +performing similar operations via shaders or CPU. > + > +We would like to make use of this HW functionality to support complex color > +transformations with no, or minimal CPU or shader load. > + > + > +How are other OSes solving this problem? > + > + > +The most widely supported use-cases regard HDR content, whether video or > +gaming. > + > +Most OSes will specify the source content format (color gamut, encoding > transfer > +function, and other metadata, such as max and average light levels) to a > driver. > +Drivers will then program their fixed-function HW accordingly to map from a > +source content buffer's space to a display's space. > + > +When fixed-function HW is not available the compositor will assemble a > shader to > +ask the GPU to perform the transformation from the source content format to > the > +display's format. > + > +A compositor's mapping function and a driver's mapping function are usually > +entirely separate concepts. On OSes where a HW vendor has no insight into > +closed-source compositor code such a vendor will tune their color management > +code to visually match the compositor's. On other OSes, where both mapping > +functions are open to an implementer they will ensure both mappings match. > + > + > +Why is Linux different? > +=== > + > +Unlike other OSes, where there is one compositor for one or more drivers, on > +Linux we have a many-to-many relationship. Many compositors; many drivers. > +In addition each compositor vendor or community has their own view of how > +color management should be done. This is what makes Linux so beautiful. > + > +This means that a HW vendor can now no longer tune their driver to one > +compositor, as tuning it to one will almost inevitably make it look very > +different from another compositor's color mapping. > + > +We need a better solution. > + > + > +Descriptive API > +=== > + > +An API that describes the source and destination colorspaces is a descriptive > +API. It describes the input and output color spaces but does not describe > +how precisely they should be mapped. Such a mapping includes many minute > +design decision that can greatly affect the look of the final result. > + > +It is not feasible to describe such mapping with enough detail to ensure the > +same result from each implementation. In fact, these mappings are a very > active > +research area. > + > + > +Prescriptive API > + > + > +A prescriptive API describes not the source and destination colorspaces. It > +instead prescribes a recipe for how to manipulate pixel values to arrive at > the > +desired outcome. > + > +This recipe is generally an order straight-forward operations, with clear > +mathematical definitions, such as 1D LUTs, 3D LUTs, matrices, or other > +operations that can be described in a precise manner. > + > + > +The Color Pipeline API > +== > + > +HW color management pipelines can significantly differ between HW > +vendors in
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On 2023-09-08 15:30, Sebastian Wick wrote: Hey Harry, Thank you and Simon for this great document. Really happy about it, but obviously I've got a few notes and questions inline. On Fri, Sep 08, 2023 at 11:02:26AM -0400, Harry Wentland wrote: Signed-off-by: Harry Wentland Cc: Ville Syrjala Cc: Pekka Paalanen Cc: Simon Ser Cc: Harry Wentland Cc: Melissa Wen Cc: Jonas Ådahl Cc: Sebastian Wick Cc: Shashank Sharma Cc: Alexander Goins Cc: Joshua Ashton Cc: Michel Dänzer Cc: Aleix Pol Cc: Xaver Hugl Cc: Victoria Brekenfeld Cc: Daniel Vetter Cc: Uma Shankar Cc: Naseer Ahmed Cc: Christopher Braga --- Documentation/gpu/rfc/color_pipeline.rst | 278 +++ 1 file changed, 278 insertions(+) create mode 100644 Documentation/gpu/rfc/color_pipeline.rst diff --git a/Documentation/gpu/rfc/color_pipeline.rst b/Documentation/gpu/rfc/color_pipeline.rst new file mode 100644 index ..bfa4a8f12087 --- /dev/null +++ b/Documentation/gpu/rfc/color_pipeline.rst @@ -0,0 +1,278 @@ + +Linux Color Pipeline API + + +What problem are we solving? + + +We would like to support pre-, and post-blending complex color transformations +in order to allow for HW-supported HDR use-cases, as well as to provide support +to color-managed applications, such as video or image editors. + +While it is possible to support an HDR output on HW supporting the Colorspace +and HDR Metadata drm_connector properties that requires the compositor or +application to render and compose the content into one final buffer intended for +display. Doing so is costly. + +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and other +operations to support color transformations. These operations are often +implemented in fixed-function HW and therefore much more power efficient than +performing similar operations via shaders or CPU. + +We would like to make use of this HW functionality to support complex color +transformations with no, or minimal CPU or shader load. + + +How are other OSes solving this problem? + + +The most widely supported use-cases regard HDR content, whether video or +gaming. + +Most OSes will specify the source content format (color gamut, encoding transfer +function, and other metadata, such as max and average light levels) to a driver. +Drivers will then program their fixed-function HW accordingly to map from a +source content buffer's space to a display's space. + +When fixed-function HW is not available the compositor will assemble a shader to +ask the GPU to perform the transformation from the source content format to the +display's format. + +A compositor's mapping function and a driver's mapping function are usually +entirely separate concepts. On OSes where a HW vendor has no insight into +closed-source compositor code such a vendor will tune their color management +code to visually match the compositor's. On other OSes, where both mapping +functions are open to an implementer they will ensure both mappings match. + + +Why is Linux different? +=== + +Unlike other OSes, where there is one compositor for one or more drivers, on +Linux we have a many-to-many relationship. Many compositors; many drivers. +In addition each compositor vendor or community has their own view of how +color management should be done. This is what makes Linux so beautiful. + +This means that a HW vendor can now no longer tune their driver to one +compositor, as tuning it to one will almost inevitably make it look very +different from another compositor's color mapping. + +We need a better solution. + + +Descriptive API +=== + +An API that describes the source and destination colorspaces is a descriptive +API. It describes the input and output color spaces but does not describe +how precisely they should be mapped. Such a mapping includes many minute +design decision that can greatly affect the look of the final result. + +It is not feasible to describe such mapping with enough detail to ensure the +same result from each implementation. In fact, these mappings are a very active +research area. + + +Prescriptive API + + +A prescriptive API describes not the source and destination colorspaces. It +instead prescribes a recipe for how to manipulate pixel values to arrive at the +desired outcome. + +This recipe is generally an order straight-forward operations, with clear +mathematical definitions, such as 1D LUTs, 3D LUTs, matrices, or other +operations that can be described in a precise manner. + + +The Color Pipeline API +== + +HW color management pipelines can significantly differ between HW +vendors in terms of availability, ordering, and capabilities of HW +blocks. This makes a common definition of color management blocks and +their ordering nigh impossible. Instead we are defining an API that +allows user space t
