Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Luca Tettamanti <kronos.it@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

Hi,
I'm attaching a first draft of my work. The first 3 patches are
infrastructure work, the fourth wires the notification handler and
retrieves the requests from the system BIOS, but it does not actually
change brightness yet.

The problem here is how to get the correct encoder: should I just scan
encoder_list checking for ATOM_DEVICE_LCD_SUPPORT and see if it has a
backlight device attached?
Hopefully there is only one encoder with it, right?

I'm also toying with the idea of creating structures matching the output
of the various ACPI methods, this would remove some ugly pointer
arithmetics, but it _might_ make it easier to read past the buffer if
one does not check the size before using the struct. What do you think?

Luca

[-- Attachment #2: 0001-drm-radeon-refactor-radeon_atif_call.patch --]
[-- Type: text/x-diff, Size: 2233 bytes --]

>From e485a3a4075c25ba1747ad61f6168c3734d5f0a9 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti <kronos.it@gmail.com>
Date: Sun, 29 Jul 2012 17:04:43 +0200
Subject: [PATCH 1/4] drm/radeon: refactor radeon_atif_call

Don't hard-code function number, this will allow to reuse the function.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_acpi.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c
index 5b32e49..8816698 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -14,10 +14,8 @@
 #include <linux/vga_switcheroo.h>
 
 /* Call the ATIF method
- *
- * Note: currently we discard the output
  */
-static int radeon_atif_call(acpi_handle handle)
+static union acpi_object* radeon_atif_call(acpi_handle handle, int function)
 {
 	acpi_status status;
 	union acpi_object atif_arg_elements[2];
@@ -28,7 +26,7 @@ static int radeon_atif_call(acpi_handle handle)
 	atif_arg.pointer = &atif_arg_elements[0];
 
 	atif_arg_elements[0].type = ACPI_TYPE_INTEGER;
-	atif_arg_elements[0].integer.value = ATIF_FUNCTION_VERIFY_INTERFACE;
+	atif_arg_elements[0].integer.value = function;
 	atif_arg_elements[1].type = ACPI_TYPE_INTEGER;
 	atif_arg_elements[1].integer.value = 0;
 
@@ -39,18 +37,18 @@ static int radeon_atif_call(acpi_handle handle)
 		DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n",
 				 acpi_format_exception(status));
 		kfree(buffer.pointer);
-		return 1;
+		return NULL;
 	}
 
-	kfree(buffer.pointer);
-	return 0;
+	return buffer.pointer;
 }
 
 /* Call all ACPI methods here */
 int radeon_acpi_init(struct radeon_device *rdev)
 {
 	acpi_handle handle;
-	int ret;
+	union acpi_object *info;
+	int ret = 0;
 
 	/* Get the device handle */
 	handle = DEVICE_ACPI_HANDLE(&rdev->pdev->dev);
@@ -60,10 +58,11 @@ int radeon_acpi_init(struct radeon_device *rdev)
 		return 0;
 
 	/* Call the ATIF method */
-	ret = radeon_atif_call(handle);
-	if (ret)
-		return ret;
+	info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE);
+	if (!info)
+		ret = -EIO;
 
-	return 0;
+	kfree(info);
+	return ret;
 }
 
-- 
1.7.10.4


[-- Attachment #3: 0002-drm-radeon-implement-radeon_atif_verify_interface.patch --]
[-- Type: text/x-diff, Size: 5331 bytes --]

>From b40c42f6808d84e36095f8b21daad28b39666628 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti <kronos.it@gmail.com>
Date: Sun, 29 Jul 2012 17:19:07 +0200
Subject: [PATCH 2/4] drm/radeon: implement radeon_atif_verify_interface

Wrap the call to VERIFY_INTERFACE and add the parsing of the support
vectors.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
---
 drivers/gpu/drm/radeon/radeon.h      |   40 ++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_acpi.c |   67 +++++++++++++++++++++++++++++++---
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index fefcca5..0db98eb 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1456,6 +1456,44 @@ struct r600_vram_scratch {
 	u64				gpu_addr;
 };
 
+/*
+ * ACPI
+ */
+struct radeon_atif_notification_cfg {
+	bool enabled;
+	int command_code;
+};
+
+struct radeon_atif_notifications {
+	bool display_switch;
+	bool expansion_mode_change;
+	bool thermal_state;
+	bool forced_power_state;
+	bool system_power_state;
+	bool display_conf_change;
+	bool px_gfx_switch;
+	bool brightness_change;
+	bool dgpu_display_event;
+};
+
+struct radeon_atif_functions {
+	bool system_params;
+	bool sbios_requests;
+	bool select_active_disp;
+	bool lid_state;
+	bool get_tv_standard;
+	bool set_tv_standard;
+	bool get_panel_expansion_mode;
+	bool set_panel_expansion_mode;
+	bool temperature_change;
+	bool graphics_device_types;
+};
+
+struct radeon_atif {
+	struct radeon_atif_notifications notifications;
+	struct radeon_atif_functions functions;
+	struct radeon_atif_notification_cfg notification_cfg;
+};
 
 /*
  * Core structure, functions and helpers.
@@ -1548,6 +1586,8 @@ struct radeon_device {
 	unsigned 		debugfs_count;
 	/* virtual memory */
 	struct radeon_vm_manager	vm_manager;
+	/* ACPI interface */
+	struct radeon_atif		atif;
 };
 
 int radeon_device_init(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c
index 8816698..d35bf8a 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -43,12 +43,68 @@ static union acpi_object* radeon_atif_call(acpi_handle handle, int function)
 	return buffer.pointer;
 }
 
