[PATCH v11 0/3] Enhancement to intel_dp_aux_backlight driver

[PATCH v11 0/3] Enhancement to intel_dp_aux_backlight driver

[Puthikorn Voravootivat]

This patch set contain 3 patches. 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:
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.0.506.g27d5fe0cd-goog

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

[PATCH v11 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.

Change-Id: I96221608e1288ffc03a0fd9c4a658809acda4aca
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 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 a0995c00fc84..6c64e1f75c4e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -97,11 +97,85 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
 	}
 }
 
+/*
+ * 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(struct intel_connector *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) {
@@ -110,18 +184,15 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		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 */
@@ -130,6 +201,17 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		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(connector, connector->panel.backlight.level);
 }
-- 
2.13.0.506.g27d5fe0cd-goog

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

[PATCH v11 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

Change-Id: If8376b09fbf83398c40037a04ddba1035c6b71fc
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 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 b6a7e363d076..3758ae1f11b4 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 34148cc8637c..643dfaf41c1f 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -52,6 +52,7 @@
 	func(int, mmio_debug); \
 	func(int, edp_vswing); \
 	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 6c64e1f75c4e..b73b3d431a82 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -250,15 +250,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;
@@ -269,6 +320,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.0.506.g27d5fe0cd-goog

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

[PATCH v11 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.

Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 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 3758ae1f11b4..d84042ddf1fc 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 643dfaf41c1f..99f68d853c18 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 b73b3d431a82..b2f4cc975a3e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -172,6 +172,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(struct intel_connector *connector)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
@@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		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.0.506.g27d5fe0cd-goog

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

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

[Pandiyan, Dhinakaran]

On Fri, 2017-06-02 at 19:04 -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.
> 
> Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> ---
>  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 3758ae1f11b4..d84042ddf1fc 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 643dfaf41c1f..99f68d853c18 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 b73b3d431a82..b2f4cc975a3e 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -172,6 +172,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(struct intel_connector *connector)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> @@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
>  		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) {

1) The Gerrit Change-ID's you've added in this version *may* be a
problem, haven't seen any patches here with those before. 
2) The PWM v/s AUX heuristics function looks likes a reasonable start
that can be adjusted later. We should be taking into account the number
of brightness levels b/w PWM pin and AUX options IMO.
3) The DPCD r/w needs optimization.

However things lgtm for now. So, 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for
this series.

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

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

[Puthikorn Voravootivat]

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

Sorry forgot to remove Gerrit Change-ID before sending the email.

Do you want me to send the same patches again with your reviewed-by tag and
the Gerrit change-Id removed?

On Mon, Jun 5, 2017 at 11:49 AM, Pandiyan, Dhinakaran <
dhinakaran.pandiyan@intel.com> wrote:

> On Fri, 2017-06-02 at 19:04 -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.
> >
> > Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
> > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> > ---
> >  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 3758ae1f11b4..d84042ddf1fc 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 643dfaf41c1f..99f68d853c18 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 b73b3d431a82..b2f4cc975a3e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -172,6 +172,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(struct intel_connector
> *connector)
> >  {
> >       struct intel_dp *intel_dp = enc_to_intel_dp(&connector->
> encoder->base);
> > @@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct
> intel_connector *connector)
> >               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) {
>
> 1) The Gerrit Change-ID's you've added in this version *may* be a
> problem, haven't seen any patches here with those before.
> 2) The PWM v/s AUX heuristics function looks likes a reasonable start
> that can be adjusted later. We should be taking into account the number
> of brightness levels b/w PWM pin and AUX options IMO.
> 3) The DPCD r/w needs optimization.
>
> However things lgtm for now. So,
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for
> this series.
>
>

[-- Attachment #1.2: Type: text/html, Size: 6558 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

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

[Pandiyan, Dhinakaran]

On Mon, 2017-06-05 at 12:43 -0700, Puthikorn Voravootivat wrote:
> Sorry forgot to remove Gerrit Change-ID before sending the email.
> 
> 
> Do you want me to send the same patches again with your reviewed-by
> tag and
> the Gerrit change-Id removed?

Thanks, please do.

-DK

> 
> On Mon, Jun 5, 2017 at 11:49 AM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com> wrote:
>         On Fri, 2017-06-02 at 19:04 -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.
>         >
>         > Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
>         > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
>         > ---
>         >  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 3758ae1f11b4..d84042ddf1fc 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 643dfaf41c1f..99f68d853c18 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 b73b3d431a82..b2f4cc975a3e 100644
>         > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         > @@ -172,6 +172,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(struct
>         intel_connector *connector)
>         >  {
>         >       struct intel_dp *intel_dp =
>         enc_to_intel_dp(&connector->encoder->base);
>         > @@ -205,6 +223,14 @@ static void
>         intel_dp_aux_enable_backlight(struct intel_connector
>         *connector)
>         >               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) {
>         
>         
>         1) The Gerrit Change-ID's you've added in this version *may*
>         be a
>         problem, haven't seen any patches here with those before.
>         2) The PWM v/s AUX heuristics function looks likes a
>         reasonable start
>         that can be adjusted later. We should be taking into account
>         the number
>         of brightness levels b/w PWM pin and AUX options IMO.
>         3) The DPCD r/w needs optimization.
>         
>         However things lgtm for now. So,
>         Reviewed-by: Dhinakaran Pandiyan
>         <dhinakaran.pandiyan@intel.com> for
>         this series.
>         
> 
> 
> _______________________________________________
> 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

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

[Patchwork]

== Series Details ==

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

== Summary ==

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

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:433s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:584s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:515s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:491s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:278  pass:263  dwarn:5   dfail:0   fail:0   skip:10  time:575s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:463s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:403s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:462s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:503s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:438s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:400s

d919ad0d077110d2d48a4b7503ddc02c3864667d drm-tip: 2017y-06m-02d-21h-49m-57s UTC integration manifest
d3c4cc5 drm/i915: Add option to support dynamic backlight via DPCD
1d25283 drm/i915: Add heuristic to determine better way to adjust brightness
e54332a drm/i915: Set PWM divider to match desired frequency in vbt

== Logs ==

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