[PATCH RESEND v11 0/3] Enhancement to intel_dp_aux_backlight driver

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

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 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 RESEND 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

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 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 RESEND 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.

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

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

[Patchwork]

== Series Details ==

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

== Summary ==

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

Test kms_busy:
        Subgroup basic-flip-default-a:
                pass       -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +1

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:443s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:579s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:483s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:588s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:433s
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:424s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:490s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:567s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:458s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:399s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:464s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:497s
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:543s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:400s

30d3326ecb407cad6c03ef6a6d3805c70ba9f0a9 drm-tip: 2017y-06m-05d-15h-21m-50s UTC integration manifest
09103d1 drm/i915: Add option to support dynamic backlight via DPCD
8c183b4 drm/i915: Add heuristic to determine better way to adjust brightness
90f7012 drm/i915: Set PWM divider to match desired frequency in vbt

== Logs ==

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

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

[Puthikorn Voravootivat]

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

Friendly ping.

Is this patch set good to go now?

On Mon, Jun 5, 2017 at 2:56 PM, Puthikorn Voravootivat <puthik@chromium.org>
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:
> 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
>
>

[-- Attachment #1.2: Type: text/html, Size: 3198 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 RESEND v11 0/3] Enhancement to intel_dp_aux_backlight driver

[Daniel Vetter]

On Mon, Jun 05, 2017 at 02:56:04PM -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:
> 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

Random unrelated observation, but shouldn't we have the dp aux backlight
driver as a generic helper in the drm core dp helper library? Doing sink
logic like this in drivers is kinda wrong ...
-Daniel
-- 
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 RESEND v11 0/3] Enhancement to intel_dp_aux_backlight driver

[Pandiyan, Dhinakaran]

On Tue, 2017-06-20 at 11:03 +0200, Daniel Vetter wrote:
> On Mon, Jun 05, 2017 at 02:56:04PM -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:
> > 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
> 
> Random unrelated observation, but shouldn't we have the dp aux backlight
> driver as a generic helper in the drm core dp helper library? Doing sink
> logic like this in drivers is kinda wrong ...
> -Daniel


We could definitely move some of the stuff to the core helpers. There is
also a bit of follow-up work needed to optimize DPCD read/writes. They
can be done together after this.

I don't know if you had a chance to look at this series in detail. But
if you did, do you see any blockers for accepting this series? 

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

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

[Daniel Vetter]

On Tue, Jun 20, 2017 at 05:46:03PM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Tue, 2017-06-20 at 11:03 +0200, Daniel Vetter wrote:
> > On Mon, Jun 05, 2017 at 02:56:04PM -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:
> > > 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
> > 
> > Random unrelated observation, but shouldn't we have the dp aux backlight
> > driver as a generic helper in the drm core dp helper library? Doing sink
> > logic like this in drivers is kinda wrong ...
> > -Daniel
> 
> 
> We could definitely move some of the stuff to the core helpers. There is
> also a bit of follow-up work needed to optimize DPCD read/writes. They
> can be done together after this.
> 
> I don't know if you had a chance to look at this series in detail. But
> if you did, do you see any blockers for accepting this series? 

No, my comment was really just a quick observation. Follow-up for that is
perfectly fine. I didn't look at the patches in detail.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Mon, Jun 05, 2017 at 02:56:04PM -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:
> 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

Ok Jani's on vacation and this address my major concern, so I'm
volunteered to merge this. But it seems to conflict against latest
drm-intel-next-queued, and against some other backlight refactoring stuff,
so I don't feel safe wrestling it myself.

Can you pls rebase?

Sorry for all this dragging out.

Thanks, Daniel

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

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