Re: [Intel-gfx] [PATCH v3 18/20] drm/i915: Use gamma LUT for RGB limited range compression
> -Original Message-
> From: Intel-gfx On Behalf Of Ville
> Syrjala
> Sent: Monday, November 14, 2022 9:08 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v3 18/20] drm/i915: Use gamma LUT for RGB limited
> range compression
>
> From: Ville Syrjälä
>
> On ilk+ and glk class hardware we current make a mess of things when we have
> to
> both generate limited range output and use the hw gamma LUT. Since we do the
> range compression using the pipe CSC unit (which is situated before the gamma
> LUT
> in the pipe) we are in fact applying the gamma to the limited range data
> instead of
> the full range data as the user intended.
>
> We can work around this by applying the range compression via the gamma LUT
> instead of using the pipe CSC for it.
> Fairly easy to do now that we have the internal post_csc_lut attachment point
> where
> we can stick our new cooked LUT.
>
> On ilk+ this only needs to be dome when using the split gamma mode or when the
Nit: Typo in "done"
Looks Good to me.
Reviewed-by: Uma Shankar
> ctm is enabled since otherwise we can simply reorder the LUT vs. CSC. On glk
> we
> need to do this any time a gamma LUT is used since no reordering is possible.
> We do lose a bit of coverage in intel_color_assert_luts(), but so be it.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 133 +
> 1 file changed, 111 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index c336524d9225..dee0382015a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -249,17 +249,44 @@ static void icl_update_output_csc(struct intel_crtc
> *crtc,
> intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), postoff[2]);
> }
>
> +static bool ilk_limited_range(const struct intel_crtc_state
> +*crtc_state) {
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> + /* icl+ have dedicated output CSC */
> + if (DISPLAY_VER(i915) >= 11)
> + return false;
> +
> + /* pre-hsw have PIPECONF_COLOR_RANGE_SELECT */
> + if (DISPLAY_VER(i915) < 7 || IS_IVYBRIDGE(i915))
> + return false;
> +
> + return crtc_state->limited_color_range; }
> +
> +static bool ilk_lut_limited_range(const struct intel_crtc_state
> +*crtc_state) {
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> + if (!ilk_limited_range(crtc_state))
> + return false;
> +
> + if (crtc_state->c8_planes)
> + return false;
> +
> + if (DISPLAY_VER(i915) == 10)
> + return crtc_state->hw.gamma_lut;
> + else
> + return crtc_state->hw.gamma_lut &&
> + (crtc_state->hw.degamma_lut || crtc_state->hw.ctm); }
> +
> static bool ilk_csc_limited_range(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> + if (!ilk_limited_range(crtc_state))
> + return false;
>
> - /*
> - * FIXME if there's a gamma LUT after the CSC, we should
> - * do the range compression using the gamma LUT instead.
> - */
> - return crtc_state->limited_color_range &&
> - (IS_HASWELL(i915) || IS_BROADWELL(i915) ||
> - IS_DISPLAY_VER(i915, 9, 10));
> + return !ilk_lut_limited_range(crtc_state);
> }
>
> static void ilk_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
> @@ -
> 603,9 +630,18 @@ create_linear_lut(struct drm_i915_private *i915, int
> lut_size)
> return blob;
> }
>
> +static u16 lut_limited_range(unsigned int value) {
> + unsigned int min = 16 << 8;
> + unsigned int max = 235 << 8;
> +
> + return value * (max - min) / 0x + min; }
> +
> static struct drm_property_blob *
> create_resized_lut(struct drm_i915_private *i915,
> -const struct drm_property_blob *blob_in, int lut_out_size)
> +const struct drm_property_blob *blob_in, int lut_out_size,
> +bool limited_color_range)
> {
> int i, lut_in_size = drm_color_lut_size(blob_in);
> struct drm_property_blob *blob_out;
> @@ -621,8 +657,18 @@ create_resized_lut(struct drm_i915_private *i915,
> lut_in = blob_in->data;
> lut_out = blob_out->data;
>
> - for (i = 0; i < lut_out_size; i++)
> - lut_out[i] = lut_in[
[Intel-gfx] [PATCH v3 18/20] drm/i915: Use gamma LUT for RGB limited range compression
From: Ville Syrjälä
On ilk+ and glk class hardware we current make a mess of
things when we have to both generate limited range output
and use the hw gamma LUT. Since we do the range compression
using the pipe CSC unit (which is situated before the gamma
LUT in the pipe) we are in fact applying the gamma to the
limited range data instead of the full range data as the
user intended.
We can work around this by applying the range compression
via the gamma LUT instead of using the pipe CSC for it.
Fairly easy to do now that we have the internal post_csc_lut
attachment point where we can stick our new cooked LUT.
On ilk+ this only needs to be dome when using the split
gamma mode or when the ctm is enabled since otherwise we can
simply reorder the LUT vs. CSC. On glk we need to do this any
time a gamma LUT is used since no reordering is possible.
We do lose a bit of coverage in intel_color_assert_luts(),
but so be it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_color.c | 133 +
1 file changed, 111 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index c336524d9225..dee0382015a5 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -249,17 +249,44 @@ static void icl_update_output_csc(struct intel_crtc *crtc,
intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), postoff[2]);
}
+static bool ilk_limited_range(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ /* icl+ have dedicated output CSC */
+ if (DISPLAY_VER(i915) >= 11)
+ return false;
+
+ /* pre-hsw have PIPECONF_COLOR_RANGE_SELECT */
+ if (DISPLAY_VER(i915) < 7 || IS_IVYBRIDGE(i915))
+ return false;
+
+ return crtc_state->limited_color_range;
+}
+
+static bool ilk_lut_limited_range(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (!ilk_limited_range(crtc_state))
+ return false;
+
+ if (crtc_state->c8_planes)
+ return false;
+
+ if (DISPLAY_VER(i915) == 10)
+ return crtc_state->hw.gamma_lut;
+ else
+ return crtc_state->hw.gamma_lut &&
+ (crtc_state->hw.degamma_lut || crtc_state->hw.ctm);
+}
+
static bool ilk_csc_limited_range(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ if (!ilk_limited_range(crtc_state))
+ return false;
- /*
-* FIXME if there's a gamma LUT after the CSC, we should
-* do the range compression using the gamma LUT instead.
-*/
- return crtc_state->limited_color_range &&
- (IS_HASWELL(i915) || IS_BROADWELL(i915) ||
-IS_DISPLAY_VER(i915, 9, 10));
+ return !ilk_lut_limited_range(crtc_state);
}
static void ilk_csc_convert_ctm(const struct intel_crtc_state *crtc_state,
@@ -603,9 +630,18 @@ create_linear_lut(struct drm_i915_private *i915, int
lut_size)
return blob;
}
+static u16 lut_limited_range(unsigned int value)
+{
+ unsigned int min = 16 << 8;
+ unsigned int max = 235 << 8;
+
+ return value * (max - min) / 0x + min;
+}
+
static struct drm_property_blob *
create_resized_lut(struct drm_i915_private *i915,
- const struct drm_property_blob *blob_in, int lut_out_size)
+ const struct drm_property_blob *blob_in, int lut_out_size,
+ bool limited_color_range)
{
int i, lut_in_size = drm_color_lut_size(blob_in);
struct drm_property_blob *blob_out;
@@ -621,8 +657,18 @@ create_resized_lut(struct drm_i915_private *i915,
lut_in = blob_in->data;
lut_out = blob_out->data;
- for (i = 0; i < lut_out_size; i++)
- lut_out[i] = lut_in[i * (lut_in_size - 1) / (lut_out_size - 1)];
+ for (i = 0; i < lut_out_size; i++) {
+ const struct drm_color_lut *entry =
+ _in[i * (lut_in_size - 1) / (lut_out_size - 1)];
+
+ if (limited_color_range) {
+ lut_out[i].red = lut_limited_range(entry->red);
+ lut_out[i].green = lut_limited_range(entry->green);
+ lut_out[i].blue = lut_limited_range(entry->blue);
+ } else {
+ lut_out[i] = *entry;
+ }
+ }
return blob_out;
}
@@ -1423,6 +1469,7 @@ void intel_color_assert_luts(const struct
intel_crtc_state *crtc_state)
crtc_state->pre_csc_lut !=
crtc_state->hw.degamma_lut &&
crtc_state->pre_csc_lut !=
i915->display.color.glk_linear_degamma_lut);
