public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT"
@ 2015-01-06 19:48 Rodrigo Vivi
  2015-01-07  8:16 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Rodrigo Vivi @ 2015-01-06 19:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Kristian Høgsberg, Rodrigo Vivi

This reverts commit 371abae844ede392066bfc21202b2e40f4a654d1.

This data seems unreliable and causing many issues and blocking other
teams and feature implementation. Safest way is to revert that for now.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88081
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88039
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 -
 drivers/gpu/drm/i915/intel_bios.c | 20 --------------------
 drivers/gpu/drm/i915/intel_bios.h | 11 -----------
 3 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 701a694..99ed307 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1411,7 +1411,6 @@ struct intel_vbt_data {
 		bool present;
 		bool active_low_pwm;
 		u8 min_brightness;	/* min_brightness/255 of max */
-		u8 controller;		/* brightness controller number */
 	} backlight;
 
 	/* MIPI DSI */
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 65b1fbc..3f17825 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -314,7 +314,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 {
 	const struct bdb_lfp_backlight_data *backlight_data;
 	const struct bdb_lfp_backlight_data_entry *entry;
-	const struct bdb_lfp_backlight_control_data *bl_ctrl_data;
 
 	backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
 	if (!backlight_data)
@@ -327,7 +326,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	}
 
 	entry = &backlight_data->data[panel_type];
-	bl_ctrl_data = &backlight_data->blc_ctl[panel_type];
 
 	dev_priv->vbt.backlight.present = entry->type == BDB_BACKLIGHT_TYPE_PWM;
 	if (!dev_priv->vbt.backlight.present) {
@@ -339,30 +337,12 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
 	dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm;
 	dev_priv->vbt.backlight.min_brightness = entry->min_brightness;
-
-	dev_priv->vbt.backlight.controller = 0;
-	if (bdb->version >= 191) {
-		dev_priv->vbt.backlight.present =
-				bl_ctrl_data->pin == BLC_CONTROL_PIN_DDI;
-		if (!dev_priv->vbt.backlight.present) {
-			DRM_DEBUG_KMS("BL control pin is not DDI (pin %u)\n",
-					bl_ctrl_data->pin);
-			return;
-		}
-		if (bl_ctrl_data->controller == 1)
-			dev_priv->vbt.backlight.controller =
-				bl_ctrl_data->controller;
-	}
-
 	DRM_DEBUG_KMS("VBT backlight PWM modulation frequency %u Hz, "
 		      "active %s, min brightness %u, level %u\n",
 		      dev_priv->vbt.backlight.pwm_freq_hz,
 		      dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
 		      dev_priv->vbt.backlight.min_brightness,
 		      backlight_data->level[panel_type]);
-
-	DRM_DEBUG_KMS("VBT BL controller %u\n",
-		dev_priv->vbt.backlight.controller);
 }
 
 /* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 9a7202e..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -402,21 +402,10 @@ struct bdb_lfp_backlight_data_entry {
 	u8 obsolete3;
 } __packed;
 
-#define BLC_CONTROL_PIN_PMIC		0
-#define BLC_CONTROL_PIN_LPSS_PWM	1
-#define BLC_CONTROL_PIN_DDI		2
-#define BLC_CONTROL_PIN_CABC		3
-
-struct bdb_lfp_backlight_control_data {
-	u8 controller:4;
-	u8 pin:4;
-} __packed;
-
 struct bdb_lfp_backlight_data {
 	u8 entry_size;
 	struct bdb_lfp_backlight_data_entry data[16];
 	u8 level[16];
-	struct bdb_lfp_backlight_control_data blc_ctl[16];
 } __packed;
 
 struct aimdb_header {
-- 
1.9.3

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT"
  2015-01-06 19:48 [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT" Rodrigo Vivi
@ 2015-01-07  8:16 ` Daniel Vetter
  2015-01-07  8:30   ` Jani Nikula
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2015-01-07  8:16 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Ben Widawsky, Deepak M, intel-gfx, Kristian Høgsberg

On Tue, Jan 06, 2015 at 02:48:15PM -0500, Rodrigo Vivi wrote:
> This reverts commit 371abae844ede392066bfc21202b2e40f4a654d1.
> 
> This data seems unreliable and causing many issues and blocking other
> teams and feature implementation. Safest way is to revert that for now.
> 
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88081
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88039
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Cc: Vandana Kannan <vandana.kannan@intel.com>
Cc: Deepak M <m.deepak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>

When submitting a revert please include the cc/submitter/reviewers from
the original patch too. I've done that now.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 -
>  drivers/gpu/drm/i915/intel_bios.c | 20 --------------------
>  drivers/gpu/drm/i915/intel_bios.h | 11 -----------
>  3 files changed, 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 701a694..99ed307 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1411,7 +1411,6 @@ struct intel_vbt_data {
>  		bool present;
>  		bool active_low_pwm;
>  		u8 min_brightness;	/* min_brightness/255 of max */
> -		u8 controller;		/* brightness controller number */
>  	} backlight;
>  
>  	/* MIPI DSI */
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 65b1fbc..3f17825 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -314,7 +314,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  {
>  	const struct bdb_lfp_backlight_data *backlight_data;
>  	const struct bdb_lfp_backlight_data_entry *entry;
> -	const struct bdb_lfp_backlight_control_data *bl_ctrl_data;
>  
>  	backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
>  	if (!backlight_data)
> @@ -327,7 +326,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	}
>  
>  	entry = &backlight_data->data[panel_type];
> -	bl_ctrl_data = &backlight_data->blc_ctl[panel_type];
>  
>  	dev_priv->vbt.backlight.present = entry->type == BDB_BACKLIGHT_TYPE_PWM;
>  	if (!dev_priv->vbt.backlight.present) {
> @@ -339,30 +337,12 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
>  	dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm;
>  	dev_priv->vbt.backlight.min_brightness = entry->min_brightness;
> -
> -	dev_priv->vbt.backlight.controller = 0;
> -	if (bdb->version >= 191) {
> -		dev_priv->vbt.backlight.present =
> -				bl_ctrl_data->pin == BLC_CONTROL_PIN_DDI;
> -		if (!dev_priv->vbt.backlight.present) {
> -			DRM_DEBUG_KMS("BL control pin is not DDI (pin %u)\n",
> -					bl_ctrl_data->pin);
> -			return;
> -		}
> -		if (bl_ctrl_data->controller == 1)
> -			dev_priv->vbt.backlight.controller =
> -				bl_ctrl_data->controller;
> -	}
> -
>  	DRM_DEBUG_KMS("VBT backlight PWM modulation frequency %u Hz, "
>  		      "active %s, min brightness %u, level %u\n",
>  		      dev_priv->vbt.backlight.pwm_freq_hz,
>  		      dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
>  		      dev_priv->vbt.backlight.min_brightness,
>  		      backlight_data->level[panel_type]);
> -
> -	DRM_DEBUG_KMS("VBT BL controller %u\n",
> -		dev_priv->vbt.backlight.controller);
>  }
>  
>  /* Try to find sdvo panel data */
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 9a7202e..a6a8710 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -402,21 +402,10 @@ struct bdb_lfp_backlight_data_entry {
>  	u8 obsolete3;
>  } __packed;
>  
> -#define BLC_CONTROL_PIN_PMIC		0
> -#define BLC_CONTROL_PIN_LPSS_PWM	1
> -#define BLC_CONTROL_PIN_DDI		2
> -#define BLC_CONTROL_PIN_CABC		3
> -
> -struct bdb_lfp_backlight_control_data {
> -	u8 controller:4;
> -	u8 pin:4;
> -} __packed;
> -
>  struct bdb_lfp_backlight_data {
>  	u8 entry_size;
>  	struct bdb_lfp_backlight_data_entry data[16];
>  	u8 level[16];
> -	struct bdb_lfp_backlight_control_data blc_ctl[16];
>  } __packed;
>  
>  struct aimdb_header {
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT"
  2015-01-07  8:16 ` Daniel Vetter
@ 2015-01-07  8:30   ` Jani Nikula
  2015-01-07  8:47     ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2015-01-07  8:30 UTC (permalink / raw)
  To: Daniel Vetter, Rodrigo Vivi
  Cc: Deepak M, intel-gfx, Kristian Høgsberg, Ben Widawsky

On Wed, 07 Jan 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jan 06, 2015 at 02:48:15PM -0500, Rodrigo Vivi wrote:
>> This reverts commit 371abae844ede392066bfc21202b2e40f4a654d1.
>> 
>> This data seems unreliable and causing many issues and blocking other
>> teams and feature implementation. Safest way is to revert that for now.
>> 
>> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88081
>> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88039

IMO s/Reference/Bugzilla/

Also

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87671

>> Cc: Ben Widawsky <ben@bwidawsk.net>
>> Cc: Kristian Høgsberg <hoegsberg@gmail.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Cc: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Deepak M <m.deepak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> When submitting a revert please include the cc/submitter/reviewers from
> the original patch too. I've done that now.

The code agrees with the spec, but the VBT on these machines quite
obviously does not agree with the spec.

Vandana, I think you're in a better position than us to figure out
what's going on with VBT vs. the spec.

Daniel, please just pick this up now.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 -
>>  drivers/gpu/drm/i915/intel_bios.c | 20 --------------------
>>  drivers/gpu/drm/i915/intel_bios.h | 11 -----------
>>  3 files changed, 32 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 701a694..99ed307 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1411,7 +1411,6 @@ struct intel_vbt_data {
>>  		bool present;
>>  		bool active_low_pwm;
>>  		u8 min_brightness;	/* min_brightness/255 of max */
>> -		u8 controller;		/* brightness controller number */
>>  	} backlight;
>>  
>>  	/* MIPI DSI */
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 65b1fbc..3f17825 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -314,7 +314,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>>  {
>>  	const struct bdb_lfp_backlight_data *backlight_data;
>>  	const struct bdb_lfp_backlight_data_entry *entry;
>> -	const struct bdb_lfp_backlight_control_data *bl_ctrl_data;
>>  
>>  	backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
>>  	if (!backlight_data)
>> @@ -327,7 +326,6 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>>  	}
>>  
>>  	entry = &backlight_data->data[panel_type];
>> -	bl_ctrl_data = &backlight_data->blc_ctl[panel_type];
>>  
>>  	dev_priv->vbt.backlight.present = entry->type == BDB_BACKLIGHT_TYPE_PWM;
>>  	if (!dev_priv->vbt.backlight.present) {
>> @@ -339,30 +337,12 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>>  	dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
>>  	dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm;
>>  	dev_priv->vbt.backlight.min_brightness = entry->min_brightness;
>> -
>> -	dev_priv->vbt.backlight.controller = 0;
>> -	if (bdb->version >= 191) {
>> -		dev_priv->vbt.backlight.present =
>> -				bl_ctrl_data->pin == BLC_CONTROL_PIN_DDI;
>> -		if (!dev_priv->vbt.backlight.present) {
>> -			DRM_DEBUG_KMS("BL control pin is not DDI (pin %u)\n",
>> -					bl_ctrl_data->pin);
>> -			return;
>> -		}
>> -		if (bl_ctrl_data->controller == 1)
>> -			dev_priv->vbt.backlight.controller =
>> -				bl_ctrl_data->controller;
>> -	}
>> -
>>  	DRM_DEBUG_KMS("VBT backlight PWM modulation frequency %u Hz, "
>>  		      "active %s, min brightness %u, level %u\n",
>>  		      dev_priv->vbt.backlight.pwm_freq_hz,
>>  		      dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
>>  		      dev_priv->vbt.backlight.min_brightness,
>>  		      backlight_data->level[panel_type]);
>> -
>> -	DRM_DEBUG_KMS("VBT BL controller %u\n",
>> -		dev_priv->vbt.backlight.controller);
>>  }
>>  
>>  /* Try to find sdvo panel data */
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
>> index 9a7202e..a6a8710 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -402,21 +402,10 @@ struct bdb_lfp_backlight_data_entry {
>>  	u8 obsolete3;
>>  } __packed;
>>  
>> -#define BLC_CONTROL_PIN_PMIC		0
>> -#define BLC_CONTROL_PIN_LPSS_PWM	1
>> -#define BLC_CONTROL_PIN_DDI		2
>> -#define BLC_CONTROL_PIN_CABC		3
>> -
>> -struct bdb_lfp_backlight_control_data {
>> -	u8 controller:4;
>> -	u8 pin:4;
>> -} __packed;
>> -
>>  struct bdb_lfp_backlight_data {
>>  	u8 entry_size;
>>  	struct bdb_lfp_backlight_data_entry data[16];
>>  	u8 level[16];
>> -	struct bdb_lfp_backlight_control_data blc_ctl[16];
>>  } __packed;
>>  
>>  struct aimdb_header {
>> -- 
>> 1.9.3
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT"
  2015-01-07  8:30   ` Jani Nikula
@ 2015-01-07  8:47     ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-01-07  8:47 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Ben Widawsky, Deepak M, intel-gfx, Kristian Høgsberg,
	Rodrigo Vivi

On Wed, Jan 07, 2015 at 10:30:46AM +0200, Jani Nikula wrote:
> On Wed, 07 Jan 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Jan 06, 2015 at 02:48:15PM -0500, Rodrigo Vivi wrote:
> >> This reverts commit 371abae844ede392066bfc21202b2e40f4a654d1.
> >> 
> >> This data seems unreliable and causing many issues and blocking other
> >> teams and feature implementation. Safest way is to revert that for now.
> >> 
> >> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88081
> >> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=88039
> 
> IMO s/Reference/Bugzilla/

Yup, Bugzilla is for bugs fixed by this patch, References: is more for fyi
stuff and mailing lists. Regressions should always have a bugzilla,
otherwise QA has let something slip through.
> 
> Also
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87671
> 
> >> Cc: Ben Widawsky <ben@bwidawsk.net>
> >> Cc: Kristian Høgsberg <hoegsberg@gmail.com>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > Cc: Vandana Kannan <vandana.kannan@intel.com>
> > Cc: Deepak M <m.deepak@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> >
> > When submitting a revert please include the cc/submitter/reviewers from
> > the original patch too. I've done that now.
> 
> The code agrees with the spec, but the VBT on these machines quite
> obviously does not agree with the spec.
> 
> Vandana, I think you're in a better position than us to figure out
> what's going on with VBT vs. the spec.
> 
> Daniel, please just pick this up now.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-07  8:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-06 19:48 [PATCH] Revert "drm/i915: Parsing LFP brightness control from VBT" Rodrigo Vivi
2015-01-07  8:16 ` Daniel Vetter
2015-01-07  8:30   ` Jani Nikula
2015-01-07  8:47     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox