On Thu, 11 Nov 2021 21:58:35 +0000 "Shankar, Uma" uma.shankar@intel.com wrote:
-----Original Message----- From: Harry Wentland harry.wentland@amd.com Sent: Friday, November 12, 2021 2:41 AM To: Shankar, Uma uma.shankar@intel.com; Ville Syrjälä ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; ppaalanen@gmail.com; brian.starkey@arm.com; sebastian@sebastianwick.net; Shashank.Sharma@amd.com Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
On 2021-11-11 15:42, Shankar, Uma wrote:
-----Original Message----- From: Ville Syrjälä ville.syrjala@linux.intel.com Sent: Thursday, November 11, 2021 10:13 PM To: Harry Wentland harry.wentland@amd.com Cc: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org; ppaalanen@gmail.com; brian.starkey@arm.com; sebastian@sebastianwick.net; Shashank.Sharma@amd.com Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
On 2021-09-06 17:38, 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 afcb4bf3826c..6403bd74324b 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -2092,6 +2092,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,- },
+};
If I understand this right, userspace would need this definition in order to populate the degamma blob. Should this sit in a UAPI header?
Are you asking whether 'struct drm_color_lut_range` is defined in any userspace visible header?
It seems to be in patch 2.
Hi Harry, Pekka and Ville, Sorry for being a bit late on the replies, got side tracked with various issues. I am back on this. Apologies for delay.
My original idea (not sure it's fully realized in this series) is to have a new GAMMA_MODE/etc. enum property on each crtc (or plane) for which each enum value points to a kernel provided blob that contains one of
these LUT descriptors.
Userspace can then query them dynamically and pick the best one for its current use case.
We have this as part of the series Ville. Patch 3 of this series creates a DEGAMMA_MODE property just for this. With that userspace can just query the blob_id's and will get the various degamma mode possible and the
respective segment and lut distributions.
This will be generic, so for userspace it should just be able to query this and parse and get the lut distribution and segment ranges.
Thanks for the explanation.
Uma, have you had a chance to sketch some of this out in IGT? I'm trying to see how userspace would do this in practice and will try to sketch an IGT test for this myself, but if you have it already we could share the effort.
Yes Harry, we do have some sample IGT's to test this. Will send those out and will copy you and all the stakeholders.
The algorithm for choosing the best one might be something like:
- prefer LUT with bpc >= FB bpc, but perhaps not needlessly high bpc
- prefer interpolated vs. direct lookup based on current needs (eg. X could prefer direct lookup to get directcolor visuals).
- prefer one with extended range values if needed
- for HDR prefer smaller step size in dark tones, for SDR perhaps prefer a more uniform step size
Or maybe we should include some kind of usage hints as well?
I think the segment range and distribution of lut should be enough for a userspace to pick the right ones, but we can add some examples in UAPI
descriptions as hints.
The range might be enough, but we're already parsing hints like "GAMMA" or "DEGAMMA". I wonder if it would make sense to add a flag for "HDR" or "SDR" as well.
What hints are GAMMA or DEGAMMA and who's parsing them? I thought they are just arbitrary names to identify the element's position in the abstract pipeline.
On Intel hardware, we differentiate this with precision and have HDR planes (they have extra Lut precision and samples) separately called out. We could add SDR/HDR FLAG as well.
What about wide color gamut SDR? That probably needs more precision than "normal" SDR but is not HDR.
I can't think of how SDR/HDR flags would work or what they would mean. Feels a bit too simple for practice. Maybe that concept should be created by a hypothetical userspace helper library instead.
Thanks, pq