[PATCH v12 0/3] Enhancement to intel_dp_aux_backlight driver

[PATCH v12 0/3] Enhancement to intel_dp_aux_backlight driver

[Puthikorn Voravootivat]

This patch set contain 3 patches which are already reviewed by DK.
Another 6 patches in previous version was already merged in v7 and v9.
- First patch sets the PWM freqency to match data in panel vbt.
- Next patch adds heuristic to determine whether we should use AUX
  or PWM pin to adjust panel backlight brightness.
- Last patch adds support for dynamic brightness.

Change log:
v12:
- Rebase

v11:
- Reorder patches in v10 to make the last patch come first
- Fix nits

v10:
- Add heuristic in patch #1
- Add _unsafe mod option in patch #1, #2
- handle frequency set error in patch #3

v9:
- Fix nits in v8

v8:
- Drop 4 patches that was already merged
- Move DP_EDP_BACKLIGHT_AUX_ENABLE_CAP check to patch #2 to avoid
  behavior change if only apply patch #1
- Add TODO to warn about enable backlight twice in patch #2
- Use DIV_ROUND_CLOSEST instead of just "/" in patch #5
- Fix bug calculate pn in patch #5
- Clarify commit  message / code comment in patch #5

v7:
- Add check in intel_dp_pwm_pin_display_control_capable in patch #4
- Add option in patch #6 to enable DPCD or not
- Change definition in patch #8 and implementation in #9 to use Khz
- Fix compiler warning from build bot in patch #9

v6:
- Address review from Dhinakaran
- Make PWM frequency to have highest value of Pn that make the
  frequency still within 25% of desired frequency.

v5:
- Split first patch in v4 to 3 patches
- Bump priority for "Correctly enable backlight brightness adjustment via DPCD"
- Make logic clearer for the case that both PWM pin and AUX are supported
- Add more log when write to register fail
- Add log when enable DBC

v4:
- Rebase / minor typo fix.

v3:
- Add new implementation of PWM frequency patch

v2:
- Drop PWM frequency patch
- Address suggestion from Jani Nikula

Puthikorn Voravootivat (3):
  drm/i915: Set PWM divider to match desired frequency in vbt
  drm/i915: Add heuristic to determine better way to adjust brightness
  drm/i915: Add option to support dynamic backlight via DPCD

 drivers/gpu/drm/i915/i915_params.c            |  12 +-
 drivers/gpu/drm/i915/i915_params.h            |   5 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 185 ++++++++++++++++++++++++--
 3 files changed, 186 insertions(+), 16 deletions(-)

-- 
2.13.1.611.g7e3b11ae1-goog

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

[PATCH v12 1/3] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

Read desired PWM frequency from panel vbt and calculate the
value for divider in DPCD address 0x724 and 0x728 to have
as many bits as possible for PWM duty cyle for granularity of
brightness adjustment while the frequency divisor is still
within 25% of the desired value.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 98 ++++++++++++++++++++++++---
 1 file changed, 90 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 228ca06d9f0b..d2830ba3162e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -98,13 +98,87 @@ intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 lev
 	}
 }
 
