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

[Pandiyan, Dhinakaran]

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

Thanks, please do.

-DK

> 
> On Mon, Jun 5, 2017 at 11:49 AM, Pandiyan, Dhinakaran
>  wrote:
> On Fri, 2017-06-02 at 19:04 -0700, Puthikorn Voravootivat
> wrote:
> > This patch adds option to enable dynamic backlight for eDP
> > panel that supports this feature via DPCD register and
> > set minimum / maximum brightness to 0% and 100% of the
> > normal brightness.
> >
> > Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
> > Signed-off-by: Puthikorn Voravootivat 
> > ---
> >  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(_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(>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] &
> 

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

[Puthikorn Voravootivat]

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

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

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

> On Fri, 2017-06-02 at 19:04 -0700, Puthikorn Voravootivat wrote:
> > This patch adds option to enable dynamic backlight for eDP
> > panel that supports this feature via DPCD register and
> > set minimum / maximum brightness to 0% and 100% of the
> > normal brightness.
> >
> > Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
> > Signed-off-by: Puthikorn Voravootivat 
> > ---
> >  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(_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(>
> 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(_dp->aux,
> >   DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf)
> < 0) {
>
> 1) The Gerrit Change-ID's you've added in this version *may* be a
> problem, haven't seen any patches here with those before.
> 2) The PWM v/s AUX heuristics function looks likes a reasonable start
> that can be adjusted later. We should be taking into account the number
> of brightness levels b/w PWM pin and AUX options IMO.
> 3) The DPCD r/w needs optimization.
>
> However things lgtm for now. So,
> Reviewed-by: Dhinakaran Pandiyan  for
> this 

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

[Pandiyan, Dhinakaran]

On Fri, 2017-06-02 at 19:04 -0700, Puthikorn Voravootivat wrote:
> This patch adds option to enable dynamic backlight for eDP
> panel that supports this feature via DPCD register and
> set minimum / maximum brightness to 0% and 100% of the
> normal brightness.
> 
> Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
> Signed-off-by: Puthikorn Voravootivat 
> ---
>  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(_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(>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(_dp->aux,
>   DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {

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

However things lgtm for now. So, 
Reviewed-by: Dhinakaran Pandiyan  for
this series.

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

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

[Puthikorn Voravootivat]

This patch adds option to enable dynamic backlight for eDP
panel that supports this feature via DPCD register and
set minimum / maximum brightness to 0% and 100% of the
normal brightness.

Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b
Signed-off-by: Puthikorn Voravootivat 
---
 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(_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(>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(_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