[Intel-gfx] [PATCH v4 0/2] drm/i915/opregion: proper handling of DIDL and CADL
Marcos Paulo de Souza
marcos.souza.org at gmail.com
Mon Aug 15 11:24:55 UTC 2016
Hi list,
I have an Asus laptop, and these two patches solved my problem with
bright hot-keys not working[1].
I applied both patches on 4.8-rc1, and the only necessary fix was
changing priv_dev->dev to priv_dev->drm in all places of for_each_*
macros touched by these patches.
Is there any chance to get this merged before 4.8 is launched? And, if
there are other problems still in need of fixes in this patch, please
let me know.
Thanks!
[1] https://bugzilla.kernel.org/show_bug.cgi?id=152091
-------------- next part --------------
>From ae6d2f8916abe9573b91b3ecb565c9585dda579a Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani.nikula at intel.com>
Date: Wed, 29 Jun 2016 18:36:41 +0300
Subject: [PATCH 1/2] drm/i915: make i915 the source of acpi device ids for
_DOD
The graphics driver is supposed to define the DIDL, which are used for
_DOD, not the BIOS. Restore that behaviour.
This is basically a revert of
commit 3143751ff51a163b77f7efd389043e038f3e008e
Author: Zhang Rui <rui.zhang at intel.com>
Date: Mon Mar 29 15:12:16 2010 +0800
drm/i915: set DIDL using the ACPI video output device _ADR method return.
which went out of its way to cater to a specific BIOS, setting up DIDL
based on _ADR method. Perhaps that approach worked on that specific
machine, but on the machines I checked the _ADR method invents the
device identifiers out of thin air if DIDL has not been set. The source
for _ADR is also supposed to be the DIDL set by the driver, not the
other way around.
With this, we'll also limit the number of outputs to what the driver
actually has.
v2: do not set ACPI_DEVICE_ID_SCHEME in the device id (Peter Wu)
Reviewed-and-tested-by: Peter Wu <peter at lekensteyn.nl>
Signed-off-by: Jani Nikula <jani.nikula at intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 3 ++
drivers/gpu/drm/i915/intel_opregion.c | 89 ++++++++++-------------------------
2 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc937a1..8656b4c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -263,6 +263,9 @@ struct intel_connector {
*/
struct intel_encoder *encoder;
+ /* ACPI device id for ACPI and driver cooperation */
+ u32 acpi_device_id;
+
/* Reads out the current hw, returning true if the connector is enabled
* and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index adca262..494559a 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -674,11 +674,11 @@ static void set_did(struct intel_opregion *opregion, int i, u32 val)
}
}
-static u32 acpi_display_type(struct drm_connector *connector)
+static u32 acpi_display_type(struct intel_connector *connector)
{
u32 display_type;
- switch (connector->connector_type) {
+ switch (connector->base.connector_type) {
case DRM_MODE_CONNECTOR_VGA:
case DRM_MODE_CONNECTOR_DVIA:
display_type = ACPI_DISPLAY_TYPE_VGA;
@@ -707,7 +707,7 @@ static u32 acpi_display_type(struct drm_connector *connector)
display_type = ACPI_DISPLAY_TYPE_OTHER;
break;
default:
- MISSING_CASE(connector->connector_type);
+ MISSING_CASE(connector->base.connector_type);
display_type = ACPI_DISPLAY_TYPE_OTHER;
break;
}
@@ -718,34 +718,10 @@ static u32 acpi_display_type(struct drm_connector *connector)
static void intel_didl_outputs(struct drm_i915_private *dev_priv)
{
struct intel_opregion *opregion = &dev_priv->opregion;
- struct pci_dev *pdev = dev_priv->drm.pdev;
- struct drm_connector *connector;
- acpi_handle handle;
- struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
- unsigned long long device_id;
- acpi_status status;
- u32 temp, max_outputs;
- int i = 0;
-
- handle = ACPI_HANDLE(&pdev->dev);
- if (!handle || acpi_bus_get_device(handle, &acpi_dev))
- return;
-
- if (acpi_is_video_device(handle))
- acpi_video_bus = acpi_dev;
- else {
- list_for_each_entry(acpi_cdev, &acpi_dev->children, node) {
- if (acpi_is_video_device(acpi_cdev->handle)) {
- acpi_video_bus = acpi_cdev;
- break;
- }
- }
- }
-
- if (!acpi_video_bus) {
- DRM_DEBUG_KMS("No ACPI video bus found\n");
- return;
- }
+ struct intel_connector *connector;
+ struct drm_device *dev = &dev_priv->drm;
+ int i = 0, max_outputs;
+ int display_index[16] = {};
/*
* In theory, did2, the extended didl, gets added at opregion version
@@ -757,46 +733,31 @@ static void intel_didl_outputs(struct drm_i915_private *dev_priv)
max_outputs = ARRAY_SIZE(opregion->acpi->didl) +
ARRAY_SIZE(opregion->acpi->did2);
- list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
- if (i >= max_outputs) {
- DRM_DEBUG_KMS("More than %u outputs detected via ACPI\n",
- max_outputs);
- return;
- }
- status = acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
- NULL, &device_id);
- if (ACPI_SUCCESS(status)) {
- if (!device_id)
- goto blind_set;
- set_did(opregion, i++, (u32)(device_id & 0x0f0f));
- }
+ for_each_intel_connector(dev, connector) {
+ u32 device_id, type;
+
+ device_id = acpi_display_type(connector);
+
+ /* Use display type specific display index. */
+ type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+ >> ACPI_DISPLAY_TYPE_SHIFT;
+ device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+ connector->acpi_device_id = device_id;
+ if (i < max_outputs)
+ set_did(opregion, i, device_id);
+ i++;
}
-end:
DRM_DEBUG_KMS("%d outputs detected\n", i);
+ if (i > max_outputs)
+ DRM_ERROR("More than %d outputs in connector list\n",
+ max_outputs);
+
/* If fewer than max outputs, the list must be null terminated */
if (i < max_outputs)
set_did(opregion, i, 0);
- return;
-
-blind_set:
- i = 0;
- list_for_each_entry(connector,
- &dev_priv->drm.mode_config.connector_list, head) {
- int display_type = acpi_display_type(connector);
-
- if (i >= max_outputs) {
- DRM_DEBUG_KMS("More than %u outputs in connector list\n",
- max_outputs);
- return;
- }
-
- temp = get_did(opregion, i);
- set_did(opregion, i, temp | (1 << 31) | display_type | i);
- i++;
- }
- goto end;
}
static void intel_setup_cadls(struct drm_i915_private *dev_priv)
--
2.7.4
-------------- next part --------------
>From a3af328f8cb36b908d37f4af0dbeb8a03c75d35c Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani.nikula at intel.com>
Date: Wed, 29 Jun 2016 18:36:42 +0300
Subject: [PATCH 2/2] drm/i915/opregion: update cadl based on actually active
outputs
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)
Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Reviewed-and-tested-by: Peter Wu <peter at lekensteyn.nl>
Signed-off-by: Jani Nikula <jani.nikula at intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_display.c | 4 ++
drivers/gpu/drm/i915/intel_opregion.c | 71 +++++++++++++++++++----------------
3 files changed, 44 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 21f9390..062e5c3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3741,6 +3741,7 @@ 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);
#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) { }
@@ -3762,6 +3763,7 @@ 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) { }
#endif
/* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dcf93b3..4062a2f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13856,6 +13856,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_atomic_track_fbs(state);
if (nonblock)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 494559a..7968587 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;
@@ -760,22 +750,37 @@ 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)
{
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;
+ struct drm_device *dev = &dev_priv->drm;
+ int i = 0, max_active = ARRAY_SIZE(opregion->acpi->cadl);
+
+ for_each_intel_crtc(dev, crtc) {
+ struct intel_encoder *encoder;
+
+ if (!crtc->active)
+ continue;
+
+ 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)
@@ -787,7 +792,7 @@ void intel_opregion_register(struct drm_i915_private *dev_priv)
if (opregion->acpi) {
intel_didl_outputs(dev_priv);
- intel_setup_cadls(dev_priv);
+ intel_opregion_update_cadl(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.
--
2.7.4
More information about the Intel-gfx
mailing list