Re: [PATCH v7 6/9] drm/i915: Add option to support dynamic backlight via DPCD

["Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>]

On Thu, 2017-05-11 at 16:02 -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.
> 
> 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 | 40 +++++++++++++++++++++++----
>  3 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 13cf3f1572ab..6eaf660e74da 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 = false,

Thanks for adding this switch, however I am not sure about doing this
via a kernel parameter. Controlling this via a drm_property is another
way. But, that would require user space changes. 

Jani, 
What are your thoughts on adding another kernel parameter. iirc there
were some concerns in the mailing list about adding new params a while
back. 


-DK


>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -255,3 +256,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(enable_dbc, i915.enable_dbc, bool, 0600);
> +MODULE_PARM_DESC(enable_dbc,
> +	"Enable support for dynamic backlight control (default:false)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index ac02efce6e22..2de3e2850b54 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -67,7 +67,8 @@
>  	func(bool, nuclear_pageflip); \
>  	func(bool, enable_dp_mst); \
>  	func(int, enable_dpcd_backlight); \
> -	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 a72893da78d0..1c5459bc20ae 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,20 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
>  		break;
>  	}
>  
> +	if (i915.enable_dbc &&
> +	    (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