Re: [PATCH 04/21] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

[Harry Wentland <harry.wentland@amd.com>]

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);
>