[PATCH v6 0/9] Enhancement to intel_dp_aux_backlight driver

[PATCH v6 0/9] Enhancement to intel_dp_aux_backlight driver

[Puthikorn Voravootivat]

This patch set contain 9 patches.
- First five patches fix bug in the driver and allow choosing which
  way to adjust brightness if both PWM pin and AUX are supported
- Next patch adds enable DBC by default
- Next patch makes the driver restore last brightness level after
  turning display off and on.
- Last two patches set the PWM freqency to match data in panel vbt.

Change log:
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 (9):
  drm/i915: Fix cap check for intel_dp_aux_backlight driver
  drm/i915: Correctly enable backlight brightness adjustment via DPCD
  drm/i915: Drop AUX backlight enable check for backlight control
  drm/i915: Allow choosing how to adjust brightness if both supported
  drm/i915: Set backlight mode before enable backlight
  drm/i915: Support dynamic backlight via DPCD register
  drm/i915: Restore brightness level in aux backlight driver
  drm: Add definition for eDP backlight frequency
  drm/i915: Set PWM divider to match desired frequency in vbt

 drivers/gpu/drm/i915/i915_params.c            |   8 +-
 drivers/gpu/drm/i915/i915_params.h            |   2 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 171 ++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h                   |   2 +
 4 files changed, 167 insertions(+), 16 deletions(-)

-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver

[Puthikorn Voravootivat]

intel_dp_aux_backlight driver should check for the
DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 6532e226db29..341bf2cb0c25 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -144,6 +144,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
 	 */
 	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
 	    (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
+	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
 	    !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
 	      (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) {
 		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 2/9] drm/i915: Correctly enable backlight brightness adjustment via DPCD

[Puthikorn Voravootivat]

intel_dp_aux_enable_backlight() assumed that the register
BACKLIGHT_BRIGHTNESS_CONTROL_MODE can only has value 01
(DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET) when initialize.

This patch fixed that by handling all cases of that register.

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 | 33 ++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 341bf2cb0c25..870c03fc0f3a 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -97,15 +97,36 @@ 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;
 
 	set_aux_backlight_enable(intel_dp, true);
 
-	if ((drm_dp_dpcd_readb(&intel_dp->aux,
-			       DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) == 1) &&
-	    ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) ==
-	     DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET))
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
-				   (dpcd_buf | DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD));
+	if (drm_dp_dpcd_readb(&intel_dp->aux,
+			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) {
+		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
+			      DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
+		return;
+	}
+
+	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");
+		}
+		break;
+
+	/* Do nothing when it is already DPCD mode */
+	case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD:
+	default:
+		break;
+	}
 }
 
 static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 3/9] drm/i915: Drop AUX backlight enable check for backlight control

[Puthikorn Voravootivat]

There are some panel that
(1) does not support display backlight enable via AUX
(2) support display backlight adjustment via AUX
(3) support display backlight enable via eDP BL_ENABLE pin

