All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Peter Frühberger" <fritsch@kodi.tv>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Implement Limited Video Range
Date: Sat, 16 Sep 2017 13:57:42 +0200	[thread overview]
Message-ID: <CAFu8+f=Bj+2QicNgmom+Lu4Zrk4-PwrLsfLdqE=DJmv-hvPBmg@mail.gmail.com> (raw)
In-Reply-To: <20161111135720.GG31595@intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 8422 bytes --]

Thanks for your comments, I tried to implement a "flag driven version"
below.

On Fri, Nov 11, 2016 at 2:57 PM, Ville Syrjälä <
ville.syrjala@linux.intel.com> wrote:

> On Fri, Nov 11, 2016 at 09:53:35AM +0100, Peter Frühberger wrote:
> > Hi,
> >
> > I was implementing this suggestion today and I think that will confuse
> > users and also devs maintaining that code. Out of the following reason:
> > compress_color_range can be true or false, it does not reference a mode,
> > but an additional setting that only influences color clamping / scaling.
> >
> > What do we expect if someone runs in Full Range and has
> > compress_color_range set to true? Also what do we expect if someone runs
> in
> > Limited Range and additionally set this flag to true? In some cases it
> > would do nothing and was not transparent.
>
> I didn't mean you should add a new property for this. Just an internal
> flag.
> [...]
>

In my opinion that makes maintainability much harder. Looking forward to
your comments.


>From f3bccf1611108247add0d85e8faed5430971cd71 Mon Sep 17 00:00:00 2001
From: fritsch <peter.fruehberger@gmail.com>
Date: Sat, 16 Sep 2017 13:54:06 +0200
Subject: [PATCH] i915: Implement uncompressed Video Range output

---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_color.c   | 12 ++++++++----
 drivers/gpu/drm/i915/intel_display.c |  6 ++++--
 drivers/gpu/drm/i915/intel_dp.c      |  3 ++-
 drivers/gpu/drm/i915/intel_drv.h     | 11 +++++++++--
 drivers/gpu/drm/i915/intel_hdmi.c    | 16 +++++++++++++---
 drivers/gpu/drm/i915/intel_modes.c   |  1 +
 7 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5aecbf795b55..70bd525317c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4230,6 +4230,7 @@ __raw_write(64, q)
 #define INTEL_BROADCAST_RGB_AUTO 0
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
+#define INTEL_BROADCAST_RGB_VIDEO 3

 static inline i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index ff9ecd211abb..b72477a43ea4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -149,7 +149,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
 			(struct drm_color_ctm *)crtc_state->ctm->data;
 		uint64_t input[9] = { 0, };

-		if (intel_crtc_state->limited_color_range) {
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range) {
 			ctm_mult_by_limited(input, ctm->matrix);
 		} else {
 			for (i = 0; i < ARRAY_SIZE(input); i++)
@@ -201,7 +202,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
 		 * into consideration.
 		 */
 		for (i = 0; i < 3; i++) {
-			if (intel_crtc_state->limited_color_range)
+			if (intel_crtc_state->limited_color_range &&
+					intel_crtc_state->compress_range)
 				coeffs[i * 3 + i] =
 					I9XX_CSC_COEFF_LIMITED_RANGE;
 			else
@@ -225,7 +227,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
 	if (INTEL_GEN(dev_priv) > 6) {
 		uint16_t postoff = 0;

-		if (intel_crtc_state->limited_color_range)
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range)
 			postoff = (16 * (1 << 12) / 255) & 0x1fff;

 		I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
@@ -236,7 +239,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
 	} else {
 		uint32_t mode = CSC_MODE_YUV_TO_RGB;

-		if (intel_crtc_state->limited_color_range)
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range)
 			mode |= CSC_BLACK_SCREEN_OFFSET;

 		I915_WRITE(PIPE_CSC_MODE(pipe), mode);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8599e425abb1..4de43eb12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7247,7 +7247,8 @@ static void i9xx_set_pipeconf(struct intel_crtc
*intel_crtc)
 		pipeconf |= PIPECONF_PROGRESSIVE;

 	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	     intel_crtc->config->limited_color_range)
+	     intel_crtc->config->limited_color_range &&
+	     intel_crtc->config->compress_range)
 		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;

 	I915_WRITE(PIPECONF(intel_crtc->pipe), pipeconf);
