All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/i915: revert DPCD backlight and DBC enabling by default
@ 2017-07-20  9:25 Jani Nikula
  2017-07-20  9:25 ` [PATCH 1/2] Revert "drm/i915: Add option to support dynamic backlight via DPCD" Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2017-07-20  9:25 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Daniel Vetter, Puthikorn Voravootivat, Jenny TC,
	Dhinakaran Pandiyan

The two commits being reverted regress machines, a production ThinkPad,
and BXT-P in CI, and there hasn't been adequate response to follow-up or
fix the issues. The regressions were reported 3½ weeks ago. The reverts
are long overdue already. Back to the drawing board.

BR,
Jani.


Cc: Jenny TC <jenny.tc@intel.com>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Puthikorn Voravootivat <puthik@chromium.org>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>


Jani Nikula (2):
  Revert "drm/i915: Add option to support dynamic backlight via DPCD"
  Revert "drm/i915: Add heuristic to determine better way to adjust
    brightness"

 drivers/gpu/drm/i915/i915_params.c            | 12 +---
 drivers/gpu/drm/i915/i915_params.h            |  5 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 87 +--------------------------
 3 files changed, 8 insertions(+), 96 deletions(-)

-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] Revert "drm/i915: Add option to support dynamic backlight via DPCD"
  2017-07-20  9:25 [PATCH 0/2] drm/i915: revert DPCD backlight and DBC enabling by default Jani Nikula
@ 2017-07-20  9:25 ` Jani Nikula
  2017-07-20  9:25 ` [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness" Jani Nikula
  2017-07-20 12:02 ` ✓ Fi.CI.BAT: success for drm/i915: revert DPCD backlight and DBC enabling by default Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2017-07-20  9:25 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Daniel Vetter, Puthikorn Voravootivat, Jenny TC,
	Dhinakaran Pandiyan

This reverts commit ae25eceab616d16a07bcaa434b84463d58a3bdc3.

The DPCD backlight commits regress a Thinkpad X1 Carbon 4th Gen and a
BXT-P (in CI). Enabling dynamic backlight boots to a black screen, and
enabling DPCD backlight leads to a black screen after suspend/resume.

References: http://mid.mail-archive.com/20170706135349.6tu3lz7uehazlnnn@boom
References: http://mid.mail-archive.com/20170627132326.f2q3yn4bh5flji4q@boom
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101619
Reported-by: David Weinehall <david.weinehall@linux.intel.com>
Fixes: ae25eceab616 ("drm/i915: Add option to support dynamic backlight via DPCD")
Cc: Jenny TC <jenny.tc@intel.com>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Puthikorn Voravootivat <puthik@chromium.org>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 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, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 88b9d3e6713a..5b5ab15d191f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -65,7 +65,6 @@ 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);
@@ -255,7 +254,3 @@ 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 057e203e6bda..0d6cf9138dc4 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -67,8 +67,7 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
-	func(bool, enable_gvt); \
-	func(bool, enable_dbc)
+	func(bool, enable_gvt)
 
 #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 b25cd88fc1c5..fea161727c6e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -173,24 +173,6 @@ 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(&intel_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(const struct intel_crtc_state *crtc_state,
 					  const struct drm_connector_state *conn_state)
 {
@@ -226,14 +208,6 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
 		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(&intel_dp->aux,
 			DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) {
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness"
  2017-07-20  9:25 [PATCH 0/2] drm/i915: revert DPCD backlight and DBC enabling by default Jani Nikula
  2017-07-20  9:25 ` [PATCH 1/2] Revert "drm/i915: Add option to support dynamic backlight via DPCD" Jani Nikula
@ 2017-07-20  9:25 ` Jani Nikula
  2017-07-20 20:23   ` Pandiyan, Dhinakaran
  2017-07-20 12:02 ` ✓ Fi.CI.BAT: success for drm/i915: revert DPCD backlight and DBC enabling by default Patchwork
  2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2017-07-20  9:25 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Daniel Vetter, Puthikorn Voravootivat, Jenny TC,
	Dhinakaran Pandiyan

This reverts commit 560a758d39c616f83ac25ff6e0816a49ebe6401c.

The DPCD backlight commits regress a Thinkpad X1 Carbon 4th Gen and a
BXT-P (in CI). Enabling dynamic backlight boots to a black screen, and
enabling DPCD backlight leads to a black screen after suspend/resume.

References: http://mid.mail-archive.com/20170706135349.6tu3lz7uehazlnnn@boom
References: http://mid.mail-archive.com/20170627132326.f2q3yn4bh5flji4q@boom
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101619
Reported-by: David Weinehall <david.weinehall@linux.intel.com>
Fixes: 560a758d39c6 ("drm/i915: Add heuristic to determine better way to adjust brightness")
Cc: Jenny TC <jenny.tc@intel.com>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Puthikorn Voravootivat <puthik@chromium.org>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c            |  7 ++-
 drivers/gpu/drm/i915/i915_params.h            |  2 +-
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 61 ++-------------------------
 3 files changed, 7 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5b5ab15d191f..14e2c2e57f96 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 = -1,
+	.enable_dpcd_backlight = false,
 	.enable_gvt = false,
 };
 
@@ -246,10 +246,9 @@ 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_unsafe(enable_dpcd_backlight, i915.enable_dpcd_backlight, int, 0600);
+module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
 MODULE_PARM_DESC(enable_dpcd_backlight,
-	"Enable support for DPCD backlight control "
-	"(-1:auto (default), 0:force disable, 1:force enabled if supported");
+	"Enable support for DPCD backlight control (default:false)");
 
 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 0d6cf9138dc4..febbfdbd30bd 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -53,7 +53,6 @@
 	func(int, edp_vswing); \
 	func(int, reset); \
 	func(unsigned int, inject_load_failure); \
-	func(int, enable_dpcd_backlight); \
 	/* leave bools at the end to not create holes */ \
 	func(bool, alpha_support); \
 	func(bool, enable_cmd_parser); \
@@ -67,6 +66,7 @@
 	func(bool, verbose_state_checks); \
 	func(bool, nuclear_pageflip); \
 	func(bool, enable_dp_mst); \
+	func(bool, 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 fea161727c6e..d2830ba3162e 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -251,66 +251,15 @@ 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)) {
+	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[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
 		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");
 		return true;
 	}
 	return false;
 }
 
-/*
- * Heuristic function whether we should use AUX for backlight adjustment or not.
- *
- * We should use AUX for backlight brightness adjustment if panel doesn't this
- * via PWM pin or using AUX is better than using PWM pin.
- *
- * The heuristic to determine that using AUX pin is better than using PWM pin is
- * that the panel support any of the feature list here.
- * - Regional backlight brightness adjustment
- * - Backlight PWM frequency set
- * - More than 8 bits resolution of brightness level
- * - Backlight enablement via AUX and not by BL_ENABLE pin
- *
- * If all above are not true, assume that using PWM pin is better.
- */
-static bool
-intel_dp_aux_display_control_heuristic(struct intel_connector *connector)
-{
-	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
-	uint8_t reg_val;
-
-	/* Panel doesn't support adjusting backlight brightness via PWN pin */
-	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
-		return true;
-
-	/* Panel supports regional backlight brightness adjustment */
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3,
-			      &reg_val) != 1) {
-		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
-			       DP_EDP_GENERAL_CAP_3);
-		return false;
-	}
-	if (reg_val > 0)
-		return true;
-
-	/* Panel supports backlight PWM frequency set */
-	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
-		return true;
-
-	/* Panel supports more than 8 bits resolution of brightness level */
-	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
-		return true;
-
-	/* Panel supports enabling backlight via AUX but not by BL_ENABLE pin */
-	if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
-	    !(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP))
-		return true;
-
-	return false;
-
-}
-
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 {
 	struct intel_panel *panel = &intel_connector->panel;
@@ -321,10 +270,6 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
 	if (!intel_dp_aux_display_control_capable(intel_connector))
 		return -ENODEV;
 
-	if (i915.enable_dpcd_backlight == -1 &&
-	    !intel_dp_aux_display_control_heuristic(intel_connector))
-		return -ENODEV;
-
 	panel->backlight.setup = intel_dp_aux_setup_backlight;
 	panel->backlight.enable = intel_dp_aux_enable_backlight;
 	panel->backlight.disable = intel_dp_aux_disable_backlight;
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: revert DPCD backlight and DBC enabling by default
  2017-07-20  9:25 [PATCH 0/2] drm/i915: revert DPCD backlight and DBC enabling by default Jani Nikula
  2017-07-20  9:25 ` [PATCH 1/2] Revert "drm/i915: Add option to support dynamic backlight via DPCD" Jani Nikula
  2017-07-20  9:25 ` [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness" Jani Nikula
@ 2017-07-20 12:02 ` Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-07-20 12:02 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: revert DPCD backlight and DBC enabling by default
URL   : https://patchwork.freedesktop.org/series/27623/
State : success

== Summary ==

Series 27623v1 drm/i915: revert DPCD backlight and DBC enabling by default
https://patchwork.freedesktop.org/api/1.0/series/27623/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:431s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:353s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:538s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:514s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:486s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:487s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:602s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:412s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:501s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:579s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:579s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:560s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:468s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:585s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:497s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:407s

948c3bddb3333e7245970c684749ba5d72370cd2 drm-tip: 2017y-07m-20d-10h-52m-43s UTC integration manifest
682e3e2 Revert "drm/i915: Add heuristic to determine better way to adjust brightness"
70cbf1b Revert "drm/i915: Add option to support dynamic backlight via DPCD"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5244/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness"
  2017-07-20  9:25 ` [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness" Jani Nikula
@ 2017-07-20 20:23   ` Pandiyan, Dhinakaran
  2017-07-21  7:01     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-20 20:23 UTC (permalink / raw)
  To: Nikula, Jani
  Cc: puthik@chromium.org, daniel.vetter@ffwll.ch,
	intel-gfx@lists.freedesktop.org, Tc, Jenny




On Thu, 2017-07-20 at 12:25 +0300, Jani Nikula wrote:
> This reverts commit 560a758d39c616f83ac25ff6e0816a49ebe6401c.
> 
> The DPCD backlight commits regress a Thinkpad X1 Carbon 4th Gen and a
> BXT-P (in CI). Enabling dynamic backlight boots to a black screen, and
> enabling DPCD backlight leads to a black screen after suspend/resume.
> 


Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for
both patches.

> References: http://mid.mail-archive.com/20170706135349.6tu3lz7uehazlnnn@boom
> References: http://mid.mail-archive.com/20170627132326.f2q3yn4bh5flji4q@boom
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101619
> Reported-by: David Weinehall <david.weinehall@linux.intel.com>
> Fixes: 560a758d39c6 ("drm/i915: Add heuristic to determine better way to adjust brightness")
> Cc: Jenny TC <jenny.tc@intel.com>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Puthikorn Voravootivat <puthik@chromium.org>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c            |  7 ++-
>  drivers/gpu/drm/i915/i915_params.h            |  2 +-
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 61 ++-------------------------
>  3 files changed, 7 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5b5ab15d191f..14e2c2e57f96 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 = -1,
> +	.enable_dpcd_backlight = false,
>  	.enable_gvt = false,
>  };
>  
> @@ -246,10 +246,9 @@ 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_unsafe(enable_dpcd_backlight, i915.enable_dpcd_backlight, int, 0600);
> +module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
>  MODULE_PARM_DESC(enable_dpcd_backlight,
> -	"Enable support for DPCD backlight control "
> -	"(-1:auto (default), 0:force disable, 1:force enabled if supported");
> +	"Enable support for DPCD backlight control (default:false)");
>  
>  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 0d6cf9138dc4..febbfdbd30bd 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -53,7 +53,6 @@
>  	func(int, edp_vswing); \
>  	func(int, reset); \
>  	func(unsigned int, inject_load_failure); \
> -	func(int, enable_dpcd_backlight); \
>  	/* leave bools at the end to not create holes */ \
>  	func(bool, alpha_support); \
>  	func(bool, enable_cmd_parser); \
> @@ -67,6 +66,7 @@
>  	func(bool, verbose_state_checks); \
>  	func(bool, nuclear_pageflip); \
>  	func(bool, enable_dp_mst); \
> +	func(bool, 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 fea161727c6e..d2830ba3162e 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -251,66 +251,15 @@ 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)) {
> +	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[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
>  		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");
>  		return true;
>  	}
>  	return false;
>  }
>  
> -/*
> - * Heuristic function whether we should use AUX for backlight adjustment or not.
> - *
> - * We should use AUX for backlight brightness adjustment if panel doesn't this
> - * via PWM pin or using AUX is better than using PWM pin.
> - *
> - * The heuristic to determine that using AUX pin is better than using PWM pin is
> - * that the panel support any of the feature list here.
> - * - Regional backlight brightness adjustment
> - * - Backlight PWM frequency set
> - * - More than 8 bits resolution of brightness level
> - * - Backlight enablement via AUX and not by BL_ENABLE pin
> - *
> - * If all above are not true, assume that using PWM pin is better.
> - */
> -static bool
> -intel_dp_aux_display_control_heuristic(struct intel_connector *connector)
> -{
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> -	uint8_t reg_val;
> -
> -	/* Panel doesn't support adjusting backlight brightness via PWN pin */
> -	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
> -		return true;
> -
> -	/* Panel supports regional backlight brightness adjustment */
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3,
> -			      &reg_val) != 1) {
> -		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
> -			       DP_EDP_GENERAL_CAP_3);
> -		return false;
> -	}
> -	if (reg_val > 0)
> -		return true;
> -
> -	/* Panel supports backlight PWM frequency set */
> -	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
> -		return true;
> -
> -	/* Panel supports more than 8 bits resolution of brightness level */
> -	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
> -		return true;
> -
> -	/* Panel supports enabling backlight via AUX but not by BL_ENABLE pin */
> -	if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
> -	    !(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP))
> -		return true;
> -
> -	return false;
> -
> -}
> -
>  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
>  {
>  	struct intel_panel *panel = &intel_connector->panel;
> @@ -321,10 +270,6 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
>  	if (!intel_dp_aux_display_control_capable(intel_connector))
>  		return -ENODEV;
>  
> -	if (i915.enable_dpcd_backlight == -1 &&
> -	    !intel_dp_aux_display_control_heuristic(intel_connector))
> -		return -ENODEV;
> -
>  	panel->backlight.setup = intel_dp_aux_setup_backlight;
>  	panel->backlight.enable = intel_dp_aux_enable_backlight;
>  	panel->backlight.disable = intel_dp_aux_disable_backlight;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness"
  2017-07-20 20:23   ` Pandiyan, Dhinakaran
@ 2017-07-21  7:01     ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2017-07-21  7:01 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran
  Cc: puthik@chromium.org, daniel.vetter@ffwll.ch,
	intel-gfx@lists.freedesktop.org, Tc, Jenny

On Thu, 20 Jul 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Thu, 2017-07-20 at 12:25 +0300, Jani Nikula wrote:
>> This reverts commit 560a758d39c616f83ac25ff6e0816a49ebe6401c.
>> 
>> The DPCD backlight commits regress a Thinkpad X1 Carbon 4th Gen and a
>> BXT-P (in CI). Enabling dynamic backlight boots to a black screen, and
>> enabling DPCD backlight leads to a black screen after suspend/resume.
>> 
>
>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for
> both patches.

Thanks for the review, both pushed, with Daniel's IRC ack on top.

BR,
Jani.

>
>> References: http://mid.mail-archive.com/20170706135349.6tu3lz7uehazlnnn@boom
>> References: http://mid.mail-archive.com/20170627132326.f2q3yn4bh5flji4q@boom
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101619
>> Reported-by: David Weinehall <david.weinehall@linux.intel.com>
>> Fixes: 560a758d39c6 ("drm/i915: Add heuristic to determine better way to adjust brightness")
>> Cc: Jenny TC <jenny.tc@intel.com>
>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>> Cc: Puthikorn Voravootivat <puthik@chromium.org>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: intel-gfx@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_params.c            |  7 ++-
>>  drivers/gpu/drm/i915/i915_params.h            |  2 +-
>>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 61 ++-------------------------
>>  3 files changed, 7 insertions(+), 63 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index 5b5ab15d191f..14e2c2e57f96 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 = -1,
>> +	.enable_dpcd_backlight = false,
>>  	.enable_gvt = false,
>>  };
>>  
>> @@ -246,10 +246,9 @@ 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_unsafe(enable_dpcd_backlight, i915.enable_dpcd_backlight, int, 0600);
>> +module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
>>  MODULE_PARM_DESC(enable_dpcd_backlight,
>> -	"Enable support for DPCD backlight control "
>> -	"(-1:auto (default), 0:force disable, 1:force enabled if supported");
>> +	"Enable support for DPCD backlight control (default:false)");
>>  
>>  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 0d6cf9138dc4..febbfdbd30bd 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -53,7 +53,6 @@
>>  	func(int, edp_vswing); \
>>  	func(int, reset); \
>>  	func(unsigned int, inject_load_failure); \
>> -	func(int, enable_dpcd_backlight); \
>>  	/* leave bools at the end to not create holes */ \
>>  	func(bool, alpha_support); \
>>  	func(bool, enable_cmd_parser); \
>> @@ -67,6 +66,7 @@
>>  	func(bool, verbose_state_checks); \
>>  	func(bool, nuclear_pageflip); \
>>  	func(bool, enable_dp_mst); \
>> +	func(bool, 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 fea161727c6e..d2830ba3162e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> @@ -251,66 +251,15 @@ 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)) {
>> +	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[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) {
>>  		DRM_DEBUG_KMS("AUX Backlight Control Supported!\n");
>>  		return true;
>>  	}
>>  	return false;
>>  }
>>  
>> -/*
>> - * Heuristic function whether we should use AUX for backlight adjustment or not.
>> - *
>> - * We should use AUX for backlight brightness adjustment if panel doesn't this
>> - * via PWM pin or using AUX is better than using PWM pin.
>> - *
>> - * The heuristic to determine that using AUX pin is better than using PWM pin is
>> - * that the panel support any of the feature list here.
>> - * - Regional backlight brightness adjustment
>> - * - Backlight PWM frequency set
>> - * - More than 8 bits resolution of brightness level
>> - * - Backlight enablement via AUX and not by BL_ENABLE pin
>> - *
>> - * If all above are not true, assume that using PWM pin is better.
>> - */
>> -static bool
>> -intel_dp_aux_display_control_heuristic(struct intel_connector *connector)
>> -{
>> -	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>> -	uint8_t reg_val;
>> -
>> -	/* Panel doesn't support adjusting backlight brightness via PWN pin */
>> -	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
>> -		return true;
>> -
>> -	/* Panel supports regional backlight brightness adjustment */
>> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3,
>> -			      &reg_val) != 1) {
>> -		DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n",
>> -			       DP_EDP_GENERAL_CAP_3);
>> -		return false;
>> -	}
>> -	if (reg_val > 0)
>> -		return true;
>> -
>> -	/* Panel supports backlight PWM frequency set */
>> -	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP)
>> -		return true;
>> -
>> -	/* Panel supports more than 8 bits resolution of brightness level */
>> -	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>> -		return true;
>> -
>> -	/* Panel supports enabling backlight via AUX but not by BL_ENABLE pin */
>> -	if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) &&
>> -	    !(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP))
>> -		return true;
>> -
>> -	return false;
>> -
>> -}
>> -
>>  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
>>  {
>>  	struct intel_panel *panel = &intel_connector->panel;
>> @@ -321,10 +270,6 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector)
>>  	if (!intel_dp_aux_display_control_capable(intel_connector))
>>  		return -ENODEV;
>>  
>> -	if (i915.enable_dpcd_backlight == -1 &&
>> -	    !intel_dp_aux_display_control_heuristic(intel_connector))
>> -		return -ENODEV;
>> -
>>  	panel->backlight.setup = intel_dp_aux_setup_backlight;
>>  	panel->backlight.enable = intel_dp_aux_enable_backlight;
>>  	panel->backlight.disable = intel_dp_aux_disable_backlight;

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-07-21  7:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-20  9:25 [PATCH 0/2] drm/i915: revert DPCD backlight and DBC enabling by default Jani Nikula
2017-07-20  9:25 ` [PATCH 1/2] Revert "drm/i915: Add option to support dynamic backlight via DPCD" Jani Nikula
2017-07-20  9:25 ` [PATCH 2/2] Revert "drm/i915: Add heuristic to determine better way to adjust brightness" Jani Nikula
2017-07-20 20:23   ` Pandiyan, Dhinakaran
2017-07-21  7:01     ` Jani Nikula
2017-07-20 12:02 ` ✓ Fi.CI.BAT: success for drm/i915: revert DPCD backlight and DBC enabling by default Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.