+static void radeon_atif_parse_notification(struct radeon_atif_notifications *n, u32 mask) {
+	n->display_switch = mask & ATIF_DISPLAY_SWITCH_REQUEST_SUPPORTED;
+	n->expansion_mode_change = mask & ATIF_EXPANSION_MODE_CHANGE_REQUEST_SUPPORTED;
+	n->thermal_state = mask & ATIF_THERMAL_STATE_CHANGE_REQUEST_SUPPORTED;
+	n->forced_power_state = mask & ATIF_FORCED_POWER_STATE_CHANGE_REQUEST_SUPPORTED;
+	n->system_power_state = mask & ATIF_SYSTEM_POWER_SOURCE_CHANGE_REQUEST_SUPPORTED;
+	n->display_conf_change = mask & ATIF_DISPLAY_CONF_CHANGE_REQUEST_SUPPORTED;
+	n->px_gfx_switch = mask & ATIF_PX_GFX_SWITCH_REQUEST_SUPPORTED;
+	n->brightness_change = mask & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST_SUPPORTED;
+	n->dgpu_display_event = mask & ATIF_DGPU_DISPLAY_EVENT_SUPPORTED;
+}
+
+static void radeon_atif_parse_functions(struct radeon_atif_functions *f, u32 mask) {
+	f->system_params = ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED;
+	f->sbios_requests = ATIF_GET_SYSTEM_BIOS_REQUESTS_SUPPORTED;
+	f->select_active_disp = ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED;
+	f->lid_state = ATIF_GET_LID_STATE_SUPPORTED;
+	f->get_tv_standard = ATIF_GET_TV_STANDARD_FROM_CMOS_SUPPORTED;
+	f->set_tv_standard = ATIF_SET_TV_STANDARD_IN_CMOS_SUPPORTED;
+	f->get_panel_expansion_mode = ATIF_GET_PANEL_EXPANSION_MODE_FROM_CMOS_SUPPORTED;
+	f->set_panel_expansion_mode = ATIF_SET_PANEL_EXPANSION_MODE_IN_CMOS_SUPPORTED;
+	f->temperature_change = ATIF_TEMPERATURE_CHANGE_NOTIFICATION_SUPPORTED;
+	f->graphics_device_types = ATIF_GET_GRAPHICS_DEVICE_TYPES_SUPPORTED;
+}
+
+static int radeon_atif_verify_interface(acpi_handle handle, struct radeon_atif *atif) {
+	union acpi_object *info;
+	u16 version;
+	u16 size;
+	u32 notification_mask;
+	u32 function_mask;
+	int err = 0;
+
+	info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE);
+
+	size = ((u16 *) info->buffer.pointer)[0];
+	if (size < 12) {
+		DRM_INFO("ATIF buffer is too small: %u\n", size);
+		err = -EINVAL;
+		goto out_err;
+	}
+
+	/* TODO: check version? */
+	version = ((u16 *) info->buffer.pointer)[1];
+	DRM_DEBUG_DRIVER("ATIF version %u\n", version);
+
+	notification_mask = ((u32 *) info->buffer.pointer)[1];
+	function_mask = ((u32 *) info->buffer.pointer)[2];
+
+	radeon_atif_parse_notification(&atif->notifications, notification_mask);
+	radeon_atif_parse_functions(&atif->functions, function_mask);
+
+out_err:
+	kfree(info);
+	return err;
+}
+
 /* Call all ACPI methods here */
 int radeon_acpi_init(struct radeon_device *rdev)
 {
 	acpi_handle handle;
-	union acpi_object *info;
-	int ret = 0;
+	int ret;
 
 	/* Get the device handle */
 	handle = DEVICE_ACPI_HANDLE(&rdev->pdev->dev);
@@ -58,11 +114,10 @@ int radeon_acpi_init(struct radeon_device *rdev)
 		return 0;
 
 	/* Call the ATIF method */
-	info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE);
-	if (!info)
-		ret = -EIO;
+	ret = radeon_atif_verify_interface(handle, &rdev->atif);
+	if (ret)
+		DRM_DEBUG_DRIVER("Call to verify_interface failed: %d\n", ret);
 