@@ -8177,7 +8178,8 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 	else
 		val |= PIPECONF_PROGRESSIVE;

-	if (intel_crtc->config->limited_color_range)
+	if (intel_crtc->config->limited_color_range &&
+			intel_crtc->config->compress_range)
 		val |= PIPECONF_COLOR_RANGE_SELECT;

 	I915_WRITE(PIPECONF(pipe), val);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..437dd0465be3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1926,7 +1926,8 @@ static void intel_dp_prepare(struct
intel_encoder *encoder,
 			trans_dp &= ~TRANS_DP_ENH_FRAMING;
 		I915_WRITE(TRANS_DP_CTL(crtc->pipe), trans_dp);
 	} else {
-		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
+		if (IS_G4X(dev_priv) && pipe_config->limited_color_range &&
+				pipe_config->compress_range)
 			intel_dp->DP |= DP_COLOR_RANGE_16_235;

 		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 307807672896..35602a1ed471 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -649,11 +649,18 @@ struct intel_crtc_state {
 	enum transcoder cpu_transcoder;

 	/*
-	 * Use reduced/limited/broadcast rbg range, compressing from the full
-	 * range fed into the crtcs.
+	 * Use reduced/limited/broadcast rbg range.
 	 */
 	bool limited_color_range;

+	/*
+	 *
+	 * Compress the input range when doing limited/broadcast rgb range
+	 * output. This is the default for Limited 16:235 Output mode.
+	 *
+	 */
+	bool compress_range;
+
 	/* Bitmask of encoder types (enum intel_output_type)
 	 * driven by the pipe.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index e6f8f30ce7bd..4963903f177e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -880,7 +880,8 @@ static void intel_hdmi_prepare(struct
intel_encoder *encoder,
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);

 	hdmi_val = SDVO_ENCODING_HDMI;
-	if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range)
+	if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range
+			&& crtc_state->compress_range)
 		hdmi_val |= HDMI_COLOR_RANGE_16_235;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
@@ -1419,9 +1420,18 @@ bool intel_hdmi_compute_config(struct
intel_encoder *encoder,
 			pipe_config->has_hdmi_sink &&
 			drm_default_rgb_quant_range(adjusted_mode) ==
 			HDMI_QUANTIZATION_RANGE_LIMITED;
+		pipe_config->compress_range = pipe_config->limited_color_range;
+	} else if (intel_conn_state->broadcast_rgb
+			== INTEL_BROADCAST_RGB_LIMITED) {
+		pipe_config->limited_color_range = true;
+		pipe_config->compress_range = true;
+	} else if (intel_conn_state->broadcast_rgb
+			== INTEL_BROADCAST_RGB_FULL) {
+		pipe_config->limited_color_range = false;
+		pipe_config->compress_range = false;
 	} else {
-		pipe_config->limited_color_range =
-			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
+		pipe_config->limited_color_range = true;
+		pipe_config->compress_range = false;
 	}

 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
diff --git a/drivers/gpu/drm/i915/intel_modes.c
b/drivers/gpu/drm/i915/intel_modes.c
index 951e834dd274..d817558410eb 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -102,6 +102,7 @@ static const struct drm_prop_enum_list
broadcast_rgb_names[] = {
 	{ INTEL_BROADCAST_RGB_AUTO, "Automatic" },
 	{ INTEL_BROADCAST_RGB_FULL, "Full" },
 	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
+	{ INTEL_BROADCAST_RGB_VIDEO, "Video 16:235 pass-through" },
 };

 void
-- 
2.11.0

[-- Attachment #1.2: Type: text/html, Size: 9417 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

      reply	other threads:[~2017-09-16 11:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-30  7:08 [PATCH] Implement Limited Video Range Peter Frühberger
2015-11-30  8:43 ` Daniel Vetter
2015-11-30 14:11   ` Ville Syrjälä
2016-11-09 20:11     ` Peter Frühberger
2016-11-09 20:25       ` Ville Syrjälä
2016-11-11  8:53         ` Peter Frühberger
2016-11-11 13:57           ` Ville Syrjälä
2017-09-16 11:57             ` Peter Frühberger [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFu8+f=Bj+2QicNgmom+Lu4Zrk4-PwrLsfLdqE=DJmv-hvPBmg@mail.gmail.com' \
    --to=fritsch@kodi.tv \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.