+/*
+ * Set PWM Frequency divider to match desired frequency in vbt.
+ * The PWM Frequency is calculated as 27Mhz / (F x P).
+ * - Where F = PWM Frequency Pre-Divider value programmed by field 7:0 of the
+ *             EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h)
+ * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 of the
+ *             EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h)
+ */
+static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+	int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
+	u8 pn, pn_min, pn_max;
+
+	/* Find desired value of (F x P)
+	 * Note that, if F x P is out of supported range, the maximum value or
+	 * minimum value will applied automatically. So no need to check that.
+	 */
+	freq = dev_priv->vbt.backlight.pwm_freq_hz;
+	DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq);
+	if (!freq) {
+		DRM_DEBUG_KMS("Use panel default backlight frequency\n");
+		return false;
+	}
+
+	fxp = DIV_ROUND_CLOSEST(KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ), freq);
+
+	/* Use highest possible value of Pn for more granularity of brightness
+	 * adjustment while satifying the conditions below.
+	 * - Pn is in the range of Pn_min and Pn_max
+	 * - F is in the range of 1 and 255
+	 * - FxP is within 25% of desired value.
+	 *   Note: 25% is arbitrary value and may need some tweak.
+	 */
+	if (drm_dp_dpcd_readb(&intel_dp->aux,
+			       DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) {
+		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n");
+		return false;
+	}
+	if (drm_dp_dpcd_readb(&intel_dp->aux,
+			       DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != 1) {
+		DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n");
+		return false;
+	}
+	pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+	pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+
+	fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4);
+	fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4);
+	if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) {
+		DRM_DEBUG_KMS("VBT defined backlight frequency out of range\n");
+		return false;
+	}
+
+	for (pn = pn_max; pn >= pn_min; pn--) {
+		f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255);
+		fxp_actual = f << pn;
+		if (fxp_min <= fxp_actual && fxp_actual <= fxp_max)
+			break;
+	}
+
+	if (drm_dp_dpcd_writeb(&intel_dp->aux,
+			       DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) {
+		DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n");
+		return false;
+	}
+	if (drm_dp_dpcd_writeb(&intel_dp->aux,
+			       DP_EDP_BACKLIGHT_FREQ_SET, (u8) f) < 0) {
+		DRM_DEBUG_KMS("Failed to write aux backlight freq\n");
+		return false;
+	}
+	return true;
+}
+
 static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state,
 					  const struct drm_connector_state *conn_state)
 {
 	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
-	uint8_t dpcd_buf = 0;
-	uint8_t edp_backlight_mode = 0;
+	uint8_t dpcd_buf, new_dpcd_buf, edp_backlight_mode;
 
 	if (drm_dp_dpcd_readb(&intel_dp->aux,
 			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) {
@@ -113,18 +187,15 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
 		return;
 	}
 
+	new_dpcd_buf = dpcd_buf;
 	edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
 
 	switch (edp_backlight_mode) {
 	case DP_EDP_BACKLIGHT_CONTROL_MODE_PWM:
 	case DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET:
 	case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT:
-		dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
-		dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
-		if (drm_dp_dpcd_writeb(&intel_dp->aux,
-			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, dpcd_buf) < 0) {
-			DRM_DEBUG_KMS("Failed to write aux backlight mode\n");
-		}
+		new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
+		new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
 		break;
 
 	/* Do nothing when it is already DPCD mode */
@@ -133,6 +204,17 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
 		break;
 	}
 
+	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
+		if (intel_dp_aux_set_pwm_freq(connector))
+			new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
+
+	if (new_dpcd_buf != dpcd_buf) {
+		if (drm_dp_dpcd_writeb(&intel_dp->aux,
+			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
+			DRM_DEBUG_KMS("Failed to write aux backlight mode\n");
+		}
+	}
+
 	set_aux_backlight_enable(intel_dp, true);
 	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
 }
-- 
2.13.1.611.g7e3b11ae1-goog

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

[PATCH v12 2/3] drm/i915: Add heuristic to determine better way to adjust brightness

[Puthikorn Voravootivat]

Add heuristic to decide that AUX or PWM pin should use for
backlight brightness adjustment and modify i915 param description
to have auto, force disable, and force enable.

The heuristic to determine that using AUX pin is better than using
PWM pin is that the panel support any of the feature list here.
- Regional backlight brightness adjustment
- Backlight PWM frequency set
- More than 8 bits resolution of brightness level
- Backlight enablement via AUX and not by BL_ENABLE pin

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c            |  7 +--
 drivers/gpu/drm/i915/i915_params.h            |  2 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 61 +++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 14e2c2e57f96..5b5ab15d191f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -63,7 +63,7 @@ struct i915_params i915 __read_mostly = {
 	.huc_firmware_path = NULL,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
-	.enable_dpcd_backlight = false,
+	.enable_dpcd_backlight = -1,
 	.enable_gvt = false,
 };
 
@@ -246,9 +246,10 @@ MODULE_PARM_DESC(enable_dp_mst,
 module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400);
 MODULE_PARM_DESC(inject_load_failure,
 	"Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
-module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
+module_param_named_unsafe(enable_dpcd_backlight, i915.enable_dpcd_backlight, int, 0600);
 MODULE_PARM_DESC(enable_dpcd_backlight,
-	"Enable support for DPCD backlight control (default:false)");
+	"Enable support for DPCD backlight control "
+	"(-1:auto (default), 0:force disable, 1:force enabled if supported");
 
 module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
 MODULE_PARM_DESC(enable_gvt,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index febbfdbd30bd..0d6cf9138dc4 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -53,6 +53,7 @@
 	func(int, edp_vswing); \
 	func(int, reset); \
 	func(unsigned int, inject_load_failure); \
+	func(int, enable_dpcd_backlight); \
 	/* leave bools at the end to not create holes */ \
 	func(bool, alpha_support); \
 	func(bool, enable_cmd_parser); \
@@ -66,7 +67,6 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
-	func(bool, enable_dpcd_backlight); \
 	func(bool, enable_gvt)
 
 #define MEMBER(T, member) T member
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index d2830ba3162e..fea161727c6e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -251,15 +251,66 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
 	/* Check the eDP Display control capabilities registers to determine if
 	 * the panel can support backlight control over the aux channel
 	 */
-	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
-	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
-	    !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
+	if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) &&
+	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) {
 		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");
 		return true;
 	}
 	return false;
 }
 