The current driver required that (1) must be support to enable (2).
This patch drops that requirement.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 870c03fc0f3a..c22712762957 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable)
 {
 	uint8_t reg_val = 0;
 
+       /* Early return when display use other mechanism to enable backlight. */
+	if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
+		return;
+
 	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
 			      &reg_val) < 0) {
 		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
@@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
 	 * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
 	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
 	    !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
 	      (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) {
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 4/9] drm/i915: Allow choosing how to adjust brightness if both supported

[Puthikorn Voravootivat]

Add option to allow choosing how to adjust brightness if
panel supports both PWM pin and AUX channel.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 drivers/gpu/drm/i915/i915_params.c            |  8 +++++---
 drivers/gpu/drm/i915/i915_params.h            |  2 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 23 ++++++++++++++++++-----
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, "
+	"1:Use DPCD if both 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..ac02efce6e22 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -66,7 +66,7 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
-	func(bool, enable_dpcd_backlight); \
+	func(int, 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 c22712762957..e82f7cb9a7af 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -167,21 +167,34 @@ 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[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_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;
 }
 
+static bool
+intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+
+	/* Check the  eDP Display control capabilities registers to determine if
+	 * the panel can support backlight control via BL_PWM_DIM eDP pin
+	 */
+	return intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP;
+}
+
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 {
 	struct intel_panel *panel = &intel_connector->panel;
 
-	if (!i915.enable_dpcd_backlight)
+	if (i915.enable_dpcd_backlight == -1)
+		return -ENODEV;
+
+	if (i915.enable_dpcd_backlight == 0 &&
+	    intel_dp_pwm_pin_display_control_capable(intel_connector))
 		return -ENODEV;
 
 	if (!intel_dp_aux_display_control_capable(intel_connector))
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 5/9] drm/i915: Set backlight mode before enable backlight

[Puthikorn Voravootivat]

We should set backlight mode register before set register to
enable the backlight.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index e82f7cb9a7af..5ef3ade7c40e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -103,8 +103,6 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 	uint8_t dpcd_buf = 0;
 	uint8_t edp_backlight_mode = 0;
 
-	set_aux_backlight_enable(intel_dp, true);
-
 	if (drm_dp_dpcd_readb(&intel_dp->aux,
 			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) {
 		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
@@ -131,6 +129,8 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 	default:
 		break;
 	}
+
+	set_aux_backlight_enable(intel_dp, true);
 }
 
 static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 6/9] drm/i915: Support dynamic backlight via DPCD register

[Puthikorn Voravootivat]

This patch enables dynamic backlight by default 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>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 39 ++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 5ef3ade7c40e..7d323af96636 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -97,10 +97,27 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
 	}
 }
 
+/*
+ * Set minimum / maximum dynamic brightness percentage. This value is expressed
+ * as the percentage of normal brightness in 5% increments.
+ */
+static void
+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");
+	}
+}
+
 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 new_dpcd_buf = 0;
 	uint8_t edp_backlight_mode = 0;
 
 	if (drm_dp_dpcd_readb(&intel_dp->aux,
@@ -110,18 +127,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 +144,19 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		break;
 	}
 
+	if (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP) {
+		new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
+		intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100);
+		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) {
+			DRM_DEBUG_KMS("Failed to write aux backlight mode\n");
+		}
+	}
+
 	set_aux_backlight_enable(intel_dp, true);
 }
 
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 7/9] drm/i915: Restore brightness level in aux backlight driver

[Puthikorn Voravootivat]

Some panel will default to zero brightness when turning the
panel off and on again. This patch restores last brightness
level back when panel is turning back on.

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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 7d323af96636..fc26fea94fd4 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -158,6 +158,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 	}
 
 	set_aux_backlight_enable(intel_dp, true);
+	intel_dp_aux_set_backlight(connector, connector->panel.backlight.level);
 }
 
 static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 8/9] drm: Add definition for eDP backlight frequency

[Puthikorn Voravootivat]

This patch adds the following definition
- Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap
  register which only use bit 0:4
- Base frequency (27 MHz) for backlight PWM frequency
  generator.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 include/drm/drm_dp_helper.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c0bd0d7651a9..810b7d5d9f2b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -572,10 +572,12 @@
 #define DP_EDP_PWMGEN_BIT_COUNT             0x724
 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN     0x725
 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX     0x726
+# define  DP_EDP_PWMGEN_BIT_COUNT_MASK      (0x1f << 0)
 
 #define DP_EDP_BACKLIGHT_CONTROL_STATUS     0x727
 
 #define DP_EDP_BACKLIGHT_FREQ_SET           0x728
+# define DP_EDP_BACKLIGHT_FREQ_BASE         27000000
 
 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB   0x72a
 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID   0x72b
-- 
2.13.0.rc2.291.g57267f2277-goog

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

[PATCH v6 9/9] 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 is still within 25%
of the desired frequency.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 +++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index fc26fea94fd4..0549ccb1bb09 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp,
 	}
 }
 
+/*
+ * 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 void 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, f, fxp_min, fxp_max, fxp_actual;
+	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;
+	}
+
+	fxp = DP_EDP_BACKLIGHT_FREQ_BASE / 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
+	 * - Effective frequency is within 25% of desired frequency.
+	 */
+	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;
+	}
+	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;
+	}
+	pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+	pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
+
+	fxp_min = fxp * 3 / 4;
+	fxp_max = 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;
+	}
+
+	for (pn = pn_max; pn > pn_min; pn--) {
+		f = clamp(fxp >> 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;
+	}
+	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;
+	}
+}
+
 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 new_dpcd_buf = 0;
 	uint8_t edp_backlight_mode = 0;
+	bool freq_cap;
 
 	if (drm_dp_dpcd_readb(&intel_dp->aux,
 			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) {
@@ -150,6 +224,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		DRM_DEBUG_KMS("Enable dynamic brightness.\n");
 	}
 
+	freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP;
+	if (freq_cap)
+		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) {
@@ -157,6 +235,9 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 		}
 	}
 
+	if (freq_cap)
+		intel_dp_aux_set_pwm_freq(connector);
+
 	set_aux_backlight_enable(intel_dp, true);
 	intel_dp_aux_set_backlight(connector, connector->panel.backlight.level);
 }
-- 
2.13.0.rc2.291.g57267f2277-goog

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

✗ Fi.CI.BAT: failure for Enhancement to intel_dp_aux_backlight driver (rev5)

[Patchwork]

== Series Details ==

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

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/intel_dp_aux_backlight.o
drivers/gpu/drm/i915/intel_dp_aux_backlight.c: In function ‘intel_dp_aux_enable_backlight’:
drivers/gpu/drm/i915/intel_dp_aux_backlight.c:128:17: error: ‘f’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  int freq, fxp, f, fxp_min, fxp_max, fxp_actual;
                 ^
cc1: all warnings being treated as errors
scripts/Makefile.build:294: recipe for target 'drivers/gpu/drm/i915/intel_dp_aux_backlight.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dp_aux_backlight.o] Error 1
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1002: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH v6 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver

[Pandiyan, Dhinakaran]

On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> intel_dp_aux_backlight driver should check for the
> DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver.
> 
> 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 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6532e226db29..341bf2cb0c25 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -144,6 +144,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
>  	 */
>  	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
>  	    (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
> +	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
>  	    !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
>  	      (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) {
>  		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");

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

Re: [PATCH v6 3/9] drm/i915: Drop AUX backlight enable check for backlight control

[Pandiyan, Dhinakaran]

On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> There are some panel that
> (1) does not support display backlight enable via AUX
> (2) support display backlight adjustment via AUX
> (3) support display backlight enable via eDP BL_ENABLE pin
> 
> The current driver required that (1) must be support to enable (2).
> This patch drops that requirement.
> 
> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 870c03fc0f3a..c22712762957 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable)
>  {
>  	uint8_t reg_val = 0;
>  
> +       /* Early return when display use other mechanism to enable backlight. */
> +	if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
> +		return;

Won't DP_EDP_BACKLIGHT_AUX_ENABLE_CAP be 1 always? The code below, in
intel_dp_aux_display_control_capable(), makes sure
DP_EDP_BACKLIGHT_PIN_ENABLE_CAP=0. The spec says at least one of these
has to be 1.


"BACKLIGHT_AUX_ENABLE_CAPABLE
1 = Indicates that the Sink device supports display backlight
enable through the BACKLIGHT_ENABLE bit in the
EDP_DISPLAY_CONTROL register (DPCD Address 00720h, bit 0).
Must be set to 1 if the BACKLIGHT_PIN_ENABLE_CAPABLE bit (bit 1)
is cleared to 0."

-DK

> +
>  	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
>  			      &reg_val) < 0) {
>  		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
> @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
>  	 * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
>  	    (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
>  	    !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
>  	      (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) {

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

Re: [PATCH v6 4/9] drm/i915: Allow choosing how to adjust brightness if both supported

[Pandiyan, Dhinakaran]

On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> Add option to allow choosing how to adjust brightness if
> panel supports both PWM pin and AUX channel.
> 
> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> ---
>  drivers/gpu/drm/i915/i915_params.c            |  8 +++++---
>  drivers/gpu/drm/i915/i915_params.h            |  2 +-
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 23 ++++++++++++++++++-----
>  3 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, "
> +	"1:Use DPCD if both 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..ac02efce6e22 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -66,7 +66,7 @@
>  	func(bool, verbose_state_checks); \
>  	func(bool, nuclear_pageflip); \
>  	func(bool, enable_dp_mst); \
> -	func(bool, enable_dpcd_backlight); \
> +	func(int, 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 c22712762957..e82f7cb9a7af 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -167,21 +167,34 @@ 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[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_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;
>  }
>  
> +static bool
> +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> +
> +	/* Check the  eDP Display control capabilities registers to determine if
> +	 * the panel can support backlight control via BL_PWM_DIM eDP pin
> +	 */
> +	return intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP;


This is not right. The bits in DPCD 0x702 don't mean anything if
DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP in 0x701 is not set.


-DK


> +}
> +
>  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
>  {
>  	struct intel_panel *panel = &intel_connector->panel;
>  
> -	if (!i915.enable_dpcd_backlight)
> +	if (i915.enable_dpcd_backlight == -1)
> +		return -ENODEV;
> +
> +	if (i915.enable_dpcd_backlight == 0 &&
> +	    intel_dp_pwm_pin_display_control_capable(intel_connector))
>  		return -ENODEV;
>  
>  	if (!intel_dp_aux_display_control_capable(intel_connector))

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

Re: [PATCH v6 8/9] drm: Add definition for eDP backlight frequency

[Pandiyan, Dhinakaran]

On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> This patch adds the following definition
> - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap
>   register which only use bit 0:4
> - Base frequency (27 MHz) for backlight PWM frequency
>   generator.
> 
> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> ---
>  include/drm/drm_dp_helper.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c0bd0d7651a9..810b7d5d9f2b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -572,10 +572,12 @@
>  #define DP_EDP_PWMGEN_BIT_COUNT             0x724
>  #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN     0x725
>  #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX     0x726
> +# define  DP_EDP_PWMGEN_BIT_COUNT_MASK      (0x1f << 0)
>  
>  #define DP_EDP_BACKLIGHT_CONTROL_STATUS     0x727
>  
>  #define DP_EDP_BACKLIGHT_FREQ_SET           0x728
> +# define DP_EDP_BACKLIGHT_FREQ_BASE         27000000

I have seen frequency values expressed in kHz in most places within drm,
I think it's best to write this also in kHz to avoid confusion. With the
units addressed,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

>  
>  #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB   0x72a
>  #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID   0x72b

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

Re: [PATCH v6 6/9] drm/i915: Support dynamic backlight via DPCD register

[Pandiyan, Dhinakaran]

On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> This patch enables dynamic backlight by default for eDP
> panel that supports this feature via DPCD register and
> set minimum / maximum brightness to 0% and 100% of the
> normal brightness.

I realized I replied to the previous version of this patch.

Should there be a switch for a feature like this that can affect image
quality?

-DK

> 
> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 39 ++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 5ef3ade7c40e..7d323af96636 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -97,10 +97,27 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
>  	}
>  }
>  
> +/*
> + * Set minimum / maximum dynamic brightness percentage. This value is expressed
> + * as the percentage of normal brightness in 5% increments.
> + */
> +static void
> +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");
> +	}
> +}
> +
>  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 new_dpcd_buf = 0;
>  	uint8_t edp_backlight_mode = 0;
>  
>  	if (drm_dp_dpcd_readb(&intel_dp->aux,
> @@ -110,18 +127,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 +144,19 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
>  		break;
>  	}
>  
> +	if (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP) {
> +		new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;
> +		intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100);
> +		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) {
> +			DRM_DEBUG_KMS("Failed to write aux backlight mode\n");
> +		}
> +	}
> +
>  	set_aux_backlight_enable(intel_dp, true);
>  }
>  

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

Re: [PATCH v6 3/9] drm/i915: Drop AUX backlight enable check for backlight control

[Puthikorn Voravootivat]

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

On Wed, May 10, 2017 at 5:39 PM, Pandiyan, Dhinakaran <
dhinakaran.pandiyan@intel.com> wrote:

> On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote:
> > There are some panel that
> > (1) does not support display backlight enable via AUX
> > (2) support display backlight adjustment via AUX
> > (3) support display backlight enable via eDP BL_ENABLE pin
> >
> > The current driver required that (1) must be support to enable (2).
> > This patch drops that requirement.
> >
> > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index 870c03fc0f3a..c22712762957 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp
> *intel_dp, bool enable)
> >  {
> >       uint8_t reg_val = 0;
> >
> > +       /* Early return when display use other mechanism to enable
> backlight. */
> > +     if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
> > +             return;
>
> Won't DP_EDP_BACKLIGHT_AUX_ENABLE_CAP be 1 always? The code below, in
> intel_dp_aux_display_control_capable(), makes sure
> DP_EDP_BACKLIGHT_PIN_ENABLE_CAP=0. The spec says at least one of these
> has to be 1.
>
> We will drop the  DP_EDP_BACKLIGHT_PIN_ENABLE_CAP != 0 check in next
patch set.
This patch adds check here to prepare for that.

>
> "BACKLIGHT_AUX_ENABLE_CAPABLE
> 1 = Indicates that the Sink device supports display backlight
> enable through the BACKLIGHT_ENABLE bit in the
> EDP_DISPLAY_CONTROL register (DPCD Address 00720h, bit 0).
> Must be set to 1 if the BACKLIGHT_PIN_ENABLE_CAPABLE bit (bit 1)
> is cleared to 0."
>
> -DK
>
> > +
> >       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_
> REGISTER,
> >                             &reg_val) < 0) {
> >               DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
> > @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct
> intel_connector *connector)
> >        * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
> >           (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)
> &&
> >           !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
> >             (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)))
> {
>
>

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

[PATCH v6 4/9] drm/i915: Allow choosing how to adjust brightness if both supported

[Puthikorn Voravootivat]

Add option to allow choosing how to adjust brightness if
panel supports both PWM pin and AUX channel.

Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
---
Fix compile error in v5

 drivers/gpu/drm/i915/i915_params.c            |  8 +++++---
 drivers/gpu/drm/i915/i915_params.h            |  2 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 17 ++++++++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, "
+	"1:Use DPCD if both 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..ac02efce6e22 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -66,7 +66,7 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
-	func(bool, enable_dpcd_backlight); \
+	func(int, 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 5b83c9737644..e82f7cb9a7af 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -175,11 +175,26 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
 	return false;
 }
 
+static bool
+intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+
+	/* Check the  eDP Display control capabilities registers to determine if
+	 * the panel can support backlight control via BL_PWM_DIM eDP pin
+	 */
+	return intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP;
+}
+
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 {
 	struct intel_panel *panel = &intel_connector->panel;
 
-	if (!i915.enable_dpcd_backlight)
+	if (i915.enable_dpcd_backlight == -1)
+		return -ENODEV;
+
+	if (i915.enable_dpcd_backlight == 0 &&
+	    intel_dp_pwm_pin_display_control_capable(intel_connector))
 		return -ENODEV;
 
 	if (!intel_dp_aux_display_control_capable(intel_connector))
-- 
2.13.0.rc1.294.g07d810a77f-goog

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