diff mbox

[v5,2/9] drm/i915: Correctly enable backlight brightness adjustment via DPCD

Message ID 20170504002836.120988-3-puthik@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Puthikorn Voravootivat May 4, 2017, 12:28 a.m. UTC
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>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 33 ++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 6 deletions(-)

Comments

Dhinakaran Pandiyan May 8, 2017, 6:07 p.m. UTC | #1
On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote:
> 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>


Spec says both 0x00 and 0x01 are possible power-on defaults. So,

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 24a905d1a74b..ad8560c5f689 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)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 24a905d1a74b..ad8560c5f689 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)