This is how a typical display color hardware pipeline looks like: +-------------------------------------------+ | RAM | | +------+ +---------+ +---------+ | | | FB 1 | | FB 2 | | FB N | | | +------+ +---------+ +---------+ | +-------------------------------------------+ | Plane Color Hardware Block | +--------------------------------------------+ | +---v-----+ +---v-------+ +---v------+ | | | Plane A | | Plane B | | Plane N | | | | DeGamma | | Degamma | | Degamma | | | +---+-----+ +---+-------+ +---+------+ | | | | | | | +---v-----+ +---v-------+ +---v------+ | | |Plane A | | Plane B | | Plane N | | | |CSC/CTM | | CSC/CTM | | CSC/CTM | | | +---+-----+ +----+------+ +----+-----+ | | | | | | | +---v-----+ +----v------+ +----v-----+ | | | Plane A | | Plane B | | Plane N | | | | Gamma | | Gamma | | Gamma | | | +---+-----+ +----+------+ +----+-----+ | | | | | | +--------------------------------------------+ +------v--------------v---------------v-------| || || || Pipe Blender || +--------------------+------------------------+ | | | | +-----------v----------+ | | | Pipe DeGamma | | | | | | | +-----------+----------+ | | | Pipe Color | | +-----------v----------+ Hardware | | | Pipe CSC/CTM | | | | | | | +-----------+----------+ | | | | | +-----------v----------+ | | | Pipe Gamma | | | | | | | +-----------+----------+ | | | | +---------------------------------------------+ | v Pipe Output
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
Userspace can take smart blending decisions and utilize these hardware supported plane color features to get accurate color profile. The same can help in consistent color quality from source to panel taking advantage of advanced color features in hardware.
These patches add the property interfaces and enable helper functions. This series adds Intel's XE_LPD hw specific plane gamma feature. We can build up and add other platform/hardware specific implementation on top of this series.
Credits: Special mention and credits to Ville Syrjala for coming up with a design for this feature and inputs. This series is based on his original design and idea.
Note: Userspace support for this new UAPI will be done on Chrome. We will notify the list once we have that ready for review.
ToDo: State readout for this feature will be added next.
Uma Shankar (21): drm: Add Enhanced Gamma and color lut range attributes drm: Add Plane Degamma Mode property drm: Add Plane Degamma Lut property drm/i915/xelpd: Define Degamma Lut range struct for HDR planes drm/i915/xelpd: Add register definitions for Plane Degamma drm/i915/xelpd: Enable plane color features drm/i915/xelpd: Add color capabilities of SDR planes drm/i915/xelpd: Program Plane Degamma Registers drm/i915/xelpd: Add plane color check to glk_plane_color_ctl drm/i915/xelpd: Initialize plane color features drm/i915/xelpd: Load plane color luts from atomic flip drm: Add Plane CTM property drm: Add helper to attach Plane ctm property drm/i915/xelpd: Define Plane CSC Registers drm/i915/xelpd: Enable Plane CSC drm: Add Plane Gamma Mode property drm: Add Plane Gamma Lut property drm/i915/xelpd: Define and Initialize Plane Gamma Lut range drm/i915/xelpd: Add register definitions for Plane Gamma drm/i915/xelpd: Program Plane Gamma Registers drm/i915/xelpd: Enable plane gamma
Documentation/gpu/drm-kms.rst | 90 +++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 12 + drivers/gpu/drm/drm_atomic_uapi.c | 38 ++ drivers/gpu/drm/drm_color_mgmt.c | 177 +++++- .../gpu/drm/i915/display/intel_atomic_plane.c | 6 + .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_color.c | 513 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_color.h | 2 + .../drm/i915/display/skl_universal_plane.c | 15 +- drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 176 +++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 81 +++ include/uapi/drm/drm_mode.h | 58 ++ 15 files changed, 1170 insertions(+), 6 deletions(-)
Existing LUT precision structure is having only 16 bit precision. This is not enough for upcoming enhanced hardwares and advance usecases like HDR processing. Hence added a new structure with 32 bit precision values.
This also defines a new structure to define color lut ranges, along with related macro definitions and enums. This will help describe multi segmented lut ranges in the hardware.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -819,6 +819,64 @@ struct hdr_output_metadata { }; };
+/* + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT + * can be used for either purpose, but not simultaneously. To expose + * modes that support gamma and degamma simultaneously the gamma mode + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA + * ranges. + */ +/* LUT is for gamma (after CTM) */ +#define DRM_MODE_LUT_GAMMA BIT(0) +/* LUT is for degamma (before CTM) */ +#define DRM_MODE_LUT_DEGAMMA BIT(1) +/* linearly interpolate between the points */ +#define DRM_MODE_LUT_INTERPOLATE BIT(2) +/* + * the last value of the previous range is the + * first value of the current range. + */ +#define DRM_MODE_LUT_REUSE_LAST BIT(3) +/* the curve must be non-decreasing */ +#define DRM_MODE_LUT_NON_DECREASING BIT(4) +/* the curve is reflected across origin for negative inputs */ +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) +/* the same curve (red) is used for blue and green channels as well */ +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6) + +struct drm_color_lut_range { + /* DRM_MODE_LUT_* */ + __u32 flags; + /* number of points on the curve */ + __u16 count; + /* input/output bits per component */ + __u8 input_bpc, output_bpc; + /* input start/end values */ + __s32 start, end; + /* output min/max values */ + __s32 min, max; +}; + +enum lut_type { + LUT_TYPE_DEGAMMA = 0, + LUT_TYPE_GAMMA = 1, +}; + +/* + * Creating 64 bit palette entries for better data + * precision. This will be required for HDR and + * similar color processing usecases. + */ +struct drm_color_lut_ext { + /* + * Data is U32.32 fixed point format. + */ + __u64 red; + __u64 green; + __u64 blue; + __u64 reserved; +}; + #define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
On Tue, 1 Jun 2021 16:21:58 +0530 Uma Shankar uma.shankar@intel.com wrote:
Existing LUT precision structure is having only 16 bit precision. This is not enough for upcoming enhanced hardwares and advance usecases like HDR processing. Hence added a new structure with 32 bit precision values.
This also defines a new structure to define color lut ranges, along with related macro definitions and enums. This will help describe multi segmented lut ranges in the hardware.
Signed-off-by: Uma Shankar uma.shankar@intel.com
include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -819,6 +819,64 @@ struct hdr_output_metadata { }; };
+/*
- DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
- can be used for either purpose, but not simultaneously. To expose
- modes that support gamma and degamma simultaneously the gamma mode
- must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
- ranges.
- */
+/* LUT is for gamma (after CTM) */ +#define DRM_MODE_LUT_GAMMA BIT(0) +/* LUT is for degamma (before CTM) */ +#define DRM_MODE_LUT_DEGAMMA BIT(1) +/* linearly interpolate between the points */ +#define DRM_MODE_LUT_INTERPOLATE BIT(2) +/*
- the last value of the previous range is the
- first value of the current range.
- */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3) +/* the curve must be non-decreasing */ +#define DRM_MODE_LUT_NON_DECREASING BIT(4) +/* the curve is reflected across origin for negative inputs */ +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) +/* the same curve (red) is used for blue and green channels as well */ +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+struct drm_color_lut_range {
- /* DRM_MODE_LUT_* */
- __u32 flags;
- /* number of points on the curve */
- __u16 count;
- /* input/output bits per component */
- __u8 input_bpc, output_bpc;
- /* input start/end values */
- __s32 start, end;
- /* output min/max values */
- __s32 min, max;
+};
+enum lut_type {
Unprefixed type name in UAPI headers is probably not a good idea.
- LUT_TYPE_DEGAMMA = 0,
- LUT_TYPE_GAMMA = 1,
+};
All the above stuff seems to be the same in your other patch series' patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what does this mean?
Thanks, pq
+/*
- Creating 64 bit palette entries for better data
- precision. This will be required for HDR and
- similar color processing usecases.
- */
+struct drm_color_lut_ext {
- /*
* Data is U32.32 fixed point format.*/- __u64 red;
- __u64 green;
- __u64 blue;
- __u64 reserved;
+};
#define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 3:04 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
On Tue, 1 Jun 2021 16:21:58 +0530 Uma Shankar uma.shankar@intel.com wrote:
Existing LUT precision structure is having only 16 bit precision. This is not enough for upcoming enhanced hardwares and advance usecases like HDR processing. Hence added a new structure with 32 bit precision values.
This also defines a new structure to define color lut ranges, along with related macro definitions and enums. This will help describe multi segmented lut ranges in the hardware.
Signed-off-by: Uma Shankar uma.shankar@intel.com
include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -819,6 +819,64 @@ struct hdr_output_metadata { }; };
+/*
- DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means
the LUT
- can be used for either purpose, but not simultaneously. To expose
- modes that support gamma and degamma simultaneously the gamma mode
- must declare distinct DRM_MODE_LUT_GAMMA and
DRM_MODE_LUT_DEGAMMA
- ranges.
- */
+/* LUT is for gamma (after CTM) */ +#define DRM_MODE_LUT_GAMMA BIT(0) +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA +BIT(1) +/* linearly interpolate between the points */ #define +DRM_MODE_LUT_INTERPOLATE BIT(2) +/*
- the last value of the previous range is the
- first value of the current range.
- */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3) +/* the curve must be non-decreasing */ #define +DRM_MODE_LUT_NON_DECREASING BIT(4) +/* the curve is reflected across origin for negative inputs */ +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) +/* the same curve (red) is used for blue and green channels as well +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+struct drm_color_lut_range {
- /* DRM_MODE_LUT_* */
- __u32 flags;
- /* number of points on the curve */
- __u16 count;
- /* input/output bits per component */
- __u8 input_bpc, output_bpc;
- /* input start/end values */
- __s32 start, end;
- /* output min/max values */
- __s32 min, max;
+};
+enum lut_type {
Unprefixed type name in UAPI headers is probably not a good idea.
Ok, will rename these.
- LUT_TYPE_DEGAMMA = 0,
- LUT_TYPE_GAMMA = 1,
+};
All the above stuff seems to be the same in your other patch series' patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what does this mean?
The concept and idea is similar and the range definition is also common. But this series focuses on plane color management while the other one is for pipe/crtc color features. Hence separated and floated them as unique series for review.
Regards, Uma Shankar
Thanks, pq
+/*
- Creating 64 bit palette entries for better data
- precision. This will be required for HDR and
- similar color processing usecases.
- */
+struct drm_color_lut_ext {
- /*
* Data is U32.32 fixed point format.*/- __u64 red;
- __u64 green;
- __u64 blue;
- __u64 reserved;
+};
#define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
On 2021-06-02 4:26 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 3:04 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
On Tue, 1 Jun 2021 16:21:58 +0530 Uma Shankar uma.shankar@intel.com wrote:
Existing LUT precision structure is having only 16 bit precision. This is not enough for upcoming enhanced hardwares and advance usecases like HDR processing. Hence added a new structure with 32 bit precision values.
This also defines a new structure to define color lut ranges, along with related macro definitions and enums. This will help describe multi segmented lut ranges in the hardware.
Signed-off-by: Uma Shankar uma.shankar@intel.com
include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -819,6 +819,64 @@ struct hdr_output_metadata { }; };
+/*
- DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means
the LUT
- can be used for either purpose, but not simultaneously. To expose
- modes that support gamma and degamma simultaneously the gamma mode
- must declare distinct DRM_MODE_LUT_GAMMA and
DRM_MODE_LUT_DEGAMMA
- ranges.
- */
+/* LUT is for gamma (after CTM) */ +#define DRM_MODE_LUT_GAMMA BIT(0) +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA +BIT(1) +/* linearly interpolate between the points */ #define +DRM_MODE_LUT_INTERPOLATE BIT(2) +/*
- the last value of the previous range is the
- first value of the current range.
- */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3) +/* the curve must be non-decreasing */ #define +DRM_MODE_LUT_NON_DECREASING BIT(4) +/* the curve is reflected across origin for negative inputs */ +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) +/* the same curve (red) is used for blue and green channels as well +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+struct drm_color_lut_range {
- /* DRM_MODE_LUT_* */
- __u32 flags;
- /* number of points on the curve */
- __u16 count;
- /* input/output bits per component */
- __u8 input_bpc, output_bpc;
- /* input start/end values */
- __s32 start, end;
- /* output min/max values */
- __s32 min, max;
+};
+enum lut_type {
Unprefixed type name in UAPI headers is probably not a good idea.
Ok, will rename these.
- LUT_TYPE_DEGAMMA = 0,
- LUT_TYPE_GAMMA = 1,
+};
All the above stuff seems to be the same in your other patch series' patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what does this mean?
The concept and idea is similar and the range definition is also common. But this series focuses on plane color management while the other one is for pipe/crtc color features. Hence separated and floated them as unique series for review.
Might be better in this case to combine both patchsets. It wasn't entirely clear to me whether I could base one patchset on top of the other (doesn't look like it) and what base to apply them on. I had success applying the plane stuff on drm-next and the crtc stuff on drm-intel.
Harry
Regards, Uma Shankar
Thanks, pq
+/*
- Creating 64 bit palette entries for better data
- precision. This will be required for HDR and
- similar color processing usecases.
- */
+struct drm_color_lut_ext {
- /*
* Data is U32.32 fixed point format.*/- __u64 red;
- __u64 green;
- __u64 blue;
- __u64 reserved;
+};
#define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-----Original Message----- From: Harry Wentland harry.wentland@amd.com Sent: Friday, June 4, 2021 8:53 PM To: Shankar, Uma uma.shankar@intel.com; Pekka Paalanen ppaalanen@gmail.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
On 2021-06-02 4:26 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 3:04 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com Subject: Re: [PATCH 01/21] drm: Add Enhanced Gamma and color lut range attributes
On Tue, 1 Jun 2021 16:21:58 +0530 Uma Shankar uma.shankar@intel.com wrote:
Existing LUT precision structure is having only 16 bit precision. This is not enough for upcoming enhanced hardwares and advance usecases like HDR processing. Hence added a new structure with 32 bit precision values.
This also defines a new structure to define color lut ranges, along with related macro definitions and enums. This will help describe multi segmented lut ranges in the hardware.
Signed-off-by: Uma Shankar uma.shankar@intel.com
include/uapi/drm/drm_mode.h | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 9b6722d45f36..d0ce48d2e732 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -819,6 +819,64 @@ struct hdr_output_metadata { }; };
+/*
- DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and
means
the LUT
- can be used for either purpose, but not simultaneously. To
- expose
- modes that support gamma and degamma simultaneously the gamma
- mode
- must declare distinct DRM_MODE_LUT_GAMMA and
DRM_MODE_LUT_DEGAMMA
- ranges.
- */
+/* LUT is for gamma (after CTM) */ +#define DRM_MODE_LUT_GAMMA BIT(0) +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA +BIT(1) +/* linearly interpolate between the points */ #define +DRM_MODE_LUT_INTERPOLATE BIT(2) +/*
- the last value of the previous range is the
- first value of the current range.
- */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3) +/* the curve must be non-decreasing */ #define +DRM_MODE_LUT_NON_DECREASING BIT(4) +/* the curve is reflected across origin for negative inputs */ +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) +/* the same curve (red) is used for blue and green channels as well +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+struct drm_color_lut_range {
- /* DRM_MODE_LUT_* */
- __u32 flags;
- /* number of points on the curve */
- __u16 count;
- /* input/output bits per component */
- __u8 input_bpc, output_bpc;
- /* input start/end values */
- __s32 start, end;
- /* output min/max values */
- __s32 min, max;
+};
+enum lut_type {
Unprefixed type name in UAPI headers is probably not a good idea.
Ok, will rename these.
- LUT_TYPE_DEGAMMA = 0,
- LUT_TYPE_GAMMA = 1,
+};
All the above stuff seems to be the same in your other patch series' patch "[PATCH 1/9] drm: Add gamma mode property". Is this series replacing the series "[PATCH 0/9] Enhance pipe color support for multi segmented luts" or what does this mean?
The concept and idea is similar and the range definition is also common. But this series focuses on plane color management while the other one
is for pipe/crtc color features.
Hence separated and floated them as unique series for review.
Might be better in this case to combine both patchsets. It wasn't entirely clear to me whether I could base one patchset on top of the other (doesn't look like it) and what base to apply them on. I had success applying the plane stuff on drm-next and the crtc stuff on drm-intel.
Sure Harry, I guess its better to combine both the series to avoid any confusions. I will send out next version with some more UAPI documentation based on the feedback received.
Regards, Uma Shankar
Harry
Regards, Uma Shankar
Thanks, pq
+/*
- Creating 64 bit palette entries for better data
- precision. This will be required for HDR and
- similar color processing usecases.
- */
+struct drm_color_lut_ext {
- /*
* Data is U32.32 fixed point format.*/- __u64 red;
- __u64 green;
- __u64 blue;
- __u64 reserved;
+};
#define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
Add Plane Degamma Mode as an enum property. Create a helper function for all plane color management features.
This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- Documentation/gpu/drm-kms.rst | 90 ++++++++++++++++++++++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 2 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 23 ++++++ 7 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -514,9 +514,99 @@ Damage Tracking Properties Color Management Properties ---------------------------
+Below is how a typical hardware pipeline for color +will look like: + +.. kernel-render:: DOT + :alt: Display Color Pipeline + :caption: Display Color Pipeline Overview + + digraph "KMS" { + node [shape=box] + + subgraph cluster_static { + style=dashed + label="Display Color Hardware Blocks" + + node [bgcolor=grey style=filled] + "Plane Degamma A" -> "Plane CSC/CTM A" + "Plane CSC/CTM A" -> "Plane Gamma A" + "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75]; + "Plane Gamma A" -> "Pipe Blender" + "Pipe Blender" -> "Pipe DeGamma" + "Pipe DeGamma" -> "Pipe CSC/CTM" + "Pipe CSC/CTM" -> "Pipe Gamma" + "Pipe Gamma" -> "Pipe Output" + } + + subgraph cluster_static { + style=dashed + + node [shape=box] + "Plane Degamma B" -> "Plane CSC/CTM B" + "Plane CSC/CTM B" -> "Plane Gamma B" + "Plane Gamma B" -> "Pipe Blender" + } + + subgraph cluster_static { + style=dashed + + node [shape=box] + "Plane Degamma C" -> "Plane CSC/CTM C" + "Plane CSC/CTM C" -> "Plane Gamma C" + "Plane Gamma C" -> "Pipe Blender" + } + + subgraph cluster_fb { + style=dashed + label="RAM" + + node [shape=box width=1.7 height=0.2] + + "FB 1" -> "Plane Degamma A" + "FB 2" -> "Plane Degamma B" + "FB 3" -> "Plane Degamma C" + } + } + +In real world usecases, + +1. Plane Degamma can be used to linearize a non linear gamma +encoded framebuffer. This is needed to do any linear math like +color space conversion. For ex, linearize frames encoded in SRGB +or by HDR curve. + +2. Later Plane CTM block can convert the content to some different +colorspace. For ex, SRGB to BT2020 etc. + +3. Plane Gamma block can be used later to re-apply the non-linear +curve. This can also be used to apply Tone Mapping for HDR usecases. + +All the layers or framebuffers need to be converted to same color +space and format before blending. The plane color hardware blocks +can help with this. Once the Data is blended, similar color processing +can be done on blended output using pipe color hardware blocks. + +DRM Properties have been created to define and expose all these +hardware blocks to userspace. A userspace application (compositor +or any color app) can use these interfaces and define policies to +efficiently use the display hardware for such color operations. + +Pipe Color Management Properties +--------------------------------- + .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c :doc: overview
+Plane Color Management Properties +--------------------------------- + +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c + :doc: Plane Color Properties + +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c + :doc: export + Tile Group Property -------------------
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a8bbb021684b..8892d03602f7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -708,6 +708,7 @@ 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)); + drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
if (plane->funcs->atomic_print_state) plane->funcs->atomic_print_state(p, state); diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index ddcf5c2c8e6a..f26b03853711 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, state->fence = NULL; state->commit = NULL; state->fb_damage_clips = NULL; + + state->color_mgmt_changed = false; } EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 438e9585b225..40fa05fa33dc 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -595,6 +595,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, state->color_encoding = val; } else if (property == plane->color_range_property) { state->color_range = val; + } else if (property == plane->degamma_mode_property) { + state->degamma_mode = val; } else if (property == config->prop_fb_damage_clips) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->fb_damage_clips, @@ -661,6 +663,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, *val = state->color_encoding; } else if (property == plane->color_range_property) { *val = state->color_range; + } else if (property == plane->degamma_mode_property) { + *val = state->degamma_mode; } else if (property == config->prop_fb_damage_clips) { *val = (state->fb_damage_clips) ? state->fb_damage_clips->base.id : 0; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6..085ed0d0db00 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -34,8 +34,8 @@ /** * DOC: overview * - * Color management or color space adjustments is supported through a set of 5 - * properties on the &drm_crtc object. They are set up by calling + * Pipe Color management or color space adjustments is supported through a + * set of 5 properties on the &drm_crtc object. They are set up by calling * drm_crtc_enable_color_mgmt(). * * "DEGAMMA_LUT”: @@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane, } EXPORT_SYMBOL(drm_plane_create_color_properties);
+/** + * DOC: Plane Color Properties + * + * Plane Color management or color space adjustments is supported + * through a set of 5 properties on the &drm_plane object. + * + * degamma_mode_property: + * Blob property which advertizes the possible degamma modes and + * lut ranges supported by the platform. This allows userspace + * to query and get the plane degamma color caps and choose the + * appropriate degamma mode and create lut values accordingly + * + */ +int drm_plane_create_color_mgmt_properties(struct drm_device *dev, + struct drm_plane *plane, + int num_values) +{ + struct drm_property *prop; + + prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, + "PLANE_DEGAMMA_MODE", num_values); + if (!prop) + return -ENOMEM; + + plane->degamma_mode_property = prop; + + return 0; +} +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties); + +void drm_plane_attach_degamma_properties(struct drm_plane *plane) +{ + if (!plane->degamma_mode_property) + return; + + drm_object_attach_property(&plane->base, + plane->degamma_mode_property, 0); +} +EXPORT_SYMBOL(drm_plane_attach_degamma_properties); + +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, + const char *name, + const struct drm_color_lut_range *ranges, + size_t length, enum lut_type type) +{ + struct drm_property_blob *blob; + struct drm_property *prop = NULL; + int num_ranges = length / sizeof(ranges[0]); + int i, ret, num_types_0; + + if (type == LUT_TYPE_DEGAMMA) + prop = plane->degamma_mode_property; + + if (!prop) + return -EINVAL; + + if (length == 0 && name) + return drm_property_add_enum(prop, 0, name); + + if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0)) + return -EINVAL; + num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_DEGAMMA)); + if (num_types_0 == 0) + return -EINVAL; + + for (i = 1; i < num_ranges; i++) { + int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_DEGAMMA)); + + /* either all ranges have DEGAMMA|GAMMA or none have it */ + if (num_types_0 != num_types) + return -EINVAL; + } + + blob = drm_property_create_blob(plane->dev, length, ranges); + if (IS_ERR(blob)) + return PTR_ERR(blob); + + ret = drm_property_add_enum(prop, blob->base.id, name); + if (ret) { + drm_property_blob_put(blob); + return ret; + } + + return 0; +} +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range); + /** * drm_color_lut_check - check validity of lookup table * @lut: property blob containing LUT to check diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e1..d4128c7daa08 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -60,7 +60,7 @@ struct drm_mode_object { void (*free_cb)(struct kref *kref); };
-#define DRM_OBJECT_MAX_PROPERTY 24 +#define DRM_OBJECT_MAX_PROPERTY 26 /** * struct drm_object_properties - property tracking for &drm_mode_object */ diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 1294610e84f4..e476a5939f8e 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -236,6 +236,15 @@ struct drm_plane_state {
/** @state: backpointer to global drm_atomic_state */ struct drm_atomic_state *state; + + /** + * @degamma_mode: This is a blob_id and exposes the platform capabilities + * wrt to various gamma modes and the respective lut ranges. This also + * helps user select a degamma mode amongst the supported ones. + */ + u32 degamma_mode; + + u8 color_mgmt_changed : 1; };
static inline struct drm_rect @@ -747,6 +756,12 @@ struct drm_plane { * scaling. */ struct drm_property *scaling_filter_property; + + /** + * @degamma_mode_property: Optional Plane property to set the LUT + * used to convert the framebuffer's colors to linear gamma. + */ + struct drm_property *degamma_mode_property; };
#define obj_to_plane(x) container_of(x, struct drm_plane, base) @@ -838,6 +853,14 @@ void drm_plane_force_disable(struct drm_plane *plane); int drm_mode_plane_set_obj_prop(struct drm_plane *plane, struct drm_property *property, uint64_t value); +int drm_plane_create_color_mgmt_properties(struct drm_device *dev, + struct drm_plane *plane, + int num_values); +void drm_plane_attach_degamma_properties(struct drm_plane *plane); +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, + const char *name, + const struct drm_color_lut_range *ranges, + size_t length, enum lut_type type);
/** * drm_plane_find - find a &drm_plane
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
Add Plane Degamma Mode as an enum property. Create a helper function for all plane color management features.
This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com
Documentation/gpu/drm-kms.rst | 90 ++++++++++++++++++++++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 2 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 23 ++++++ 7 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -514,9 +514,99 @@ Damage Tracking Properties Color Management Properties
+Below is how a typical hardware pipeline for color +will look like:
+.. kernel-render:: DOT
- :alt: Display Color Pipeline
- :caption: Display Color Pipeline Overview
- digraph "KMS" {
node [shape=box]subgraph cluster_static {style=dashedlabel="Display Color Hardware Blocks"node [bgcolor=grey style=filled]"Plane Degamma A" -> "Plane CSC/CTM A""Plane CSC/CTM A" -> "Plane Gamma A""Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];"Plane Gamma A" -> "Pipe Blender""Pipe Blender" -> "Pipe DeGamma""Pipe DeGamma" -> "Pipe CSC/CTM""Pipe CSC/CTM" -> "Pipe Gamma""Pipe Gamma" -> "Pipe Output"}
It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline, between the FB and Plane degamma, i.e. YCbCr coefficients > plane degamma > csc > ...
One problem with this view is that not all HW will support all (or any) of these CM blocks on all planes. For example, on AMD HW cursors are very different from other planes and don't really have full CM support.
subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma B" -> "Plane CSC/CTM B""Plane CSC/CTM B" -> "Plane Gamma B""Plane Gamma B" -> "Pipe Blender"}subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma C" -> "Plane CSC/CTM C""Plane CSC/CTM C" -> "Plane Gamma C""Plane Gamma C" -> "Pipe Blender"}subgraph cluster_fb {style=dashedlabel="RAM"node [shape=box width=1.7 height=0.2]"FB 1" -> "Plane Degamma A""FB 2" -> "Plane Degamma B""FB 3" -> "Plane Degamma C"}- }
+In real world usecases,
+1. Plane Degamma can be used to linearize a non linear gamma +encoded framebuffer. This is needed to do any linear math like +color space conversion. For ex, linearize frames encoded in SRGB +or by HDR curve.
+2. Later Plane CTM block can convert the content to some different +colorspace. For ex, SRGB to BT2020 etc.
+3. Plane Gamma block can be used later to re-apply the non-linear +curve. This can also be used to apply Tone Mapping for HDR usecases.
This would mean you're blending in gamma space which is likely not what most compositors expect. There are numerous articles that describe why blending in gamma space is problematic, such as [1]
[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html
To blend in linear space this should be configured to do
Plane Degamma > Plane CTM > CRTC Gamma
I think it would also be good if we moved away from calling this gamma. It's really only gamma for legacy SDR scenarios. For HDR cases I would never expect these to use gamma and even though the sRGB transfer function is based on gamma functions it's complicated [2].
[2] https://en.wikipedia.org/wiki/SRGB
A better way to describe these would be as "transfer function" and "inverse transfer function." The space at various stages could then be described as linear or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.
Harry
+All the layers or framebuffers need to be converted to same color +space and format before blending. The plane color hardware blocks +can help with this. Once the Data is blended, similar color processing +can be done on blended output using pipe color hardware blocks.
+DRM Properties have been created to define and expose all these +hardware blocks to userspace. A userspace application (compositor +or any color app) can use these interfaces and define policies to +efficiently use the display hardware for such color operations.
+Pipe Color Management Properties +---------------------------------
.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c :doc: overview
+Plane Color Management Properties +---------------------------------
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: Plane Color Properties
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: export
Tile Group Property
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a8bbb021684b..8892d03602f7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -708,6 +708,7 @@ 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));
drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
if (plane->funcs->atomic_print_state) plane->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index ddcf5c2c8e6a..f26b03853711 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, state->fence = NULL; state->commit = NULL; state->fb_damage_clips = NULL;
- state->color_mgmt_changed = false;
} EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 438e9585b225..40fa05fa33dc 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -595,6 +595,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, state->color_encoding = val; } else if (property == plane->color_range_property) { state->color_range = val;
- } else if (property == plane->degamma_mode_property) {
state->degamma_mode = val;@@ -661,6 +663,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, *val = state->color_encoding; } else if (property == plane->color_range_property) { *val = state->color_range;
- } else if (property == plane->degamma_mode_property) {
*val = state->degamma_mode;diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6..085ed0d0db00 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -34,8 +34,8 @@ /**
- DOC: overview
- Color management or color space adjustments is supported through a set of 5
- properties on the &drm_crtc object. They are set up by calling
- Pipe Color management or color space adjustments is supported through a
- set of 5 properties on the &drm_crtc object. They are set up by calling
- drm_crtc_enable_color_mgmt().
- "DEGAMMA_LUT”:
@@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane, } EXPORT_SYMBOL(drm_plane_create_color_properties);
+/**
- DOC: Plane Color Properties
- Plane Color management or color space adjustments is supported
- through a set of 5 properties on the &drm_plane object.
- degamma_mode_property:
Blob property which advertizes the possible degamma modes and
lut ranges supported by the platform. This allows userspace
to query and get the plane degamma color caps and choose the
appropriate degamma mode and create lut values accordingly
- */
+int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
struct drm_plane *plane,int num_values)+{
- struct drm_property *prop;
- prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
"PLANE_DEGAMMA_MODE", num_values);- if (!prop)
return -ENOMEM;- plane->degamma_mode_property = prop;
- return 0;
+} +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
+void drm_plane_attach_degamma_properties(struct drm_plane *plane) +{
- if (!plane->degamma_mode_property)
return;- drm_object_attach_property(&plane->base,
plane->degamma_mode_property, 0);+} +EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
const char *name,const struct drm_color_lut_range *ranges,size_t length, enum lut_type type)+{
- struct drm_property_blob *blob;
- struct drm_property *prop = NULL;
- int num_ranges = length / sizeof(ranges[0]);
- int i, ret, num_types_0;
- if (type == LUT_TYPE_DEGAMMA)
prop = plane->degamma_mode_property;- if (!prop)
return -EINVAL;- if (length == 0 && name)
return drm_property_add_enum(prop, 0, name);- if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
return -EINVAL;- num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
DRM_MODE_LUT_DEGAMMA));- if (num_types_0 == 0)
return -EINVAL;- for (i = 1; i < num_ranges; i++) {
int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_DEGAMMA));/* either all ranges have DEGAMMA|GAMMA or none have it */if (num_types_0 != num_types)return -EINVAL;- }
- blob = drm_property_create_blob(plane->dev, length, ranges);
- if (IS_ERR(blob))
return PTR_ERR(blob);- ret = drm_property_add_enum(prop, blob->base.id, name);
- if (ret) {
drm_property_blob_put(blob);return ret;- }
- return 0;
+} +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
/**
- drm_color_lut_check - check validity of lookup table
- @lut: property blob containing LUT to check
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e1..d4128c7daa08 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -60,7 +60,7 @@ struct drm_mode_object { void (*free_cb)(struct kref *kref); };
-#define DRM_OBJECT_MAX_PROPERTY 24 +#define DRM_OBJECT_MAX_PROPERTY 26 /**
- struct drm_object_properties - property tracking for &drm_mode_object
*/ diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 1294610e84f4..e476a5939f8e 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -236,6 +236,15 @@ struct drm_plane_state {
/** @state: backpointer to global drm_atomic_state */ struct drm_atomic_state *state;
- /**
* @degamma_mode: This is a blob_id and exposes the platform capabilities* wrt to various gamma modes and the respective lut ranges. This also* helps user select a degamma mode amongst the supported ones.*/- u32 degamma_mode;
- u8 color_mgmt_changed : 1;
};
static inline struct drm_rect @@ -747,6 +756,12 @@ struct drm_plane { * scaling. */ struct drm_property *scaling_filter_property;
- /**
* @degamma_mode_property: Optional Plane property to set the LUT* used to convert the framebuffer's colors to linear gamma.*/- struct drm_property *degamma_mode_property;
};
#define obj_to_plane(x) container_of(x, struct drm_plane, base) @@ -838,6 +853,14 @@ void drm_plane_force_disable(struct drm_plane *plane); int drm_mode_plane_set_obj_prop(struct drm_plane *plane, struct drm_property *property, uint64_t value); +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
struct drm_plane *plane,int num_values);+void drm_plane_attach_degamma_properties(struct drm_plane *plane); +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
const char *name,const struct drm_color_lut_range *ranges,size_t length, enum lut_type type);/**
- drm_plane_find - find a &drm_plane
On Fri, 4 Jun 2021 14:24:28 -0400 Harry Wentland harry.wentland@amd.com wrote:
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
Add Plane Degamma Mode as an enum property. Create a helper function for all plane color management features.
This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com
Documentation/gpu/drm-kms.rst | 90 ++++++++++++++++++++++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 2 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 23 ++++++ 7 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -514,9 +514,99 @@ Damage Tracking Properties Color Management Properties
+Below is how a typical hardware pipeline for color +will look like:
+.. kernel-render:: DOT
- :alt: Display Color Pipeline
- :caption: Display Color Pipeline Overview
- digraph "KMS" {
node [shape=box]subgraph cluster_static {style=dashedlabel="Display Color Hardware Blocks"node [bgcolor=grey style=filled]"Plane Degamma A" -> "Plane CSC/CTM A""Plane CSC/CTM A" -> "Plane Gamma A""Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];"Plane Gamma A" -> "Pipe Blender""Pipe Blender" -> "Pipe DeGamma""Pipe DeGamma" -> "Pipe CSC/CTM""Pipe CSC/CTM" -> "Pipe Gamma""Pipe Gamma" -> "Pipe Output"}It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline, between the FB and Plane degamma, i.e. YCbCr coefficients > plane degamma > csc > ...
One problem with this view is that not all HW will support all (or any) of these CM blocks on all planes. For example, on AMD HW cursors are very different from other planes and don't really have full CM support.
subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma B" -> "Plane CSC/CTM B""Plane CSC/CTM B" -> "Plane Gamma B""Plane Gamma B" -> "Pipe Blender"}subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma C" -> "Plane CSC/CTM C""Plane CSC/CTM C" -> "Plane Gamma C""Plane Gamma C" -> "Pipe Blender"}subgraph cluster_fb {style=dashedlabel="RAM"node [shape=box width=1.7 height=0.2]"FB 1" -> "Plane Degamma A""FB 2" -> "Plane Degamma B""FB 3" -> "Plane Degamma C"}- }
+In real world usecases,
+1. Plane Degamma can be used to linearize a non linear gamma +encoded framebuffer. This is needed to do any linear math like +color space conversion. For ex, linearize frames encoded in SRGB +or by HDR curve.
+2. Later Plane CTM block can convert the content to some different +colorspace. For ex, SRGB to BT2020 etc.
+3. Plane Gamma block can be used later to re-apply the non-linear +curve. This can also be used to apply Tone Mapping for HDR usecases.
This would mean you're blending in gamma space which is likely not what most compositors expect. There are numerous articles that describe why blending in gamma space is problematic, such as [1]
[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html
To blend in linear space this should be configured to do
Plane Degamma > Plane CTM > CRTC Gamma
I think it would also be good if we moved away from calling this gamma. It's really only gamma for legacy SDR scenarios. For HDR cases I would never expect these to use gamma and even though the sRGB transfer function is based on gamma functions it's complicated [2].
[2] https://en.wikipedia.org/wiki/SRGB
A better way to describe these would be as "transfer function" and "inverse transfer function." The space at various stages could then be described as linear or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.
Hi,
incidentally, I, Sebastian and Vitaly are discussing on what to call these pieces in Weston: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582#note_9472...
They might not even be the transfer functions but just some curves, e.g. optimized to keep a 3D LUT small without sacrificing precision.
Thanks, pq
+All the layers or framebuffers need to be converted to same color +space and format before blending. The plane color hardware blocks +can help with this. Once the Data is blended, similar color processing +can be done on blended output using pipe color hardware blocks.
+DRM Properties have been created to define and expose all these +hardware blocks to userspace. A userspace application (compositor +or any color app) can use these interfaces and define policies to +efficiently use the display hardware for such color operations.
+Pipe Color Management Properties +---------------------------------
.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c :doc: overview
+Plane Color Management Properties +---------------------------------
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: Plane Color Properties
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: export
Tile Group Property
-----Original Message----- From: Harry Wentland harry.wentland@amd.com Sent: Friday, June 4, 2021 11:54 PM To: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org Cc: Modem, Bhanuprakash bhanuprakash.modem@intel.com; Cyr, Aric Aric.Cyr@amd.com Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
Add Plane Degamma Mode as an enum property. Create a helper function for all plane color management features.
This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com
Documentation/gpu/drm-kms.rst | 90 ++++++++++++++++++++++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 2 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 23 ++++++ 7 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -514,9 +514,99 @@ Damage Tracking Properties Color Management Properties
+Below is how a typical hardware pipeline for color will look like:
+.. kernel-render:: DOT
- :alt: Display Color Pipeline
- :caption: Display Color Pipeline Overview
- digraph "KMS" {
node [shape=box]subgraph cluster_static {style=dashedlabel="Display Color Hardware Blocks"node [bgcolor=grey style=filled]"Plane Degamma A" -> "Plane CSC/CTM A""Plane CSC/CTM A" -> "Plane Gamma A""Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];"Plane Gamma A" -> "Pipe Blender""Pipe Blender" -> "Pipe DeGamma""Pipe DeGamma" -> "Pipe CSC/CTM""Pipe CSC/CTM" -> "Pipe Gamma""Pipe Gamma" -> "Pipe Output"}It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline, between the FB and Plane degamma, i.e. YCbCr coefficients > plane degamma > csc > ...
One problem with this view is that not all HW will support all (or any) of these CM blocks on all planes. For example, on AMD HW cursors are very different from other planes and don't really have full CM support.
subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma B" -> "Plane CSC/CTM B""Plane CSC/CTM B" -> "Plane Gamma B""Plane Gamma B" -> "Pipe Blender"}subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma C" -> "Plane CSC/CTM C""Plane CSC/CTM C" -> "Plane Gamma C""Plane Gamma C" -> "Pipe Blender"}subgraph cluster_fb {style=dashedlabel="RAM"node [shape=box width=1.7 height=0.2]"FB 1" -> "Plane Degamma A""FB 2" -> "Plane Degamma B""FB 3" -> "Plane Degamma C"}- }
+In real world usecases,
+1. Plane Degamma can be used to linearize a non linear gamma encoded +framebuffer. This is needed to do any linear math like color space +conversion. For ex, linearize frames encoded in SRGB or by HDR curve.
+2. Later Plane CTM block can convert the content to some different +colorspace. For ex, SRGB to BT2020 etc.
+3. Plane Gamma block can be used later to re-apply the non-linear +curve. This can also be used to apply Tone Mapping for HDR usecases.
This would mean you're blending in gamma space which is likely not what most compositors expect. There are numerous articles that describe why blending in gamma space is problematic, such as [1]
[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal- blend.html
To blend in linear space this should be configured to do
Plane Degamma > Plane CTM > CRTC Gamma
I think it would also be good if we moved away from calling this gamma. It's really only gamma for legacy SDR scenarios. For HDR cases I would never expect these to use gamma and even though the sRGB transfer function is based on gamma functions it's complicated [2].
[2] https://en.wikipedia.org/wiki/SRGB
A better way to describe these would be as "transfer function" and "inverse transfer function." The space at various stages could then be described as linear or non- linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.
Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we can rename these to have them named generically, as gamma lut actually can be used for Tone mapping as well and its not just meant for applying a fixed non liner curve only. However naming based on transfer function is also not optimum as Tone mapping is not related to transfer function. May be we can name them as: linear_lut and nl_tone_mapper_lut.
Other ideas for names are welcome.
Regards, Uma Shankar
Harry
+All the layers or framebuffers need to be converted to same color +space and format before blending. The plane color hardware blocks can +help with this. Once the Data is blended, similar color processing +can be done on blended output using pipe color hardware blocks.
+DRM Properties have been created to define and expose all these +hardware blocks to userspace. A userspace application (compositor or +any color app) can use these interfaces and define policies to +efficiently use the display hardware for such color operations.
+Pipe Color Management Properties +---------------------------------
.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c :doc: overview
+Plane Color Management Properties +---------------------------------
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: Plane Color Properties
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
- :doc: export
Tile Group Property
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a8bbb021684b..8892d03602f7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -708,6 +708,7 @@ 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));
- drm_printf(p, "\tcolor_mgmt_changed=%d\n",
+state->color_mgmt_changed);
if (plane->funcs->atomic_print_state) plane->funcs->atomic_print_state(p, state); diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index ddcf5c2c8e6a..f26b03853711 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
state->fence = NULL; state->commit = NULL; state->fb_damage_clips = NULL;
- state->color_mgmt_changed = false;
} EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 438e9585b225..40fa05fa33dc 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -595,6 +595,8 @@ static int drm_atomic_plane_set_property(struct
drm_plane *plane,
state->color_encoding = val;} else if (property == plane->color_range_property) { state->color_range = val;
- } else if (property == plane->degamma_mode_property) {
state->degamma_mode = val;@@ -661,6 +663,8 @@ drm_atomic_plane_get_property(struct drm_plane
*plane,
*val = state->color_encoding;} else if (property == plane->color_range_property) { *val = state->color_range;
- } else if (property == plane->degamma_mode_property) {
*val = state->degamma_mode;a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6..085ed0d0db00 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -34,8 +34,8 @@ /**
- DOC: overview
- Color management or color space adjustments is supported through a
set of 5
- properties on the &drm_crtc object. They are set up by calling
- Pipe Color management or color space adjustments is supported
- through a
- set of 5 properties on the &drm_crtc object. They are set up by
- calling
- drm_crtc_enable_color_mgmt().
- "DEGAMMA_LUT”:
@@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane, } EXPORT_SYMBOL(drm_plane_create_color_properties);
+/**
- DOC: Plane Color Properties
- Plane Color management or color space adjustments is supported
- through a set of 5 properties on the &drm_plane object.
- degamma_mode_property:
Blob property which advertizes the possible degamma modes and
lut ranges supported by the platform. This allows userspace
to query and get the plane degamma color caps and choose the
appropriate degamma mode and create lut values accordingly
- */
+int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
struct drm_plane *plane,int num_values)+{
- struct drm_property *prop;
- prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
"PLANE_DEGAMMA_MODE", num_values);- if (!prop)
return -ENOMEM;- plane->degamma_mode_property = prop;
- return 0;
+} +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
+void drm_plane_attach_degamma_properties(struct drm_plane *plane) {
- if (!plane->degamma_mode_property)
return;- drm_object_attach_property(&plane->base,
plane->degamma_mode_property, 0); }+EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
*plane,
const char *name,const struct drm_color_lut_range*ranges,
size_t length, enum lut_type type){
- struct drm_property_blob *blob;
- struct drm_property *prop = NULL;
- int num_ranges = length / sizeof(ranges[0]);
- int i, ret, num_types_0;
- if (type == LUT_TYPE_DEGAMMA)
prop = plane->degamma_mode_property;- if (!prop)
return -EINVAL;- if (length == 0 && name)
return drm_property_add_enum(prop, 0, name);- if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
return -EINVAL;- num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
DRM_MODE_LUT_DEGAMMA));- if (num_types_0 == 0)
return -EINVAL;- for (i = 1; i < num_ranges; i++) {
int num_types = hweight8(ranges[i].flags &(DRM_MODE_LUT_GAMMA |
DRM_MODE_LUT_DEGAMMA));/* either all ranges have DEGAMMA|GAMMA or none have it */if (num_types_0 != num_types)return -EINVAL;- }
- blob = drm_property_create_blob(plane->dev, length, ranges);
- if (IS_ERR(blob))
return PTR_ERR(blob);- ret = drm_property_add_enum(prop, blob->base.id, name);
- if (ret) {
drm_property_blob_put(blob);return ret;- }
- return 0;
+} +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
/**
- drm_color_lut_check - check validity of lookup table
- @lut: property blob containing LUT to check diff --git
a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e1..d4128c7daa08 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -60,7 +60,7 @@ struct drm_mode_object { void (*free_cb)(struct kref *kref); };
-#define DRM_OBJECT_MAX_PROPERTY 24 +#define DRM_OBJECT_MAX_PROPERTY 26 /**
- struct drm_object_properties - property tracking for &drm_mode_object
*/ diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 1294610e84f4..e476a5939f8e 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -236,6 +236,15 @@ struct drm_plane_state {
/** @state: backpointer to global drm_atomic_state */ struct drm_atomic_state *state;
- /**
* @degamma_mode: This is a blob_id and exposes the platform capabilities* wrt to various gamma modes and the respective lut ranges. This also* helps user select a degamma mode amongst the supported ones.*/- u32 degamma_mode;
- u8 color_mgmt_changed : 1;
};
static inline struct drm_rect @@ -747,6 +756,12 @@ struct drm_plane { * scaling. */ struct drm_property *scaling_filter_property;
- /**
* @degamma_mode_property: Optional Plane property to set the LUT* used to convert the framebuffer's colors to linear gamma.*/- struct drm_property *degamma_mode_property;
};
#define obj_to_plane(x) container_of(x, struct drm_plane, base) @@ -838,6 +853,14 @@ void drm_plane_force_disable(struct drm_plane *plane); int drm_mode_plane_set_obj_prop(struct drm_plane *plane, struct drm_property *property, uint64_t value); +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
struct drm_plane *plane,int num_values);+void drm_plane_attach_degamma_properties(struct drm_plane *plane); +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
*plane,
const char *name,const struct drm_color_lut_range*ranges,
size_t length, enum lut_type type);/**
- drm_plane_find - find a &drm_plane
On Mon, 7 Jun 2021 17:34:06 +0000 "Shankar, Uma" uma.shankar@intel.com wrote:
-----Original Message----- From: Harry Wentland harry.wentland@amd.com Sent: Friday, June 4, 2021 11:54 PM To: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org Cc: Modem, Bhanuprakash bhanuprakash.modem@intel.com; Cyr, Aric Aric.Cyr@amd.com Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
Add Plane Degamma Mode as an enum property. Create a helper function for all plane color management features.
This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com
Documentation/gpu/drm-kms.rst | 90 ++++++++++++++++++++++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 2 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 23 ++++++ 7 files changed, 212 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -514,9 +514,99 @@ Damage Tracking Properties Color Management Properties
+Below is how a typical hardware pipeline for color will look like:
+.. kernel-render:: DOT
- :alt: Display Color Pipeline
- :caption: Display Color Pipeline Overview
- digraph "KMS" {
node [shape=box]subgraph cluster_static {style=dashedlabel="Display Color Hardware Blocks"node [bgcolor=grey style=filled]"Plane Degamma A" -> "Plane CSC/CTM A""Plane CSC/CTM A" -> "Plane Gamma A""Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];"Plane Gamma A" -> "Pipe Blender""Pipe Blender" -> "Pipe DeGamma""Pipe DeGamma" -> "Pipe CSC/CTM""Pipe CSC/CTM" -> "Pipe Gamma""Pipe Gamma" -> "Pipe Output"}It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline, between the FB and Plane degamma, i.e. YCbCr coefficients > plane degamma > csc > ...
One problem with this view is that not all HW will support all (or any) of these CM blocks on all planes. For example, on AMD HW cursors are very different from other planes and don't really have full CM support.
subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma B" -> "Plane CSC/CTM B""Plane CSC/CTM B" -> "Plane Gamma B""Plane Gamma B" -> "Pipe Blender"}subgraph cluster_static {style=dashednode [shape=box]"Plane Degamma C" -> "Plane CSC/CTM C""Plane CSC/CTM C" -> "Plane Gamma C""Plane Gamma C" -> "Pipe Blender"}subgraph cluster_fb {style=dashedlabel="RAM"node [shape=box width=1.7 height=0.2]"FB 1" -> "Plane Degamma A""FB 2" -> "Plane Degamma B""FB 3" -> "Plane Degamma C"}- }
+In real world usecases,
+1. Plane Degamma can be used to linearize a non linear gamma encoded +framebuffer. This is needed to do any linear math like color space +conversion. For ex, linearize frames encoded in SRGB or by HDR curve.
+2. Later Plane CTM block can convert the content to some different +colorspace. For ex, SRGB to BT2020 etc.
+3. Plane Gamma block can be used later to re-apply the non-linear +curve. This can also be used to apply Tone Mapping for HDR usecases.
This would mean you're blending in gamma space which is likely not what most compositors expect. There are numerous articles that describe why blending in gamma space is problematic, such as [1]
[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal- blend.html
To blend in linear space this should be configured to do
Plane Degamma > Plane CTM > CRTC Gamma
I think it would also be good if we moved away from calling this gamma. It's really only gamma for legacy SDR scenarios. For HDR cases I would never expect these to use gamma and even though the sRGB transfer function is based on gamma functions it's complicated [2].
[2] https://en.wikipedia.org/wiki/SRGB
A better way to describe these would be as "transfer function" and "inverse transfer function." The space at various stages could then be described as linear or non- linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.
Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we can rename these to have them named generically, as gamma lut actually can be used for Tone mapping as well and its not just meant for applying a fixed non liner curve only. However naming based on transfer function is also not optimum as Tone mapping is not related to transfer function. May be we can name them as: linear_lut and nl_tone_mapper_lut.
Other ideas for names are welcome.
I would suggest to avoid any kind of naming that explicitly refers to gamma, EOTF, EOTF^-1, tone map, or anything else that has a specific meaning. Likewise, the diagram should not claim that the pixel data at certain stages of the pipeline is in certain form, e.g. linear, non-linear, or in certain color space. Doing that would semantically restrict what userspace can do. After all, kernel UAPI design is mechanism, not policy, right?
However, to make the pipeline understandable, the documentation could show example configurations, where you *can* say that this LUT implements EOTF, pixels are light-linear at that point, the color space at another point is XXX, and so on.
What exactly the LUT is, are pixel values linear or non-linear, and what the color space is at any step are something that userspace will define through providing the parameters for each processing step in the pipeline.
Unless, your hardware has been designed for a certain policy and deviating from that policy should be discouraged?
So what to name these:
That's the question we are struggling with at Weston too. You can seek inspiration from CMM data structures (e.g. Little CMS 2) or from ICC specifications. Just need to be careful to not pick a term that has more meaning than kernel UAPI wants.
I personally like the terms "color curve" for 1D LUT -like things, and "color mapping" for matrix or 3D LUT -like things. I'm using the term "color transformation" for going from one encoding, color space, gamut, and dynamic range to another with a specific render intent, which may require a sequence of several color curves and color mappings to achieve. But that's just me.
Why bother with all this indirection:
Use cases that want blending in non-linear spaces are one thing, but probably less useful with HDR. Instead, I would imagine that doing numerical optimization on a complete pipeline to achieve the best precision given hardware limitations or minimizing memory consumption or other, may result in different curves and mapping tables than simply plugging in the EOTFs and CSCs into their intended places.
I have actually started to think that when Weston's DRM-backend is trying to fit a window on a KMS plane, it would need to get the color pipeline capabilities of that specific plane and ask the color manager component to optimize the color transformation against the exact KMS capabilities, evaluating whether a sufficient precision can be achieved. Then the color manager gives back a description of how the KMS plane would need to be configured, and the DRM-backend continues with an atomic TEST_ONLY commit to see if it would work. The color manager would quite likely delegate that optimization to a CMM, like Little CMS 2 if possible. But this is all still so much hand-waving and the KMS UAPI is not really there yet, that I will mostly just ignore this idea for now and do something much simpler.
Or maybe all this optimization has already been done when designing the new hardware, and we really do need to simply plug in the EOTFs and CSCs without thinking?
Then again, if hardware changes every generation, that implies the optimization is not complete, and there might be something to gain from software optimization still. Old hardware always exists, too.
Thanks, pq
Add Plane Degamma Lut as a blob property. User will calculate the lut values, create the blob and send it to driver using this property. Lut calculation will be based on the gamma mode chosen out of the gamma mode exposed.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/drm_atomic_state_helper.c | 4 ++++ drivers/gpu/drm/drm_atomic_uapi.c | 10 ++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++ include/drm/drm_plane.h | 14 ++++++++++++++ 4 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index f26b03853711..6e358067cb7a 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -312,6 +312,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, state->commit = NULL; state->fb_damage_clips = NULL;
+ if (state->degamma_lut) + drm_property_blob_get(state->degamma_lut); + state->color_mgmt_changed = false; } EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state); @@ -359,6 +362,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state) drm_crtc_commit_put(state->commit);
drm_property_blob_put(state->fb_damage_clips); + drm_property_blob_put(state->degamma_lut); } EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 40fa05fa33dc..ce3cb65d415e 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -597,6 +597,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, state->color_range = val; } else if (property == plane->degamma_mode_property) { state->degamma_mode = val; + } else if (property == plane->degamma_lut_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->degamma_lut, + val, -1, sizeof(struct drm_color_lut_ext), + &replaced); + state->color_mgmt_changed |= replaced; + return ret; } else if (property == config->prop_fb_damage_clips) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->fb_damage_clips, @@ -665,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane, *val = state->color_range; } else if (property == plane->degamma_mode_property) { *val = state->degamma_mode; + } else if (property == plane->degamma_lut_property) { + *val = (state->degamma_lut) ? + state->degamma_lut->base.id : 0; } else if (property == config->prop_fb_damage_clips) { *val = (state->fb_damage_clips) ? state->fb_damage_clips->base.id : 0; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 085ed0d0db00..29d0fc1e52b5 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -596,6 +596,12 @@ EXPORT_SYMBOL(drm_plane_create_color_properties); * to query and get the plane degamma color caps and choose the * appropriate degamma mode and create lut values accordingly * + * degamma_lut_property: + * Blob property which allows a userspace to provide LUT values + * to apply degamma curve using the h/w plane degamma processing + * engine, thereby making the content as linear for further color + * processing. + * */ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, struct drm_plane *plane, @@ -610,6 +616,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
plane->degamma_mode_property = prop;
+ prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, + "PLANE_DEGAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + + plane->degamma_lut_property = prop; + return 0; } EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties); @@ -621,6 +634,12 @@ void drm_plane_attach_degamma_properties(struct drm_plane *plane)
drm_object_attach_property(&plane->base, plane->degamma_mode_property, 0); + + if (!plane->degamma_lut_property) + return; + + drm_object_attach_property(&plane->base, + plane->degamma_lut_property, 0); } EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index e476a5939f8e..bbd0033ed1d2 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -244,6 +244,14 @@ struct drm_plane_state { */ u32 degamma_mode;
+ /* @degamma_lut: + * + * Lookup table for converting framebuffer pixel data before apply the + * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The + * blob (if not NULL) is an array of &struct drm_color_lut_ext. + */ + struct drm_property_blob *degamma_lut; + u8 color_mgmt_changed : 1; };
@@ -762,6 +770,12 @@ struct drm_plane { * used to convert the framebuffer's colors to linear gamma. */ struct drm_property *degamma_mode_property; + + /** + * @degamma_lut_property: Optional Plane property to set the LUT + * used to convert the framebuffer's colors to linear gamma. + */ + struct drm_property *degamma_lut_property; };
#define obj_to_plane(x) container_of(x, struct drm_plane, base)
Define the structure with XE_LPD degamma lut ranges. HDR and SDR planes have different capabilities, implemented respective structure for the HDR planes.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251b..c735d06a6b54 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state) } }
+ /* FIXME input bpc? */ +__maybe_unused +static const struct drm_color_lut_range d13_degamma_hdr[] = { + /* segment 1 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_NON_DECREASING), + .count = 128, + .input_bpc = 24, .output_bpc = 16, + .start = 0, .end = (1 << 24) - 1, + .min = 0, .max = (1 << 24) - 1, + }, + /* segment 2 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = (1 << 24) - 1, .end = 1 << 24, + .min = 0, .max = (1 << 27) - 1, + }, + /* Segment 3 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = 1 << 24, .end = 3 << 24, + .min = 0, .max = (1 << 27) - 1, + }, + /* Segment 4 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = 3 << 24, .end = 7 << 24, + .min = 0, .max = (1 << 27) - 1, + }, +}; + void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
On 2021-06-01 6:52 a.m., Uma Shankar wrote:
Define the structure with XE_LPD degamma lut ranges. HDR and SDR planes have different capabilities, implemented respective structure for the HDR planes.
Signed-off-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251b..c735d06a6b54 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state) } }
- /* FIXME input bpc? */
+__maybe_unused +static const struct drm_color_lut_range d13_degamma_hdr[] = {
- /* segment 1 */
- {
.flags = (DRM_MODE_LUT_GAMMA |
Why are these using DRM_MODE_LUT_GAMMA and not DRM_MODE_LUT_DEGAMMA when the lut_type for this LUT is LUT_TYPE_DEGAMMA?
DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_NON_DECREASING),.count = 128,.input_bpc = 24, .output_bpc = 16,
Why do we need more than 16 bpc for LUT? FP16 is enough to represent HDR in linear space. Wouldn't 16 bpc be enough?
.start = 0, .end = (1 << 24) - 1,.min = 0, .max = (1 << 24) - 1,- },
- /* segment 2 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = (1 << 24) - 1, .end = 1 << 24,.min = 0, .max = (1 << 27) - 1,
How can max be 1 << 27 if input_bpc is 24?
Harry
- },
- /* Segment 3 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = 1 << 24, .end = 3 << 24,.min = 0, .max = (1 << 27) - 1,- },
- /* Segment 4 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = 3 << 24, .end = 7 << 24,.min = 0, .max = (1 << 27) - 1,- },
+};
void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Harry Wentland Sent: Monday, June 28, 2021 8:45 PM To: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org Cc: Modem, Bhanuprakash bhanuprakash.modem@intel.com Subject: Re: [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
On 2021-06-01 6:52 a.m., Uma Shankar wrote:
Define the structure with XE_LPD degamma lut ranges. HDR and SDR planes have different capabilities, implemented respective structure for the HDR planes.
Signed-off-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251b..c735d06a6b54 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2093,6 +2093,58 @@ static void icl_read_luts(struct intel_crtc_state
*crtc_state)
} }
- /* FIXME input bpc? */
+__maybe_unused +static const struct drm_color_lut_range d13_degamma_hdr[] = {
- /* segment 1 */
- {
.flags = (DRM_MODE_LUT_GAMMA |Why are these using DRM_MODE_LUT_GAMMA and not DRM_MODE_LUT_DEGAMMA when the lut_type for this LUT is LUT_TYPE_DEGAMMA?
Thanks Harry for the comments.
Yeah this is an oversight, will fix this.
DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_NON_DECREASING),.count = 128,.input_bpc = 24, .output_bpc = 16,Why do we need more than 16 bpc for LUT? FP16 is enough to represent HDR in linear space. Wouldn't 16 bpc be enough?
Pipe sometimes works internally on higher precision (just to take care of rounding etc.), later the extra data gets dropped at the end of the pipe. So from source side you are right, 16bpc is enough but the lut precision can go higher.
.start = 0, .end = (1 << 24) - 1,.min = 0, .max = (1 << 24) - 1,- },
- /* segment 2 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = (1 << 24) - 1, .end = 1 << 24,.min = 0, .max = (1 << 27) - 1,How can max be 1 << 27 if input_bpc is 24?
This is to take care of > 1.0 section. 1.0 to 3.0 and 3.0 to 7.0. So we have 3.24 format for Lut to take care of this.
Also, I have an action to update the series with UAPI doc and new naming for the property. My apologies for being late on that one. Will update and send that out soon.
Thanks & Regards, Uma Shankar
Harry
- },
- /* Segment 3 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = 1 << 24, .end = 3 << 24,.min = 0, .max = (1 << 27) - 1,- },
- /* Segment 4 */
- {
.flags = (DRM_MODE_LUT_GAMMA |DRM_MODE_LUT_REFLECT_NEGATIVE |DRM_MODE_LUT_INTERPOLATE |DRM_MODE_LUT_REUSE_LAST |DRM_MODE_LUT_NON_DECREASING),.count = 1,.input_bpc = 24, .output_bpc = 16,.start = 3 << 24, .end = 7 << 24,.min = 0, .max = (1 << 27) - 1,- },
+};
void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
Add macros to define Plane Degamma registers
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 24307c49085f..9431913969f3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -262,6 +262,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) INTEL_INFO(dev_priv)->cursor_offsets[PIPE_A] + (reg) + \ DISPLAY_MMIO_BASE(dev_priv))
+/* Plane Gamma Registers */ +#define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) + #define __MASKED_FIELD(mask, value) ((mask) << 16 | (value)) #define _MASKED_FIELD(mask, value) ({ \ if (__builtin_constant_p(mask)) \ @@ -11300,6 +11303,55 @@ enum skl_power_gate { _PAL_PREC_MULTI_SEG_DATA_A, \ _PAL_PREC_MULTI_SEG_DATA_B)
+/* Display13 Plane Degmma Reg */ +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_A 0x701d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_B 0x711d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_A 0x702d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_B 0x712d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_A, \ + _PLANE_PRE_CSC_GAMC_INDEX_ENH_1_B) +#define _PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_A, \ + _PLANE_PRE_CSC_GAMC_INDEX_ENH_2_B) +#define PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe), \ + _PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe)) + +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A 0x701d4 +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_B 0x711d4 +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2_A 0x702d4 +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2_B 0x712d4 +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A, \ + _PLANE_PRE_CSC_GAMC_DATA_ENH_1_B) +#define _PLANE_PRE_CSC_GAMC_DATA_ENH_2(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_ENH_2_A, \ + _PLANE_PRE_CSC_GAMC_DATA_ENH_2_B) +#define PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_ENH_1(pipe), \ + _PLANE_PRE_CSC_GAMC_DATA_ENH_2(pipe)) + +#define _PLANE_PRE_CSC_GAMC_INDEX_1_A 0x704d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_1_B 0x714d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_2_A 0x705d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_2_B 0x715d0 +#define _PLANE_PRE_CSC_GAMC_INDEX_1(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_1_A, \ + _PLANE_PRE_CSC_GAMC_INDEX_1_B) +#define _PLANE_PRE_CSC_GAMC_INDEX_2(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_INDEX_2_A, \ + _PLANE_PRE_CSC_GAMC_INDEX_2_B) +#define PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_1(pipe), \ + _PLANE_PRE_CSC_GAMC_INDEX_2(pipe)) + +#define _PLANE_PRE_CSC_GAMC_DATA_1_A 0x704d4 +#define _PLANE_PRE_CSC_GAMC_DATA_1_B 0x714d4 +#define _PLANE_PRE_CSC_GAMC_DATA_2_A 0x705d4 +#define _PLANE_PRE_CSC_GAMC_DATA_2_B 0x715d4 +#define _PLANE_PRE_CSC_GAMC_DATA_1(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_1_A, \ + _PLANE_PRE_CSC_GAMC_DATA_1_B) +#define _PLANE_PRE_CSC_GAMC_DATA_2(pipe) _PIPE(pipe, _PLANE_PRE_CSC_GAMC_DATA_2_A, \ + _PLANE_PRE_CSC_GAMC_DATA_2_B) +#define PLANE_PRE_CSC_GAMC_DATA(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_1(pipe), \ + _PLANE_PRE_CSC_GAMC_DATA_2(pipe)) + /* pipe CSC & degamma/gamma LUTs on CHV */ #define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) #define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904)
Enable and initialize plane color features. Also initialize the color features of HDR planes.
Signed-off-by: Uma Shankar uma.shankar@intel.com Signed-off-by: Bhanuprakash Modem bhanuprakash.modem@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 22 +++++++++++++++++++++- drivers/gpu/drm/i915/display/intel_color.h | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ 3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index c735d06a6b54..00a0d45632fe 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -25,6 +25,7 @@ #include "intel_color.h" #include "intel_de.h" #include "intel_display_types.h" +#include <drm/drm_plane.h>
#define CTM_COEFF_SIGN (1ULL << 63)
@@ -2094,7 +2095,6 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state) }
/* FIXME input bpc? */ -__maybe_unused static const struct drm_color_lut_range d13_degamma_hdr[] = { /* segment 1 */ { @@ -2145,6 +2145,26 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = { }, };
+int intel_plane_color_init(struct drm_plane *plane) +{ + struct drm_i915_private *dev_priv = to_i915(plane->dev); + int ret = 0; + + if (DISPLAY_VER(dev_priv) >= 13) { + drm_plane_create_color_mgmt_properties(plane->dev, plane, 2); + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma", + NULL, 0, + LUT_TYPE_DEGAMMA); + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma", + d13_degamma_hdr, + sizeof(d13_degamma_hdr), + LUT_TYPE_DEGAMMA); + drm_plane_attach_degamma_properties(plane); + } + + return ret; +} + void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h index 173727aaa24d..b8850bb1b0c9 100644 --- a/drivers/gpu/drm/i915/display/intel_color.h +++ b/drivers/gpu/drm/i915/display/intel_color.h @@ -10,6 +10,7 @@
struct intel_crtc_state; struct intel_crtc; +struct drm_plane; struct drm_property_blob;
void intel_color_init(struct intel_crtc *crtc); @@ -21,5 +22,6 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat bool intel_color_lut_equal(struct drm_property_blob *blob1, struct drm_property_blob *blob2, u32 gamma_mode, u32 bit_precision); +int intel_plane_color_init(struct drm_plane *plane);
#endif /* __INTEL_COLOR_H__ */ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0f6d27da69ac..f955f039edba 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -326,6 +326,9 @@ struct drm_i915_display_funcs { */ void (*load_luts)(const struct intel_crtc_state *crtc_state); void (*read_luts)(struct intel_crtc_state *crtc_state); + /* Add Plane Color callbacks */ + void (*load_plane_csc_matrix)(const struct drm_plane_state *plane_state); + void (*load_plane_luts)(const struct drm_plane_state *plane_state); };
struct intel_dmc {
Add the Color capabilities of SDR planes.
Signed-off-by: Uma Shankar uma.shankar@intel.com Signed-off-by: Bhanuprakash Modem bhanuprakash.modem@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 67 ++++++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 00a0d45632fe..4908e825f3cc 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -25,6 +25,7 @@ #include "intel_color.h" #include "intel_de.h" #include "intel_display_types.h" +#include "intel_sprite.h" #include <drm/drm_plane.h>
#define CTM_COEFF_SIGN (1ULL << 63) @@ -2145,6 +2146,57 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = { }, };
+ /* FIXME input bpc? */ +static const struct drm_color_lut_range d13_degamma_sdr[] = { + /* segment 1 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_NON_DECREASING), + .count = 32, + .input_bpc = 16, .output_bpc = 16, + .start = 0, .end = (1 << 16) - (1 << 16) / 33, + .min = 0, .max = (1 << 16) - 1, + }, + /* segment 2 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 16, .output_bpc = 16, + .start = (1 << 16) - (1 << 16) / 33, .end = 1 << 16, + .min = 0, .max = 1 << 16, + }, + /* Segment 3 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 16, .output_bpc = 16, + .start = 1 << 16, .end = 3 << 16, + .min = 0, .max = (8 << 16) - 1, + }, + /* Segment 4 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 16, .output_bpc = 16, + .start = 3 << 16, .end = 7 << 16, + .min = 0, .max = (8 << 16) - 1, + }, +}; + int intel_plane_color_init(struct drm_plane *plane) { struct drm_i915_private *dev_priv = to_i915(plane->dev); @@ -2155,10 +2207,17 @@ int intel_plane_color_init(struct drm_plane *plane) ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma", NULL, 0, LUT_TYPE_DEGAMMA); - ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma", - d13_degamma_hdr, - sizeof(d13_degamma_hdr), - LUT_TYPE_DEGAMMA); + if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma", + d13_degamma_hdr, + sizeof(d13_degamma_hdr), + LUT_TYPE_DEGAMMA); + else + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, + "plane degamma", + d13_degamma_sdr, + sizeof(d13_degamma_sdr), + LUT_TYPE_DEGAMMA); drm_plane_attach_degamma_properties(plane); }
Extract the LUT and program plane degamma registers.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 116 +++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 2 + 2 files changed, 118 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 4908e825f3cc..4e5733573fd8 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -126,6 +126,29 @@ static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state *crtc_state lut_is_legacy(crtc_state->hw.gamma_lut); }
+/* + * Added to accommodate enhanced LUT precision. + * Max LUT precision is 32 bits. + */ +static u64 drm_color_lut_extract_ext(u64 user_input, u32 bit_precision) +{ + u64 val = user_input & 0xffffffff; + u32 max; + + if (bit_precision > 32) + return 0; + + max = 0xffffffff >> (32 - bit_precision); + /* Round only if we're not using full precision. */ + if (bit_precision < 32) { + val += 1UL << (32 - bit_precision - 1); + val >>= 32 - bit_precision; + } + + return ((user_input & 0xffffffff00000000) | + clamp_val(val, 0, max)); +} + /* * When using limited range, multiply the matrix given by userspace by * the matrix that we would use for the limited range. @@ -2197,6 +2220,97 @@ static const struct drm_color_lut_range d13_degamma_sdr[] = { }, };
+static void d13_program_plane_degamma_lut(const struct drm_plane_state *state, + struct drm_color_lut_ext *degamma_lut, + u32 offset) +{ + struct drm_i915_private *dev_priv = to_i915(state->plane->dev); + enum pipe pipe = to_intel_plane(state->plane)->pipe; + enum plane_id plane = to_intel_plane(state->plane)->id; + u32 i, lut_size; + + if (icl_is_hdr_plane(dev_priv, plane)) { + lut_size = 128; + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), + PLANE_PAL_PREC_AUTO_INCREMENT); + + if (degamma_lut) { + for (i = 0; i < lut_size; i++) { + u64 word = drm_color_lut_extract_ext(degamma_lut[i].green, 24); + u32 lut_val = (word & 0xffffff); + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), + lut_val); + } + + /* Program the max register to clamp values > 1.0. */ + while (i < 131) + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), + degamma_lut[i++].green); + } else { + for (i = 0; i < lut_size; i++) { + u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1); + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), v); + } + + do { + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), + 1 << 24); + } while (i++ < 130); + } + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0); + } else { + lut_size = 32; + + /* + * First 3 planes are HDR, so reduce by 3 to get to the right + * SDR plane offset + */ + plane = plane - 3; + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, 0), + PLANE_PAL_PREC_AUTO_INCREMENT); + + if (degamma_lut) { + for (i = 0; i < lut_size; i++) + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), + degamma_lut[i].green); + /* Program the max register to clamp values > 1.0. */ + while (i < 35) + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), + degamma_lut[i++].green); + } else { + for (i = 0; i < lut_size; i++) { + u32 v = (i * ((1 << 16) - 1)) / (lut_size - 1); + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), v); + } + + do { + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), + 1 << 16); + } while (i++ < 34); + } + + intel_de_write(dev_priv, PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, 0), 0); + } +} + +static void d13_plane_load_luts(const struct drm_plane_state *plane_state) +{ + const struct drm_property_blob *degamma_lut_blob = + plane_state->degamma_lut; + struct drm_color_lut_ext *degamma_lut = NULL; + + if (degamma_lut_blob) { + degamma_lut = degamma_lut_blob->data; + d13_program_plane_degamma_lut(plane_state, degamma_lut, 0); + } +} + int intel_plane_color_init(struct drm_plane *plane) { struct drm_i915_private *dev_priv = to_i915(plane->dev); @@ -2218,6 +2332,8 @@ int intel_plane_color_init(struct drm_plane *plane) d13_degamma_sdr, sizeof(d13_degamma_sdr), LUT_TYPE_DEGAMMA); + + dev_priv->display.load_plane_luts = d13_plane_load_luts; drm_plane_attach_degamma_properties(plane); }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9431913969f3..ede7dca440e2 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7189,6 +7189,7 @@ enum { #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) #define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ +#define PLANE_PRE_CSC_GAMMA_ENABLE (1 << 14) #define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17) #define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601 (1 << 17) #define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709 (2 << 17) @@ -11315,6 +11316,7 @@ enum skl_power_gate { #define PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, i) \ _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_INDEX_ENH_1(pipe), \ _PLANE_PRE_CSC_GAMC_INDEX_ENH_2(pipe)) +#define PLANE_PAL_PREC_AUTO_INCREMENT (1 << 10)
#define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_A 0x701d4 #define _PLANE_PRE_CSC_GAMC_DATA_ENH_1_B 0x711d4
Extended glk_plane_color_ctl to have plane color checks. This helps enabling the degamma or gamma block based on user inputs.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/skl_universal_plane.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 92a4fd508e92..ae439dca4b3c 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -960,6 +960,11 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, u32 plane_color_ctl = 0;
plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE; + + /* FIXME needs hw.degamma_lut */ + if (plane_state->uapi.degamma_lut) + plane_color_ctl |= PLANE_PRE_CSC_GAMMA_ENABLE; + plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
Initialize plane color features for XE_LPD.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/intel_atomic_plane.h | 1 + drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index dc4d05e75e1c..c809f522a710 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -65,5 +65,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state, bool can_position); void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); +int intel_plane_color_init(struct drm_plane *plane);
#endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index ae439dca4b3c..aadb984fdf77 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -2184,6 +2184,8 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv, BIT(DRM_SCALING_FILTER_DEFAULT) | BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
+ intel_plane_color_init(&plane->base); + drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
return plane;
Load plane color luts as part of atomic plane updates. This will be done only if the plane color luts are changed.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/intel_atomic_plane.c | 3 +++ drivers/gpu/drm/i915/display/intel_atomic_plane.h | 1 + drivers/gpu/drm/i915/display/intel_color.c | 9 +++++++++ 3 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 36f52a1d7552..5de9c98beaf6 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -513,6 +513,9 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state, struct intel_plane_state *new_plane_state = intel_atomic_get_new_plane_state(state, plane);
+ if (new_plane_state->uapi.color_mgmt_changed) + intel_color_load_plane_luts(&new_plane_state->uapi); + if (new_plane_state->uapi.visible || new_plane_state->planar_slave) { intel_update_plane(plane, new_crtc_state, new_plane_state); diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index c809f522a710..1ba3b524cee2 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -66,5 +66,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state, void intel_plane_set_invisible(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); int intel_plane_color_init(struct drm_plane *plane); +void intel_color_load_plane_luts(const struct drm_plane_state *plane_state);
#endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 4e5733573fd8..6d57a47d8a60 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -22,6 +22,7 @@ * */
+#include "intel_atomic_plane.h" #include "intel_color.h" #include "intel_de.h" #include "intel_display_types.h" @@ -2311,6 +2312,14 @@ static void d13_plane_load_luts(const struct drm_plane_state *plane_state) } }
+void intel_color_load_plane_luts(const struct drm_plane_state *plane_state) +{ + struct drm_device *dev = plane_state->plane->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + + dev_priv->display.load_plane_luts(plane_state); +} + int intel_plane_color_init(struct drm_plane *plane) { struct drm_i915_private *dev_priv = to_i915(plane->dev);
Add a blob property for plane CSC usage.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/drm_atomic_state_helper.c | 3 +++ drivers/gpu/drm/drm_atomic_uapi.c | 10 ++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 11 +++++++++++ include/drm/drm_plane.h | 15 +++++++++++++++ 4 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 6e358067cb7a..fafb8af1c9cb 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -314,6 +314,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
if (state->degamma_lut) drm_property_blob_get(state->degamma_lut); + if (state->ctm) + drm_property_blob_get(state->ctm);
state->color_mgmt_changed = false; } @@ -363,6 +365,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
drm_property_blob_put(state->fb_damage_clips); drm_property_blob_put(state->degamma_lut); + drm_property_blob_put(state->ctm); } EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index ce3cb65d415e..6af2afe362ff 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -604,6 +604,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, &replaced); state->color_mgmt_changed |= replaced; return ret; + } else if (property == plane->ctm_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->ctm, + val, + sizeof(struct drm_color_ctm), -1, + &replaced); + state->color_mgmt_changed |= replaced; + return ret; } else if (property == config->prop_fb_damage_clips) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->fb_damage_clips, @@ -675,6 +683,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, } else if (property == plane->degamma_lut_property) { *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; + } else if (property == plane->ctm_property) { + *val = (state->ctm) ? state->ctm->base.id : 0; } else if (property == config->prop_fb_damage_clips) { *val = (state->fb_damage_clips) ? state->fb_damage_clips->base.id : 0; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 29d0fc1e52b5..83832adf3adf 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -602,6 +602,10 @@ EXPORT_SYMBOL(drm_plane_create_color_properties); * engine, thereby making the content as linear for further color * processing. * + * ctm_property: + * Blob property which allows a userspace to provide CTM coefficients + * to do color space conversion or any other enhancement by doing a + * matrix multiplication using the h/w CTM processing engine */ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, struct drm_plane *plane, @@ -623,6 +627,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
plane->degamma_lut_property = prop;
+ prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, + "PLANE_CTM", 0); + if (!prop) + return -ENOMEM; + + plane->ctm_property = prop; + return 0; } EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties); diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index bbd0033ed1d2..83d11918333b 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -252,6 +252,14 @@ struct drm_plane_state { */ struct drm_property_blob *degamma_lut;
+ /** + * @ctm: + * + * Color transformation matrix. See drm_plane_enable_color_mgmt(). The + * blob (if not NULL) is a &struct drm_color_ctm. + */ + struct drm_property_blob *ctm; + u8 color_mgmt_changed : 1; };
@@ -776,6 +784,13 @@ struct drm_plane { * used to convert the framebuffer's colors to linear gamma. */ struct drm_property *degamma_lut_property; + + /** + * @plane_ctm_property: Optional Plane property to set the + * matrix used to convert colors after the lookup in the + * degamma LUT. + */ + struct drm_property *ctm_property; };
#define obj_to_plane(x) container_of(x, struct drm_plane, base)
Add a DRM helper to attach ctm property.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/drm_color_mgmt.c | 10 ++++++++++ include/drm/drm_plane.h | 1 + 2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 83832adf3adf..5c3138497b9c 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -654,6 +654,16 @@ void drm_plane_attach_degamma_properties(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
+void drm_plane_attach_ctm_property(struct drm_plane *plane) +{ + if (!plane->ctm_property) + return; + + drm_object_attach_property(&plane->base, + plane->ctm_property, 0); +} +EXPORT_SYMBOL(drm_plane_attach_ctm_property); + int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, const char *name, const struct drm_color_lut_range *ranges, diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 83d11918333b..4557f59cf3cf 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -886,6 +886,7 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, struct drm_plane *plane, int num_values); void drm_plane_attach_degamma_properties(struct drm_plane *plane); +void drm_plane_attach_ctm_property(struct drm_plane *plane); int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, const char *name, const struct drm_color_lut_range *ranges,
Define Register macros for plane CSC.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ede7dca440e2..df8500a86e9d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7397,6 +7397,49 @@ enum { #define PLANE_COLOR_CTL(pipe, plane) \ _MMIO_PLANE(plane, _PLANE_COLOR_CTL_1(pipe), _PLANE_COLOR_CTL_2(pipe))
+/* Plane CSC Registers */ +#define _PLANE_CSC_RY_GY_1_A 0x70210 +#define _PLANE_CSC_RY_GY_2_A 0x70310 + +#define _PLANE_CSC_RY_GY_1_B 0x71210 +#define _PLANE_CSC_RY_GY_2_B 0x71310 + +#define _PLANE_CSC_RY_GY_1(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_1_A, \ + _PLANE_CSC_RY_GY_1_B) +#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \ + _PLANE_INPUT_CSC_RY_GY_2_B) +#define PLANE_CSC_COEFF(pipe, plane, index) _MMIO_PLANE(plane, \ + _PLANE_CSC_RY_GY_1(pipe) + (index) * 4, \ + _PLANE_CSC_RY_GY_2(pipe) + (index) * 4) + +#define _PLANE_CSC_PREOFF_HI_1_A 0x70228 +#define _PLANE_CSC_PREOFF_HI_2_A 0x70328 + +#define _PLANE_CSC_PREOFF_HI_1_B 0x71228 +#define _PLANE_CSC_PREOFF_HI_2_B 0x71328 + +#define _PLANE_CSC_PREOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_1_A, \ + _PLANE_CSC_PREOFF_HI_1_B) +#define _PLANE_CSC_PREOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_2_A, \ + _PLANE_CSC_PREOFF_HI_2_B) +#define PLANE_CSC_PREOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_PREOFF_HI_1(pipe) + \ + (index) * 4, _PLANE_CSC_PREOFF_HI_2(pipe) + \ + (index) * 4) + +#define _PLANE_CSC_POSTOFF_HI_1_A 0x70234 +#define _PLANE_CSC_POSTOFF_HI_2_A 0x70334 + +#define _PLANE_CSC_POSTOFF_HI_1_B 0x71234 +#define _PLANE_CSC_POSTOFF_HI_2_B 0x71334 + +#define _PLANE_CSC_POSTOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_1_A, \ + _PLANE_CSC_POSTOFF_HI_1_B) +#define _PLANE_CSC_POSTOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_2_A, \ + _PLANE_CSC_POSTOFF_HI_2_B) +#define PLANE_CSC_POSTOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_POSTOFF_HI_1(pipe) + \ + (index) * 4, _PLANE_CSC_POSTOFF_HI_2(pipe) + \ + (index) * 4) + #define _SEL_FETCH_PLANE_BASE_1_A 0x70890 #define _SEL_FETCH_PLANE_BASE_2_A 0x708B0 #define _SEL_FETCH_PLANE_BASE_3_A 0x708D0
Implement plane CSC for ICL+
Signed-off-by: Uma Shankar uma.shankar@intel.com --- .../gpu/drm/i915/display/intel_atomic_plane.c | 5 +- drivers/gpu/drm/i915/display/intel_color.c | 82 +++++++++++++++++++ .../drm/i915/display/skl_universal_plane.c | 4 + drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 5de9c98beaf6..ec7646790892 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -499,6 +499,7 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state, intel_atomic_get_new_crtc_state(state, crtc); struct skl_ddb_entry entries_y[I915_MAX_PLANES]; struct skl_ddb_entry entries_uv[I915_MAX_PLANES]; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); u32 update_mask = new_crtc_state->update_planes; struct intel_plane *plane;
@@ -513,8 +514,10 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state, struct intel_plane_state *new_plane_state = intel_atomic_get_new_plane_state(state, plane);
- if (new_plane_state->uapi.color_mgmt_changed) + if (new_plane_state->uapi.color_mgmt_changed) { intel_color_load_plane_luts(&new_plane_state->uapi); + dev_priv->display.load_plane_csc_matrix(&new_plane_state->uapi); + }
if (new_plane_state->uapi.visible || new_plane_state->planar_slave) { diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 6d57a47d8a60..8b4f653b213d 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2119,6 +2119,83 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state) } }
+static void icl_load_plane_csc_matrix(const struct drm_plane_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->plane->dev); + enum pipe pipe = to_intel_plane(state->plane)->pipe; + enum plane_id plane = to_intel_plane(state->plane)->id; + struct drm_color_ctm *ctm; + const u64 *input; + u16 coeffs[9] = {}; + u16 postoff = 0; + int i; + + if (!icl_is_hdr_plane(dev_priv, plane) || !state->ctm) + return; + + ctm = state->ctm->data; + input = ctm->matrix; + + /* + * Convert fixed point S31.32 input to format supported by the + * hardware. + */ + for (i = 0; i < ARRAY_SIZE(coeffs); i++) { + u64 abs_coeff = ((1ULL << 63) - 1) & input[i]; + + /* + * Clamp input value to min/max supported by + * hardware. + */ + abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1); + + /* sign bit */ + if (CTM_COEFF_NEGATIVE(input[i])) + coeffs[i] |= 1 << 15; + + if (abs_coeff < CTM_COEFF_0_125) + coeffs[i] |= (3 << 12) | + ILK_CSC_COEFF_FP(abs_coeff, 12); + else if (abs_coeff < CTM_COEFF_0_25) + coeffs[i] |= (2 << 12) | + ILK_CSC_COEFF_FP(abs_coeff, 11); + else if (abs_coeff < CTM_COEFF_0_5) + coeffs[i] |= (1 << 12) | + ILK_CSC_COEFF_FP(abs_coeff, 10); + else if (abs_coeff < CTM_COEFF_1_0) + coeffs[i] |= ILK_CSC_COEFF_FP(abs_coeff, 9); + else if (abs_coeff < CTM_COEFF_2_0) + coeffs[i] |= (7 << 12) | + ILK_CSC_COEFF_FP(abs_coeff, 8); + else + coeffs[i] |= (6 << 12) | + ILK_CSC_COEFF_FP(abs_coeff, 7); + } + + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 0), + coeffs[0] << 16 | coeffs[1]); + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 1), + coeffs[2] << 16); + + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 2), + coeffs[3] << 16 | coeffs[4]); + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 3), + coeffs[5] << 16); + + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 4), + coeffs[6] << 16 | coeffs[7]); + intel_de_write(dev_priv, PLANE_CSC_COEFF(pipe, plane, 5), + coeffs[8] << 16); + + intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 0), 0); + intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 1), 0); + intel_de_write(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 2), 0); + + intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 0), postoff); + intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 1), postoff); + intel_de_write(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 2), postoff); +} + /* FIXME input bpc? */ static const struct drm_color_lut_range d13_degamma_hdr[] = { /* segment 1 */ @@ -2343,7 +2420,12 @@ int intel_plane_color_init(struct drm_plane *plane) LUT_TYPE_DEGAMMA);
dev_priv->display.load_plane_luts = d13_plane_load_luts; + dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix; + drm_plane_attach_degamma_properties(plane); + + if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) + drm_plane_attach_ctm_property(plane); }
return ret; diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index aadb984fdf77..6ba670b6a5c9 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -965,6 +965,10 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, if (plane_state->uapi.degamma_lut) plane_color_ctl |= PLANE_PRE_CSC_GAMMA_ENABLE;
+ /* FIXME needs hw.ctm */ + if (plane_state->uapi.ctm) + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; + plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index df8500a86e9d..a8e35357aea0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7187,6 +7187,7 @@ enum { #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */ #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-ICL */ #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) +#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << 21) /* ICL+ */ #define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ #define PLANE_PRE_CSC_GAMMA_ENABLE (1 << 14)
Add Plane Gamma Mode as a blob property. This is an enum property with values as blob_id's and exposes the various gamma modes supported and the lut ranges. Getting the blob id in userspace, user can get the mode supported and also the range of gamma mode supported with number of lut coefficients. It can then set one of the modes using this enum property.
Lut values will be sent through a separate GAMMA_LUT blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com Signed-off-by: Bhanuprakash Modem bhanuprakash.modem@intel.com --- drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_color_mgmt.c | 26 ++++++++++++++++++++++++++ include/drm/drm_plane.h | 14 ++++++++++++++ 3 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 6af2afe362ff..6e3958491d10 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -612,6 +612,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, &replaced); state->color_mgmt_changed |= replaced; return ret; + } else if (property == plane->gamma_mode_property) { + state->gamma_mode = val; } else if (property == config->prop_fb_damage_clips) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->fb_damage_clips, @@ -685,6 +687,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, state->degamma_lut->base.id : 0; } else if (property == plane->ctm_property) { *val = (state->ctm) ? state->ctm->base.id : 0; + } else if (property == plane->gamma_mode_property) { + *val = state->gamma_mode; } else if (property == config->prop_fb_damage_clips) { *val = (state->fb_damage_clips) ? state->fb_damage_clips->base.id : 0; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 5c3138497b9c..02367e691cf3 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -606,6 +606,13 @@ EXPORT_SYMBOL(drm_plane_create_color_properties); * Blob property which allows a userspace to provide CTM coefficients * to do color space conversion or any other enhancement by doing a * matrix multiplication using the h/w CTM processing engine + * + * gamma_mode_property: + * Blob property which advertizes the possible gamma modes and + * lut ranges supported by the platform. This allows userspace + * to query and get the plane gamma color caps and choose the + * appropriate gamma mode and create lut values accordingly + * */ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, struct drm_plane *plane, @@ -634,6 +641,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
plane->ctm_property = prop;
+ prop = drm_property_create(dev, DRM_MODE_PROP_ENUM, + "PLANE_GAMMA_MODE", num_values); + if (!prop) + return -ENOMEM; + + plane->gamma_mode_property = prop; + return 0; } EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties); @@ -664,6 +678,16 @@ void drm_plane_attach_ctm_property(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_attach_ctm_property);
+void drm_plane_attach_gamma_properties(struct drm_plane *plane) +{ + if (!plane->gamma_mode_property) + return; + + drm_object_attach_property(&plane->base, + plane->gamma_mode_property, 0); +} +EXPORT_SYMBOL(drm_plane_attach_gamma_properties); + int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, const char *name, const struct drm_color_lut_range *ranges, @@ -676,6 +700,8 @@ int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
if (type == LUT_TYPE_DEGAMMA) prop = plane->degamma_mode_property; + else + prop = plane->gamma_mode_property;
if (!prop) return -EINVAL; diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 4557f59cf3cf..a7b7c8599702 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -260,6 +260,13 @@ struct drm_plane_state { */ struct drm_property_blob *ctm;
+ /** + * @gamma_mode: This is a blob_id and exposes the platform capabilities + * wrt to various gamma modes and the respective lut ranges. This also + * helps user select a gamma mode amongst the supported ones. + */ + u32 gamma_mode; + u8 color_mgmt_changed : 1; };
@@ -791,6 +798,12 @@ struct drm_plane { * degamma LUT. */ struct drm_property *ctm_property; + + /** + * @gamma_mode_property: Optional Plane property to set the LUT + * used to convert the framebuffer's colors to non-linear gamma. + */ + struct drm_property *gamma_mode_property; };
#define obj_to_plane(x) container_of(x, struct drm_plane, base) @@ -887,6 +900,7 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, int num_values); void drm_plane_attach_degamma_properties(struct drm_plane *plane); void drm_plane_attach_ctm_property(struct drm_plane *plane); +void drm_plane_attach_gamma_properties(struct drm_plane *plane); int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane, const char *name, const struct drm_color_lut_range *ranges,
Add Plane Gamma Lut as a blob property.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/drm_atomic_state_helper.c | 3 +++ drivers/gpu/drm/drm_atomic_uapi.c | 10 ++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 18 ++++++++++++++++++ include/drm/drm_plane.h | 14 ++++++++++++++ 4 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index fafb8af1c9cb..7ddf6e4b956b 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -316,6 +316,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, drm_property_blob_get(state->degamma_lut); if (state->ctm) drm_property_blob_get(state->ctm); + if (state->gamma_lut) + drm_property_blob_get(state->gamma_lut);
state->color_mgmt_changed = false; } @@ -366,6 +368,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state) drm_property_blob_put(state->fb_damage_clips); drm_property_blob_put(state->degamma_lut); drm_property_blob_put(state->ctm); + drm_property_blob_put(state->gamma_lut); } EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 6e3958491d10..4f5b7f76208d 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -614,6 +614,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, return ret; } else if (property == plane->gamma_mode_property) { state->gamma_mode = val; + } else if (property == plane->gamma_lut_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->gamma_lut, + val, -1, sizeof(struct drm_color_lut_ext), + &replaced); + state->color_mgmt_changed |= replaced; + return ret; } else if (property == config->prop_fb_damage_clips) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->fb_damage_clips, @@ -689,6 +696,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane, *val = (state->ctm) ? state->ctm->base.id : 0; } else if (property == plane->gamma_mode_property) { *val = state->gamma_mode; + } else if (property == plane->gamma_lut_property) { + *val = (state->gamma_lut) ? + state->gamma_lut->base.id : 0; } else if (property == config->prop_fb_damage_clips) { *val = (state->fb_damage_clips) ? state->fb_damage_clips->base.id : 0; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 02367e691cf3..b5b3ff7f654d 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -613,6 +613,11 @@ EXPORT_SYMBOL(drm_plane_create_color_properties); * to query and get the plane gamma color caps and choose the * appropriate gamma mode and create lut values accordingly * + * gamma_lut_property: + * Blob property which allows a userspace to provide LUT values + * to apply gamma curve using the h/w plane degamma processing + * engine, thereby making the content as non-linear. + * */ int drm_plane_create_color_mgmt_properties(struct drm_device *dev, struct drm_plane *plane, @@ -648,6 +653,13 @@ int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
plane->gamma_mode_property = prop;
+ prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, + "PLANE_GAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + + plane->gamma_lut_property = prop; + return 0; } EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties); @@ -685,6 +697,12 @@ void drm_plane_attach_gamma_properties(struct drm_plane *plane)
drm_object_attach_property(&plane->base, plane->gamma_mode_property, 0); + + if (!plane->gamma_lut_property) + return; + + drm_object_attach_property(&plane->base, + plane->gamma_lut_property, 0); } EXPORT_SYMBOL(drm_plane_attach_gamma_properties);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index a7b7c8599702..8989bb1aa46c 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -267,6 +267,14 @@ struct drm_plane_state { */ u32 gamma_mode;
+ /* @gamma_lut: + * + * Lookup table for converting framebuffer pixel data after applying the + * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The + * blob (if not NULL) is an array of &struct drm_color_lut_ext. + */ + struct drm_property_blob *gamma_lut; + u8 color_mgmt_changed : 1; };
@@ -804,6 +812,12 @@ struct drm_plane { * used to convert the framebuffer's colors to non-linear gamma. */ struct drm_property *gamma_mode_property; + + /** + * @gamma_lut_property: Optional Plane property to set the LUT + * used to convert the framebuffer's colors to non-linear gamma. + */ + struct drm_property *gamma_lut_property; };
#define obj_to_plane(x) container_of(x, struct drm_plane, base)
Define the structure with XE_LPD gamma lut ranges. HDR and SDR planes have different capabilities, implemented respective structure for the HDR planes. Degamma and GAMMA has same Lut caps for SDR planes, extended the same.
Initialize the mode range caps as well.
Signed-off-by: Uma Shankar uma.shankar@intel.com Signed-off-by: Bhanuprakash Modem bhanuprakash.modem@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 112 ++++++++++++++++++--- 1 file changed, 99 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8b4f653b213d..7f091dd0bb19 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2248,7 +2248,7 @@ static const struct drm_color_lut_range d13_degamma_hdr[] = { };
/* FIXME input bpc? */ -static const struct drm_color_lut_range d13_degamma_sdr[] = { +static const struct drm_color_lut_range d13_gamma_degamma_sdr[] = { /* segment 1 */ { .flags = (DRM_MODE_LUT_GAMMA | @@ -2298,6 +2298,63 @@ static const struct drm_color_lut_range d13_degamma_sdr[] = { }, };
+ /* FIXME input bpc? */ +static const struct drm_color_lut_range d13_gamma_hdr[] = { + /* + * ToDo: Add Segment 1 + * There is an optional fine segment added with 9 lut values + * Will be added later + */ + + /* segment 2 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_NON_DECREASING), + .count = 32, + .input_bpc = 24, .output_bpc = 16, + .start = 0, .end = (1 << 24) - 1, + .min = 0, .max = (1 << 24) - 1, + }, + /* segment 3 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = (1 << 24) - 1, .end = 1 << 24, + .min = 0, .max = 1 << 24, + }, + /* Segment 4 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = 1 << 24, .end = 3 << 24, + .min = 0, .max = (3 << 24), + }, + /* Segment 5 */ + { + .flags = (DRM_MODE_LUT_GAMMA | + DRM_MODE_LUT_REFLECT_NEGATIVE | + DRM_MODE_LUT_INTERPOLATE | + DRM_MODE_LUT_REUSE_LAST | + DRM_MODE_LUT_NON_DECREASING), + .count = 1, + .input_bpc = 24, .output_bpc = 16, + .start = 3 << 24, .end = 7 << 24, + .min = 0, .max = (7 << 24), + }, +}; + static void d13_program_plane_degamma_lut(const struct drm_plane_state *state, struct drm_color_lut_ext *degamma_lut, u32 offset) @@ -2407,26 +2464,55 @@ int intel_plane_color_init(struct drm_plane *plane) ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no degamma", NULL, 0, LUT_TYPE_DEGAMMA); - if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) + if (ret) + return ret; + + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "no gamma", + NULL, 0, + LUT_TYPE_GAMMA); + if (ret) + return ret; + + if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) { ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma", d13_degamma_hdr, sizeof(d13_degamma_hdr), LUT_TYPE_DEGAMMA); - else - ret = drm_plane_color_add_gamma_degamma_mode_range(plane, - "plane degamma", - d13_degamma_sdr, - sizeof(d13_degamma_sdr), + if (ret) + return ret; + + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane gamma", + d13_gamma_hdr, + sizeof(d13_gamma_hdr), + LUT_TYPE_GAMMA); + if (ret) + return ret; + } else { + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane degamma", + d13_gamma_degamma_sdr, + sizeof(d13_gamma_degamma_sdr), LUT_TYPE_DEGAMMA); + if (ret) + return ret; + + ret = drm_plane_color_add_gamma_degamma_mode_range(plane, "plane gamma", + d13_gamma_degamma_sdr, + sizeof(d13_gamma_degamma_sdr), + LUT_TYPE_GAMMA); + if (ret) + return ret; + } + }
- dev_priv->display.load_plane_luts = d13_plane_load_luts; - dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix; + dev_priv->display.load_plane_luts = d13_plane_load_luts; + dev_priv->display.load_plane_csc_matrix = icl_load_plane_csc_matrix;
- drm_plane_attach_degamma_properties(plane); + drm_plane_attach_degamma_properties(plane);
- if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) - drm_plane_attach_ctm_property(plane); - } + if (icl_is_hdr_plane(dev_priv, to_intel_plane(plane)->id)) + drm_plane_attach_ctm_property(plane); + + drm_plane_attach_gamma_properties(plane);
return ret; }
Add macros to define Plane Gamma registers
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 73 +++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a8e35357aea0..2ebc92104f64 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -11398,6 +11398,79 @@ enum skl_power_gate { _MMIO_PLANE_GAMC(plane, i, _PLANE_PRE_CSC_GAMC_DATA_1(pipe), \ _PLANE_PRE_CSC_GAMC_DATA_2(pipe))
+/* Display13 Plane Gamma Reg */ +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_A 0x70160 +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_B 0x71160 +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_A 0x70260 +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_B 0x71260 +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_A, \ + _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1_B) +#define _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_A, \ + _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2_B) +#define PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_1(pipe), \ + _PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH_2(pipe)) + +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_A 0x70164 +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_B 0x71164 +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_A 0x70264 +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_B 0x71264 +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_A, \ + _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1_B) +#define _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_A, \ + _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2_B) +#define PLANE_POST_CSC_GAMC_SEG0_DATA_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_1(pipe), \ + _PLANE_POST_CSC_GAMC_SEG0_DATA_ENH_2(pipe)) + +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1_A 0x701d8 +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1_B 0x711d8 +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2_A 0x702d8 +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2_B 0x712d8 +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_ENH_1_A, \ + _PLANE_POST_CSC_GAMC_INDEX_ENH_1_B) +#define _PLANE_POST_CSC_GAMC_INDEX_ENH_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_ENH_2_A, \ + _PLANE_POST_CSC_GAMC_INDEX_ENH_2_B) +#define PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_INDEX_ENH_1(pipe), \ + _PLANE_POST_CSC_GAMC_INDEX_ENH_2(pipe)) + +#define _PLANE_POST_CSC_GAMC_DATA_ENH_1_A 0x701dc +#define _PLANE_POST_CSC_GAMC_DATA_ENH_1_B 0x711dc +#define _PLANE_POST_CSC_GAMC_DATA_ENH_2_A 0x702dc +#define _PLANE_POST_CSC_GAMC_DATA_ENH_2_B 0x712dc +#define _PLANE_POST_CSC_GAMC_DATA_ENH_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_ENH_1_A, \ + _PLANE_POST_CSC_GAMC_DATA_ENH_1_B) +#define _PLANE_POST_CSC_GAMC_DATA_ENH_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_ENH_2_A, \ + _PLANE_POST_CSC_GAMC_DATA_ENH_2_B) +#define PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_DATA_ENH_1(pipe), \ + _PLANE_POST_CSC_GAMC_DATA_ENH_2(pipe)) + +#define _PLANE_POST_CSC_GAMC_INDEX_1_A 0x704d8 +#define _PLANE_POST_CSC_GAMC_INDEX_1_B 0x714d8 +#define _PLANE_POST_CSC_GAMC_INDEX_2_A 0x705d8 +#define _PLANE_POST_CSC_GAMC_INDEX_2_B 0x715d8 +#define _PLANE_POST_CSC_GAMC_INDEX_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_1_A, \ + _PLANE_POST_CSC_GAMC_INDEX_1_B) +#define _PLANE_POST_CSC_GAMC_INDEX_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_INDEX_2_A, \ + _PLANE_POST_CSC_GAMC_INDEX_2_B) +#define PLANE_POST_CSC_GAMC_INDEX(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_INDEX_1(pipe), \ + _PLANE_POST_CSC_GAMC_INDEX_2(pipe)) + +#define _PLANE_POST_CSC_GAMC_DATA_1_A 0x704dc +#define _PLANE_POST_CSC_GAMC_DATA_1_B 0x714dc +#define _PLANE_POST_CSC_GAMC_DATA_2_A 0x705dc +#define _PLANE_POST_CSC_GAMC_DATA_2_B 0x715dc +#define _PLANE_POST_CSC_GAMC_DATA_1(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_1_A, \ + _PLANE_POST_CSC_GAMC_DATA_1_B) +#define _PLANE_POST_CSC_GAMC_DATA_2(pipe) _PIPE(pipe, _PLANE_POST_CSC_GAMC_DATA_2_A, \ + _PLANE_POST_CSC_GAMC_DATA_2_B) +#define PLANE_POST_CSC_GAMC_DATA(pipe, plane, i) \ + _MMIO_PLANE_GAMC(plane, i, _PLANE_POST_CSC_GAMC_DATA_1(pipe), \ + _PLANE_POST_CSC_GAMC_DATA_2(pipe)) + /* pipe CSC & degamma/gamma LUTs on CHV */ #define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) #define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904)
Extract the LUT and program plane gamma registers. Enabled multi segmented lut as well.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/intel_color.c | 89 ++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 9 ++- 2 files changed, 94 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 7f091dd0bb19..daf2148fb2df 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -27,6 +27,9 @@ #include "intel_de.h" #include "intel_display_types.h" #include "intel_sprite.h" + +#include "skl_universal_plane.h" + #include <drm/drm_plane.h>
#define CTM_COEFF_SIGN (1ULL << 63) @@ -2434,16 +2437,102 @@ static void d13_program_plane_degamma_lut(const struct drm_plane_state *state, } }
+static void d13_program_plane_gamma_lut(const struct drm_plane_state *state, + struct drm_color_lut_ext *gamma_lut, + u32 offset) +{ + struct drm_i915_private *dev_priv = to_i915(state->plane->dev); + enum pipe pipe = to_intel_plane(state->plane)->pipe; + enum plane_id plane = to_intel_plane(state->plane)->id; + u32 i, lut_size; + + if (icl_is_hdr_plane(dev_priv, plane)) { + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0), + offset | PLANE_PAL_PREC_AUTO_INCREMENT); + if (gamma_lut) { + lut_size = 32; + for (i = 0; i < lut_size; i++) { + u64 word = drm_color_lut_extract_ext(gamma_lut[i].green, 24); + u32 lut_val = (word & 0xffffff); + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), + lut_val); + } + + do { + /* Program the max register to clamp values > 1.0. */ + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), + gamma_lut[i].green); + } while (i++ < 34); + } else { + lut_size = 32; + for (i = 0; i < lut_size; i++) { + u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1); + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), v); + } + + do { + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), + 1 << 24); + } while (i++ < 34); + } + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0); + } else { + lut_size = 32; + /* + * First 3 planes are HDR, so reduce by 3 to get to the right + * SDR plane offset + */ + plane = plane - 3; + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0), + offset | PLANE_PAL_PREC_AUTO_INCREMENT); + + if (gamma_lut) { + for (i = 0; i < lut_size; i++) + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), + gamma_lut[i].green & 0xffff); + /* Program the max register to clamp values > 1.0. */ + while (i < 35) + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), + gamma_lut[i++].green & 0x3ffff); + } else { + for (i = 0; i < lut_size; i++) { + u32 v = (i * ((1 << 16) - 1)) / (lut_size - 1); + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), v); + } + + do { + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), + (1 << 16)); + } while (i++ < 34); + } + + intel_de_write(dev_priv, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0), 0); + } +} + static void d13_plane_load_luts(const struct drm_plane_state *plane_state) { const struct drm_property_blob *degamma_lut_blob = plane_state->degamma_lut; + const struct drm_property_blob *gamma_lut_blob = + plane_state->gamma_lut; struct drm_color_lut_ext *degamma_lut = NULL; + struct drm_color_lut_ext *gamma_lut = NULL;
if (degamma_lut_blob) { degamma_lut = degamma_lut_blob->data; d13_program_plane_degamma_lut(plane_state, degamma_lut, 0); } + + if (gamma_lut_blob) { + gamma_lut = gamma_lut_blob->data; + d13_program_plane_gamma_lut(plane_state, gamma_lut, 0); + } }
void intel_color_load_plane_luts(const struct drm_plane_state *plane_state) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2ebc92104f64..6ab4adbe7f56 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7187,10 +7187,11 @@ enum { #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */ #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-ICL */ #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) -#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << 21) /* ICL+ */ -#define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ -#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ -#define PLANE_PRE_CSC_GAMMA_ENABLE (1 << 14) +#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << 21) /* ICL+ */ +#define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ +#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ +#define PLANE_POST_CSC_GAMMA_MULTI_SEGMENT_ENABLE (1 << 15) +#define PLANE_PRE_CSC_GAMMA_ENABLE (1 << 14) #define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17) #define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601 (1 << 17) #define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709 (2 << 17)
Enable plane gamma feature in check callbacks. Decide based on the user input.
Signed-off-by: Uma Shankar uma.shankar@intel.com --- drivers/gpu/drm/i915/display/skl_universal_plane.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 6ba670b6a5c9..5d527d12ec45 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -959,7 +959,9 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); u32 plane_color_ctl = 0;
- plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE; + /* FIXME needs hw.gamma_lut */ + if (!plane_state->uapi.gamma_lut) + plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
/* FIXME needs hw.degamma_lut */ if (plane_state->uapi.degamma_lut)
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like: +-------------------------------------------+ | RAM | | +------+ +---------+ +---------+ | | | FB 1 | | FB 2 | | FB N | | | +------+ +---------+ +---------+ | +-------------------------------------------+ | Plane Color Hardware Block | +--------------------------------------------+ | +---v-----+ +---v-------+ +---v------+ | | | Plane A | | Plane B | | Plane N | | | | DeGamma | | Degamma | | Degamma | | | +---+-----+ +---+-------+ +---+------+ | | | | | | | +---v-----+ +---v-------+ +---v------+ | | |Plane A | | Plane B | | Plane N | | | |CSC/CTM | | CSC/CTM | | CSC/CTM | | | +---+-----+ +----+------+ +----+-----+ | | | | | | | +---v-----+ +----v------+ +----v-----+ | | | Plane A | | Plane B | | Plane N | | | | Gamma | | Gamma | | Gamma | | | +---+-----+ +----+------+ +----+-----+ | | | | | | +--------------------------------------------+ +------v--------------v---------------v-------| || || || Pipe Blender || +--------------------+------------------------+ | | | | +-----------v----------+ | | | Pipe DeGamma | | | | | | | +-----------+----------+ | | | Pipe Color | | +-----------v----------+ Hardware | | | Pipe CSC/CTM | | | | | | | +-----------+----------+ | | | | | +-----------v----------+ | | | Pipe Gamma | | | | | | | +-----------+----------+ | | | | +---------------------------------------------+ | v Pipe Output
Hi,
this is an excellent picture. I have long been wanting schematics like that in the DRM UAPI documentation. Another note on that: https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html
But the schematic for DRM UAPI documentation needs to be written in terms of the abstract KMS pipeline with property names spelled out, like in what Ville sketched in that email.
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html
Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
Thanks, pq
Userspace can take smart blending decisions and utilize these hardware supported plane color features to get accurate color profile. The same can help in consistent color quality from source to panel taking advantage of advanced color features in hardware.
These patches add the property interfaces and enable helper functions. This series adds Intel's XE_LPD hw specific plane gamma feature. We can build up and add other platform/hardware specific implementation on top of this series.
Credits: Special mention and credits to Ville Syrjala for coming up with a design for this feature and inputs. This series is based on his original design and idea.
Note: Userspace support for this new UAPI will be done on Chrome. We will notify the list once we have that ready for review.
ToDo: State readout for this feature will be added next.
Uma Shankar (21): drm: Add Enhanced Gamma and color lut range attributes drm: Add Plane Degamma Mode property drm: Add Plane Degamma Lut property drm/i915/xelpd: Define Degamma Lut range struct for HDR planes drm/i915/xelpd: Add register definitions for Plane Degamma drm/i915/xelpd: Enable plane color features drm/i915/xelpd: Add color capabilities of SDR planes drm/i915/xelpd: Program Plane Degamma Registers drm/i915/xelpd: Add plane color check to glk_plane_color_ctl drm/i915/xelpd: Initialize plane color features drm/i915/xelpd: Load plane color luts from atomic flip drm: Add Plane CTM property drm: Add helper to attach Plane ctm property drm/i915/xelpd: Define Plane CSC Registers drm/i915/xelpd: Enable Plane CSC drm: Add Plane Gamma Mode property drm: Add Plane Gamma Lut property drm/i915/xelpd: Define and Initialize Plane Gamma Lut range drm/i915/xelpd: Add register definitions for Plane Gamma drm/i915/xelpd: Program Plane Gamma Registers drm/i915/xelpd: Enable plane gamma
Documentation/gpu/drm-kms.rst | 90 +++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 12 + drivers/gpu/drm/drm_atomic_uapi.c | 38 ++ drivers/gpu/drm/drm_color_mgmt.c | 177 +++++- .../gpu/drm/i915/display/intel_atomic_plane.c | 6 + .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_color.c | 513 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_color.h | 2 + .../drm/i915/display/skl_universal_plane.c | 15 +- drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 176 +++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 81 +++ include/uapi/drm/drm_mode.h | 58 ++ 15 files changed, 1170 insertions(+), 6 deletions(-)
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 2:59 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com; Harry Wentland harry.wentland@amd.com Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like: +-------------------------------------------+ | RAM | | +------+ +---------+ +---------+ | | | FB 1 | | FB 2 | | FB N | | | +------+ +---------+ +---------+ | +-------------------------------------------+ | Plane Color Hardware Block | +--------------------------------------------+ | +---v-----+ +---v-------+ +---v------+ | | | Plane A | | Plane B | | Plane N | | | | DeGamma | | Degamma | | Degamma | | | +---+-----+ +---+-------+ +---+------+ | | | | | | | +---v-----+ +---v-------+ +---v------+ | | |Plane A | | Plane B | | Plane N | | | |CSC/CTM | | CSC/CTM | | CSC/CTM | | | +---+-----+ +----+------+ +----+-----+ | | | | | | | +---v-----+ +----v------+ +----v-----+ | | | Plane A | | Plane B | | Plane N | | | | Gamma | | Gamma | | Gamma | | | +---+-----+ +----+------+ +----+-----+ | | | | | | +--------------------------------------------+ +------v--------------v---------------v-------| || || || Pipe Blender || +--------------------+------------------------+ | | | | +-----------v----------+ | | | Pipe DeGamma | | | | | | | +-----------+----------+ | | | Pipe Color | | +-----------v----------+ Hardware | | | Pipe CSC/CTM | | | | | | | +-----------+----------+ | | | | | +-----------v----------+ | | | Pipe Gamma | | | | | | | +-----------+----------+ | | | | +---------------------------------------------+ | v Pipe Output
Hi,
this is an excellent picture. I have long been wanting schematics like that in the DRM UAPI documentation. Another note on that: https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html
But the schematic for DRM UAPI documentation needs to be written in terms of the abstract KMS pipeline with property names spelled out, like in what Ville sketched in that email.
Sure Pekka, I can add that.
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html
Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
This is a good suggestion to have the whole pipeline and properties documented along with the exact usages. We may end with 2 properties almost doing similar work but needed due to underlying hardware, but we can get that properly documented and defined.
I will discuss with Harry and Ville as well to define this.
Regards, Uma Shankar
Thanks, pq
Userspace can take smart blending decisions and utilize these hardware supported plane color features to get accurate color profile. The same can help in consistent color quality from source to panel taking advantage of advanced color features in hardware.
These patches add the property interfaces and enable helper functions. This series adds Intel's XE_LPD hw specific plane gamma feature. We can build up and add other platform/hardware specific implementation on top of this series.
Credits: Special mention and credits to Ville Syrjala for coming up with a design for this feature and inputs. This series is based on his original design and idea.
Note: Userspace support for this new UAPI will be done on Chrome. We will notify the list once we have that ready for review.
ToDo: State readout for this feature will be added next.
Uma Shankar (21): drm: Add Enhanced Gamma and color lut range attributes drm: Add Plane Degamma Mode property drm: Add Plane Degamma Lut property drm/i915/xelpd: Define Degamma Lut range struct for HDR planes drm/i915/xelpd: Add register definitions for Plane Degamma drm/i915/xelpd: Enable plane color features drm/i915/xelpd: Add color capabilities of SDR planes drm/i915/xelpd: Program Plane Degamma Registers drm/i915/xelpd: Add plane color check to glk_plane_color_ctl drm/i915/xelpd: Initialize plane color features drm/i915/xelpd: Load plane color luts from atomic flip drm: Add Plane CTM property drm: Add helper to attach Plane ctm property drm/i915/xelpd: Define Plane CSC Registers drm/i915/xelpd: Enable Plane CSC drm: Add Plane Gamma Mode property drm: Add Plane Gamma Lut property drm/i915/xelpd: Define and Initialize Plane Gamma Lut range drm/i915/xelpd: Add register definitions for Plane Gamma drm/i915/xelpd: Program Plane Gamma Registers drm/i915/xelpd: Enable plane gamma
Documentation/gpu/drm-kms.rst | 90 +++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 12 + drivers/gpu/drm/drm_atomic_uapi.c | 38 ++ drivers/gpu/drm/drm_color_mgmt.c | 177 +++++- .../gpu/drm/i915/display/intel_atomic_plane.c | 6 + .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_color.c | 513 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_color.h | 2 + .../drm/i915/display/skl_universal_plane.c | 15 +- drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 176 +++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 81 +++ include/uapi/drm/drm_mode.h | 58 ++ 15 files changed, 1170 insertions(+), 6 deletions(-)
On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 2:59 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com; Harry Wentland harry.wentland@amd.com Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like: +-------------------------------------------+ | RAM | | +------+ +---------+ +---------+ | | | FB 1 | | FB 2 | | FB N | | | +------+ +---------+ +---------+ | +-------------------------------------------+ | Plane Color Hardware Block | +--------------------------------------------+ | +---v-----+ +---v-------+ +---v------+ | | | Plane A | | Plane B | | Plane N | | | | DeGamma | | Degamma | | Degamma | | | +---+-----+ +---+-------+ +---+------+ | | | | | | | +---v-----+ +---v-------+ +---v------+ | | |Plane A | | Plane B | | Plane N | | | |CSC/CTM | | CSC/CTM | | CSC/CTM | | | +---+-----+ +----+------+ +----+-----+ | | | | | | | +---v-----+ +----v------+ +----v-----+ | | | Plane A | | Plane B | | Plane N | | | | Gamma | | Gamma | | Gamma | | | +---+-----+ +----+------+ +----+-----+ | | | | | | +--------------------------------------------+ +------v--------------v---------------v-------| || || || Pipe Blender || +--------------------+------------------------+ | | | | +-----------v----------+ | | | Pipe DeGamma | | | | | | | +-----------+----------+ | | | Pipe Color | | +-----------v----------+ Hardware | | | Pipe CSC/CTM | | | | | | | +-----------+----------+ | | | | | +-----------v----------+ | | | Pipe Gamma | | | | | | | +-----------+----------+ | | | | +---------------------------------------------+ | v Pipe Output
Hi,
this is an excellent picture. I have long been wanting schematics like that in the DRM UAPI documentation. Another note on that: https://lists.freedesktop.org/archives/dri-devel/2021-May/307310.html%3E%3E%... But the schematic for DRM UAPI documentation needs to be written in terms of the abstract KMS pipeline with property names spelled out, like in what Ville sketched in that email.
Sure Pekka, I can add that.
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html%3E%3E%... Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
This is a good suggestion to have the whole pipeline and properties documented along with the exact usages. We may end with 2 properties almost doing similar work but needed due to underlying hardware, but we can get that properly documented and defined.
I will discuss with Harry and Ville as well to define this.
Just wanted to let you know that I've seen and read through both of Shankar's patchsets and had some thoughts but haven't found the time to respond. I will respond soon.
I very much agree with Pekka. We need to make sure this all plays well together and is well documented. Maybe a library to deal with DRM KMS color management/HDR would even be helpful. Not sure yet how I feel about that.
Harry
Regards, Uma Shankar
Thanks, pq
Userspace can take smart blending decisions and utilize these hardware supported plane color features to get accurate color profile. The same can help in consistent color quality from source to panel taking advantage of advanced color features in hardware.
These patches add the property interfaces and enable helper functions. This series adds Intel's XE_LPD hw specific plane gamma feature. We can build up and add other platform/hardware specific implementation on top of this series.
Credits: Special mention and credits to Ville Syrjala for coming up with a design for this feature and inputs. This series is based on his original design and idea.
Note: Userspace support for this new UAPI will be done on Chrome. We will notify the list once we have that ready for review.
ToDo: State readout for this feature will be added next.
Uma Shankar (21): drm: Add Enhanced Gamma and color lut range attributes drm: Add Plane Degamma Mode property drm: Add Plane Degamma Lut property drm/i915/xelpd: Define Degamma Lut range struct for HDR planes drm/i915/xelpd: Add register definitions for Plane Degamma drm/i915/xelpd: Enable plane color features drm/i915/xelpd: Add color capabilities of SDR planes drm/i915/xelpd: Program Plane Degamma Registers drm/i915/xelpd: Add plane color check to glk_plane_color_ctl drm/i915/xelpd: Initialize plane color features drm/i915/xelpd: Load plane color luts from atomic flip drm: Add Plane CTM property drm: Add helper to attach Plane ctm property drm/i915/xelpd: Define Plane CSC Registers drm/i915/xelpd: Enable Plane CSC drm: Add Plane Gamma Mode property drm: Add Plane Gamma Lut property drm/i915/xelpd: Define and Initialize Plane Gamma Lut range drm/i915/xelpd: Add register definitions for Plane Gamma drm/i915/xelpd: Program Plane Gamma Registers drm/i915/xelpd: Enable plane gamma
Documentation/gpu/drm-kms.rst | 90 +++ drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_atomic_state_helper.c | 12 + drivers/gpu/drm/drm_atomic_uapi.c | 38 ++ drivers/gpu/drm/drm_color_mgmt.c | 177 +++++- .../gpu/drm/i915/display/intel_atomic_plane.c | 6 + .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_color.c | 513 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_color.h | 2 + .../drm/i915/display/skl_universal_plane.c | 15 +- drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 176 +++++- include/drm/drm_mode_object.h | 2 +- include/drm/drm_plane.h | 81 +++ include/uapi/drm/drm_mode.h | 58 ++ 15 files changed, 1170 insertions(+), 6 deletions(-)
On Wed, 2 Jun 2021 19:42:19 -0400 Harry Wentland harry.wentland@amd.com wrote:
On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 2:59 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com; Harry Wentland harry.wentland@amd.com Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like:
...
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html%3E%3E%... Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
This is a good suggestion to have the whole pipeline and properties documented along with the exact usages. We may end with 2 properties almost doing similar work but needed due to underlying hardware, but we can get that properly documented and defined.
I will discuss with Harry and Ville as well to define this.
Just wanted to let you know that I've seen and read through both of Shankar's patchsets and had some thoughts but haven't found the time to respond. I will respond soon.
Hi Harry,
awesome!
I very much agree with Pekka. We need to make sure this all plays well together and is well documented. Maybe a library to deal with DRM KMS color management/HDR would even be helpful. Not sure yet how I feel about that.
That is an excellent question. While I am working on Weston CM&HDR, I already have issues with how to represent the color related transformations. These new hardware features exposed here are nothing I have prepared for, and would probably need changes to accommodate.
The main Weston roadmap is drafted in https://gitlab.freedesktop.org/wayland/weston/-/issues/467
The MR that introduces the concept of a color transformation, and also the whole beginnings of color management, is https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582
In that MR, there is a patch introducing struct weston_color_transform: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?com...
The design idea there is that libweston shall have what I call "color manager" module. That module handles all the policy decisions about color, it uses a CMM (Little CMS 2 in this case) for all the color profile computations, and based on all information it has available from display EDID, ICC profile files, Wayland clients via the CM&HDR protocol extension and more, it will ultimately produce weston_color_transform objects.
weston_color_transform is a complete description of how to map a pixel in one color model/space/encoding into another, maybe with user preferred tuning/tone-mapping. E.g. from client content to the output's blending space (output space but light-linear), or from output's blending space to output's framebuffer space or maybe even monitor wire space.
The mapping described by weston_color_transform shall be implemented by libweston's GL-renderer or by the DRM-backend using KMS properties, whatever works for each case. So the description cannot be opaque, it has to map to GLSL shaders (easy) and KMS properties (???).
Now the problem is, what should weston_color_transform look like?
The current design has two steps in a color transform: - Transfer function: identity, the traditional set of three 1D LUTs, or something else. - Color mapping: identity, a 3D LUT, or something else.
"Something else" is a placeholder for whatever we want to have, but the problem in adding new types of transfer function or color mapping representations (e.g. the fancy new GAMMA_MODEs) is how will the color manager create the parameters for those?
If we have ICC profiles as the original data, then we are probably limited to what LCMS2 can produce. The issue with ICC profiles is that they may contain 3D LUTs themselves, so not what I would call a parametric model. OTOH, if we have, say, enumerated operations defined by various HDR standards, we have to code those ourselves and then producing whatever fancy representation is less of a problem.
Maybe that is how it has to be. If the color transformations are defined by ICC profiles, we might be stuck with old-school KMS color properties, but HDR stuff that doesn't rely on ICC can use the fancier KMS properties. I'm sure interesting questions will arise when e.g. you have the monitor in HDR mode, described with standard HDR terms, and then you have application content described with an ICC profile (maybe SDR, maybe not).
We can always get a 3D LUT out of LCMS2, so theoretically it would be possible to get a huge LUT and then optimise whatever parameterised model you have to that data set. But I worry that might be too costly to do in-flight, at least in a way that blocks the compositor. Maybe do what I hear shader compilers do: produce an unoptimal model fast, then compute an optimised model asynchronously and replace when ready. And disk cache(?).
A library probably makes sense in the long run, but for now, I would have no idea at all what it should look like.
Thanks, pq
On 2021-06-03 10:47, Pekka Paalanen wrote:
On Wed, 2 Jun 2021 19:42:19 -0400 Harry Wentland harry.wentland@amd.com wrote:
On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 2:59 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com; Harry Wentland harry.wentland@amd.com Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like:
...
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html%3E%3E%... Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
This is a good suggestion to have the whole pipeline and properties documented along with the exact usages. We may end with 2 properties almost doing similar work but needed due to underlying hardware, but we can get that properly documented and defined.
I will discuss with Harry and Ville as well to define this.
Just wanted to let you know that I've seen and read through both of Shankar's patchsets and had some thoughts but haven't found the time to respond. I will respond soon.
Hi Harry,
awesome!
I very much agree with Pekka. We need to make sure this all plays well together and is well documented. Maybe a library to deal with DRM KMS color management/HDR would even be helpful. Not sure yet how I feel about that.
That is an excellent question. While I am working on Weston CM&HDR, I already have issues with how to represent the color related transformations. These new hardware features exposed here are nothing I have prepared for, and would probably need changes to accommodate.
The main Weston roadmap is drafted in https://gitlab.freedesktop.org/wayland/weston/-/issues/467
The MR that introduces the concept of a color transformation, and also the whole beginnings of color management, is https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582
In that MR, there is a patch introducing struct weston_color_transform: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?com...
The design idea there is that libweston shall have what I call "color manager" module. That module handles all the policy decisions about color, it uses a CMM (Little CMS 2 in this case) for all the color profile computations, and based on all information it has available from display EDID, ICC profile files, Wayland clients via the CM&HDR protocol extension and more, it will ultimately produce weston_color_transform objects.
weston_color_transform is a complete description of how to map a pixel in one color model/space/encoding into another, maybe with user preferred tuning/tone-mapping. E.g. from client content to the output's blending space (output space but light-linear), or from output's blending space to output's framebuffer space or maybe even monitor wire space.
The mapping described by weston_color_transform shall be implemented by libweston's GL-renderer or by the DRM-backend using KMS properties, whatever works for each case. So the description cannot be opaque, it has to map to GLSL shaders (easy) and KMS properties (???).
Now the problem is, what should weston_color_transform look like?
The current design has two steps in a color transform:
- Transfer function: identity, the traditional set of three 1D LUTs, or something else.
- Color mapping: identity, a 3D LUT, or something else.
"Something else" is a placeholder for whatever we want to have, but the problem in adding new types of transfer function or color mapping representations (e.g. the fancy new GAMMA_MODEs) is how will the color manager create the parameters for those?
I think the weston_color_transform is going a bit in the wrong direction. While the 3D LUT can describe everything if it has enough precision it indeed makes sense to apply a transform before to get the required precision down. It doesn't have to be a TF though and we really don't care what it is as long as in the end the content is in the correct color space and dynamic range. This might be enough to get something off the ground right now though.
In the long run however it probably makes more sense to convert the color transform to a complete pipeline of enumerated, parametric and numerical elements together with some helpers to lower (enumerated > parametric > numerical) and fuse elements (to the point that you can always convert the pipeline to a 3D LUT). The color manager ideally should provide a pipeline with the highest abstraction and avoid fusing elements if it would result in a lose of information. This is a lot more complex but it also gives us much better chances of finding a way to offload the transform.
AFAIR lcms uses such a model and gives you access to the pipeline. If we want to be independent of lcms we would need our own descriptions and possibly lower some lcms elements to our own stuff. I'm also not sure how good lcms is at retaining the high level description if possible.
If we have ICC profiles as the original data, then we are probably limited to what LCMS2 can produce. The issue with ICC profiles is that they may contain 3D LUTs themselves, so not what I would call a parametric model. OTOH, if we have, say, enumerated operations defined by various HDR standards, we have to code those ourselves and then producing whatever fancy representation is less of a problem.
Maybe that is how it has to be. If the color transformations are defined by ICC profiles, we might be stuck with old-school KMS color properties, but HDR stuff that doesn't rely on ICC can use the fancier KMS properties. I'm sure interesting questions will arise when e.g. you have the monitor in HDR mode, described with standard HDR terms, and then you have application content described with an ICC profile (maybe SDR, maybe not).
We can always get a 3D LUT out of LCMS2, so theoretically it would be possible to get a huge LUT and then optimise whatever parameterised model you have to that data set. But I worry that might be too costly to do in-flight, at least in a way that blocks the compositor. Maybe do what I hear shader compilers do: produce an unoptimal model fast, then compute an optimised model asynchronously and replace when ready. And disk cache(?).
A library probably makes sense in the long run, but for now, I would have no idea at all what it should look like.
Thanks, pq
On Thu, 03 Jun 2021 14:30:41 +0200 Sebastian Wick sebastian@sebastianwick.net wrote:
On 2021-06-03 10:47, Pekka Paalanen wrote:
On Wed, 2 Jun 2021 19:42:19 -0400 Harry Wentland harry.wentland@amd.com wrote:
On 2021-06-02 4:22 p.m., Shankar, Uma wrote:
-----Original Message----- From: Pekka Paalanen ppaalanen@gmail.com Sent: Wednesday, June 2, 2021 2:59 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, Bhanuprakash bhanuprakash.modem@intel.com; Harry Wentland harry.wentland@amd.com Subject: Re: [PATCH 00/21] Add Support for Plane Color Lut and CSC features
On Tue, 1 Jun 2021 16:21:57 +0530 Uma Shankar uma.shankar@intel.com wrote:
This is how a typical display color hardware pipeline looks like:
...
This patch series adds properties for plane color features. It adds properties for degamma used to linearize data and CSC used for gamut conversion. It also includes Gamma support used to again non-linearize data as per panel supported color space. These can be utilize by user space to convert planes from one format to another, one color space to another etc.
This is very much welcome!
There is also the thread: https://lists.freedesktop.org/archives/dri-devel/2021-May/306726.html%3E%3E%... Everything mentioned will interact with each other by changing what the abstract KMS pixel pipeline does. I think you and Harry should probably look at each others' suggestions and see how to fit them all into a single abstract KMS pipeline.
People are adding new pieces into KMS left and right, and I fear we lose sight of how everything will actually work together when all KMS properties are supposed to be generic and potentially present simultaneously. This is why I would very much like to have that *whole* abstract KMS pipeline documented with *everything*. Otherwise it is coming really hard fast to figure out how generic userspace should use all these KMS properties together.
Or if there cannot be a single abstract KMS pipeline, then sure, have multiple, as long as they are documented and how userspace will know which pipeline it is dealing with, and what things are mutually exclusive so we can avoid writing userspace code for combinations that will never exist.
This is a good suggestion to have the whole pipeline and properties documented along with the exact usages. We may end with 2 properties almost doing similar work but needed due to underlying hardware, but we can get that properly documented and defined.
I will discuss with Harry and Ville as well to define this.
Just wanted to let you know that I've seen and read through both of Shankar's patchsets and had some thoughts but haven't found the time to respond. I will respond soon.
Hi Harry,
awesome!
I very much agree with Pekka. We need to make sure this all plays well together and is well documented. Maybe a library to deal with DRM KMS color management/HDR would even be helpful. Not sure yet how I feel about that.
That is an excellent question. While I am working on Weston CM&HDR, I already have issues with how to represent the color related transformations. These new hardware features exposed here are nothing I have prepared for, and would probably need changes to accommodate.
The main Weston roadmap is drafted in https://gitlab.freedesktop.org/wayland/weston/-/issues/467
The MR that introduces the concept of a color transformation, and also the whole beginnings of color management, is https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582
In that MR, there is a patch introducing struct weston_color_transform: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582/diffs?com...
The design idea there is that libweston shall have what I call "color manager" module. That module handles all the policy decisions about color, it uses a CMM (Little CMS 2 in this case) for all the color profile computations, and based on all information it has available from display EDID, ICC profile files, Wayland clients via the CM&HDR protocol extension and more, it will ultimately produce weston_color_transform objects.
weston_color_transform is a complete description of how to map a pixel in one color model/space/encoding into another, maybe with user preferred tuning/tone-mapping. E.g. from client content to the output's blending space (output space but light-linear), or from output's blending space to output's framebuffer space or maybe even monitor wire space.
The mapping described by weston_color_transform shall be implemented by libweston's GL-renderer or by the DRM-backend using KMS properties, whatever works for each case. So the description cannot be opaque, it has to map to GLSL shaders (easy) and KMS properties (???).
Now the problem is, what should weston_color_transform look like?
The current design has two steps in a color transform:
- Transfer function: identity, the traditional set of three 1D LUTs, or something else.
- Color mapping: identity, a 3D LUT, or something else.
"Something else" is a placeholder for whatever we want to have, but the problem in adding new types of transfer function or color mapping representations (e.g. the fancy new GAMMA_MODEs) is how will the color manager create the parameters for those?
I think the weston_color_transform is going a bit in the wrong direction. While the 3D LUT can describe everything if it has enough precision it indeed makes sense to apply a transform before to get the required precision down. It doesn't have to be a TF though and we really don't care what it is as long as in the end the content is in the correct color space and dynamic range. This might be enough to get something off the ground right now though.
Ok, please leave these comments in https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582 and propose something? A new name for what I now called "transfer function"? It is desperately needing comments.
In the long run however it probably makes more sense to convert the color transform to a complete pipeline of enumerated, parametric and numerical elements together with some helpers to lower (enumerated > parametric > numerical) and fuse elements (to the point that you can always convert the pipeline to a 3D LUT). The color manager ideally should provide a pipeline with the highest abstraction and avoid fusing elements if it would result in a lose of information. This is a lot more complex but it also gives us much better chances of finding a way to offload the transform.
AFAIR lcms uses such a model and gives you access to the pipeline. If we want to be independent of lcms we would need our own descriptions and possibly lower some lcms elements to our own stuff. I'm also not sure how good lcms is at retaining the high level description if possible.
Yes, I think LCMS and even ICC use such a model. Access to the pipeline in LCMS I don't know about yet. You can construct, but inspect? I also think you can plug custom code in LCMS into the pipeline, so it can be totally arbitrary and also opaque.
Using our own description is what I'm doing, since I am keeping LCMS contained in the color manager plugin.
Lowering that into what can fit in KMS properties though is a huge problem. It can reduce precision, and how much precision are you good to lose? That's a policy decision, and I'd like to contain policy in color manager.
I would rather model weston_color_transform based on what KMS can do than based on what CMM can do. That way the hard problems are left for the color manager, while the rest of libweston can just implement the model directly as much as it fits e.g. in KMS.
Thanks, pq
dri-devel@lists.freedesktop.org
-
Harry Wentland -
Pekka Paalanen -
Sebastian Wick -
Shankar, Uma -
Uma Shankar