-	kfree(info);
 	return ret;
 }
 
-- 
1.7.10.4


[-- Attachment #4: 0003-drm-radeon-implement-wrapper-for-GET_SYSTEM_PARAMS.patch --]
[-- Type: text/x-diff, Size: 3101 bytes --]

>From 9386cd89a085a83d2b3393e1e641c49d4e48cddd Mon Sep 17 00:00:00 2001
From: Luca Tettamanti <kronos.it@gmail.com>
Date: Sun, 29 Jul 2012 17:30:54 +0200
Subject: [PATCH 3/4] drm/radeon: implement wrapper for GET_SYSTEM_PARAMS

Use GET_SYSTEM_PARAMS for retrieving the configuration for the system
BIOS notifications.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_acpi.c |   77 +++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c
index d35bf8a..94f8255 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -13,6 +13,11 @@
 
 #include <linux/vga_switcheroo.h>
 
+#define ATIF_NOTIFICATION_MASK	0x3
+#define ATIF_NOTIFICATION_NONE	0
+#define ATIF_NOTIFICATION_81	1
+#define ATIF_NOTIFICATION_N	2
+
 /* Call the ATIF method
  */
 static union acpi_object* radeon_atif_call(acpi_handle handle, int function)
@@ -100,10 +105,58 @@ out_err:
 	return err;
 }
 
+static int radeon_atif_get_notification_params(acpi_handle handle,
+		struct radeon_atif_notification_cfg *n)
+{
+	union acpi_object *info;
+	u8 *buffer;
+	u16 size;
+	u32 valid_mask;
+	u32 flags;
+	int err = 0;
+
+	info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_PARAMETERS);
+	if (!info) {
+		err = -EIO;
+		goto out;
+	}
+
+	buffer = info->buffer.pointer;
+
+	size = *(u16 *)buffer;
+	if (size < 10) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	valid_mask = *(u32 *)(buffer + 2);
+	flags = (*(u32 *)(buffer + 6)) & valid_mask;
+	flags &= ATIF_NOTIFICATION_MASK;
+
+	if (flags == ATIF_NOTIFICATION_NONE) {
+		n->enabled = false;
+		n->command_code = 0;
+	} else if (flags == ATIF_NOTIFICATION_81) {
+		n->enabled = true;
+		n->command_code = 0x81;
+	} else {
+		if (size < 11) {
+			err = -EINVAL;
+			goto out;
+		}
+		n->command_code = *(u8 *)(buffer + 10);
+	}
+
+out:
+	kfree(info);
+	return err;
+}
+
 /* Call all ACPI methods here */
 int radeon_acpi_init(struct radeon_device *rdev)
 {
 	acpi_handle handle;
+	struct radeon_atif *atif = &rdev->atif;
 	int ret;
 
 	/* Get the device handle */
@@ -114,10 +167,30 @@ int radeon_acpi_init(struct radeon_device *rdev)
 		return 0;
 
 	/* Call the ATIF method */
-	ret = radeon_atif_verify_interface(handle, &rdev->atif);
-	if (ret)
+	ret = radeon_atif_verify_interface(handle, atif);
+	if (ret) {
 		DRM_DEBUG_DRIVER("Call to verify_interface failed: %d\n", ret);
+		goto out;
+	}
+
+	if (atif->functions.sbios_requests && !atif->functions.system_params) {
+		/* XXX check this workraround, if sbios request function is 
+		 * present we have to see how it's configured in the system
+		 * params
+		 */
+		atif->functions.system_params = true;
+	}
+
+	if (atif->functions.system_params) {
+		ret = radeon_atif_get_notification_params(handle, &atif->notification_cfg);
+		if (ret) {
+			DRM_DEBUG_DRIVER("Call to GET_SYSTEM_PARAMS failed: %d\n", ret);
+			/* Disable notification */
+			atif->notification_cfg.enabled = false;
+		}
+	}
 
+out:
 	return ret;
 }
 