+/*
+ * Heuristic function whether we should use AUX for backlight adjustment or not.
+ *
+ * We should use AUX for backlight brightness adjustment if panel doesn't this
+ * via PWM pin or using AUX is better than using PWM pin.
+ *
+ * The heuristic to determine that using AUX pin is better than using PWM pin is
+ * that the panel support any of the feature list here.
+ * - Regional backlight brightness adjustment
+ * - Backlight PWM frequency set
+ * - More than 8 bits resolution of brightness level
+ * - Backlight enablement via AUX and not by BL_ENABLE pin
+ *
+ * If all above are not true, assume that using PWM pin is better.
+ */
+static bool
+intel_dp_aux_display_control_heuristic(struct intel_connector *connector)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+	uint8_t reg_val;
+
+	/* Panel doesn't support adjusting backlight brightness via PWN pin */
+	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
+		return true;
+
+	/* Panel supports regional backlight brightness adjustment */
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3,
+			      &reg_val) != 1) {
+		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
+			       DP_EDP_GENERAL_CAP_3);
+		return false;
+	}
+	if (reg_val > 0)
+		return true;
+
+	/* Panel supports backlight PWM frequency set */
+	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
+		return true;
+
+	/* Panel supports more than 8 bits resolution of brightness level */
+	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
+		return true;
+
+	/* Panel supports enabling backlight via AUX but not by BL_ENABLE pin */
+	if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
+	    !(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP))
+		return true;
+
+	return false;
+
+}
+
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 {
 	struct intel_panel *panel = &intel_connector->panel;
@@ -270,6 +321,10 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 	if (!intel_dp_aux_display_control_capable(intel_connector))
 		return -ENODEV;
 
+	if (i915.enable_dpcd_backlight == -1 &&
+	    !intel_dp_aux_display_control_heuristic(intel_connector))
+		return -ENODEV;
+
 	panel->backlight.setup = intel_dp_aux_setup_backlight;
 	panel->backlight.enable = intel_dp_aux_enable_backlight;
 	panel->backlight.disable = intel_dp_aux_disable_backlight;
-- 
2.13.1.611.g7e3b11ae1-goog

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

[PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD

[Puthikorn Voravootivat]

This patch adds option to enable dynamic backlight for eDP
panel that supports this feature via DPCD register and
set minimum / maximum brightness to 0% and 100% of the
normal brightness.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c            |  5 +++++
 drivers/gpu/drm/i915/i915_params.h            |  3 ++-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 26 ++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5b5ab15d191f..88b9d3e6713a 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = {
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = -1,
 	.enable_gvt = false,
+	.enable_dbc = true,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -254,3 +255,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
 module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
 MODULE_PARM_DESC(enable_gvt,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+module_param_named_unsafe(enable_dbc, i915.enable_dbc, bool, 0600);
+MODULE_PARM_DESC(enable_dbc,
+	"Enable support for dynamic backlight control (default:true)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 0d6cf9138dc4..057e203e6bda 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -67,7 +67,8 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
-	func(bool, enable_gvt)
+	func(bool, enable_gvt); \
+	func(bool, enable_dbc)
 
 #define MEMBER(T, member) T member
 struct i915_params {
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index fea161727c6e..b25cd88fc1c5 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -173,6 +173,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
 	return true;
 }
 
+/*
+* Set minimum / maximum dynamic brightness percentage. This value is expressed
+* as the percentage of normal brightness in 5% increments.
+*/
+static bool
+intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp,
+					   u32 min, u32 max)
+{
+	u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) };
+
+	if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET,
+			  dbc, sizeof(dbc)) < 0) {
+		DRM_DEBUG_KMS("Failed to write aux DBC brightness level\n");
+		return false;
+	}
+	return true;
+}
+
 static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state,
 					  const struct drm_connector_state *conn_state)
 {
@@ -208,6 +226,14 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
 		if (intel_dp_aux_set_pwm_freq(connector))
 			new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
 
+	if (i915.enable_dbc &&
+	    (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {
+		if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) {
+			new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
+			DRM_DEBUG_KMS("Enable dynamic brightness.\n");
+		}
+	}
+
 	if (new_dpcd_buf != dpcd_buf) {
 		if (drm_dp_dpcd_writeb(&intel_dp->aux,
 			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
-- 
2.13.1.611.g7e3b11ae1-goog

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

✓ Fi.CI.BAT: success for Enhancement to intel_dp_aux_backlight driver (rev12)

[Patchwork]

== Series Details ==

Series: Enhancement to intel_dp_aux_backlight driver (rev12)
URL   : https://patchwork.freedesktop.org/series/21086/
State : success

== Summary ==

Series 21086v12 Enhancement to intel_dp_aux_backlight driver
https://patchwork.freedesktop.org/api/1.0/series/21086/revisions/12/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-bsw-n3050) fdo#101517
Test prime_busy:
        Subgroup basic-wait-after-default:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101515 +1

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101517 https://bugs.freedesktop.org/show_bug.cgi?id=101517
fdo#101515 https://bugs.freedesktop.org/show_bug.cgi?id=101515

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:438s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:430s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:519s
fi-byt-j1900     total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  time:491s
fi-byt-n2820     total:279  pass:249  dwarn:2   dfail:0   fail:0   skip:28  time:479s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:589s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:440s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:484s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:569s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:585s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:461s
fi-skl-6700hq    total:279  pass:223  dwarn:1   dfail:0   fail:30  skip:24  time:340s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:474s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:401s

cd6f0ef4478545aa014d92cabe4d794bfe54fe33 drm-tip: 2017y-06m-22d-16h-48m-46s UTC integration manifest
e30b3ba drm/i915: Add option to support dynamic backlight via DPCD
59a5270 drm/i915: Add heuristic to determine better way to adjust brightness
0389d86 drm/i915: Set PWM divider to match desired frequency in vbt

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5030/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v12 0/3] Enhancement to intel_dp_aux_backlight driver

[Daniel Vetter]

On Thu, Jun 22, 2017 at 12:03:36PM -0700, Puthikorn Voravootivat wrote:
> This patch set contain 3 patches which are already reviewed by DK.
> Another 6 patches in previous version was already merged in v7 and v9.
> - First patch sets the PWM freqency to match data in panel vbt.
> - Next patch adds heuristic to determine whether we should use AUX
>   or PWM pin to adjust panel backlight brightness.
> - Last patch adds support for dynamic brightness.
> 
> Change log:
> v12:
> - Rebase

Thanks for doing the rebase, applied.
-Daniel

> 
> v11:
> - Reorder patches in v10 to make the last patch come first
> - Fix nits
> 
> v10:
> - Add heuristic in patch #1
> - Add _unsafe mod option in patch #1, #2
> - handle frequency set error in patch #3
> 
> v9:
> - Fix nits in v8
> 
> v8:
> - Drop 4 patches that was already merged
> - Move DP_EDP_BACKLIGHT_AUX_ENABLE_CAP check to patch #2 to avoid
>   behavior change if only apply patch #1
> - Add TODO to warn about enable backlight twice in patch #2
> - Use DIV_ROUND_CLOSEST instead of just "/" in patch #5
> - Fix bug calculate pn in patch #5
> - Clarify commit  message / code comment in patch #5
> 
> v7:
> - Add check in intel_dp_pwm_pin_display_control_capable in patch #4
> - Add option in patch #6 to enable DPCD or not
> - Change definition in patch #8 and implementation in #9 to use Khz
> - Fix compiler warning from build bot in patch #9
> 
> v6:
> - Address review from Dhinakaran
> - Make PWM frequency to have highest value of Pn that make the
>   frequency still within 25% of desired frequency.
> 
> v5:
> - Split first patch in v4 to 3 patches
> - Bump priority for "Correctly enable backlight brightness adjustment via DPCD"
> - Make logic clearer for the case that both PWM pin and AUX are supported
> - Add more log when write to register fail
> - Add log when enable DBC
> 
> v4:
> - Rebase / minor typo fix.
> 
> v3:
> - Add new implementation of PWM frequency patch
> 
> v2:
> - Drop PWM frequency patch
> - Address suggestion from Jani Nikula
> 
> Puthikorn Voravootivat (3):
>   drm/i915: Set PWM divider to match desired frequency in vbt
>   drm/i915: Add heuristic to determine better way to adjust brightness
>   drm/i915: Add option to support dynamic backlight via DPCD
> 
>  drivers/gpu/drm/i915/i915_params.c            |  12 +-
>  drivers/gpu/drm/i915/i915_params.h            |   5 +-
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 185 ++++++++++++++++++++++++--
>  3 files changed, 186 insertions(+), 16 deletions(-)
> 
> -- 
> 2.13.1.611.g7e3b11ae1-goog
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD

[David Weinehall]

On Thu, Jun 22, 2017 at 12:03:39PM -0700, Puthikorn Voravootivat wrote:
> This patch adds option to enable dynamic backlight for eDP
> panel that supports this feature via DPCD register and
> set minimum / maximum brightness to 0% and 100% of the
> normal brightness.

This patch causes a regression on my ThinkPad X1 Carbon Gen 4;
with enable_dbc = true the backlight stays off; setting i915.enable_dbc=0
on boot makes things work properly again.

> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c            |  5 +++++
>  drivers/gpu/drm/i915/i915_params.h            |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 26 ++++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5b5ab15d191f..88b9d3e6713a 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = {
>  	.inject_load_failure = 0,
>  	.enable_dpcd_backlight = -1,
>  	.enable_gvt = false,
> +	.enable_dbc = true,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -254,3 +255,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
>  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
>  MODULE_PARM_DESC(enable_gvt,
>  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> +
> +module_param_named_unsafe(enable_dbc, i915.enable_dbc, bool, 0600);
> +MODULE_PARM_DESC(enable_dbc,
> +	"Enable support for dynamic backlight control (default:true)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 0d6cf9138dc4..057e203e6bda 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -67,7 +67,8 @@
>  	func(bool, verbose_state_checks); \
>  	func(bool, nuclear_pageflip); \
>  	func(bool, enable_dp_mst); \
> -	func(bool, enable_gvt)
> +	func(bool, enable_gvt); \
> +	func(bool, enable_dbc)
>  
>  #define MEMBER(T, member) T member
>  struct i915_params {
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index fea161727c6e..b25cd88fc1c5 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -173,6 +173,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
>  	return true;
>  }
>  
> +/*
> +* Set minimum / maximum dynamic brightness percentage. This value is expressed
> +* as the percentage of normal brightness in 5% increments.
> +*/
> +static bool
> +intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp,
> +					   u32 min, u32 max)
> +{
> +	u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) };
> +
> +	if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET,
> +			  dbc, sizeof(dbc)) < 0) {
> +		DRM_DEBUG_KMS("Failed to write aux DBC brightness level\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
>  static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state,
>  					  const struct drm_connector_state *conn_state)
>  {
> @@ -208,6 +226,14 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
>  		if (intel_dp_aux_set_pwm_freq(connector))
>  			new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
>  
> +	if (i915.enable_dbc &&
> +	    (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {
> +		if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) {
> +			new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
> +			DRM_DEBUG_KMS("Enable dynamic brightness.\n");
> +		}
> +	}
> +
>  	if (new_dpcd_buf != dpcd_buf) {
>  		if (drm_dp_dpcd_writeb(&intel_dp->aux,
>  			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
> -- 
> 2.13.1.611.g7e3b11ae1-goog
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD

[David Weinehall]

On Mon, Jun 26, 2017 at 05:18:19PM +0300, David Weinehall wrote:
> On Thu, Jun 22, 2017 at 12:03:39PM -0700, Puthikorn Voravootivat wrote:
> > This patch adds option to enable dynamic backlight for eDP
> > panel that supports this feature via DPCD register and
> > set minimum / maximum brightness to 0% and 100% of the
> > normal brightness.
> 
> This patch causes a regression on my ThinkPad X1 Carbon Gen 4;
> with enable_dbc = true the backlight stays off; setting i915.enable_dbc=0
> on boot makes things work properly again.

Some more testing indicates that while i915.enable_dbc=0 solves the
initial issue of backlight not coming on, it's not enough--after
suspend/resume the backlight stays off.

Passing i915.enable_dpcd_backlight=0 seems to work both for the initial
case of backlight not coming on at boot, and the issue with backlight
not coming on after suspend/resume.

So:

DBC:1, DPCD: -1: blank screen on boot
DBC:0, DPCD: -1: screen OK on boot, blank after suspend/resume
DBC:1, DPCD:  0: screen OK on boot, screen OK after suspend/resume
DBC:0, DPCD:  0: screen OK on boot, screen OK after suspend/resume

So it seems that at least for the ThinkPad X1 Carbon Gen 4 (possibly
limited to the 2560x1440 model) DPCD backlight isn't supported.

Or possibly something is wrong with the DPCD backlight patch.

> > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_params.c            |  5 +++++
> >  drivers/gpu/drm/i915/i915_params.h            |  3 ++-
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 26 ++++++++++++++++++++++++++
> >  3 files changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index 5b5ab15d191f..88b9d3e6713a 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = {
> >  	.inject_load_failure = 0,
> >  	.enable_dpcd_backlight = -1,
> >  	.enable_gvt = false,
> > +	.enable_dbc = true,
> >  };
> >  
> >  module_param_named(modeset, i915.modeset, int, 0400);
> > @@ -254,3 +255,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
> >  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
> >  MODULE_PARM_DESC(enable_gvt,
> >  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> > +
> > +module_param_named_unsafe(enable_dbc, i915.enable_dbc, bool, 0600);
> > +MODULE_PARM_DESC(enable_dbc,
> > +	"Enable support for dynamic backlight control (default:true)");
> > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > index 0d6cf9138dc4..057e203e6bda 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -67,7 +67,8 @@
> >  	func(bool, verbose_state_checks); \
> >  	func(bool, nuclear_pageflip); \
> >  	func(bool, enable_dp_mst); \
> > -	func(bool, enable_gvt)
> > +	func(bool, enable_gvt); \
> > +	func(bool, enable_dbc)
> >  
> >  #define MEMBER(T, member) T member
> >  struct i915_params {
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index fea161727c6e..b25cd88fc1c5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -173,6 +173,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
> >  	return true;
> >  }
> >  
> > +/*
> > +* Set minimum / maximum dynamic brightness percentage. This value is expressed
> > +* as the percentage of normal brightness in 5% increments.
> > +*/
> > +static bool
> > +intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp,
> > +					   u32 min, u32 max)
> > +{
> > +	u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) };
> > +
> > +	if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET,
> > +			  dbc, sizeof(dbc)) < 0) {
> > +		DRM_DEBUG_KMS("Failed to write aux DBC brightness level\n");
> > +		return false;
> > +	}
> > +	return true;
> > +}
> > +
> >  static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state,
> >  					  const struct drm_connector_state *conn_state)
> >  {
> > @@ -208,6 +226,14 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
> >  		if (intel_dp_aux_set_pwm_freq(connector))
> >  			new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
> >  
> > +	if (i915.enable_dbc &&
> > +	    (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {
> > +		if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) {
> > +			new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
> > +			DRM_DEBUG_KMS("Enable dynamic brightness.\n");
> > +		}
> > +	}
> > +
> >  	if (new_dpcd_buf != dpcd_buf) {
> >  		if (drm_dp_dpcd_writeb(&intel_dp->aux,
> >  			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
> > -- 
> > 2.13.1.611.g7e3b11ae1-goog
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD

[Pandiyan, Dhinakaran]

On Tue, 2017-06-27 at 16:23 +0300, David Weinehall wrote:
> On Mon, Jun 26, 2017 at 05:18:19PM +0300, David Weinehall wrote:
> > On Thu, Jun 22, 2017 at 12:03:39PM -0700, Puthikorn Voravootivat wrote:
> > > This patch adds option to enable dynamic backlight for eDP
> > > panel that supports this feature via DPCD register and
> > > set minimum / maximum brightness to 0% and 100% of the
> > > normal brightness.
> > 
> > This patch causes a regression on my ThinkPad X1 Carbon Gen 4;
> > with enable_dbc = true the backlight stays off; setting i915.enable_dbc=0
> > on boot makes things work properly again.
> 
> Some more testing indicates that while i915.enable_dbc=0 solves the
> initial issue of backlight not coming on, it's not enough--after
> suspend/resume the backlight stays off.
> 
> Passing i915.enable_dpcd_backlight=0 seems to work both for the initial
> case of backlight not coming on at boot, and the issue with backlight
> not coming on after suspend/resume.
> 
> So:
> 
> DBC:1, DPCD: -1: blank screen on boot
> DBC:0, DPCD: -1: screen OK on boot, blank after suspend/resume
> DBC:1, DPCD:  0: screen OK on boot, screen OK after suspend/resume
> DBC:0, DPCD:  0: screen OK on boot, screen OK after suspend/resume
> 
> So it seems that at least for the ThinkPad X1 Carbon Gen 4 (possibly
> limited to the 2560x1440 model) DPCD backlight isn't supported.
> 

Hi David, 

Thanks for the bug report. It's come a bit sooner than I expected
though.

We shouldn't be enabling DPCD backlight control if the panel does not
support it. I'd like to look at the dmesg with drm.debug=0xE. We debug
log the panel capabilities early during boot and that will be useful.
Can you also provide the output of i915_dpcd in debugfs for the eDP
connector?

I guess it makes sense to file an FDO and attach the logs there. 



-DK

> Or possibly something is wrong with the DPCD backlight patch.
> 
> > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_params.c            |  5 +++++
> > >  drivers/gpu/drm/i915/i915_params.h            |  3 ++-
> > >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 26 ++++++++++++++++++++++++++
> > >  3 files changed, 33 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > > index 5b5ab15d191f..88b9d3e6713a 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = {
> > >  	.inject_load_failure = 0,
> > >  	.enable_dpcd_backlight = -1,
> > >  	.enable_gvt = false,
> > > +	.enable_dbc = true,
> > >  };
> > >  
> > >  module_param_named(modeset, i915.modeset, int, 0400);
> > > @@ -254,3 +255,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
> > >  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
> > >  MODULE_PARM_DESC(enable_gvt,
> > >  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> > > +
> > > +module_param_named_unsafe(enable_dbc, i915.enable_dbc, bool, 0600);
> > > +MODULE_PARM_DESC(enable_dbc,
> > > +	"Enable support for dynamic backlight control (default:true)");
> > > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > > index 0d6cf9138dc4..057e203e6bda 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.h
> > > +++ b/drivers/gpu/drm/i915/i915_params.h
> > > @@ -67,7 +67,8 @@
> > >  	func(bool, verbose_state_checks); \
> > >  	func(bool, nuclear_pageflip); \
> > >  	func(bool, enable_dp_mst); \
> > > -	func(bool, enable_gvt)
> > > +	func(bool, enable_gvt); \
> > > +	func(bool, enable_dbc)
> > >  
> > >  #define MEMBER(T, member) T member
> > >  struct i915_params {
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > index fea161727c6e..b25cd88fc1c5 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > @@ -173,6 +173,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
> > >  	return true;
> > >  }
> > >  
> > > +/*
> > > +* Set minimum / maximum dynamic brightness percentage. This value is expressed
> > > +* as the percentage of normal brightness in 5% increments.
> > > +*/
> > > +static bool
> > > +intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp,
> > > +					   u32 min, u32 max)
> > > +{
> > > +	u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) };
> > > +
> > > +	if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET,
> > > +			  dbc, sizeof(dbc)) < 0) {
> > > +		DRM_DEBUG_KMS("Failed to write aux DBC brightness level\n");
> > > +		return false;
> > > +	}
> > > +	return true;
> > > +}
> > > +
> > >  static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state,
> > >  					  const struct drm_connector_state *conn_state)
> > >  {
> > > @@ -208,6 +226,14 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
> > >  		if (intel_dp_aux_set_pwm_freq(connector))
> > >  			new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
> > >  
> > > +	if (i915.enable_dbc &&
> > > +	    (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {
> > > +		if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) {
> > > +			new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
> > > +			DRM_DEBUG_KMS("Enable dynamic brightness.\n");
> > > +		}
> > > +	}
> > > +
> > >  	if (new_dpcd_buf != dpcd_buf) {
> > >  		if (drm_dp_dpcd_writeb(&intel_dp->aux,
> > >  			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
> > > -- 
> > > 2.13.1.611.g7e3b11ae1-goog
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD

[David Weinehall]

On Tue, Jun 27, 2017 at 10:29:33PM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Tue, 2017-06-27 at 16:23 +0300, David Weinehall wrote:
> > On Mon, Jun 26, 2017 at 05:18:19PM +0300, David Weinehall wrote:
> > > On Thu, Jun 22, 2017 at 12:03:39PM -0700, Puthikorn Voravootivat wrote:
> > > > This patch adds option to enable dynamic backlight for eDP
> > > > panel that supports this feature via DPCD register and
> > > > set minimum / maximum brightness to 0% and 100% of the
> > > > normal brightness.
> > > 
> > > This patch causes a regression on my ThinkPad X1 Carbon Gen 4;
> > > with enable_dbc = true the backlight stays off; setting i915.enable_dbc=0
> > > on boot makes things work properly again.
> > 
> > Some more testing indicates that while i915.enable_dbc=0 solves the
> > initial issue of backlight not coming on, it's not enough--after
> > suspend/resume the backlight stays off.
> > 
> > Passing i915.enable_dpcd_backlight=0 seems to work both for the initial
> > case of backlight not coming on at boot, and the issue with backlight
> > not coming on after suspend/resume.
> > 
> > So:
> > 
> > DBC:1, DPCD: -1: blank screen on boot
> > DBC:0, DPCD: -1: screen OK on boot, blank after suspend/resume
> > DBC:1, DPCD:  0: screen OK on boot, screen OK after suspend/resume
> > DBC:0, DPCD:  0: screen OK on boot, screen OK after suspend/resume
> > 
> > So it seems that at least for the ThinkPad X1 Carbon Gen 4 (possibly
> > limited to the 2560x1440 model) DPCD backlight isn't supported.
> > 
> 
> Hi David, 
> 
> Thanks for the bug report. It's come a bit sooner than I expected
> though.
> 
> We shouldn't be enabling DPCD backlight control if the panel does not
> support it. I'd like to look at the dmesg with drm.debug=0xE. We debug
> log the panel capabilities early during boot and that will be useful.
> Can you also provide the output of i915_dpcd in debugfs for the eDP
> connector?
> 
> I guess it makes sense to file an FDO and attach the logs there. 

https://bugs.freedesktop.org/show_bug.cgi?id=101619


Kind regards, David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel