[2/2] drm/i915/opregion: update cadl based on actually active outputs

Submitted by Jani Nikula on Aug. 25, 2016, 12:53 p.m.

Details

Message ID 124734c3648b19fc2f3f4f3f7cac7aefef588471.1472129339.git.jani.nikula@intel.com
State New
Headers show
Series "drm/i915/opregion: proper handling of DIDL and CADL" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Jani Nikula Aug. 25, 2016, 12:53 p.m.
Previously we've just shoved the first eight devices in DIDL to CADL
(list of active outputs). Some of the active outputs may have been left
outside of CADL. The problem is, some BIOS implementations prevent
laptop brightness hotkey propagation if the flat panel is not active.

Now that we have connector to acpi device id mapping covered, we can
update CADL based on which outputs are actually active.

v3: actually git add the dev->dev_priv change.

v4: update cadl in intel_shared_dpll_commit() if intel_state->modeset
    (Maarten)

v5: use crtc mask, not ->active to determine active outputs (Maarten)
    Move first cadl update to after hardware readout (Maarten)

Cc: Peter Wu <peter@lekensteyn.nl>
Cc: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>
Cc: Jan-Marek Glogowski <glogow@fbihome.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Cc: Paolo Stivanin <paolostivanin@fastmail.fm>
Tested-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Reviewed-and-tested-by: Peter Wu <peter@lekensteyn.nl>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  4 ++
 drivers/gpu/drm/i915/intel_display.c  |  6 +++
 drivers/gpu/drm/i915/intel_opregion.c | 71 +++++++++++++++++++----------------
 3 files changed, 48 insertions(+), 33 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 04b4fd6c32e4..bc7825e16526 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3610,6 +3610,8 @@  extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 extern int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
 					 pci_power_t state);
 extern int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
+extern void intel_opregion_update_cadl(struct drm_i915_private *dev_priv,
+				       unsigned int active_crtcs);
 #else
 static inline int intel_opregion_setup(struct drm_i915_private *dev) { return 0; }
 static inline void intel_opregion_register(struct drm_i915_private *dev_priv) { }
@@ -3631,6 +3633,8 @@  static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
 {
 	return -ENODEV;
 }
+static inline void intel_opregion_update_cadl(struct drm_i915_private *dev_priv,
+					      unsigned int active_crtcs) { }
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e4e6141b38c0..c3e8eae8cbd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14537,6 +14537,10 @@  static int intel_atomic_commit(struct drm_device *dev,
 	dev_priv->wm.distrust_bios_wm = false;
 	dev_priv->wm.skl_results = intel_state->wm_results;
 	intel_shared_dpll_commit(state);
+
+	if (intel_state->modeset)
+		intel_opregion_update_cadl(dev_priv, intel_state->active_crtcs);
+
 	intel_atomic_track_fbs(state);
 
 	if (nonblock)
@@ -16927,6 +16931,8 @@  intel_modeset_setup_hw_state(struct drm_device *dev)
 	intel_display_set_init_power(dev_priv, false);
 
 	intel_fbc_init_pipe_state(dev_priv);
+
+	intel_opregion_update_cadl(dev_priv, dev_priv->active_crtcs);
 }
 
 void intel_display_resume(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index b2caf151c3fa..4239ac230a41 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -642,24 +642,6 @@  static struct notifier_block intel_opregion_notifier = {
  * (version 3)
  */
 
-static u32 get_did(struct intel_opregion *opregion, int i)
-{
-	u32 did;
-
-	if (i < ARRAY_SIZE(opregion->acpi->didl)) {
-		did = opregion->acpi->didl[i];
-	} else {
-		i -= ARRAY_SIZE(opregion->acpi->didl);
-
-		if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->did2)))
-			return 0;
-
-		did = opregion->acpi->did2[i];
-	}
-
-	return did;
-}
-
 static void set_did(struct intel_opregion *opregion, int i, u32 val)
 {
 	if (i < ARRAY_SIZE(opregion->acpi->didl)) {
@@ -674,6 +656,14 @@  static void set_did(struct intel_opregion *opregion, int i, u32 val)
 	}
 }
 
+static void set_cad(struct intel_opregion *opregion, int i, u32 val)
+{
+	if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->cadl)))
+		return;
+
+	opregion->acpi->cadl[i] = val;
+}
+
 static u32 acpi_display_type(struct intel_connector *connector)
 {
 	u32 display_type;
@@ -759,22 +749,38 @@  static void intel_didl_outputs(struct drm_i915_private *dev_priv)
 		set_did(opregion, i, 0);
 }
 
-static void intel_setup_cadls(struct drm_i915_private *dev_priv)
+/* Update CADL to reflect active outputs. */
+void intel_opregion_update_cadl(struct drm_i915_private *dev_priv,
+				unsigned int active_crtcs)
 {
+	struct drm_device *dev = &dev_priv->drm;
 	struct intel_opregion *opregion = &dev_priv->opregion;
-	int i = 0;
-	u32 disp_id;
-
-	/* Initialize the CADL field by duplicating the DIDL values.
-	 * Technically, this is not always correct as display outputs may exist,
-	 * but not active. This initialization is necessary for some Clevo
-	 * laptops that check this field before processing the brightness and
-	 * display switching hotkeys. Just like DIDL, CADL is NULL-terminated if
-	 * there are less than eight devices. */
-	do {
-		disp_id = get_did(opregion, i);
-		opregion->acpi->cadl[i] = disp_id;
-	} while (++i < 8 && disp_id != 0);
+	struct intel_crtc *crtc;
+	int i = 0, max_active = ARRAY_SIZE(opregion->acpi->cadl);
+
+	if (!opregion->acpi)
+		return;
+
+	for_each_intel_crtc_mask(dev, crtc, active_crtcs) {
+		struct intel_encoder *encoder;
+
+		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+			struct intel_connector *connector;
+
+			for_each_connector_on_encoder(dev, &encoder->base, connector) {
+				if (i >= max_active) {
+					DRM_DEBUG_KMS("too many outputs active\n");
+					return;
+				}
+
+				set_cad(opregion, i++, connector->acpi_device_id);
+			}
+		}
+	}
+
+	/* If fewer than max active outputs, the list must be null terminated */
+	if (i < max_active)
+		set_cad(opregion, i, 0);
 }
 
 void intel_opregion_register(struct drm_i915_private *dev_priv)
@@ -786,7 +792,6 @@  void intel_opregion_register(struct drm_i915_private *dev_priv)
 
 	if (opregion->acpi) {
 		intel_didl_outputs(dev_priv);
-		intel_setup_cadls(dev_priv);
 
 		/* Notify BIOS we are ready to handle ACPI video ext notifs.
 		 * Right now, all the events are handled by the ACPI video module.

Comments

Op 25-08-16 om 14:53 schreef Jani Nikula:
> Previously we've just shoved the first eight devices in DIDL to CADL
> (list of active outputs). Some of the active outputs may have been left
> outside of CADL. The problem is, some BIOS implementations prevent
> laptop brightness hotkey propagation if the flat panel is not active.
>
> Now that we have connector to acpi device id mapping covered, we can
> update CADL based on which outputs are actually active.
>
> v3: actually git add the dev->dev_priv change.
>
> v4: update cadl in intel_shared_dpll_commit() if intel_state->modeset
>     (Maarten)
>
> v5: use crtc mask, not ->active to determine active outputs (Maarten)
>     Move first cadl update to after hardware readout (Maarten)
Much better! I don't dare to do a thorough review of the non-atomicy parts, but idea looks good.

Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>