-- 
1.7.10.4


[-- Attachment #5: 0004-drm-radeon-implement-skeleton-handler-for-ACPI-event.patch --]
[-- Type: text/x-diff, Size: 3696 bytes --]

>From 29937c81f1449d136c85145ceeeb7de24bfcc4c8 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti <kronos.it@gmail.com>
Date: Sun, 29 Jul 2012 21:00:51 +0200
Subject: [PATCH 4/4] drm/radeon: implement skeleton handler for ACPI events

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_acpi.c |   72 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_acpi.h |    5 +++
 drivers/gpu/drm/radeon/radeon_pm.c   |    4 +-
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c
index 94f8255..8b1fed7 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -18,6 +18,11 @@
 #define ATIF_NOTIFICATION_81	1
 #define ATIF_NOTIFICATION_N	2
 
+struct radeon_atif_sbios_requests {
+	bool brightness_change;
+	int brightness_target;
+};
+
 /* Call the ATIF method
  */
 static union acpi_object* radeon_atif_call(acpi_handle handle, int function)
@@ -152,6 +157,73 @@ out:
 	return err;
 }
 
+static int radeon_atif_get_sbios_requests(acpi_handle handle,
+		struct radeon_atif_sbios_requests *req)
+{
+	union acpi_object *info;
+	void *buffer;
+	u16 size;
+	u32 pending;
+	int count = 0;
+
+	memset(req, 0, sizeof(*req));
+
+	info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS);
+	if (!info)
+		return -EIO;
+
+	buffer = info->buffer.pointer;
+
+	size = *(u16 *)buffer;
+	if (size < 0xd) {
+		count = -EINVAL;
+		goto out;
+	}
+
+	pending = *(u32 *)(buffer + 2);
+
+	if (pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST_SUPPORTED) {
+		count++;
+		req->brightness_change = true;
+		req->brightness_target = *(u8 *)(buffer + 12);
+	}
+
+	/* TODO check other requests */
+
+out:
+	kfree(info);
+	return count;
+}
+
+int radeon_atif_handler(struct radeon_device *rdev, struct acpi_bus_event *event)
+{
+	struct radeon_atif *atif = &rdev->atif;
+	struct radeon_atif_sbios_requests req;
+	acpi_handle handle;
+	int count;
+
+	if (!atif->notification_cfg.enabled ||
+			event->type != atif->notification_cfg.command_code)
+		/* Not our event */
+		return NOTIFY_DONE;
+
+	/* Check pending SBIOS requests */
+	handle = DEVICE_ACPI_HANDLE(&rdev->pdev->dev);
+	count = radeon_atif_get_sbios_requests(handle, &req);
+
+	if (count <= 0)
+		return NOTIFY_DONE;
+
+	DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", count);
+
+	if (req.brightness_change) {
+		/* TODO */
+		printk(KERN_INFO "Changing brightness to %d\n", req.brightness_target);
+	}
+
+	return NOTIFY_OK;
+}
+
 /* Call all ACPI methods here */
 int radeon_acpi_init(struct radeon_device *rdev)
 {
diff --git a/drivers/gpu/drm/radeon/radeon_acpi.h b/drivers/gpu/drm/radeon/radeon_acpi.h
index a42288d..513894e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.h
+++ b/drivers/gpu/drm/radeon/radeon_acpi.h
@@ -24,6 +24,11 @@
 #ifndef RADEON_ACPI_H
 #define RADEON_ACPI_H
 
+struct radeon_device;
+struct acpi_bus_event;
+
+int radeon_atif_handler(struct radeon_device *rdev, struct acpi_bus_event *event);
+
 /* AMD hw uses four ACPI control methods:
  * 1. ATIF
  * ARG0: (ACPI_INTEGER) function code
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 5b37e28..8621748 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -22,6 +22,7 @@
  */
 #include "drmP.h"
 #include "radeon.h"
+#include "radeon_acpi.h"
 #include "avivod.h"
 #include "atom.h"
 #ifdef CONFIG_ACPI
@@ -95,7 +96,8 @@ static int radeon_acpi_event(struct notifier_block *nb,
 		}
 	}
 
-	return NOTIFY_OK;
+	/* Check for pending SBIOS requests */
+	return radeon_atif_handler(rdev, entry);
 }
 #endif
 
-- 
1.7.10.4


[-- Attachment #6: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel