* [PATCH] drm/radeon: add new AMD ACPI header and update relevant code @ 2012-07-25 17:38 alexdeucher 2012-07-26 12:58 ` Luca Tettamanti 0 siblings, 1 reply; 47+ messages in thread From: alexdeucher @ 2012-07-25 17:38 UTC (permalink / raw) To: airlied, dri-devel; +Cc: Alex Deucher From: Alex Deucher <alexander.deucher@amd.com> Add a new header that defines the AMD ACPI interface used for laptops, PowerXpress, and chipset specific functionality and update the current code to use it. Todo: - properly verify the ACPI interfaces - hook up and handle ACPI notifications - make PX code more robust - implement PCIe Gen and width switching using ACPI Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 3 +- drivers/gpu/drm/radeon/radeon_acpi.h | 439 ++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_atpx_handler.c | 35 +-- 3 files changed, 455 insertions(+), 22 deletions(-) create mode 100644 drivers/gpu/drm/radeon/radeon_acpi.h diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 3516a60..5b32e49 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -9,6 +9,7 @@ #include "drm_sarea.h" #include "drm_crtc_helper.h" #include "radeon.h" +#include "radeon_acpi.h" #include <linux/vga_switcheroo.h> @@ -27,7 +28,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 = 0; + atif_arg_elements[0].integer.value = ATIF_FUNCTION_VERIFY_INTERFACE; atif_arg_elements[1].type = ACPI_TYPE_INTEGER; atif_arg_elements[1].integer.value = 0; diff --git a/drivers/gpu/drm/radeon/radeon_acpi.h b/drivers/gpu/drm/radeon/radeon_acpi.h new file mode 100644 index 0000000..a42288d --- /dev/null +++ b/drivers/gpu/drm/radeon/radeon_acpi.h @@ -0,0 +1,439 @@ +/* + * Copyright 2012 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#ifndef RADEON_ACPI_H +#define RADEON_ACPI_H + +/* AMD hw uses four ACPI control methods: + * 1. ATIF + * ARG0: (ACPI_INTEGER) function code + * ARG1: (ACPI_BUFFER) parameter buffer, 256 bytes + * OUTPUT: (ACPI_BUFFER) output buffer, 256 bytes + * ATIF provides an entry point for the gfx driver to interact with the sbios. + * The AMD ACPI notification mechanism uses Notify (VGA, 0x81) or a custom + * notification. Which notification is used as indicated by the ATIF Control + * Method GET_SYSTEM_PARAMETERS. When the driver receives Notify (VGA, 0x81) or + * a custom notification it invokes ATIF Control Method GET_SYSTEM_BIOS_REQUESTS + * to identify pending System BIOS requests and associated parameters. For + * example, if one of the pending requests is DISPLAY_SWITCH_REQUEST, the driver + * will perform display device detection and invoke ATIF Control Method + * SELECT_ACTIVE_DISPLAYS. + * + * 2. ATPX + * ARG0: (ACPI_INTEGER) function code + * ARG1: (ACPI_BUFFER) parameter buffer, 256 bytes + * OUTPUT: (ACPI_BUFFER) output buffer, 256 bytes + * ATPX methods are used on PowerXpress systems to handle mux switching and + * discrete GPU power control. + * + * 3. ATRM + * ARG0: (ACPI_INTEGER) offset of vbios rom data + * ARG1: (ACPI_BUFFER) size of the buffer to fill (up to 4K). + * OUTPUT: (ACPI_BUFFER) output buffer + * ATRM provides an interfacess to access the discrete GPU vbios image on + * PowerXpress systems with multiple GPUs. + * + * 4. ATCS + * ARG0: (ACPI_INTEGER) function code + * ARG1: (ACPI_BUFFER) parameter buffer, 256 bytes + * OUTPUT: (ACPI_BUFFER) output buffer, 256 bytes + * ATCS provides an interface to AMD chipset specific functionality. + * + */ +/* ATIF */ +#define ATIF_FUNCTION_VERIFY_INTERFACE 0x0 +/* ARG0: ATIF_FUNCTION_VERIFY_INTERFACE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * WORD - version + * DWORD - supported notifications mask + * DWORD - supported functions bit vector + */ +/* Notifications mask */ +# define ATIF_DISPLAY_SWITCH_REQUEST_SUPPORTED (1 << 0) +# define ATIF_EXPANSION_MODE_CHANGE_REQUEST_SUPPORTED (1 << 1) +# define ATIF_THERMAL_STATE_CHANGE_REQUEST_SUPPORTED (1 << 2) +# define ATIF_FORCED_POWER_STATE_CHANGE_REQUEST_SUPPORTED (1 << 3) +# define ATIF_SYSTEM_POWER_SOURCE_CHANGE_REQUEST_SUPPORTED (1 << 4) +# define ATIF_DISPLAY_CONF_CHANGE_REQUEST_SUPPORTED (1 << 5) +# define ATIF_PX_GFX_SWITCH_REQUEST_SUPPORTED (1 << 6) +# define ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST_SUPPORTED (1 << 7) +# define ATIF_DGPU_DISPLAY_EVENT_SUPPORTED (1 << 8) +/* supported functions vector */ +# define ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED (1 << 0) +# define ATIF_GET_SYSTEM_BIOS_REQUESTS_SUPPORTED (1 << 1) +# define ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) +# define ATIF_GET_LID_STATE_SUPPORTED (1 << 3) +# define ATIF_GET_TV_STANDARD_FROM_CMOS_SUPPORTED (1 << 4) +# define ATIF_SET_TV_STANDARD_IN_CMOS_SUPPORTED (1 << 5) +# define ATIF_GET_PANEL_EXPANSION_MODE_FROM_CMOS_SUPPORTED (1 << 6) +# define ATIF_SET_PANEL_EXPANSION_MODE_IN_CMOS_SUPPORTED (1 << 7) +# define ATIF_TEMPERATURE_CHANGE_NOTIFICATION_SUPPORTED (1 << 12) +# define ATIF_GET_GRAPHICS_DEVICE_TYPES_SUPPORTED (1 << 14) +#define ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags + * + * OR + * + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags + * BYTE - notify command code + * + * flags + * bits 1:0: + * 0 - Notify(VGA, 0x81) is not used for notification + * 1 - Notify(VGA, 0x81) is used for notification + * 2 - Notify(VGA, n) is used for notification where + * n (0xd0-0xd9) is specified in notify command code. + * bit 2: + * 1 - lid changes not reported though int10 + */ +#define ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS 0x2 +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - pending sbios requests + * BYTE - panel expansion mode + * BYTE - thermal state: target gfx controller + * BYTE - thermal state: state id (0: exit state, non-0: state) + * BYTE - forced power state: target gfx controller + * BYTE - forced power state: state id + * BYTE - system power source + * BYTE - panel backlight level (0-255) + */ +/* pending sbios requests */ +# define ATIF_DISPLAY_SWITCH_REQUEST (1 << 0) +# define ATIF_EXPANSION_MODE_CHANGE_REQUEST (1 << 1) +# define ATIF_THERMAL_STATE_CHANGE_REQUEST (1 << 2) +# define ATIF_FORCED_POWER_STATE_CHANGE_REQUEST (1 << 3) +# define ATIF_SYSTEM_POWER_SOURCE_CHANGE_REQUEST (1 << 4) +# define ATIF_DISPLAY_CONF_CHANGE_REQUEST (1 << 5) +# define ATIF_PX_GFX_SWITCH_REQUEST (1 << 6) +# define ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST (1 << 7) +# define ATIF_DGPU_DISPLAY_EVENT (1 << 8) +/* panel expansion mode */ +# define ATIF_PANEL_EXPANSION_DISABLE 0 +# define ATIF_PANEL_EXPANSION_FULL 1 +# define ATIF_PANEL_EXPANSION_ASPECT 2 +/* target gfx controller */ +# define ATIF_TARGET_GFX_SINGLE 0 +# define ATIF_TARGET_GFX_PX_IGPU 1 +# define ATIF_TARGET_GFX_PX_DGPU 2 +/* system power source */ +# define ATIF_POWER_SOURCE_AC 1 +# define ATIF_POWER_SOURCE_DC 2 +# define ATIF_POWER_SOURCE_RESTRICTED_AC_1 3 +# define ATIF_POWER_SOURCE_RESTRICTED_AC_2 4 +#define ATIF_FUNCTION_SELECT_ACTIVE_DISPLAYS 0x3 +/* ARG0: ATIF_FUNCTION_SELECT_ACTIVE_DISPLAYS + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - selected displays + * WORD - connected displays + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * WORD - selected displays + */ +# define ATIF_LCD1 (1 << 0) +# define ATIF_CRT1 (1 << 1) +# define ATIF_TV (1 << 2) +# define ATIF_DFP1 (1 << 3) +# define ATIF_CRT2 (1 << 4) +# define ATIF_LCD2 (1 << 5) +# define ATIF_DFP2 (1 << 7) +# define ATIF_CV (1 << 8) +# define ATIF_DFP3 (1 << 9) +# define ATIF_DFP4 (1 << 10) +# define ATIF_DFP5 (1 << 11) +# define ATIF_DFP6 (1 << 12) +#define ATIF_FUNCTION_GET_LID_STATE 0x4 +/* ARG0: ATIF_FUNCTION_GET_LID_STATE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * BYTE - lid state (0: open, 1: closed) + * + * GET_LID_STATE only works at boot and resume, for general lid + * status, use the kernel provided status + */ +#define ATIF_FUNCTION_GET_TV_STANDARD_FROM_CMOS 0x5 +/* ARG0: ATIF_FUNCTION_GET_TV_STANDARD_FROM_CMOS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * BYTE - 0 + * BYTE - TV standard + */ +# define ATIF_TV_STD_NTSC 0 +# define ATIF_TV_STD_PAL 1 +# define ATIF_TV_STD_PALM 2 +# define ATIF_TV_STD_PAL60 3 +# define ATIF_TV_STD_NTSCJ 4 +# define ATIF_TV_STD_PALCN 5 +# define ATIF_TV_STD_PALN 6 +# define ATIF_TV_STD_SCART_RGB 9 +#define ATIF_FUNCTION_SET_TV_STANDARD_IN_CMOS 0x6 +/* ARG0: ATIF_FUNCTION_SET_TV_STANDARD_IN_CMOS + * ARG1: + * WORD - structure size in bytes (includes size field) + * BYTE - 0 + * BYTE - TV standard + * OUTPUT: none + */ +#define ATIF_FUNCTION_GET_PANEL_EXPANSION_MODE_FROM_CMOS 0x7 +/* ARG0: ATIF_FUNCTION_GET_PANEL_EXPANSION_MODE_FROM_CMOS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * BYTE - panel expansion mode + */ +#define ATIF_FUNCTION_SET_PANEL_EXPANSION_MODE_IN_CMOS 0x8 +/* ARG0: ATIF_FUNCTION_SET_PANEL_EXPANSION_MODE_IN_CMOS + * ARG1: + * WORD - structure size in bytes (includes size field) + * BYTE - panel expansion mode + * OUTPUT: none + */ +#define ATIF_FUNCTION_TEMPERATURE_CHANGE_NOTIFICATION 0xD +/* ARG0: ATIF_FUNCTION_TEMPERATURE_CHANGE_NOTIFICATION + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - gfx controller id + * BYTE - current temperature (degress Celsius) + * OUTPUT: none + */ +#define ATIF_FUNCTION_GET_GRAPHICS_DEVICE_TYPES 0xF +/* ARG0: ATIF_FUNCTION_GET_GRAPHICS_DEVICE_TYPES + * ARG1: none + * OUTPUT: + * WORD - number of gfx devices + * WORD - device structure size in bytes (excludes device size field) + * DWORD - flags \ + * WORD - bus number } repeated structure + * WORD - device number / + */ +/* flags */ +# define ATIF_PX_REMOVABLE_GRAPHICS_DEVICE (1 << 0) +# define ATIF_XGP_PORT (1 << 1) +# define ATIF_VGA_ENABLED_GRAPHICS_DEVICE (1 << 2) +# define ATIF_XGP_PORT_IN_DOCK (1 << 3) + +/* ATPX */ +#define ATPX_FUNCTION_VERIFY_INTERFACE 0x0 +/* ARG0: ATPX_FUNCTION_VERIFY_INTERFACE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * WORD - version + * DWORD - supported functions bit vector + */ +/* supported functions vector */ +# define ATPX_GET_PX_PARAMETERS_SUPPORTED (1 << 0) +# define ATPX_POWER_CONTROL_SUPPORTED (1 << 1) +# define ATPX_DISPLAY_MUX_CONTROL_SUPPORTED (1 << 2) +# define ATPX_I2C_MUX_CONTROL_SUPPORTED (1 << 3) +# define ATPX_GRAPHICS_DEVICE_SWITCH_START_NOTIFICATION_SUPPORTED (1 << 4) +# define ATPX_GRAPHICS_DEVICE_SWITCH_END_NOTIFICATION_SUPPORTED (1 << 5) +# define ATPX_GET_DISPLAY_CONNECTORS_MAPPING_SUPPORTED (1 << 7) +# define ATPX_GET_DISPLAY_DETECTION_PORTS_SUPPORTED (1 << 8) +#define ATPX_FUNCTION_GET_PX_PARAMETERS 0x1 +/* ARG0: ATPX_FUNCTION_GET_PX_PARAMETERS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags + */ +/* flags */ +# define ATPX_LVDS_I2C_AVAILABLE_TO_BOTH_GPUS (1 << 0) +# define ATPX_CRT1_I2C_AVAILABLE_TO_BOTH_GPUS (1 << 1) +# define ATPX_DVI1_I2C_AVAILABLE_TO_BOTH_GPUS (1 << 2) +# define ATPX_CRT1_RGB_SIGNAL_MUXED (1 << 3) +# define ATPX_TV_SIGNAL_MUXED (1 << 4) +# define ATPX_DFP_SIGNAL_MUXED (1 << 5) +# define ATPX_SEPARATE_MUX_FOR_I2C (1 << 6) +# define ATPX_DYNAMIC_PX_SUPPORTED (1 << 7) +# define ATPX_ACF_NOT_SUPPORTED (1 << 8) +# define ATPX_FIXED_NOT_SUPPORTED (1 << 9) +# define ATPX_DYNAMIC_DGPU_POWER_OFF_SUPPORTED (1 << 10) +# define ATPX_DGPU_REQ_POWER_FOR_DISPLAYS (1 << 11) +#define ATPX_FUNCTION_POWER_CONTROL 0x2 +/* ARG0: ATPX_FUNCTION_POWER_CONTROL + * ARG1: + * WORD - structure size in bytes (includes size field) + * BYTE - dGPU power state (0: power off, 1: power on) + * OUTPUT: none + */ +#define ATPX_FUNCTION_DISPLAY_MUX_CONTROL 0x3 +/* ARG0: ATPX_FUNCTION_DISPLAY_MUX_CONTROL + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - display mux control (0: iGPU, 1: dGPU) + * OUTPUT: none + */ +# define ATPX_INTEGRATED_GPU 0 +# define ATPX_DISCRETE_GPU 1 +#define ATPX_FUNCTION_I2C_MUX_CONTROL 0x4 +/* ARG0: ATPX_FUNCTION_I2C_MUX_CONTROL + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - i2c/aux/hpd mux control (0: iGPU, 1: dGPU) + * OUTPUT: none + */ +#define ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_START_NOTIFICATION 0x5 +/* ARG0: ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_START_NOTIFICATION + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - target gpu (0: iGPU, 1: dGPU) + * OUTPUT: none + */ +#define ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_END_NOTIFICATION 0x6 +/* ARG0: ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_END_NOTIFICATION + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - target gpu (0: iGPU, 1: dGPU) + * OUTPUT: none + */ +#define ATPX_FUNCTION_GET_DISPLAY_CONNECTORS_MAPPING 0x8 +/* ARG0: ATPX_FUNCTION_GET_DISPLAY_CONNECTORS_MAPPING + * ARG1: none + * OUTPUT: + * WORD - number of display connectors + * WORD - connector structure size in bytes (excludes connector size field) + * BYTE - flags \ + * BYTE - ATIF display vector bit position } repeated + * BYTE - adapter id (0: iGPU, 1-n: dGPU ordered by pcie bus number) } structure + * WORD - connector ACPI id / + */ +/* flags */ +# define ATPX_DISPLAY_OUTPUT_SUPPORTED_BY_ADAPTER_ID_DEVICE (1 << 0) +# define ATPX_DISPLAY_HPD_SUPPORTED_BY_ADAPTER_ID_DEVICE (1 << 1) +# define ATPX_DISPLAY_I2C_SUPPORTED_BY_ADAPTER_ID_DEVICE (1 << 2) +#define ATPX_FUNCTION_GET_DISPLAY_DETECTION_PORTS 0x9 +/* ARG0: ATPX_FUNCTION_GET_DISPLAY_DETECTION_PORTS + * ARG1: none + * OUTPUT: + * WORD - number of HPD/DDC ports + * WORD - port structure size in bytes (excludes port size field) + * BYTE - ATIF display vector bit position \ + * BYTE - hpd id } reapeated structure + * BYTE - ddc id / + * + * available on A+A systems only + */ +/* hpd id */ +# define ATPX_HPD_NONE 0 +# define ATPX_HPD1 1 +# define ATPX_HPD2 2 +# define ATPX_HPD3 3 +# define ATPX_HPD4 4 +# define ATPX_HPD5 5 +# define ATPX_HPD6 6 +/* ddc id */ +# define ATPX_DDC_NONE 0 +# define ATPX_DDC1 1 +# define ATPX_DDC2 2 +# define ATPX_DDC3 3 +# define ATPX_DDC4 4 +# define ATPX_DDC5 5 +# define ATPX_DDC6 6 +# define ATPX_DDC7 7 +# define ATPX_DDC8 8 + +/* ATCS */ +#define ATCS_FUNCTION_VERIFY_INTERFACE 0x0 +/* ARG0: ATCS_FUNCTION_VERIFY_INTERFACE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * WORD - version + * DWORD - supported functions bit vector + */ +/* supported functions vector */ +# define ATCS_GET_EXTERNAL_STATE_SUPPORTED (1 << 0) +# define ATCS_PCIE_PERFORMANCE_REQUEST_SUPPORTED (1 << 1) +# define ATCS_PCIE_DEVICE_READY_NOTIFICATION_SUPPORTED (1 << 2) +# define ATCS_SET_PCIE_BUS_WIDTH_SUPPORTED (1 << 3) +#define ATCS_FUNCTION_GET_EXTERNAL_STATE 0x1 +/* ARG0: ATCS_FUNCTION_GET_EXTERNAL_STATE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags (0: undocked, 1: docked) + */ +/* flags */ +# define ATCS_DOCKED (1 << 0) +#define ATCS_FUNCTION_PCIE_PERFORMANCE_REQUEST 0x2 +/* ARG0: ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_END_NOTIFICATION + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - client id (bit 2-0: func num, 7-3: dev num, 15-8: bus num) + * WORD - valid flags mask + * WORD - flags + * BYTE - request type + * BYTE - performance request + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * BYTE - return value + */ +/* flags */ +# define ATCS_ADVERTISE_CAPS (1 << 0) +# define ATCS_WAIT_FOR_COMPLETION (1 << 1) +/* request type */ +# define ATCS_PCIE_LINK_SPEED 1 +/* performance request */ +# define ATCS_REMOVE 0 +# define ATCS_FORCE_LOW_POWER 1 +# define ATCS_PERF_LEVEL_1 2 /* PCIE Gen 1 */ +# define ATCS_PERF_LEVEL_2 3 /* PCIE Gen 2 */ +# define ATCS_PERF_LEVEL_3 4 /* PCIE Gen 3 */ +/* return value */ +# define ATCS_REQUEST_REFUSED 1 +# define ATCS_REQUEST_COMPLETE 2 +# define ATCS_REQUEST_IN_PROGRESS 3 +#define ATCS_FUNCTION_PCIE_DEVICE_READY_NOTIFICATION 0x3 +/* ARG0: ATCS_FUNCTION_PCIE_DEVICE_READY_NOTIFICATION + * ARG1: none + * OUTPUT: none + */ +#define ATCS_FUNCTION_SET_PCIE_BUS_WIDTH 0x4 +/* ARG0: ATCS_FUNCTION_SET_PCIE_BUS_WIDTH + * ARG1: + * WORD - structure size in bytes (includes size field) + * WORD - client id (bit 2-0: func num, 7-3: dev num, 15-8: bus num) + * BYTE - number of active lanes + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * BYTE - number of active lanes + */ + +#endif diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c index 98724fc..ab1c4a9 100644 --- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c +++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c @@ -12,18 +12,7 @@ #include <acpi/acpi_bus.h> #include <linux/pci.h> -#define ATPX_VERSION 0 -#define ATPX_GPU_PWR 2 -#define ATPX_MUX_SELECT 3 -#define ATPX_I2C_MUX_SELECT 4 -#define ATPX_SWITCH_START 5 -#define ATPX_SWITCH_END 6 - -#define ATPX_INTEGRATED 0 -#define ATPX_DISCRETE 1 - -#define ATPX_MUX_IGD 0 -#define ATPX_MUX_DISCRETE 1 +#include "radeon_acpi.h" static struct radeon_atpx_priv { bool atpx_detected; @@ -92,10 +81,10 @@ static int radeon_atpx_get_version(acpi_handle handle) atpx_arg.pointer = &atpx_arg_elements[0]; atpx_arg_elements[0].type = ACPI_TYPE_INTEGER; - atpx_arg_elements[0].integer.value = ATPX_VERSION; + atpx_arg_elements[0].integer.value = ATPX_FUNCTION_VERIFY_INTERFACE; atpx_arg_elements[1].type = ACPI_TYPE_INTEGER; - atpx_arg_elements[1].integer.value = ATPX_VERSION; + atpx_arg_elements[1].integer.value = 0; status = acpi_evaluate_object(handle, NULL, &atpx_arg, &buffer); if (ACPI_FAILURE(status)) { @@ -145,27 +134,31 @@ static int radeon_atpx_execute(acpi_handle handle, int cmd_id, u16 value) static int radeon_atpx_set_discrete_state(acpi_handle handle, int state) { - return radeon_atpx_execute(handle, ATPX_GPU_PWR, state); + return radeon_atpx_execute(handle, ATPX_FUNCTION_POWER_CONTROL, state); } static int radeon_atpx_switch_mux(acpi_handle handle, int mux_id) { - return radeon_atpx_execute(handle, ATPX_MUX_SELECT, mux_id); + return radeon_atpx_execute(handle, ATPX_FUNCTION_DISPLAY_MUX_CONTROL, mux_id); } static int radeon_atpx_switch_i2c_mux(acpi_handle handle, int mux_id) { - return radeon_atpx_execute(handle, ATPX_I2C_MUX_SELECT, mux_id); + return radeon_atpx_execute(handle, ATPX_FUNCTION_I2C_MUX_CONTROL, mux_id); } static int radeon_atpx_switch_start(acpi_handle handle, int gpu_id) { - return radeon_atpx_execute(handle, ATPX_SWITCH_START, gpu_id); + return radeon_atpx_execute(handle, + ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_START_NOTIFICATION, + gpu_id); } static int radeon_atpx_switch_end(acpi_handle handle, int gpu_id) { - return radeon_atpx_execute(handle, ATPX_SWITCH_END, gpu_id); + return radeon_atpx_execute(handle, + ATPX_FUNCTION_GRAPHICS_DEVICE_SWITCH_END_NOTIFICATION, + gpu_id); } static int radeon_atpx_switchto(enum vga_switcheroo_client_id id) @@ -173,9 +166,9 @@ static int radeon_atpx_switchto(enum vga_switcheroo_client_id id) int gpu_id; if (id == VGA_SWITCHEROO_IGD) - gpu_id = ATPX_INTEGRATED; + gpu_id = ATPX_INTEGRATED_GPU; else - gpu_id = ATPX_DISCRETE; + gpu_id = ATPX_DISCRETE_GPU; radeon_atpx_switch_start(radeon_atpx_priv.atpx_handle, gpu_id); radeon_atpx_switch_mux(radeon_atpx_priv.atpx_handle, gpu_id); -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-25 17:38 [PATCH] drm/radeon: add new AMD ACPI header and update relevant code alexdeucher @ 2012-07-26 12:58 ` Luca Tettamanti 2012-07-26 15:35 ` Alex Deucher 2012-07-27 2:50 ` joeyli 0 siblings, 2 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-26 12:58 UTC (permalink / raw) To: alexdeucher; +Cc: airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi On Wed, Jul 25, 2012 at 01:38:09PM -0400, alexdeucher@gmail.com wrote: > From: Alex Deucher <alexander.deucher@amd.com> > > Add a new header that defines the AMD ACPI interface used > for laptops, PowerXpress, and chipset specific functionality > and update the current code to use it. Great! Now my DSDT makes sense ;) > Todo: > - properly verify the ACPI interfaces > - hook up and handle ACPI notifications I see a problem here, I've hacked up a standalone test module, but: > +#define ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 [...] > + * flags > + * bits 1:0: > + * 0 - Notify(VGA, 0x81) is not used for notification > + * 1 - Notify(VGA, 0x81) is used for notification My system has this bit set, and the brightness control method does send the notification. My module register itself with register_acpi_notifier and gets 2 notifications, one with ACPI_VIDEO_NOTIFY_PROBE (0x81) and the other with ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS (or DEC, depending on what I press). The standard acpi "video" module, however, in response to ACPI_VIDEO_NOTIFY_PROBE generates a key press sending KEY_SWITCHVIDEOMODE. This greatly confuses KDE which messes up my dual screen configuration; I guess that the spurious KEY_SWITCHVIDEOMODE may be problematic also with other DEs... In more detail what happens is the following: - I press the brightness hotkey, firmware generates a notification on the relevant device (_SB.PCI0.PEG0.VGA.LCD) - ACPI video module gets the ACPI_VIDEO_NOTIFY_{DEC,INC}_BRIGHTNESS notification and tries to adjust the brightness with acpi_video_device_lcd_set_level, which in turns calls _BCM - _BCM sets the relevant bits in the AMD-specific structure and does a Notify (VGA, 0x81) - again ACPI video module gets the nodification (in this case ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE - KDE seems this and muck with the screen configuration :( - meanwhile the brightness notification is propagated, the hypothetical radeon driver does its magic to adjust the screen. My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to the acpi notifier chain, and allow the handlers to veto the key press (like it's done for ACPI_VIDEO_NOTIFY_SWITCH). Zhang Rui what do you think about this? The other missing bit is how to actually change the brightness... Alex, do you know what registers to poke? My card is a: 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Thames XT/GL [Radeon HD 600M Series] [1002:6840] on a Toshiba L855. Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 12:58 ` Luca Tettamanti @ 2012-07-26 15:35 ` Alex Deucher 2012-07-26 19:33 ` Luca Tettamanti 2012-07-28 14:39 ` Pali Rohár 2012-07-27 2:50 ` joeyli 1 sibling, 2 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-26 15:35 UTC (permalink / raw) To: Luca Tettamanti; +Cc: airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi [-- Attachment #1: Type: text/plain, Size: 2892 bytes --] On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Wed, Jul 25, 2012 at 01:38:09PM -0400, alexdeucher@gmail.com wrote: >> From: Alex Deucher <alexander.deucher@amd.com> >> >> Add a new header that defines the AMD ACPI interface used >> for laptops, PowerXpress, and chipset specific functionality >> and update the current code to use it. > > Great! Now my DSDT makes sense ;) > >> Todo: >> - properly verify the ACPI interfaces >> - hook up and handle ACPI notifications > > I see a problem here, I've hacked up a standalone test module, but: > >> +#define ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 > [...] >> + * flags >> + * bits 1:0: >> + * 0 - Notify(VGA, 0x81) is not used for notification >> + * 1 - Notify(VGA, 0x81) is used for notification > > My system has this bit set, and the brightness control method does send > the notification. > My module register itself with register_acpi_notifier and gets 2 > notifications, one with ACPI_VIDEO_NOTIFY_PROBE (0x81) and the other > with ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS (or DEC, depending on what I > press). > > The standard acpi "video" module, however, in response to > ACPI_VIDEO_NOTIFY_PROBE generates a key press sending > KEY_SWITCHVIDEOMODE. > > This greatly confuses KDE which messes up my dual screen configuration; > I guess that the spurious KEY_SWITCHVIDEOMODE may be problematic also > with other DEs... > > In more detail what happens is the following: > - I press the brightness hotkey, firmware generates a notification on > the relevant device (_SB.PCI0.PEG0.VGA.LCD) > - ACPI video module gets the ACPI_VIDEO_NOTIFY_{DEC,INC}_BRIGHTNESS > notification and tries to adjust the brightness with > acpi_video_device_lcd_set_level, which in turns calls _BCM > - _BCM sets the relevant bits in the AMD-specific structure and does a > Notify (VGA, 0x81) > - again ACPI video module gets the nodification (in this case > ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE > - KDE seems this and muck with the screen configuration :( > - meanwhile the brightness notification is propagated, the hypothetical > radeon driver does its magic to adjust the screen. > > My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to the > acpi notifier chain, and allow the handlers to veto the key press (like > it's done for ACPI_VIDEO_NOTIFY_SWITCH). > > Zhang Rui what do you think about this? > > The other missing bit is how to actually change the brightness... Alex, > do you know what registers to poke? You need to check if the GPU controls the backlight or the system does. I think the attached patches should point you in the right direction. Alex > > My card is a: > > 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Thames XT/GL [Radeon HD 600M Series] [1002:6840] > > on a Toshiba L855. > > Luca [-- Attachment #2: 0003-drm-radeon-add-backlight-control-for-atom-devices.patch --] [-- Type: text/x-patch, Size: 10963 bytes --] From 31f6dff07ecb4cf092292345405b2151c98f0504 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Thu, 26 Jul 2012 11:32:03 -0400 Subject: [PATCH 3/3] drm/radeon: add backlight control for atom devices On systems that use the build in GPU backlight controller, we can use atom tables to change the brightness level. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/radeon/atombios_encoders.c | 231 ++++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_connectors.c | 15 -- drivers/gpu/drm/radeon/radeon_encoders.c | 18 ++- 3 files changed, 248 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c index 7dfc62f..60c86ce 100644 --- a/drivers/gpu/drm/radeon/atombios_encoders.c +++ b/drivers/gpu/drm/radeon/atombios_encoders.c @@ -28,9 +28,238 @@ #include "radeon_drm.h" #include "radeon.h" #include "atom.h" +#include <linux/backlight.h> extern int atom_debug; +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) + +static u8 +radeon_atom_get_backlight_level_from_reg(struct radeon_device *rdev) +{ + u8 backlight_level; + u32 bios_2_scratch; + + if (rdev->family >= CHIP_R600) + bios_2_scratch = RREG32(R600_BIOS_2_SCRATCH); + else + bios_2_scratch = RREG32(RADEON_BIOS_2_SCRATCH); + + backlight_level = ((bios_2_scratch & ATOM_S2_CURRENT_BL_LEVEL_MASK) >> + ATOM_S2_CURRENT_BL_LEVEL_SHIFT); + + return backlight_level; +} + +static void +radeon_atom_set_backlight_level_to_reg(struct radeon_device *rdev, + u8 backlight_level) +{ + u32 bios_2_scratch; + + if (rdev->family >= CHIP_R600) + bios_2_scratch = RREG32(R600_BIOS_2_SCRATCH); + else + bios_2_scratch = RREG32(RADEON_BIOS_2_SCRATCH); + + bios_2_scratch &= ~ATOM_S2_CURRENT_BL_LEVEL_MASK; + bios_2_scratch |= ((backlight_level << ATOM_S2_CURRENT_BL_LEVEL_SHIFT) & + ATOM_S2_CURRENT_BL_LEVEL_MASK); + + if (rdev->family >= CHIP_R600) + WREG32(R600_BIOS_2_SCRATCH, bios_2_scratch); + else + WREG32(RADEON_BIOS_2_SCRATCH, bios_2_scratch); +} + +static void +atombios_set_panel_brightness(struct radeon_encoder *radeon_encoder) +{ + struct drm_encoder *encoder = &radeon_encoder->base; + struct drm_device *dev = radeon_encoder->base.dev; + struct radeon_device *rdev = dev->dev_private; + struct radeon_encoder_atom_dig *dig; + DISPLAY_DEVICE_OUTPUT_CONTROL_PS_ALLOCATION args; + int index; + + if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) { + dig = radeon_encoder->enc_priv; + radeon_atom_set_backlight_level_to_reg(rdev, dig->backlight_level); + + switch (radeon_encoder->encoder_id) { + case ENCODER_OBJECT_ID_INTERNAL_LVDS: + case ENCODER_OBJECT_ID_INTERNAL_LVTM1: + index = GetIndexIntoMasterTable(COMMAND, LCD1OutputControl); + if (dig->backlight_level == 0) { + args.ucAction = ATOM_LCD_BLOFF; + atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args); + } else { + args.ucAction = ATOM_LCD_BL_BRIGHTNESS_CONTROL; + atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args); + args.ucAction = ATOM_LCD_BLON; + atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args); + } + break; + case ENCODER_OBJECT_ID_INTERNAL_UNIPHY: + case ENCODER_OBJECT_ID_INTERNAL_KLDSCP_LVTMA: + case ENCODER_OBJECT_ID_INTERNAL_UNIPHY1: + case ENCODER_OBJECT_ID_INTERNAL_UNIPHY2: + if (dig->backlight_level == 0) + atombios_dig_transmitter_setup(encoder, ATOM_TRANSMITTER_ACTION_LCD_BLOFF, 0, 0); + else { + atombios_dig_transmitter_setup(encoder, ATOM_TRANSMITTER_ACTION_BL_BRIGHTNESS_CONTROL, 0, 0); + atombios_dig_transmitter_setup(encoder, ATOM_TRANSMITTER_ACTION_LCD_BLON, 0, 0); + } + break; + default: + break; + } + } +} + +static u8 radeon_atom_bl_level(struct backlight_device *bd) +{ + u8 level; + + /* Convert brightness to hardware level */ + if (bd->props.brightness < 0) + level = 0; + else if (bd->props.brightness > RADEON_MAX_BL_LEVEL) + level = RADEON_MAX_BL_LEVEL; + else + level = bd->props.brightness; + + return level; +} + +static int radeon_atom_backlight_update_status(struct backlight_device *bd) +{ + struct radeon_backlight_privdata *pdata = bl_get_data(bd); + struct radeon_encoder *radeon_encoder = pdata->encoder; + + if (radeon_encoder->enc_priv) { + struct radeon_encoder_atom_dig *dig = radeon_encoder->enc_priv; + dig->backlight_level = radeon_atom_bl_level(bd); + atombios_set_panel_brightness(radeon_encoder); + } + + return 0; +} + +static int radeon_atom_backlight_get_brightness(struct backlight_device *bd) +{ + struct radeon_backlight_privdata *pdata = bl_get_data(bd); + struct radeon_encoder *radeon_encoder = pdata->encoder; + struct drm_device *dev = radeon_encoder->base.dev; + struct radeon_device *rdev = dev->dev_private; + + return radeon_atom_get_backlight_level_from_reg(rdev); +} + +static const struct backlight_ops radeon_atom_backlight_ops = { + .get_brightness = radeon_atom_backlight_get_brightness, + .update_status = radeon_atom_backlight_update_status, +}; + +void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder, + struct drm_connector *drm_connector) +{ + struct drm_device *dev = radeon_encoder->base.dev; + struct radeon_device *rdev = dev->dev_private; + struct backlight_device *bd; + struct backlight_properties props; + struct radeon_backlight_privdata *pdata; + struct radeon_encoder_atom_dig *dig; + u8 backlight_level; + + if (!radeon_encoder->enc_priv) + return; + + if (!rdev->is_atom_bios) + return; + + if (!rdev->mode_info.gpu_controls_bl) + return; + + pdata = kmalloc(sizeof(struct radeon_backlight_privdata), GFP_KERNEL); + if (!pdata) { + DRM_ERROR("Memory allocation failed\n"); + goto error; + } + + memset(&props, 0, sizeof(props)); + props.max_brightness = RADEON_MAX_BL_LEVEL; + props.type = BACKLIGHT_RAW; + bd = backlight_device_register("radeon_bl", &drm_connector->kdev, + pdata, &radeon_atom_backlight_ops, &props); + if (IS_ERR(bd)) { + DRM_ERROR("Backlight registration failed\n"); + goto error; + } + + pdata->encoder = radeon_encoder; + + backlight_level = radeon_atom_get_backlight_level_from_reg(rdev); + + dig = radeon_encoder->enc_priv; + dig->bl_dev = bd; + + bd->props.brightness = radeon_atom_backlight_get_brightness(bd); + bd->props.power = FB_BLANK_UNBLANK; + backlight_update_status(bd); + + DRM_INFO("radeon atom DIG backlight initialized\n"); + + return; + +error: + kfree(pdata); + return; +} + +static void radeon_atom_backlight_exit(struct radeon_encoder *radeon_encoder) +{ + struct drm_device *dev = radeon_encoder->base.dev; + struct radeon_device *rdev = dev->dev_private; + struct backlight_device *bd = NULL; + struct radeon_encoder_atom_dig *dig; + + if (!radeon_encoder->enc_priv) + return; + + if (!rdev->is_atom_bios) + return; + + if (!rdev->mode_info.gpu_controls_bl) + return; + + dig = radeon_encoder->enc_priv; + bd = dig->bl_dev; + dig->bl_dev = NULL; + + if (bd) { + struct radeon_legacy_backlight_privdata *pdata; + + pdata = bl_get_data(bd); + backlight_device_unregister(bd); + kfree(pdata); + + DRM_INFO("radeon atom LVDS backlight unloaded\n"); + } +} + +#else /* !CONFIG_BACKLIGHT_CLASS_DEVICE */ + +void radeon_atom_backlight_init(struct radeon_encoder *encoder) +{ +} + +static void radeon_atom_backlight_exit(struct radeon_encoder *encoder) +{ +} + +#endif + /* evil but including atombios.h is much worse */ bool radeon_atom_get_tv_timings(struct radeon_device *rdev, int index, struct drm_display_mode *mode); @@ -2272,6 +2501,8 @@ static const struct drm_encoder_helper_funcs radeon_atom_dac_helper_funcs = { void radeon_enc_destroy(struct drm_encoder *encoder) { struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder); + if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) + radeon_atom_backlight_exit(radeon_encoder); kfree(radeon_encoder->enc_priv); drm_encoder_cleanup(encoder); kfree(radeon_encoder); diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 895e628..748c5f2 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -40,10 +40,6 @@ radeon_atombios_connected_scratch_regs(struct drm_connector *connector, struct drm_encoder *encoder, bool connected); -extern void -radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder, - struct drm_connector *drm_connector); - void radeon_connector_hotplug(struct drm_connector *connector) { struct drm_device *dev = connector->dev; @@ -2008,15 +2004,4 @@ radeon_add_legacy_connector(struct drm_device *dev, connector->polled = DRM_CONNECTOR_POLL_HPD; connector->display_info.subpixel_order = subpixel_order; drm_sysfs_connector_add(connector); - if (connector_type == DRM_MODE_CONNECTOR_LVDS) { - struct drm_encoder *drm_encoder; - - list_for_each_entry(drm_encoder, &dev->mode_config.encoder_list, head) { - struct radeon_encoder *radeon_encoder; - - radeon_encoder = to_radeon_encoder(drm_encoder); - if (radeon_encoder->encoder_id == ENCODER_OBJECT_ID_INTERNAL_LVDS) - radeon_legacy_backlight_init(radeon_encoder, connector); - } - } } diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c b/drivers/gpu/drm/radeon/radeon_encoders.c index 7467069..93b0d64 100644 --- a/drivers/gpu/drm/radeon/radeon_encoders.c +++ b/drivers/gpu/drm/radeon/radeon_encoders.c @@ -29,6 +29,14 @@ #include "radeon.h" #include "atom.h" +extern void +radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder, + struct drm_connector *drm_connector); +extern void +radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder, + struct drm_connector *drm_connector); + + static uint32_t radeon_encoder_clones(struct drm_encoder *encoder) { struct drm_device *dev = encoder->dev; @@ -153,6 +161,7 @@ radeon_get_encoder_enum(struct drm_device *dev, uint32_t supported_device, uint8 void radeon_link_encoder_connector(struct drm_device *dev) { + struct radeon_device *rdev = dev->dev_private; struct drm_connector *connector; struct radeon_connector *radeon_connector; struct drm_encoder *encoder; @@ -163,8 +172,15 @@ radeon_link_encoder_connector(struct drm_device *dev) radeon_connector = to_radeon_connector(connector); list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { radeon_encoder = to_radeon_encoder(encoder); - if (radeon_encoder->devices & radeon_connector->devices) + if (radeon_encoder->devices & radeon_connector->devices) { drm_mode_connector_attach_encoder(connector, encoder); + if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) { + if (rdev->is_atom_bios) + radeon_atom_backlight_init(radeon_encoder, connector); + else + radeon_legacy_backlight_init(radeon_encoder, connector); + } + } } } } -- 1.7.7.5 [-- Attachment #3: 0002-drm-radeon-rework-legacy-backlight-control.patch --] [-- Type: text/x-patch, Size: 3443 bytes --] From f55f2a468c48494b8111bf732a9f8d3586d8541e Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Thu, 26 Jul 2012 11:05:22 -0400 Subject: [PATCH 2/3] drm/radeon: rework legacy backlight control To better enable sharing with atom backlight control. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 19 ++++++------------- drivers/gpu/drm/radeon/radeon_mode.h | 11 +++++++++++ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c index 670e991..9910fe4 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c @@ -271,13 +271,6 @@ static const struct drm_encoder_helper_funcs radeon_legacy_lvds_helper_funcs = { #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) -#define MAX_RADEON_LEVEL 0xFF - -struct radeon_backlight_privdata { - struct radeon_encoder *encoder; - uint8_t negative; -}; - static uint8_t radeon_legacy_lvds_level(struct backlight_device *bd) { struct radeon_backlight_privdata *pdata = bl_get_data(bd); @@ -286,13 +279,13 @@ static uint8_t radeon_legacy_lvds_level(struct backlight_device *bd) /* Convert brightness to hardware level */ if (bd->props.brightness < 0) level = 0; - else if (bd->props.brightness > MAX_RADEON_LEVEL) - level = MAX_RADEON_LEVEL; + else if (bd->props.brightness > RADEON_MAX_BL_LEVEL) + level = RADEON_MAX_BL_LEVEL; else level = bd->props.brightness; if (pdata->negative) - level = MAX_RADEON_LEVEL - level; + level = RADEON_MAX_BL_LEVEL - level; return level; } @@ -336,7 +329,7 @@ static int radeon_legacy_backlight_get_brightness(struct backlight_device *bd) backlight_level = (RREG32(RADEON_LVDS_GEN_CNTL) >> RADEON_LVDS_BL_MOD_LEVEL_SHIFT) & 0xff; - return pdata->negative ? MAX_RADEON_LEVEL - backlight_level : backlight_level; + return pdata->negative ? RADEON_MAX_BL_LEVEL - backlight_level : backlight_level; } static const struct backlight_ops radeon_backlight_ops = { @@ -370,7 +363,7 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder, } memset(&props, 0, sizeof(props)); - props.max_brightness = MAX_RADEON_LEVEL; + props.max_brightness = RADEON_MAX_BL_LEVEL; props.type = BACKLIGHT_RAW; bd = backlight_device_register("radeon_bl", &drm_connector->kdev, pdata, &radeon_backlight_ops, &props); @@ -449,7 +442,7 @@ static void radeon_legacy_backlight_exit(struct radeon_encoder *radeon_encoder) } if (bd) { - struct radeon_legacy_backlight_privdata *pdata; + struct radeon_backlight_privdata *pdata; pdata = bl_get_data(bd); backlight_device_unregister(bd); diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index 33cbf97..46d7801 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -256,6 +256,17 @@ struct radeon_mode_info { bool gpu_controls_bl; }; +#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) + +#define RADEON_MAX_BL_LEVEL 0xFF + +struct radeon_backlight_privdata { + struct radeon_encoder *encoder; + uint8_t negative; +}; + +#endif + #define MAX_H_CODE_TIMING_LEN 32 #define MAX_V_CODE_TIMING_LEN 32 -- 1.7.7.5 [-- Attachment #4: 0001-drm-radeon-track-whether-the-GPU-controls-the-backli.patch --] [-- Type: text/x-patch, Size: 1652 bytes --] From a56ac560e34032ca803e0909c8a92a9037046790 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Thu, 26 Jul 2012 09:50:57 -0400 Subject: [PATCH 1/3] drm/radeon: track whether the GPU controls the backlight A table in the vbios tells us whether the GPU backlight controller is used or not. If the bit is set, the GPU backlight controller is used; if it is not set, an off-chip backlight controller is used. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/radeon/radeon_atombios.c | 4 ++++ drivers/gpu/drm/radeon/radeon_mode.h | 2 ++ 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c index b1e3820..92267a8 100644 --- a/drivers/gpu/drm/radeon/radeon_atombios.c +++ b/drivers/gpu/drm/radeon/radeon_atombios.c @@ -1254,6 +1254,10 @@ bool radeon_atom_get_clock_info(struct drm_device *dev) if (rdev->clock.max_pixel_clock == 0) rdev->clock.max_pixel_clock = 40000; + /* not technically a clock, but... */ + if (firmware_info->info.usFirmwareCapability.sbfAccess.GPUControlsBL) + rdev->mode_info.gpu_controls_bl = true; + return true; } diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index f380d59..33cbf97 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -252,6 +252,8 @@ struct radeon_mode_info { /* pointer to fbdev info structure */ struct radeon_fbdev *rfbdev; + /* GPU controls backlight */ + bool gpu_controls_bl; }; #define MAX_H_CODE_TIMING_LEN 32 -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 15:35 ` Alex Deucher @ 2012-07-26 19:33 ` Luca Tettamanti 2012-07-26 19:42 ` Alex Deucher 2012-07-28 14:39 ` Pali Rohár 1 sibling, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-26 19:33 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, linux-acpi, Zhang Rui, dri-devel On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: > On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > The other missing bit is how to actually change the brightness... Alex, > > do you know what registers to poke? > > You need to check if the GPU controls the backlight or the system > does. I think the attached patches should point you in the right > direction. Yep :) 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : USHORT GPUControlsBL:1 = 0x0001 (1) The panel is using the INTERNAL_UNIPHY encoder, and I see the UNIPHYTransmitterControl command table. Interaction with video.ko is still a bit messy... Do you already have code for handling the notifications? I'll work on it in the weekend otherwise ;) Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 19:33 ` Luca Tettamanti @ 2012-07-26 19:42 ` Alex Deucher 2012-07-26 19:58 ` Alex Deucher 2012-07-28 14:56 ` Luca Tettamanti 0 siblings, 2 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-26 19:42 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, linux-acpi, Zhang Rui, dri-devel On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: >> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > The other missing bit is how to actually change the brightness... Alex, >> > do you know what registers to poke? >> >> You need to check if the GPU controls the backlight or the system >> does. I think the attached patches should point you in the right >> direction. > > Yep :) > > 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : > 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : > USHORT GPUControlsBL:1 = 0x0001 (1) > > The panel is using the INTERNAL_UNIPHY encoder, and I see the > UNIPHYTransmitterControl command table. > > Interaction with video.ko is still a bit messy... > > Do you already have code for handling the notifications? I'll work on it > in the weekend otherwise ;) I don't have patches for that. Please feel free to work on it :) Thanks, Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 19:42 ` Alex Deucher @ 2012-07-26 19:58 ` Alex Deucher 2012-07-28 14:56 ` Luca Tettamanti 1 sibling, 0 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-26 19:58 UTC (permalink / raw) To: Luca Tettamanti; +Cc: airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi On Thu, Jul 26, 2012 at 3:42 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: >>> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >>> > The other missing bit is how to actually change the brightness... Alex, >>> > do you know what registers to poke? >>> >>> You need to check if the GPU controls the backlight or the system >>> does. I think the attached patches should point you in the right >>> direction. >> >> Yep :) >> >> 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : >> 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : >> USHORT GPUControlsBL:1 = 0x0001 (1) >> >> The panel is using the INTERNAL_UNIPHY encoder, and I see the >> UNIPHYTransmitterControl command table. >> >> Interaction with video.ko is still a bit messy... >> >> Do you already have code for handling the notifications? I'll work on it >> in the weekend otherwise ;) > > I don't have patches for that. Please feel free to work on it :) One thing worth checking, the sbios may write the requested backlight level to the bios scratch reg, in which case the driver only has to execute the BL_BRIGHTNESS action rather than writing the requested level to the register first. Alex > > Thanks, > > Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 19:42 ` Alex Deucher 2012-07-26 19:58 ` Alex Deucher @ 2012-07-28 14:56 ` Luca Tettamanti 2012-07-28 21:29 ` Alex Deucher 2012-07-29 3:51 ` joeyli 1 sibling, 2 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-28 14:56 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel On Thu, Jul 26, 2012 at 03:42:26PM -0400, Alex Deucher wrote: > On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: > >> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > >> > The other missing bit is how to actually change the brightness... Alex, > >> > do you know what registers to poke? > >> > >> You need to check if the GPU controls the backlight or the system > >> does. I think the attached patches should point you in the right > >> direction. > > > > Yep :) > > > > 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : > > 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : > > USHORT GPUControlsBL:1 = 0x0001 (1) > > > > The panel is using the INTERNAL_UNIPHY encoder, and I see the > > UNIPHYTransmitterControl command table. > > > > Interaction with video.ko is still a bit messy... > > > > Do you already have code for handling the notifications? I'll work on it > > in the weekend otherwise ;) > > I don't have patches for that. Please feel free to work on it :) I just found the first problem (probably a BIOS bug): ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( I intended to use the method to set up the notification handler but now my BIOS says that it's not there even if it is... Can I assume some default values (e.g. notifications are enabled and will use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something different)? thanks, Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-28 14:56 ` Luca Tettamanti @ 2012-07-28 21:29 ` Alex Deucher 2012-07-29 13:06 ` Luca Tettamanti 2012-07-29 19:33 ` [PATCH] drm/radeon: add new AMD ACPI header and update relevant code Luca Tettamanti 2012-07-29 3:51 ` joeyli 1 sibling, 2 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-28 21:29 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Thu, Jul 26, 2012 at 03:42:26PM -0400, Alex Deucher wrote: >> On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: >> >> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> >> > The other missing bit is how to actually change the brightness... Alex, >> >> > do you know what registers to poke? >> >> >> >> You need to check if the GPU controls the backlight or the system >> >> does. I think the attached patches should point you in the right >> >> direction. >> > >> > Yep :) >> > >> > 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : >> > 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : >> > USHORT GPUControlsBL:1 = 0x0001 (1) >> > >> > The panel is using the INTERNAL_UNIPHY encoder, and I see the >> > UNIPHYTransmitterControl command table. >> > >> > Interaction with video.ko is still a bit messy... >> > >> > Do you already have code for handling the notifications? I'll work on it >> > in the weekend otherwise ;) >> >> I don't have patches for that. Please feel free to work on it :) > > I just found the first problem (probably a BIOS bug): > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > I intended to use the method to set up the notification handler but now > my BIOS says that it's not there even if it is... > Can I assume some default values (e.g. notifications are enabled and will > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > different)? The spec says that the bits in the supported functions vector mean that if bit n is set, function n+1 exists, but it's possible that the spec is wrong and it's actually a 1 to 1 mapping; if bit n is set, function n is supported. In which case the the supported functions vector bits should be: +/* supported functions vector */ +# define ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED (1 << 1) +# define ATIF_GET_SYSTEM_BIOS_REQUESTS_SUPPORTED (1 << 2) +# define ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 3) +# define ATIF_GET_LID_STATE_SUPPORTED (1 << 4) +# define ATIF_GET_TV_STANDARD_FROM_CMOS_SUPPORTED (1 << 5) +# define ATIF_SET_TV_STANDARD_IN_CMOS_SUPPORTED (1 << 6) +# define ATIF_GET_PANEL_EXPANSION_MODE_FROM_CMOS_SUPPORTED (1 << 7) +# define ATIF_SET_PANEL_EXPANSION_MODE_IN_CMOS_SUPPORTED (1 << 8) +# define ATIF_TEMPERATURE_CHANGE_NOTIFICATION_SUPPORTED (1 << 13) +# define ATIF_GET_GRAPHICS_DEVICE_TYPES_SUPPORTED (1 << 15) See if that lines up better. I'm still new to these ACPI interfaces so I'm not an expert yet. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-28 21:29 ` Alex Deucher @ 2012-07-29 13:06 ` Luca Tettamanti 2012-07-30 14:20 ` Alex Deucher 2012-07-29 19:33 ` [PATCH] drm/radeon: add new AMD ACPI header and update relevant code Luca Tettamanti 1 sibling, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-29 13:06 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: > On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > I just found the first problem (probably a BIOS bug): > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > > I intended to use the method to set up the notification handler but now > > my BIOS says that it's not there even if it is... > > Can I assume some default values (e.g. notifications are enabled and will > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > > different)? > > The spec says that the bits in the supported functions vector mean > that if bit n is set, function n+1 exists, Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? Maybe if the bit n is set then functions 0..n are available? That would (almost) match what I see... > but it's possible that the > spec is wrong and it's actually a 1 to 1 mapping; if bit n is set, > function n is supported. In which case the the supported functions > vector bits should be: > +/* supported functions vector */ > +# define ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED (1 << 1) > +# define ATIF_GET_SYSTEM_BIOS_REQUESTS_SUPPORTED (1 << 2) > +# define ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 3) > +# define ATIF_GET_LID_STATE_SUPPORTED (1 << 4) > +# define ATIF_GET_TV_STANDARD_FROM_CMOS_SUPPORTED (1 << 5) > +# define ATIF_SET_TV_STANDARD_IN_CMOS_SUPPORTED (1 << 6) > +# define ATIF_GET_PANEL_EXPANSION_MODE_FROM_CMOS_SUPPORTED (1 << 7) > +# define ATIF_SET_PANEL_EXPANSION_MODE_IN_CMOS_SUPPORTED (1 << 8) > +# define ATIF_TEMPERATURE_CHANGE_NOTIFICATION_SUPPORTED (1 << 13) > +# define ATIF_GET_GRAPHICS_DEVICE_TYPES_SUPPORTED (1 << 15) > > See if that lines up better. Not really... the value returned by VERIFY_INTERFACE is 0x2, but in the DSDT I see: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS ATIF_FUNCTION_GET_TV_STANDARD_FROM_CMOS ATIF_FUNCTION_SET_TV_STANDARD_IN_CMOS The implementation of the first one makes sense, the second is used for brightness control. The other two _might_ be a leftover (the machine does not have an analog TV out). > I'm still new to these ACPI interfaces > so I'm not an expert yet. I've been exposed to a lot of ACPI code (I wrote the asus_atk0110 driver), in my experience the DSDT is full of crap: code copied&pasted from other machines, leftover no longer used, and other stuff that's plainly wrong. Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-29 13:06 ` Luca Tettamanti @ 2012-07-30 14:20 ` Alex Deucher 2012-07-30 20:24 ` Luca Tettamanti 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-30 14:20 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: >> On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > I just found the first problem (probably a BIOS bug): >> > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the >> > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( >> > I intended to use the method to set up the notification handler but now >> > my BIOS says that it's not there even if it is... >> > Can I assume some default values (e.g. notifications are enabled and will >> > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something >> > different)? >> >> The spec says that the bits in the supported functions vector mean >> that if bit n is set, function n+1 exists, > > Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would > mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? > > Maybe if the bit n is set then functions 0..n are available? That would > (almost) match what I see... >From the spec: Supported DWORD Bit vector providing supported functions information. Each bit marks Functions Bit support for one specific function of the ATIF method. Bit n, if set, Vector indicates that Function n+1 is supported. I don't know how wonky bioses in the wild are however. I can see what our internal teams do in these sort of cases. > >> but it's possible that the >> spec is wrong and it's actually a 1 to 1 mapping; if bit n is set, >> function n is supported. In which case the the supported functions >> vector bits should be: >> +/* supported functions vector */ >> +# define ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED (1 << 1) >> +# define ATIF_GET_SYSTEM_BIOS_REQUESTS_SUPPORTED (1 << 2) >> +# define ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 3) >> +# define ATIF_GET_LID_STATE_SUPPORTED (1 << 4) >> +# define ATIF_GET_TV_STANDARD_FROM_CMOS_SUPPORTED (1 << 5) >> +# define ATIF_SET_TV_STANDARD_IN_CMOS_SUPPORTED (1 << 6) >> +# define ATIF_GET_PANEL_EXPANSION_MODE_FROM_CMOS_SUPPORTED (1 << 7) >> +# define ATIF_SET_PANEL_EXPANSION_MODE_IN_CMOS_SUPPORTED (1 << 8) >> +# define ATIF_TEMPERATURE_CHANGE_NOTIFICATION_SUPPORTED (1 << 13) >> +# define ATIF_GET_GRAPHICS_DEVICE_TYPES_SUPPORTED (1 << 15) >> >> See if that lines up better. > > Not really... the value returned by VERIFY_INTERFACE is 0x2, but in the > DSDT I see: > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS > ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS > ATIF_FUNCTION_GET_TV_STANDARD_FROM_CMOS > ATIF_FUNCTION_SET_TV_STANDARD_IN_CMOS > > The implementation of the first one makes sense, the second is used for > brightness control. The other two _might_ be a leftover (the machine > does not have an analog TV out). Yeah, probably unless there is a TV-out on a docking station or something like that. The TV-out port should show up in the connector table in the vbios even if it has a port on the docking station however. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 14:20 ` Alex Deucher @ 2012-07-30 20:24 ` Luca Tettamanti 2012-07-30 20:30 ` Alex Deucher 2012-07-30 20:45 ` Alex Deucher 0 siblings, 2 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-30 20:24 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel [-- Attachment #1: Type: text/plain, Size: 2310 bytes --] On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote: > On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: > >> On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > >> > I just found the first problem (probably a BIOS bug): > >> > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > >> > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > >> > I intended to use the method to set up the notification handler but now > >> > my BIOS says that it's not there even if it is... > >> > Can I assume some default values (e.g. notifications are enabled and will > >> > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > >> > different)? > >> > >> The spec says that the bits in the supported functions vector mean > >> that if bit n is set, function n+1 exists, > > > > Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would > > mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? > > > > Maybe if the bit n is set then functions 0..n are available? That would > > (almost) match what I see... > > From the spec: > > Supported DWORD Bit vector providing supported functions > information. Each bit marks > Functions Bit support for one specific function of the > ATIF method. Bit n, if set, > Vector indicates that Function n+1 is supported. Sorry, I still don't understand it... what's "Function n+1" in this context? Does this mean that if bit n is set then the function defined as 1 << (n+1) is supported? > I don't know how wonky bioses in the wild are however. I can see what > our internal teams do in these sort of cases. That would be helpful :) Note that on this machine (Toshiba L855-10W) brightness control under windows does not work with the stock catalyst driver, it works only with the (oldish) driver supplied by Toshiba. Anyway, I'm attaching v2 of my patches, I've incorporated the suggestions and some bits of code from joeyli, and now brightness control is actually implemented. Still missing is the issue of event 0x81 and the conflict with video.ko; the handler should probably return NOTIFY_BAD to veto the keypress. Luca [-- Attachment #2: 0001-drm-radeon-refactor-radeon_atif_call.patch --] [-- Type: text/plain, Size: 2875 bytes --] >From f0f8699eabee0d47b93fba14f8126b821cc106a5 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. v2: add support for the 2nd parameter (from Lee, Chun-Yi <jlee@suse.com>). Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 38 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 5b32e49..158e514 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -14,23 +14,30 @@ #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, + struct acpi_buffer *params) { acpi_status status; union acpi_object atif_arg_elements[2]; struct acpi_object_list atif_arg; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; atif_arg.count = 2; 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[1].type = ACPI_TYPE_INTEGER; - atif_arg_elements[1].integer.value = 0; + atif_arg_elements[0].integer.value = function; + + if (params) { + atif_arg_elements[1].type = ACPI_TYPE_BUFFER; + atif_arg_elements[1].buffer.length = params->length; + atif_arg_elements[1].buffer.pointer = params->pointer; + } else { + /* We need a second fake parameter */ + atif_arg_elements[1].type = ACPI_TYPE_INTEGER; + atif_arg_elements[1].integer.value = 0; + } status = acpi_evaluate_object(handle, "ATIF", &atif_arg, &buffer); @@ -39,18 +46,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 +67,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, NULL); + 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/plain, Size: 5884 bytes --] >From 427002ddf653b0abd0fb820b09322bf2a0b281af Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:11:58 +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. v2: use a packed struct for handling the output of ACPI calls, hides ugly pointer arithmetics (Lee, Chun-Yi <jlee@suse.com>). Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon.h | 40 +++++++++++++++++ drivers/gpu/drm/radeon/radeon_acpi.c | 79 +++++++++++++++++++++++++++++++--- 2 files changed, 113 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 158e514..53c519d 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -13,6 +13,13 @@ #include <linux/vga_switcheroo.h> +struct atif_verify_interface { + u16 size; /* structure size in bytes (includes size field) */ + u16 version; /* version */ + u32 notification_mask; /* supported notifications mask */ + u32 function_bits; /* supported functions bit vector */ +} __packed; + /* Call the ATIF method */ static union acpi_object *radeon_atif_call(acpi_handle handle, int function, @@ -52,12 +59,73 @@ 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; + struct atif_verify_interface output; + size_t size; + int err = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL); + if (!info) + return -EIO; + + memset(&output, 0, sizeof(output)); + + size = *(u16 *) info->buffer.pointer; + if (size < 12) { + DRM_INFO("ATIF buffer is too small: %lu\n", size); + err = -EINVAL; + goto out; + } + size = min(sizeof(output), size); + + memcpy(&output, info->buffer.pointer, size); + + /* TODO: check version? */ + DRM_DEBUG_DRIVER("ATIF version %u\n", output.version); + + radeon_atif_parse_notification(&atif->notifications, output.notification_mask); + radeon_atif_parse_functions(&atif->functions, output.function_bits); + +out: + 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); @@ -67,11 +135,10 @@ int radeon_acpi_init(struct radeon_device *rdev) return 0; /* Call the ATIF method */ - info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL); - 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/plain, Size: 3383 bytes --] >From e83d04d1b60c5cf46611f6afc9680024e1dad73b Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:16:06 +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. v2: packed struct (Lee, Chun-Yi <jlee@suse.com>) Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 84 +++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 53c519d..3d025dd 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -20,6 +20,18 @@ struct atif_verify_interface { u32 function_bits; /* supported functions bit vector */ } __packed; +struct atif_system_params { + u16 size; + u32 valid_mask; + u32 flags; + u8 command_code; +} __packed; + +#define ATIF_NOTIFY_MASK 0x3 +#define ATIF_NOTIFY_NONE 0 +#define ATIF_NOTIFY_81 1 +#define ATIF_NOTIFY_N 2 + /* Call the ATIF method */ static union acpi_object *radeon_atif_call(acpi_handle handle, int function, @@ -121,10 +133,56 @@ out: return err; } +static int radeon_atif_get_notification_params(acpi_handle handle, + struct radeon_atif_notification_cfg *n) +{ + union acpi_object *info; + struct atif_system_params params; + size_t size; + int err = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_PARAMETERS, NULL); + if (!info) { + err = -EIO; + goto out; + } + + size = *(u16 *) info->buffer.pointer; + if (size < 10) { + err = -EINVAL; + goto out; + } + + memset(¶ms, 0, sizeof(params)); + size = min(sizeof(params), size); + memcpy(¶ms, info->buffer.pointer, size); + + params.flags = params.flags & params.valid_mask; + + if ((params.flags & ATIF_NOTIFY_MASK) == ATIF_NOTIFY_NONE) { + n->enabled = false; + n->command_code = 0; + } else if ((params.flags & ATIF_NOTIFY_MASK) == ATIF_NOTIFY_81) { + n->enabled = true; + n->command_code = 0x81; + } else { + if (size < 11) { + err = -EINVAL; + goto out; + } + n->command_code = params.command_code; + } + +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 */ @@ -135,10 +193,32 @@ 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-handler-for-ACPI-event.patch --] [-- Type: text/plain, Size: 7163 bytes --] >From 1d48218e22427dbeeca3e31fd18c78c4fbd35969 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:20:35 +0200 Subject: [PATCH 4/4] drm/radeon: implement handler for ACPI event Set up an handler for ACPI events and respond to brightness change requests from the system BIOS. Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/atombios_encoders.c | 2 +- drivers/gpu/drm/radeon/radeon_acpi.c | 119 +++++++++++++++++++++++++++- drivers/gpu/drm/radeon/radeon_acpi.h | 6 ++ drivers/gpu/drm/radeon/radeon_mode.h | 2 + drivers/gpu/drm/radeon/radeon_pm.c | 4 +- 5 files changed, 127 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c index c2d3552..e4152d6 100644 --- a/drivers/gpu/drm/radeon/atombios_encoders.c +++ b/drivers/gpu/drm/radeon/atombios_encoders.c @@ -72,7 +72,7 @@ radeon_atom_set_backlight_level_to_reg(struct radeon_device *rdev, WREG32(RADEON_BIOS_2_SCRATCH, bios_2_scratch); } -static void +void atombios_set_panel_brightness(struct radeon_encoder *radeon_encoder) { struct drm_encoder *encoder = &radeon_encoder->base; diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 3d025dd..d1a45e0 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -3,6 +3,7 @@ #include <linux/slab.h> #include <acpi/acpi_drivers.h> #include <acpi/acpi_bus.h> +#include <acpi/video.h> #include "drmP.h" #include "drm.h" @@ -10,6 +11,7 @@ #include "drm_crtc_helper.h" #include "radeon.h" #include "radeon_acpi.h" +#include "atom.h" #include <linux/vga_switcheroo.h> @@ -21,10 +23,22 @@ struct atif_verify_interface { } __packed; struct atif_system_params { - u16 size; - u32 valid_mask; - u32 flags; - u8 command_code; + u16 size; /* structure size in bytes (includes size field) */ + u32 valid_mask; /* valid flags mask */ + u32 flags; /* flags */ + u8 command_code; /* notify command code */ +} __packed; + +struct atif_sbios_requests { + u16 size; /* structure size in bytes (includes size field) */ + u32 pending; /* pending sbios requests */ + u8 panel_exp_mode; /* panel expansion mode */ + u8 thermal_gfx; /* thermal state: target gfx controller */ + u8 thermal_state; /* thermal state: state id (0: exit state, non-0: state) */ + u8 forced_power_gfx; /* forced power state: target gfx controller */ + u8 forced_power_state; /* forced power state: state id */ + u8 system_power_src; /* system power source */ + u8 backlight_level; /* panel backlight level (0-255) */ } __packed; #define ATIF_NOTIFY_MASK 0x3 @@ -32,6 +46,11 @@ struct atif_system_params { #define ATIF_NOTIFY_81 1 #define ATIF_NOTIFY_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, @@ -178,6 +197,98 @@ out: return err; } +static int radeon_atif_get_sbios_requests(acpi_handle handle, + struct atif_sbios_requests *req) +{ + union acpi_object *info; + size_t size; + int count = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS, NULL); + if (!info) + return -EIO; + + size = *(u16 *)info->buffer.pointer; + if (size < 0xd) { + count = -EINVAL; + goto out; + } + memset(req, 0, sizeof(*req)); + + size = min(sizeof(*req), size); + memcpy(req, info->buffer.pointer, size); + + count = hweight32(req->pending); + +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 atif_sbios_requests req; + acpi_handle handle; + int count; + + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + + 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.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) { + struct drm_encoder *tmp; + struct radeon_encoder *target = NULL; + + DRM_DEBUG_DRIVER("Changing brightness to %d\n", + req.backlight_level); + + /* Find the backlight controller */ + list_for_each_entry(tmp, &rdev->ddev->mode_config.encoder_list, + head) { + struct radeon_encoder *enc = to_radeon_encoder(tmp); + struct radeon_encoder_atom_dig *dig = enc->enc_priv; + + if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) && + dig->bl_dev != NULL) { + target = enc; + break; + } + } + + if (target) { + struct radeon_encoder_atom_dig *dig = target->enc_priv; + dig->backlight_level = req.backlight_level; + + atombios_set_panel_brightness(target); + + backlight_force_update(dig->bl_dev, + BACKLIGHT_UPDATE_HOTKEY); + } else { + /* This should never happen */ + dev_warn(rdev->dev, "Brightness change requested, " + "but not suitable encoder found\n"); + } + } + /* TODO: check other events */ + + 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..df1f162 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.h +++ b/drivers/gpu/drm/radeon/radeon_acpi.h @@ -24,6 +24,12 @@ #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_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index 129ed8e..ce91d62 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -697,6 +697,8 @@ void radeon_panel_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode); void atom_rv515_force_tv_scaler(struct radeon_device *rdev, struct radeon_crtc *radeon_crtc); +void atombios_set_panel_brightness(struct radeon_encoder *radeon_encoder); + /* legacy tv */ void radeon_legacy_tv_adjust_crtc_reg(struct drm_encoder *encoder, uint32_t *h_total_disp, uint32_t *h_sync_strt_wid, 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 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 20:24 ` Luca Tettamanti @ 2012-07-30 20:30 ` Alex Deucher 2012-07-30 20:36 ` Luca Tettamanti 2012-07-30 20:45 ` Alex Deucher 1 sibling, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-30 20:30 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Mon, Jul 30, 2012 at 4:24 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote: >> On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: >> >> On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> >> > I just found the first problem (probably a BIOS bug): >> >> > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the >> >> > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( >> >> > I intended to use the method to set up the notification handler but now >> >> > my BIOS says that it's not there even if it is... >> >> > Can I assume some default values (e.g. notifications are enabled and will >> >> > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something >> >> > different)? >> >> >> >> The spec says that the bits in the supported functions vector mean >> >> that if bit n is set, function n+1 exists, >> > >> > Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would >> > mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? >> > >> > Maybe if the bit n is set then functions 0..n are available? That would >> > (almost) match what I see... >> >> From the spec: >> >> Supported DWORD Bit vector providing supported functions >> information. Each bit marks >> Functions Bit support for one specific function of the >> ATIF method. Bit n, if set, >> Vector indicates that Function n+1 is supported. > > Sorry, I still don't understand it... what's "Function n+1" in this > context? > Does this mean that if bit n is set then the function defined as > 1 << (n+1) is supported? It means if bit n is set in teh supported function vector, function n+1 is supported. E.g., if bit 1 is set, function 2 (ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS) is supported. If bit 3 is set function 4 (ATIF_FUNCTION_GET_LID_STATE) is supported, etc. Alex > >> I don't know how wonky bioses in the wild are however. I can see what >> our internal teams do in these sort of cases. > > That would be helpful :) > Note that on this machine (Toshiba L855-10W) brightness control under > windows does not work with the stock catalyst driver, it works only with > the (oldish) driver supplied by Toshiba. > > Anyway, I'm attaching v2 of my patches, I've incorporated the > suggestions and some bits of code from joeyli, and now brightness > control is actually implemented. > > Still missing is the issue of event 0x81 and the conflict with video.ko; > the handler should probably return NOTIFY_BAD to veto the keypress. > > Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 20:30 ` Alex Deucher @ 2012-07-30 20:36 ` Luca Tettamanti 0 siblings, 0 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-30 20:36 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel On Mon, Jul 30, 2012 at 10:30 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Mon, Jul 30, 2012 at 4:24 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote: >>> Supported DWORD Bit vector providing supported functions >>> information. Each bit marks >>> Functions Bit support for one specific function of the >>> ATIF method. Bit n, if set, >>> Vector indicates that Function n+1 is supported. >> >> Sorry, I still don't understand it... what's "Function n+1" in this >> context? >> Does this mean that if bit n is set then the function defined as >> 1 << (n+1) is supported? > > It means if bit n is set in teh supported function vector, function > n+1 is supported. E.g., if bit 1 is set, function 2 > (ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS) is supported. If bit 3 is > set function 4 (ATIF_FUNCTION_GET_LID_STATE) is supported, etc. Great, just had an epiphany ;-) "n+1" refers to the value that's passed down to ATIF, I was still thinking in terms of bitmasks... Ok, so my code is correct, BIOS is botched... meh. L ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 20:24 ` Luca Tettamanti 2012-07-30 20:30 ` Alex Deucher @ 2012-07-30 20:45 ` Alex Deucher 2012-07-31 9:16 ` Luca Tettamanti 1 sibling, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-30 20:45 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Mon, Jul 30, 2012 at 4:24 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote: >> On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: >> >> On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> >> > I just found the first problem (probably a BIOS bug): >> >> > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the >> >> > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( >> >> > I intended to use the method to set up the notification handler but now >> >> > my BIOS says that it's not there even if it is... >> >> > Can I assume some default values (e.g. notifications are enabled and will >> >> > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something >> >> > different)? >> >> >> >> The spec says that the bits in the supported functions vector mean >> >> that if bit n is set, function n+1 exists, >> > >> > Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would >> > mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? >> > >> > Maybe if the bit n is set then functions 0..n are available? That would >> > (almost) match what I see... >> >> From the spec: >> >> Supported DWORD Bit vector providing supported functions >> information. Each bit marks >> Functions Bit support for one specific function of the >> ATIF method. Bit n, if set, >> Vector indicates that Function n+1 is supported. > > Sorry, I still don't understand it... what's "Function n+1" in this > context? > Does this mean that if bit n is set then the function defined as > 1 << (n+1) is supported? > >> I don't know how wonky bioses in the wild are however. I can see what >> our internal teams do in these sort of cases. > > That would be helpful :) > Note that on this machine (Toshiba L855-10W) brightness control under > windows does not work with the stock catalyst driver, it works only with > the (oldish) driver supplied by Toshiba. > > Anyway, I'm attaching v2 of my patches, I've incorporated the > suggestions and some bits of code from joeyli, and now brightness > control is actually implemented. Regarding patches 3 and 4, it might be easier to just store a pointer to the relevant encoder when you verify ATIF rather than walking the encoder list every time. Alex > > Still missing is the issue of event 0x81 and the conflict with video.ko; > the handler should probably return NOTIFY_BAD to veto the keypress. > > Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 20:45 ` Alex Deucher @ 2012-07-31 9:16 ` Luca Tettamanti 2012-07-31 13:58 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-31 9:16 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel On Mon, Jul 30, 2012 at 10:45 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > Regarding patches 3 and 4, it might be easier to just store a pointer > to the relevant encoder when you verify ATIF rather than walking the > encoder list every time. Makes sense, I was unsure about the lifetime of the encoders, but AFAICS they're destroyed only when the module in unloaded. Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-31 9:16 ` Luca Tettamanti @ 2012-07-31 13:58 ` Alex Deucher 2012-07-31 20:05 ` Luca Tettamanti 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-31 13:58 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Tue, Jul 31, 2012 at 5:16 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Mon, Jul 30, 2012 at 10:45 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> Regarding patches 3 and 4, it might be easier to just store a pointer >> to the relevant encoder when you verify ATIF rather than walking the >> encoder list every time. > > Makes sense, I was unsure about the lifetime of the encoders, but > AFAICS they're destroyed only when the module in unloaded. They are present for the life of the driver. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-31 13:58 ` Alex Deucher @ 2012-07-31 20:05 ` Luca Tettamanti 2012-07-31 21:33 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-31 20:05 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel [-- Attachment #1: Type: text/plain, Size: 952 bytes --] On Tue, Jul 31, 2012 at 09:58:04AM -0400, Alex Deucher wrote: > On Tue, Jul 31, 2012 at 5:16 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > On Mon, Jul 30, 2012 at 10:45 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > >> Regarding patches 3 and 4, it might be easier to just store a pointer > >> to the relevant encoder when you verify ATIF rather than walking the > >> encoder list every time. Done. > > Makes sense, I was unsure about the lifetime of the encoders, but > > AFAICS they're destroyed only when the module in unloaded. > > They are present for the life of the driver. I had to move to call to radeon_acpi_init after radeon_modeset_init, otherwise the encoders are not present yet (the tear down code path is correct, acpi first, then modeset). Latest and greatest version is attached; I fixed notifications when using custom command codes (tested by Pali Rohár) and implemented your suggestion. Luca [-- Attachment #2: 0001-drm-radeon-refactor-radeon_atif_call.patch --] [-- Type: text/plain, Size: 2875 bytes --] >From f0f8699eabee0d47b93fba14f8126b821cc106a5 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. v2: add support for the 2nd parameter (from Lee, Chun-Yi <jlee@suse.com>). Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 38 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 5b32e49..158e514 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -14,23 +14,30 @@ #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, + struct acpi_buffer *params) { acpi_status status; union acpi_object atif_arg_elements[2]; struct acpi_object_list atif_arg; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; atif_arg.count = 2; 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[1].type = ACPI_TYPE_INTEGER; - atif_arg_elements[1].integer.value = 0; + atif_arg_elements[0].integer.value = function; + + if (params) { + atif_arg_elements[1].type = ACPI_TYPE_BUFFER; + atif_arg_elements[1].buffer.length = params->length; + atif_arg_elements[1].buffer.pointer = params->pointer; + } else { + /* We need a second fake parameter */ + atif_arg_elements[1].type = ACPI_TYPE_INTEGER; + atif_arg_elements[1].integer.value = 0; + } status = acpi_evaluate_object(handle, "ATIF", &atif_arg, &buffer); @@ -39,18 +46,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 +67,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, NULL); + 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/plain, Size: 5884 bytes --] >From 427002ddf653b0abd0fb820b09322bf2a0b281af Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:11:58 +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. v2: use a packed struct for handling the output of ACPI calls, hides ugly pointer arithmetics (Lee, Chun-Yi <jlee@suse.com>). Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon.h | 40 +++++++++++++++++ drivers/gpu/drm/radeon/radeon_acpi.c | 79 +++++++++++++++++++++++++++++++--- 2 files changed, 113 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 158e514..53c519d 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -13,6 +13,13 @@ #include <linux/vga_switcheroo.h> +struct atif_verify_interface { + u16 size; /* structure size in bytes (includes size field) */ + u16 version; /* version */ + u32 notification_mask; /* supported notifications mask */ + u32 function_bits; /* supported functions bit vector */ +} __packed; + /* Call the ATIF method */ static union acpi_object *radeon_atif_call(acpi_handle handle, int function, @@ -52,12 +59,73 @@ 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; + struct atif_verify_interface output; + size_t size; + int err = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL); + if (!info) + return -EIO; + + memset(&output, 0, sizeof(output)); + + size = *(u16 *) info->buffer.pointer; + if (size < 12) { + DRM_INFO("ATIF buffer is too small: %lu\n", size); + err = -EINVAL; + goto out; + } + size = min(sizeof(output), size); + + memcpy(&output, info->buffer.pointer, size); + + /* TODO: check version? */ + DRM_DEBUG_DRIVER("ATIF version %u\n", output.version); + + radeon_atif_parse_notification(&atif->notifications, output.notification_mask); + radeon_atif_parse_functions(&atif->functions, output.function_bits); + +out: + 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); @@ -67,11 +135,10 @@ int radeon_acpi_init(struct radeon_device *rdev) return 0; /* Call the ATIF method */ - info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL); - 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/plain, Size: 3383 bytes --] >From e83d04d1b60c5cf46611f6afc9680024e1dad73b Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:16:06 +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. v2: packed struct (Lee, Chun-Yi <jlee@suse.com>) Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 84 +++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 53c519d..3d025dd 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -20,6 +20,18 @@ struct atif_verify_interface { u32 function_bits; /* supported functions bit vector */ } __packed; +struct atif_system_params { + u16 size; + u32 valid_mask; + u32 flags; + u8 command_code; +} __packed; + +#define ATIF_NOTIFY_MASK 0x3 +#define ATIF_NOTIFY_NONE 0 +#define ATIF_NOTIFY_81 1 +#define ATIF_NOTIFY_N 2 + /* Call the ATIF method */ static union acpi_object *radeon_atif_call(acpi_handle handle, int function, @@ -121,10 +133,56 @@ out: return err; } +static int radeon_atif_get_notification_params(acpi_handle handle, + struct radeon_atif_notification_cfg *n) +{ + union acpi_object *info; + struct atif_system_params params; + size_t size; + int err = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_PARAMETERS, NULL); + if (!info) { + err = -EIO; + goto out; + } + + size = *(u16 *) info->buffer.pointer; + if (size < 10) { + err = -EINVAL; + goto out; + } + + memset(¶ms, 0, sizeof(params)); + size = min(sizeof(params), size); + memcpy(¶ms, info->buffer.pointer, size); + + params.flags = params.flags & params.valid_mask; + + if ((params.flags & ATIF_NOTIFY_MASK) == ATIF_NOTIFY_NONE) { + n->enabled = false; + n->command_code = 0; + } else if ((params.flags & ATIF_NOTIFY_MASK) == ATIF_NOTIFY_81) { + n->enabled = true; + n->command_code = 0x81; + } else { + if (size < 11) { + err = -EINVAL; + goto out; + } + n->command_code = params.command_code; + } + +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 */ @@ -135,10 +193,32 @@ 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-handler-for-ACPI-event.patch --] [-- Type: text/plain, Size: 10417 bytes --] >From 88a3e35a0495cfff7d1a8ea23b779c621b3e1faa Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Mon, 30 Jul 2012 21:20:35 +0200 Subject: [PATCH 4/4] drm/radeon: implement handler for ACPI event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set up an handler for ACPI events and respond to brightness change requests from the system BIOS. v2: fix notification when using device-specific command codes (tested by Pali Rohár <pali.rohar@gmail.com>); cache the encoder controlling the backlight during the initialization to avoid searching it every time (suggested by Alex Deucher). Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/atombios_encoders.c | 2 +- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_acpi.c | 132 +++++++++++++++++++++++++++- drivers/gpu/drm/radeon/radeon_acpi.h | 6 ++ drivers/gpu/drm/radeon/radeon_kms.c | 16 ++-- drivers/gpu/drm/radeon/radeon_mode.h | 2 + drivers/gpu/drm/radeon/radeon_pm.c | 4 +- 7 files changed, 152 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c index c2d3552..e4152d6 100644 --- a/drivers/gpu/drm/radeon/atombios_encoders.c +++ b/drivers/gpu/drm/radeon/atombios_encoders.c @@ -72,7 +72,7 @@ radeon_atom_set_backlight_level_to_reg(struct radeon_device *rdev, WREG32(RADEON_BIOS_2_SCRATCH, bios_2_scratch); } -static void +void atombios_set_panel_brightness(struct radeon_encoder *radeon_encoder) { struct drm_encoder *encoder = &radeon_encoder->base; diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 0db98eb..a57795b 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1493,6 +1493,7 @@ struct radeon_atif { struct radeon_atif_notifications notifications; struct radeon_atif_functions functions; struct radeon_atif_notification_cfg notification_cfg; + struct radeon_encoder *backlight_ctl; }; /* diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 3d025dd..9a97914 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -3,6 +3,7 @@ #include <linux/slab.h> #include <acpi/acpi_drivers.h> #include <acpi/acpi_bus.h> +#include <acpi/video.h> #include "drmP.h" #include "drm.h" @@ -10,6 +11,7 @@ #include "drm_crtc_helper.h" #include "radeon.h" #include "radeon_acpi.h" +#include "atom.h" #include <linux/vga_switcheroo.h> @@ -21,10 +23,22 @@ struct atif_verify_interface { } __packed; struct atif_system_params { - u16 size; - u32 valid_mask; - u32 flags; - u8 command_code; + u16 size; /* structure size in bytes (includes size field) */ + u32 valid_mask; /* valid flags mask */ + u32 flags; /* flags */ + u8 command_code; /* notify command code */ +} __packed; + +struct atif_sbios_requests { + u16 size; /* structure size in bytes (includes size field) */ + u32 pending; /* pending sbios requests */ + u8 panel_exp_mode; /* panel expansion mode */ + u8 thermal_gfx; /* thermal state: target gfx controller */ + u8 thermal_state; /* thermal state: state id (0: exit state, non-0: state) */ + u8 forced_power_gfx; /* forced power state: target gfx controller */ + u8 forced_power_state; /* forced power state: state id */ + u8 system_power_src; /* system power source */ + u8 backlight_level; /* panel backlight level (0-255) */ } __packed; #define ATIF_NOTIFY_MASK 0x3 @@ -157,6 +171,8 @@ static int radeon_atif_get_notification_params(acpi_handle handle, size = min(sizeof(params), size); memcpy(¶ms, info->buffer.pointer, size); + DRM_DEBUG_DRIVER("SYSTEM_PARAMS: mask = %#x, flags = %#x\n", + params.flags, params.valid_mask); params.flags = params.flags & params.valid_mask; if ((params.flags & ATIF_NOTIFY_MASK) == ATIF_NOTIFY_NONE) { @@ -174,10 +190,91 @@ static int radeon_atif_get_notification_params(acpi_handle handle, } out: + DRM_DEBUG_DRIVER("Notification %s, command code = %#x\n", + (n->enabled ? "enabled" : "disabled"), + n->command_code); kfree(info); return err; } +static int radeon_atif_get_sbios_requests(acpi_handle handle, + struct atif_sbios_requests *req) +{ + union acpi_object *info; + size_t size; + int count = 0; + + info = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS, NULL); + if (!info) + return -EIO; + + size = *(u16 *)info->buffer.pointer; + if (size < 0xd) { + count = -EINVAL; + goto out; + } + memset(req, 0, sizeof(*req)); + + size = min(sizeof(*req), size); + memcpy(req, info->buffer.pointer, size); + DRM_DEBUG_DRIVER("SBIOS pending requests: %#x\n", req->pending); + + count = hweight32(req->pending); + +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 atif_sbios_requests req; + acpi_handle handle; + int count; + + DRM_DEBUG_DRIVER("event, device_class = %s, type = %#x\n", + event->device_class, event->type); + + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + + 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.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) { + struct radeon_encoder *enc = atif->backlight_ctl; + + if (enc) { + struct radeon_encoder_atom_dig *dig = enc->enc_priv; + dig->backlight_level = req.backlight_level; + + DRM_DEBUG_DRIVER("Changing brightness to %d\n", + req.backlight_level); + + atombios_set_panel_brightness(enc); + + backlight_force_update(dig->bl_dev, + BACKLIGHT_UPDATE_HOTKEY); + } + } + /* TODO: check other events */ + + return NOTIFY_OK; +} + /* Call all ACPI methods here */ int radeon_acpi_init(struct radeon_device *rdev) { @@ -199,6 +296,33 @@ int radeon_acpi_init(struct radeon_device *rdev) goto out; } + if (atif->notifications.brightness_change) { + struct drm_encoder *tmp; + struct radeon_encoder *target = NULL; + + /* Find the encoder controlling the brightness */ + list_for_each_entry(tmp, &rdev->ddev->mode_config.encoder_list, + head) { + struct radeon_encoder *enc = to_radeon_encoder(tmp); + struct radeon_encoder_atom_dig *dig = enc->enc_priv; + + if ((enc->devices & (ATOM_DEVICE_LCD_SUPPORT)) && + dig->bl_dev != NULL) { + target = enc; + break; + } + } + + atif->backlight_ctl = target; + if (!target) { + /* Brightness change notification is enabled, but we + * didn't find a backlight controller, this should + * never happen. + */ + DRM_ERROR("Cannot find a backlight controller\n"); + } + } + 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 diff --git a/drivers/gpu/drm/radeon/radeon_acpi.h b/drivers/gpu/drm/radeon/radeon_acpi.h index a42288d..df1f162 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.h +++ b/drivers/gpu/drm/radeon/radeon_acpi.h @@ -24,6 +24,12 @@ #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_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 5c58d7d..a13b2eb 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -78,11 +78,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags) goto out; } - /* Call ACPI methods */ - acpi_status = radeon_acpi_init(rdev); - if (acpi_status) - dev_dbg(&dev->pdev->dev, "Error during ACPI methods call\n"); - /* Again modeset_init should fail only on fatal error * otherwise it should provide enough functionalities * for shadowfb to run @@ -90,6 +85,17 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags) r = radeon_modeset_init(rdev); if (r) dev_err(&dev->pdev->dev, "Fatal error during modeset init\n"); + + /* Call ACPI methods: require modeset init + * but failure is not fatal + */ + if (!r) { + acpi_status = radeon_acpi_init(rdev); + if (acpi_status) + dev_dbg(&dev->pdev->dev, + "Error during ACPI methods call\n"); + } + out: if (r) radeon_driver_unload_kms(dev); diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index 129ed8e..ce91d62 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -697,6 +697,8 @@ void radeon_panel_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode); void atom_rv515_force_tv_scaler(struct radeon_device *rdev, struct radeon_crtc *radeon_crtc); +void atombios_set_panel_brightness(struct radeon_encoder *radeon_encoder); + /* legacy tv */ void radeon_legacy_tv_adjust_crtc_reg(struct drm_encoder *encoder, uint32_t *h_total_disp, uint32_t *h_sync_strt_wid, 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 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-31 20:05 ` Luca Tettamanti @ 2012-07-31 21:33 ` Alex Deucher 2012-08-01 8:57 ` Luca Tettamanti 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti 0 siblings, 2 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-31 21:33 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Tue, Jul 31, 2012 at 4:05 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Tue, Jul 31, 2012 at 09:58:04AM -0400, Alex Deucher wrote: >> On Tue, Jul 31, 2012 at 5:16 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> > On Mon, Jul 30, 2012 at 10:45 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> >> Regarding patches 3 and 4, it might be easier to just store a pointer >> >> to the relevant encoder when you verify ATIF rather than walking the >> >> encoder list every time. > > Done. > >> > Makes sense, I was unsure about the lifetime of the encoders, but >> > AFAICS they're destroyed only when the module in unloaded. >> >> They are present for the life of the driver. > > I had to move to call to radeon_acpi_init after radeon_modeset_init, > otherwise the encoders are not present yet (the tear down code path is > correct, acpi first, then modeset). > Latest and greatest version is attached; I fixed notifications when > using custom command codes (tested by Pali Rohár) and implemented your > suggestion. Patches look good. I picked them up and combined them with may patches plus a few other small fixes. They are available here: http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches Let me know what you think. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-31 21:33 ` Alex Deucher @ 2012-08-01 8:57 ` Luca Tettamanti 2012-08-01 13:56 ` Alex Deucher 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti 1 sibling, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-08-01 8:57 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel [-- Attachment #1: Type: text/plain, Size: 367 bytes --] On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote: > Patches look good. I picked them up and combined them with may > patches plus a few other small fixes. They are available here: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches > Let me know what you think. Looks ok, I lost one fix along the road though, I'm attaching the patch. Luca [-- Attachment #2: 0001-drm-radeon-fix-enable-notifications-with-device-spec.patch --] [-- Type: text/plain, Size: 795 bytes --] >From 0f71d5b56b9e5eee3194b5b926767511281ea0a6 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Wed, 1 Aug 2012 10:53:19 +0200 Subject: [PATCH] drm/radeon: fix, enable notifications with device specific command code Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 14ae8aa..a812b9a 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -214,6 +214,7 @@ static int radeon_atif_get_notification_params(acpi_handle handle, err = -EINVAL; goto out; } + n->enabled = true; n->command_code = params.command_code; } -- 1.7.10.4 [-- Attachment #3: Type: text/plain, Size: 159 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-08-01 8:57 ` Luca Tettamanti @ 2012-08-01 13:56 ` Alex Deucher 2012-08-02 15:03 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-08-01 13:56 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote: >> Patches look good. I picked them up and combined them with may >> patches plus a few other small fixes. They are available here: >> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches >> Let me know what you think. > > Looks ok, I lost one fix along the road though, I'm attaching the patch. Thanks, I rolled it into your original patch. New tree: http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches Alex > > Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-08-01 13:56 ` Alex Deucher @ 2012-08-02 15:03 ` Alex Deucher 2012-08-02 16:31 ` Luca Tettamanti 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-08-02 15:03 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel I admit I'm not really an ACPI expert, but thinking about this more, I'm wondering if maybe we should just send the appropriate brightness change, switch display, etc. event to userspace rather than handling it directly in the radeon driver, then let userspace callback down via the bl interface, etc. With backlight for example, does handling it in the kernel driver as per your patch prevent userspace from seeing the brightness up/down event? Wouldn't that break things like OSD brightness displays and such? Alex On Wed, Aug 1, 2012 at 9:56 AM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote: >>> Patches look good. I picked them up and combined them with may >>> patches plus a few other small fixes. They are available here: >>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches >>> Let me know what you think. >> >> Looks ok, I lost one fix along the road though, I'm attaching the patch. > > Thanks, I rolled it into your original patch. New tree: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches > > Alex > >> >> Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-08-02 15:03 ` Alex Deucher @ 2012-08-02 16:31 ` Luca Tettamanti 2012-08-02 16:33 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-08-02 16:31 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > I admit I'm not really an ACPI expert, but thinking about this more, > I'm wondering if maybe we should just send the appropriate brightness > change, switch display, etc. event to userspace rather than handling > it directly in the radeon driver, then let userspace callback down via > the bl interface, etc. With backlight for example, does handling it > in the kernel driver as per your patch prevent userspace from seeing > the brightness up/down event? Wouldn't that break things like OSD > brightness displays and such? No, the event is sent to userspace by the standard ACPI video driver, it works as before. Changing brightness usually goes like this: 1) user presses a hotkey 2) a notification is generated (0x86 or 0x87) 3) video.ko handles the notification and calls into ACPI to change the level (_BCM) and firmware does its magic 4) a key press (brightness up/down) is sent to userspace With ATIF step 3 does not actually change the brightness, it just send out another event (VIDEO_PROBE, or one of the device specific ones) so we need to take care of that too. The rest of the process, including the delivery of the key presses, stays the same. Luca ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-08-02 16:31 ` Luca Tettamanti @ 2012-08-02 16:33 ` Alex Deucher 2012-08-02 20:54 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-08-02 16:33 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> I admit I'm not really an ACPI expert, but thinking about this more, >> I'm wondering if maybe we should just send the appropriate brightness >> change, switch display, etc. event to userspace rather than handling >> it directly in the radeon driver, then let userspace callback down via >> the bl interface, etc. With backlight for example, does handling it >> in the kernel driver as per your patch prevent userspace from seeing >> the brightness up/down event? Wouldn't that break things like OSD >> brightness displays and such? > > No, the event is sent to userspace by the standard ACPI video driver, > it works as before. > Changing brightness usually goes like this: > 1) user presses a hotkey > 2) a notification is generated (0x86 or 0x87) > 3) video.ko handles the notification and calls into ACPI to change the > level (_BCM) and firmware does its magic > 4) a key press (brightness up/down) is sent to userspace > > With ATIF step 3 does not actually change the brightness, it just send > out another event (VIDEO_PROBE, or one of the device specific ones) so > we need to take care of that too. The rest of the process, including > the delivery of the key presses, stays the same. Excellent! thanks for clarifying. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-08-02 16:33 ` Alex Deucher @ 2012-08-02 20:54 ` Alex Deucher 0 siblings, 0 replies; 47+ messages in thread From: Alex Deucher @ 2012-08-02 20:54 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Thu, Aug 2, 2012 at 12:33 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: >> On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >>> I admit I'm not really an ACPI expert, but thinking about this more, >>> I'm wondering if maybe we should just send the appropriate brightness >>> change, switch display, etc. event to userspace rather than handling >>> it directly in the radeon driver, then let userspace callback down via >>> the bl interface, etc. With backlight for example, does handling it >>> in the kernel driver as per your patch prevent userspace from seeing >>> the brightness up/down event? Wouldn't that break things like OSD >>> brightness displays and such? >> >> No, the event is sent to userspace by the standard ACPI video driver, >> it works as before. >> Changing brightness usually goes like this: >> 1) user presses a hotkey >> 2) a notification is generated (0x86 or 0x87) >> 3) video.ko handles the notification and calls into ACPI to change the >> level (_BCM) and firmware does its magic >> 4) a key press (brightness up/down) is sent to userspace >> >> With ATIF step 3 does not actually change the brightness, it just send >> out another event (VIDEO_PROBE, or one of the device specific ones) so >> we need to take care of that too. The rest of the process, including >> the delivery of the key presses, stays the same. Updated tree with fixes to a few existing patches and improved support for ATPX and initial support for ATCS: http://cgit.freedesktop.org/~agd5f/linux/?h=acpi_patches Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-07-31 21:33 ` Alex Deucher 2012-08-01 8:57 ` Luca Tettamanti @ 2012-08-01 13:49 ` Luca Tettamanti 2012-08-01 14:02 ` Alex Deucher ` (2 more replies) 1 sibling, 3 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-08-01 13:49 UTC (permalink / raw) To: Alex Deucher Cc: airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Zhang Rui, Len Brown AMD ACPI interface may overload the standard event ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the userspace because the user did not press the mode switch key (the spurious keypress confuses the DE which usually changes the display configuration and messes up a dual-screen setup). This patch gives the radeon driver the chance to examine the event and block the keypress if the event is an "AMD event". Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- Any comment from ACPI front? drivers/acpi/video.c | 10 ++++++++-- drivers/gpu/drm/radeon/radeon_acpi.c | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..a8592c4 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) acpi_video_device_enumerate(video); acpi_video_device_rebind(video); acpi_bus_generate_proc_event(device, event, 0); - keycode = KEY_SWITCHVIDEOMODE; + /* This event is also overloaded by AMD ACPI interface, don't + * send the key press if the event has been handled elsewhere + * (e.g. radeon DRM driver). + */ + if (!acpi_notifier_call_chain(device, event, 0)) + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) + if (event != ACPI_VIDEO_NOTIFY_SWITCH && + event != ACPI_VIDEO_NOTIFY_PROBE) acpi_notifier_call_chain(device, event, 0); if (keycode) { diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 96de08d..ee0d29e 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, } /* TODO: check other events */ - return NOTIFY_OK; + /* We've handled the event, stop the notifier chain. The ACPI interface + * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to + * userspace if the event was generated only to signal a SBIOS + * request. + */ + return NOTIFY_BAD; } /* Call all ACPI methods here */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti @ 2012-08-01 14:02 ` Alex Deucher 2012-08-01 14:50 ` joeyli 2012-08-02 0:45 ` Zhang Rui 2 siblings, 0 replies; 47+ messages in thread From: Alex Deucher @ 2012-08-01 14:02 UTC (permalink / raw) To: Luca Tettamanti Cc: airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Zhang Rui, Len Brown On Wed, Aug 1, 2012 at 9:49 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > AMD ACPI interface may overload the standard event > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > userspace because the user did not press the mode switch key (the > spurious keypress confuses the DE which usually changes the > display configuration and messes up a dual-screen setup). > This patch gives the radeon driver the chance to examine the event and > block the keypress if the event is an "AMD event". > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> Looks good to me, but I'm certainly not an ACPI expert. Acked-by: Alex Deucher <alexander.deucher@amd.com> ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti 2012-08-01 14:02 ` Alex Deucher @ 2012-08-01 14:50 ` joeyli 2012-08-02 0:45 ` Zhang Rui 2 siblings, 0 replies; 47+ messages in thread From: joeyli @ 2012-08-01 14:50 UTC (permalink / raw) To: Luca Tettamanti Cc: Alex Deucher, airlied, dri-devel, Alex Deucher, linux-acpi, Zhang Rui, Len Brown 於 三，2012-08-01 於 15:49 +0200，Luca Tettamanti 提到： > AMD ACPI interface may overload the standard event > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > userspace because the user did not press the mode switch key (the > spurious keypress confuses the DE which usually changes the > display configuration and messes up a dual-screen setup). > This patch gives the radeon driver the chance to examine the event and > block the keypress if the event is an "AMD event". > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> Tested-by: Lee, Chun-Yi <jlee@suse.com> This patch works to me on my HP notebook to avoid (VGA, 0x81) notify event issued when AC-power unpluged. Thanks Joey Lee > --- > Any comment from ACPI front? > > drivers/acpi/video.c | 10 ++++++++-- > drivers/gpu/drm/radeon/radeon_acpi.c | 7 ++++++- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index 1e0a9e1..a8592c4 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) > acpi_video_device_enumerate(video); > acpi_video_device_rebind(video); > acpi_bus_generate_proc_event(device, event, 0); > - keycode = KEY_SWITCHVIDEOMODE; > + /* This event is also overloaded by AMD ACPI interface, don't > + * send the key press if the event has been handled elsewhere > + * (e.g. radeon DRM driver). > + */ > + if (!acpi_notifier_call_chain(device, event, 0)) > + keycode = KEY_SWITCHVIDEOMODE; > break; > > case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ > @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) > break; > } > > - if (event != ACPI_VIDEO_NOTIFY_SWITCH) > + if (event != ACPI_VIDEO_NOTIFY_SWITCH && > + event != ACPI_VIDEO_NOTIFY_PROBE) > acpi_notifier_call_chain(device, event, 0); > > if (keycode) { > diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c > index 96de08d..ee0d29e 100644 > --- a/drivers/gpu/drm/radeon/radeon_acpi.c > +++ b/drivers/gpu/drm/radeon/radeon_acpi.c > @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, > } > /* TODO: check other events */ > > - return NOTIFY_OK; > + /* We've handled the event, stop the notifier chain. The ACPI interface > + * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to > + * userspace if the event was generated only to signal a SBIOS > + * request. > + */ > + return NOTIFY_BAD; > } > > /* Call all ACPI methods here */ -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti 2012-08-01 14:02 ` Alex Deucher 2012-08-01 14:50 ` joeyli @ 2012-08-02 0:45 ` Zhang Rui 2012-08-02 13:46 ` Luca Tettamanti 2 siblings, 1 reply; 47+ messages in thread From: Zhang Rui @ 2012-08-02 0:45 UTC (permalink / raw) To: Luca Tettamanti Cc: Alex Deucher, airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Len Brown On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > AMD ACPI interface may overload the standard event > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > userspace because the user did not press the mode switch key (the > spurious keypress confuses the DE which usually changes the > display configuration and messes up a dual-screen setup). > This patch gives the radeon driver the chance to examine the event and > block the keypress if the event is an "AMD event". > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > --- > Any comment from ACPI front? > it looks good to me. But I'm wondering if we can use the following code for ACPI part, which looks cleaner. I know this may change the behavior of other events, but in theory, we should not send any input event if we know something wrong in kernel. what do you think? diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..8ad1f53 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + keycode = 0; if (keycode) { input_report_key(input, keycode, 1); > drivers/acpi/video.c | 10 ++++++++-- > drivers/gpu/drm/radeon/radeon_acpi.c | 7 ++++++- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index 1e0a9e1..a8592c4 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) > acpi_video_device_enumerate(video); > acpi_video_device_rebind(video); > acpi_bus_generate_proc_event(device, event, 0); > - keycode = KEY_SWITCHVIDEOMODE; > + /* This event is also overloaded by AMD ACPI interface, don't > + * send the key press if the event has been handled elsewhere > + * (e.g. radeon DRM driver). > + */ > + if (!acpi_notifier_call_chain(device, event, 0)) > + keycode = KEY_SWITCHVIDEOMODE; > break; > > case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ > @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) > break; > } > > - if (event != ACPI_VIDEO_NOTIFY_SWITCH) > + if (event != ACPI_VIDEO_NOTIFY_SWITCH && > + event != ACPI_VIDEO_NOTIFY_PROBE) > acpi_notifier_call_chain(device, event, 0); > > if (keycode) { > diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c > index 96de08d..ee0d29e 100644 > --- a/drivers/gpu/drm/radeon/radeon_acpi.c > +++ b/drivers/gpu/drm/radeon/radeon_acpi.c > @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, > } > /* TODO: check other events */ > > - return NOTIFY_OK; > + /* We've handled the event, stop the notifier chain. The ACPI interface > + * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to > + * userspace if the event was generated only to signal a SBIOS > + * request. > + */ > + return NOTIFY_BAD; > } > > /* Call all ACPI methods here */ -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-02 0:45 ` Zhang Rui @ 2012-08-02 13:46 ` Luca Tettamanti 2012-08-03 1:40 ` Zhang Rui 0 siblings, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-08-02 13:46 UTC (permalink / raw) To: Zhang Rui Cc: Alex Deucher, airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Len Brown [-- Attachment #1: Type: text/plain, Size: 1253 bytes --] On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > > AMD ACPI interface may overload the standard event > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > > userspace because the user did not press the mode switch key (the > > spurious keypress confuses the DE which usually changes the > > display configuration and messes up a dual-screen setup). > > This patch gives the radeon driver the chance to examine the event and > > block the keypress if the event is an "AMD event". > > > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > > --- > > Any comment from ACPI front? > > > it looks good to me. > But I'm wondering if we can use the following code for ACPI part, which > looks cleaner. > I know this may change the behavior of other events, but in theory, we > should not send any input event if we know something wrong in kernel. > > what do you think? I like it, it's cleaner. I've split the patch in two pieces (one for video, the other for radeon) and adopted your suggestion. BTW, I'm leaving for vacation in a few hours, I'll be offline till mid August. Luca [-- Attachment #2: 0001-ACPI-video-allow-events-handlers-to-veto-the-keypres.patch --] [-- Type: text/plain, Size: 1959 bytes --] >From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Thu, 2 Aug 2012 15:30:27 +0200 Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress The standard video events may be overloaded for device specific purposes. For example AMD ACPI interface overloads ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the userspace because the user did not press the mode switch key (the spurious keypress confuses the DE which usually changes the display configuration and messes up a dual-screen setup). This patch gives the handlers the chance to examine the event and block the keypress if the event is device specific. v2: refactor as suggested by Zhang Rui <rui.zhang@intel.com> Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/acpi/video.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..d75642a 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + /* Something vetoed the keypress. */ + keycode = 0; if (keycode) { input_report_key(input, keycode, 1); -- 1.7.10.4 [-- Attachment #3: 0002-drm-radeon-block-the-keypress-on-ATIF-events.patch --] [-- Type: text/plain, Size: 1193 bytes --] >From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti <kronos.it@gmail.com> Date: Thu, 2 Aug 2012 15:33:03 +0200 Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS requests; block the keypress in this case since the user did not actually press the mode switch key. Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> --- drivers/gpu/drm/radeon/radeon_acpi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 96de08d..ee0d29e 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, } /* TODO: check other events */ - return NOTIFY_OK; + /* We've handled the event, stop the notifier chain. The ACPI interface + * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to + * userspace if the event was generated only to signal a SBIOS + * request. + */ + return NOTIFY_BAD; } /* Call all ACPI methods here */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-02 13:46 ` Luca Tettamanti @ 2012-08-03 1:40 ` Zhang Rui 2012-08-03 1:45 ` Alex Deucher 0 siblings, 1 reply; 47+ messages in thread From: Zhang Rui @ 2012-08-03 1:40 UTC (permalink / raw) To: Luca Tettamanti Cc: Alex Deucher, airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Len Brown On 四, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: > On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > > > AMD ACPI interface may overload the standard event > > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > > > userspace because the user did not press the mode switch key (the > > > spurious keypress confuses the DE which usually changes the > > > display configuration and messes up a dual-screen setup). > > > This patch gives the radeon driver the chance to examine the event and > > > block the keypress if the event is an "AMD event". > > > > > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > > > --- > > > Any comment from ACPI front? > > > > > it looks good to me. > > But I'm wondering if we can use the following code for ACPI part, which > > looks cleaner. > > I know this may change the behavior of other events, but in theory, we > > should not send any input event if we know something wrong in kernel. > > > > what do you think? > > I like it, it's cleaner. > I've split the patch in two pieces (one for video, the other for > radeon) and adopted your suggestion. > Great. Acked-by: Zhang Rui <rui.zhang@intel.com> hmm, who should take these two patches? and which tree the second patch is based on? thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-03 1:40 ` Zhang Rui @ 2012-08-03 1:45 ` Alex Deucher 2012-08-03 2:06 ` Zhang Rui 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-08-03 1:45 UTC (permalink / raw) To: Zhang Rui Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Len Brown On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui <rui.zhang@intel.com> wrote: > On 四, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: >> > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: >> > > AMD ACPI interface may overload the standard event >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the >> > > userspace because the user did not press the mode switch key (the >> > > spurious keypress confuses the DE which usually changes the >> > > display configuration and messes up a dual-screen setup). >> > > This patch gives the radeon driver the chance to examine the event and >> > > block the keypress if the event is an "AMD event". >> > > >> > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> >> > > --- >> > > Any comment from ACPI front? >> > > >> > it looks good to me. >> > But I'm wondering if we can use the following code for ACPI part, which >> > looks cleaner. >> > I know this may change the behavior of other events, but in theory, we >> > should not send any input event if we know something wrong in kernel. >> > >> > what do you think? >> >> I like it, it's cleaner. >> I've split the patch in two pieces (one for video, the other for >> radeon) and adopted your suggestion. >> > Great. > Acked-by: Zhang Rui <rui.zhang@intel.com> > > hmm, who should take these two patches? I'm happy to take the patches. > and which tree the second patch is based on? I've got a tree with all the radeon ACPI patches on the acpi_patches branches of my git tree: git://people.freedesktop.org/~agd5f/linux Alex > > thanks, > rui > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events 2012-08-03 1:45 ` Alex Deucher @ 2012-08-03 2:06 ` Zhang Rui 0 siblings, 0 replies; 47+ messages in thread From: Zhang Rui @ 2012-08-03 2:06 UTC (permalink / raw) To: Alex Deucher Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, joeyli, linux-acpi, Len Brown On 四, 2012-08-02 at 21:45 -0400, Alex Deucher wrote: > On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui <rui.zhang@intel.com> wrote: > > On 四, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: > >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > >> > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > >> > > AMD ACPI interface may overload the standard event > >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > >> > > userspace because the user did not press the mode switch key (the > >> > > spurious keypress confuses the DE which usually changes the > >> > > display configuration and messes up a dual-screen setup). > >> > > This patch gives the radeon driver the chance to examine the event and > >> > > block the keypress if the event is an "AMD event". > >> > > > >> > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > >> > > --- > >> > > Any comment from ACPI front? > >> > > > >> > it looks good to me. > >> > But I'm wondering if we can use the following code for ACPI part, which > >> > looks cleaner. > >> > I know this may change the behavior of other events, but in theory, we > >> > should not send any input event if we know something wrong in kernel. > >> > > >> > what do you think? > >> > >> I like it, it's cleaner. > >> I've split the patch in two pieces (one for video, the other for > >> radeon) and adopted your suggestion. > >> > > Great. > > Acked-by: Zhang Rui <rui.zhang@intel.com> > > > > hmm, who should take these two patches? > > I'm happy to take the patches. > > > and which tree the second patch is based on? > > I've got a tree with all the radeon ACPI patches on the acpi_patches > branches of my git tree: > git://people.freedesktop.org/~agd5f/linux > great. thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-28 21:29 ` Alex Deucher 2012-07-29 13:06 ` Luca Tettamanti @ 2012-07-29 19:33 ` Luca Tettamanti 2012-07-30 14:29 ` Alex Deucher 1 sibling, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-29 19:33 UTC (permalink / raw) To: Alex Deucher; +Cc: Alex Deucher, joeyli, dri-devel [-- 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 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-29 19:33 ` [PATCH] drm/radeon: add new AMD ACPI header and update relevant code Luca Tettamanti @ 2012-07-30 14:29 ` Alex Deucher 0 siblings, 0 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-30 14:29 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, joeyli, dri-devel On Sun, Jul 29, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > 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? Yeah, that should work. > Hopefully there is only one encoder with it, right? There's only one backlight controller and there should only be one encoder with it enabled on it. > > 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? That's fine with me. We do something similar with atombios structs. Also, feel free to add stuff to radeon_acpi.h if you think it's useful. Alex ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-28 14:56 ` Luca Tettamanti 2012-07-28 21:29 ` Alex Deucher @ 2012-07-29 3:51 ` joeyli 2012-07-29 13:10 ` Luca Tettamanti 1 sibling, 1 reply; 47+ messages in thread From: joeyli @ 2012-07-29 3:51 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, dri-devel Hi Luca, 於 六，2012-07-28 於 16:56 +0200，Luca Tettamanti 提到： > On Thu, Jul 26, 2012 at 03:42:26PM -0400, Alex Deucher wrote: > > On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > > On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote: > > >> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote: > > >> > The other missing bit is how to actually change the brightness... Alex, > > >> > do you know what registers to poke? > > >> > > >> You need to check if the GPU controls the backlight or the system > > >> does. I think the attached patches should point you in the right > > >> direction. > > > > > > Yep :) > > > > > > 0050: ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability : > > > 0050: (union) ATOM_FIRMWARE_CAPABILITY sbfAccess : > > > USHORT GPUControlsBL:1 = 0x0001 (1) > > > > > > The panel is using the INTERNAL_UNIPHY encoder, and I see the > > > UNIPHYTransmitterControl command table. > > > > > > Interaction with video.ko is still a bit messy... > > > > > > Do you already have code for handling the notifications? I'll work on it > > > in the weekend otherwise ;) > > > > I don't have patches for that. Please feel free to work on it :) > > I just found the first problem (probably a BIOS bug): > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > I intended to use the method to set up the notification handler but now > my BIOS says that it's not there even if it is... > Can I assume some default values (e.g. notifications are enabled and will > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > different)? > > thanks, > Luca > Did you check your DSDT for there have some "Notify (VGA, 0x81)" statement in AFN0..AFN15? If YES, I think that means your machine in case the 0x81 is for ATI used by default. On the other hand, I am also trying to write patch for avoid my AC-power problem. Like your idea, I think just add radeon-acpi to acpi notifier chain then acpi/video feed event to chain before issue KEY code like Matthew's code for ACPI_VIDEO_NOTIFY_SWITCH with intel_opregion on 0x80. The following code for reference: diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..fc138fd 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -91,6 +91,8 @@ static int acpi_video_bus_add(struct acpi_device *device); static int acpi_video_bus_remove(struct acpi_device *device, int type); static void acpi_video_bus_notify(struct acpi_device *device, u32 event); +static u16 video_notify_block_map; + static const struct acpi_device_id video_device_ids[] = { {ACPI_VIDEO_HID, 0}, {"", 0}, @@ -1457,7 +1459,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) acpi_video_device_enumerate(video); acpi_video_device_rebind(video); acpi_bus_generate_proc_event(device, event, 0); - keycode = KEY_SWITCHVIDEOMODE; + if (!acpi_notifier_call_chain(device, event, 0)) + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ @@ -1479,7 +1482,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) + if (event != ACPI_VIDEO_NOTIFY_SWITCH || + event != ACPI_VIDEO_NOTIFY_PROBE) acpi_notifier_call_chain(device, event, 0); if (keycode) { Thanks a lot! Joey Lee _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-29 3:51 ` joeyli @ 2012-07-29 13:10 ` Luca Tettamanti 2012-07-30 8:32 ` joeyli 0 siblings, 1 reply; 47+ messages in thread From: Luca Tettamanti @ 2012-07-29 13:10 UTC (permalink / raw) To: joeyli; +Cc: Alex Deucher, dri-devel On Sun, Jul 29, 2012 at 11:51:48AM +0800, joeyli wrote: > Hi Luca, > > 於 六，2012-07-28 於 16:56 +0200，Luca Tettamanti 提到： > > I just found the first problem (probably a BIOS bug): > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > > I intended to use the method to set up the notification handler but now > > my BIOS says that it's not there even if it is... > > Can I assume some default values (e.g. notifications are enabled and will > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > > different)? > > > > Did you check your DSDT for there have some "Notify (VGA, 0x81)" > statement in AFN0..AFN15? > If YES, I think that means your machine in case the 0x81 is for ATI used > by default. Yes, my point is that the nofication is there, but since GET_SYSTEM_PARAMETERS is not announced I have not way to check it. IOW, what is implemented in the DSDT does not match what is announced by VERIFY_INTERFACE. For reference this is the DSDT: http://pastebin.com/KKS7ZsTt Luca _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-29 13:10 ` Luca Tettamanti @ 2012-07-30 8:32 ` joeyli 2012-07-30 14:16 ` Luca Tettamanti 0 siblings, 1 reply; 47+ messages in thread From: joeyli @ 2012-07-30 8:32 UTC (permalink / raw) To: Luca Tettamanti; +Cc: Alex Deucher, dri-devel 於 日，2012-07-29 於 15:10 +0200，Luca Tettamanti 提到： > On Sun, Jul 29, 2012 at 11:51:48AM +0800, joeyli wrote: > > Hi Luca, > > > > 於 六，2012-07-28 於 16:56 +0200，Luca Tettamanti 提到： > > > I just found the first problem (probably a BIOS bug): > > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > > > I intended to use the method to set up the notification handler but now > > > my BIOS says that it's not there even if it is... > > > Can I assume some default values (e.g. notifications are enabled and will > > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > > > different)? > > > > > > > Did you check your DSDT for there have some "Notify (VGA, 0x81)" > > statement in AFN0..AFN15? > > If YES, I think that means your machine in case the 0x81 is for ATI used > > by default. > > Yes, my point is that the nofication is there, but since > GET_SYSTEM_PARAMETERS is not announced I have not way to check it. > IOW, what is implemented in the DSDT does not match what is announced by > VERIFY_INTERFACE. > For reference this is the DSDT: http://pastebin.com/KKS7ZsTt > > Luca > Yes, saw the problem in your DSDT: Method (AF00, 0, NotSerialized) { CreateWordField (ATIB, Zero, SSZE) ... Store (0x80, NMSK) Store (0x02, SFUN) <=== 10b, bit 0 is 0 Return (ATIB) } But, AF01 still supported in ATIF on this machine, maybe we should still try GET_SYSTEM_PARAMETERS even the function vector set to 0? No idea... On the other hand, My patch to avoid 0x81 event conflict with acpi/video driver like below. This patch woks on my notebook. Your patches do much more things then mine, so I think my patch just for reference. There have a problem is: If we want use acpi_notifier_call_chain to check does event consume by any notifier in acpi's blocking notifier chain, then we need return NOTIFY_BAD in radeon_acpi but not NOTIFY_OK. So, I suggest radeon_acpi should register to acpi notifier chain by itself but not append on radeon_acpi_event in radeon_pm. And, suggest also check the device class is ACPI_VIDEO_CLASS like following: +static int radeon_acpi_video_event(struct notifier_block *nb, ... + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + Thanks a lot! Joey Lee >From 9c0c42ab8f37dafd3b512ca395b64bf39819d26b Mon Sep 17 00:00:00 2001 From: Lee, Chun-Yi <jlee@suse.com> Date: Mon, 30 Jul 2012 16:17:05 +0800 Subject: [PATCH] drm/radeon avoid 0x81 acpi event conflict with acpi video driver drm/radeon avoid 0x81 acpi event conflict with acpi video driver Signed-off-by: Lee, Chun-Yi <jlee@suse.com> --- drivers/acpi/video.c | 6 +- drivers/gpu/drm/radeon/radeon_acpi.c | 150 ++++++++++++++++++++++++++++++---- 2 files changed, 139 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..e32492d 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1457,7 +1457,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) acpi_video_device_enumerate(video); acpi_video_device_rebind(video); acpi_bus_generate_proc_event(device, event, 0); - keycode = KEY_SWITCHVIDEOMODE; + if (!acpi_notifier_call_chain(device, event, 0)) + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ @@ -1479,7 +1480,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) + if (event != ACPI_VIDEO_NOTIFY_SWITCH || + event != ACPI_VIDEO_NOTIFY_PROBE) acpi_notifier_call_chain(device, event, 0); if (keycode) { diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 5b32e49..37ed5e1 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -3,6 +3,7 @@ #include <linux/slab.h> #include <acpi/acpi_drivers.h> #include <acpi/acpi_bus.h> +#include <acpi/video.h> #include "drmP.h" #include "drm.h" @@ -13,36 +14,150 @@ #include <linux/vga_switcheroo.h> +struct atif_verify_output { + u16 size; /* structure size in bytes (includes size field) */ + u16 version; /* version */ + u32 notif_mask; /* supported notifications mask */ + u32 func_bits; /* supported functions bit vector */ +} __attribute__((packed)); + +static struct atif_verify_output *verify_output; + +struct atif_get_sysparams_output { + u16 size; /* structure size in bytes (includes size field) */ + u32 valid_flags_mask; /* valid flags mask */ + u32 flags; /* flags */ + u8 notify_code; /* notify command code */ +} __attribute__((packed)); + +static u8 notify_code; + /* Call the ATIF method * * Note: currently we discard the output */ -static int radeon_atif_call(acpi_handle handle) +static int radeon_atif_call(acpi_handle handle, int function, struct acpi_buffer *params, struct acpi_buffer *output) { acpi_status status; union acpi_object atif_arg_elements[2]; struct acpi_object_list atif_arg; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL}; + union acpi_object *obj; - atif_arg.count = 2; + atif_arg.count = 1; 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[1].type = ACPI_TYPE_INTEGER; - atif_arg_elements[1].integer.value = 0; + atif_arg_elements[0].integer.value = function; - status = acpi_evaluate_object(handle, "ATIF", &atif_arg, &buffer); + if (params) { + atif_arg.count = 2; + atif_arg_elements[1].type = ACPI_TYPE_BUFFER; + atif_arg_elements[1].buffer.length = params->length; + atif_arg_elements[1].buffer.pointer = params->pointer; + } - /* Fail only if calling the method fails and ATIF is supported */ - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { - DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n", - acpi_format_exception(status)); - kfree(buffer.pointer); + status = acpi_evaluate_object(handle, "ATIF", &atif_arg, output); + if (ACPI_FAILURE(status)) return 1; + + obj = (union acpi_object *) output->pointer; + if (!obj) + return 1; + else if (obj->type != ACPI_TYPE_BUFFER) { + kfree(obj); + return 1; + } else if (obj->buffer.length != 256) { + DRM_DEBUG_DRIVER("Unknown ATIF output buffer length %d\n", obj->buffer.length); + kfree(obj); + return 1; + } + + return 0; +} + +static int radeon_atif_verify(acpi_handle handle) +{ + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + struct atif_verify_output *voutput; + union acpi_object *obj; + int ret; + + /* evaluate the ATIF verify function */ + ret = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL, &output); + if (ret) { + kfree(output.pointer); + return ret; + } + + obj = (union acpi_object *) output.pointer; + + voutput = (struct atif_verify_output *) obj->buffer.pointer; + verify_output = kzalloc(sizeof(struct atif_verify_output), GFP_KERNEL); /* TODO: kfree */ + verify_output->size = voutput->size; + verify_output->version = voutput->version; + verify_output->notif_mask = voutput->notif_mask; + verify_output->func_bits = voutput->func_bits; + + kfree(output.pointer); + return 0; +} + +static int radeon_acpi_video_event(struct notifier_block *nb, + unsigned long val, void *data) +{ + /* The only video events relevant to opregion are 0x81 or n. These indicate + either a docking event, lid switch or display switch request. In + Linux, these are handled by the dock, button and video drivers. + */ + + struct acpi_bus_event *event = data; + int ret = NOTIFY_BAD; /* Question: for fill acpi's blocking notifier chains */ + + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + + if (event->type != notify_code) + return NOTIFY_DONE; + + /* TODO: run anything should take care by radeon */ + + return ret; +} + +static struct notifier_block radeon_acpi_notifier = { + .notifier_call = radeon_acpi_video_event, +}; + +static int radeon_atif_get_system_param(acpi_handle handle) +{ + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + struct atif_get_sysparams_output *params_output; + union acpi_object *obj; + u32 flags; + int ret; + + /* evaluate the ATIF get system parameters function */ + ret = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_PARAMETERS, NULL, &output); + if (ret) { + kfree(output.pointer); + return ret; } - kfree(buffer.pointer); + obj = (union acpi_object *) output.pointer; + + params_output = (struct atif_get_sysparams_output *) obj->buffer.pointer; + flags = params_output->flags; + + if (flags) { + if (flags == 1) + notify_code = 0x81; + else if (flags == 2) + notify_code = params_output->notify_code; + + register_acpi_notifier(&radeon_acpi_notifier); + } + + kfree(output.pointer); return 0; } @@ -59,11 +174,16 @@ int radeon_acpi_init(struct radeon_device *rdev) if (!ASIC_IS_AVIVO(rdev) || !rdev->bios || !handle) return 0; - /* Call the ATIF method */ - ret = radeon_atif_call(handle); + /* Call the ATIF verify function */ + ret = radeon_atif_verify(handle); if (ret) return ret; + if (verify_output->func_bits & ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED) + ret = radeon_atif_get_system_param(handle); + if (ret) + return ret; + return 0; } -- 1.7.7 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-30 8:32 ` joeyli @ 2012-07-30 14:16 ` Luca Tettamanti 0 siblings, 0 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-30 14:16 UTC (permalink / raw) To: joeyli; +Cc: Alex Deucher, dri-devel On Mon, Jul 30, 2012 at 04:32:47PM +0800, joeyli wrote: > 於 日，2012-07-29 於 15:10 +0200，Luca Tettamanti 提到： > > On Sun, Jul 29, 2012 at 11:51:48AM +0800, joeyli wrote: > > > Hi Luca, > > > > > > 於 六，2012-07-28 於 16:56 +0200，Luca Tettamanti 提到： > > > > I just found the first problem (probably a BIOS bug): > > > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > > > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > > > > I intended to use the method to set up the notification handler but now > > > > my BIOS says that it's not there even if it is... > > > > Can I assume some default values (e.g. notifications are enabled and will > > > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > > > > different)? > > > > > > > > > > Did you check your DSDT for there have some "Notify (VGA, 0x81)" > > > statement in AFN0..AFN15? > > > If YES, I think that means your machine in case the 0x81 is for ATI used > > > by default. > > > > Yes, my point is that the nofication is there, but since > > GET_SYSTEM_PARAMETERS is not announced I have not way to check it. > > IOW, what is implemented in the DSDT does not match what is announced by > > VERIFY_INTERFACE. > > For reference this is the DSDT: http://pastebin.com/KKS7ZsTt > > > > Luca > > > > Yes, saw the problem in your DSDT: > > Method (AF00, 0, NotSerialized) > { > CreateWordField (ATIB, Zero, SSZE) > ... > Store (0x80, NMSK) > Store (0x02, SFUN) <=== 10b, bit 0 is 0 > Return (ATIB) > } > > But, AF01 still supported in ATIF on this machine, maybe we should still > try GET_SYSTEM_PARAMETERS even the function vector set to 0? > No idea... That's what I'm doing right now... if SBIOS_REQUESTS is supported I try and call GET_SYSTEM_PARAMETERS even if it's not announced. > On the other hand, > My patch to avoid 0x81 event conflict with acpi/video driver like below. > This patch woks on my notebook. Your patches do much more things then > mine, so I think my patch just for reference. I ignored the event handling for now... I'd like to hear something back from ACPI camp before committing to this solution. > There have a problem is: > If we want use acpi_notifier_call_chain to check does event consume by > any notifier in acpi's blocking notifier chain, then we need return > NOTIFY_BAD in radeon_acpi but not NOTIFY_OK. > > So, I suggest radeon_acpi should register to acpi notifier chain by > itself but not append on radeon_acpi_event in radeon_pm. It shouldn't matter, once I change radeon_atif_handler to return NOTIFY_BAD the call chain will be stopped anyway. > And, > suggest also check the device class is ACPI_VIDEO_CLASS like following: > > +static int radeon_acpi_video_event(struct notifier_block *nb, > ... > + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) > + return NOTIFY_DONE; > + Will do. I'll use the package structs in the next iteration. Luca _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 15:35 ` Alex Deucher 2012-07-26 19:33 ` Luca Tettamanti @ 2012-07-28 14:39 ` Pali Rohár 1 sibling, 0 replies; 47+ messages in thread From: Pali Rohár @ 2012-07-28 14:39 UTC (permalink / raw) To: linux-acpi Alex Deucher <alexdeucher <at> gmail.com> writes: > > On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti wrote: > > The other missing bit is how to actually change the brightness... Alex, > > do you know what registers to poke? > > You need to check if the GPU controls the backlight or the system > does. I think the attached patches should point you in the right > direction. > > Alex > Hi, thank you very much for patches. With these patches now I'm able to adjust brightness on my notebook HP EliteBook 8460p with Radeon 6470M card via file /sys/class/backlight/radeon_bl/brightness It has only one problem: When I enable Ambient Ligth Sensor via hp-wmi.ko driver (echo 1 > /sys/devices/platform/hp-wmi/als which auto adjust some brightness level based on sensor data on HW level) and I set radeon brightness to 0 or 255 (min or max value) screen is turned off. Values 1..254 are OK. What about to disable values 0 and 255 in radeon brightness driver? ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-26 12:58 ` Luca Tettamanti 2012-07-26 15:35 ` Alex Deucher @ 2012-07-27 2:50 ` joeyli 2012-07-27 3:31 ` Alex Deucher 1 sibling, 1 reply; 47+ messages in thread From: joeyli @ 2012-07-27 2:50 UTC (permalink / raw) To: Luca Tettamanti Cc: alexdeucher, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi 於 四，2012-07-26 於 14:58 +0200，Luca Tettamanti 提到： > - again ACPI video module gets the nodification (in this case > ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE > - KDE seems this and muck with the screen configuration :( > - meanwhile the brightness notification is propagated, the > hypothetical > radeon driver does its magic to adjust the screen. > > My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to > the > acpi notifier chain, and allow the handlers to veto the key press > (like > it's done for ACPI_VIDEO_NOTIFY_SWITCH). I welcome this approach! On some ATI machine's DSDT also issue ACPI_VIDEO_NOTIFY_PROBE when AC-power unplug, because BIOS want to nodify video driver do some power saving stuff. It causes KEY_SWITCHVIDEOMODE issued by acpi/video driver when AC-power unplug. At least acpi/video driver need to know this 0x81 event is filed by BIOS to radeon-acpi for notify but not do video mode switch. That means the radeon drm need take the video switch responsibility. Thanks a lot! Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 2:50 ` joeyli @ 2012-07-27 3:31 ` Alex Deucher 2012-07-27 4:46 ` joeyli 0 siblings, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-27 3:31 UTC (permalink / raw) To: joeyli; +Cc: dri-devel, linux-acpi, Alex Deucher, Zhang Rui On Thu, Jul 26, 2012 at 10:50 PM, joeyli <jlee@suse.com> wrote: > 於 四，2012-07-26 於 14:58 +0200，Luca Tettamanti 提到： >> - again ACPI video module gets the nodification (in this case >> ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE >> - KDE seems this and muck with the screen configuration :( >> - meanwhile the brightness notification is propagated, the >> hypothetical >> radeon driver does its magic to adjust the screen. >> >> My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to >> the >> acpi notifier chain, and allow the handlers to veto the key press >> (like >> it's done for ACPI_VIDEO_NOTIFY_SWITCH). > > I welcome this approach! > > On some ATI machine's DSDT also issue ACPI_VIDEO_NOTIFY_PROBE when > AC-power unplug, because BIOS want to nodify video driver do some power > saving stuff. > It causes KEY_SWITCHVIDEOMODE issued by acpi/video driver when AC-power > unplug. > > At least acpi/video driver need to know this 0x81 event is filed by BIOS > to radeon-acpi for notify but not do video mode switch. That means the > radeon drm need take the video switch responsibility. Probably we'd just want the radeon acpi handler to just forward the events to userspace so that the user can choose what to do with it (xrandr command, etc.), otherwise we'll need to define policy in the driver. Alex > > > Thanks a lot! > Joey Lee > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 3:31 ` Alex Deucher @ 2012-07-27 4:46 ` joeyli 2012-07-27 9:02 ` Luca Tettamanti 2012-07-27 13:21 ` Alex Deucher 0 siblings, 2 replies; 47+ messages in thread From: joeyli @ 2012-07-27 4:46 UTC (permalink / raw) To: Alex Deucher Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi 於 四，2012-07-26 於 23:31 -0400，Alex Deucher 提到： > On Thu, Jul 26, 2012 at 10:50 PM, joeyli <jlee@suse.com> wrote: > > 於 四，2012-07-26 於 14:58 +0200，Luca Tettamanti 提到： > >> - again ACPI video module gets the nodification (in this case > >> ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE > >> - KDE seems this and muck with the screen configuration :( > >> - meanwhile the brightness notification is propagated, the > >> hypothetical > >> radeon driver does its magic to adjust the screen. > >> > >> My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to > >> the > >> acpi notifier chain, and allow the handlers to veto the key press > >> (like > >> it's done for ACPI_VIDEO_NOTIFY_SWITCH). > > > > I welcome this approach! > > > > On some ATI machine's DSDT also issue ACPI_VIDEO_NOTIFY_PROBE when > > AC-power unplug, because BIOS want to nodify video driver do some power > > saving stuff. > > It causes KEY_SWITCHVIDEOMODE issued by acpi/video driver when AC-power > > unplug. > > > > At least acpi/video driver need to know this 0x81 event is filed by BIOS > > to radeon-acpi for notify but not do video mode switch. That means the > > radeon drm need take the video switch responsibility. > > Probably we'd just want the radeon acpi handler to just forward the > events to userspace so that the user can choose what to do with it > (xrandr command, etc.), otherwise we'll need to define policy in the > driver. > > Alex > Any kernel module can issue KEY_SWITCHVIDEOMODE to user space, then gnome-settings-daemon(on Gnome) and krandr(on KDE) will call xrandr library to switch video mode. The 0x81 ACPI event for acpi/video driver is ACPI_VIDEO_NOTIFY_PROBE, means need issue KEY_SWITCHVIDEOMODE. But, 0x81 for radeon is a general notification event. I didn't see probe state in GET_SYSTEM_BIOS_REQUESTS, how can we distinguish this 0x81 is a ACPI_VIDEO_NOTIFY_PROBE or a ATI general notification event? + * flags + * bits 1:0: + * 0 - Notify(VGA, 0x81) is not used for notification + * 1 - Notify(VGA, 0x81) is used for notification Per the above flags, when we detect bit set to 1, means 0x81 used for radeon-acpi to be a general notification event. My question is: what's the event number for ACPI_VIDEO_NOTIFY_PROBE on this AMD/ATI machine when 0x81 not available for acpi/video? Thanks a lot! Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 4:46 ` joeyli @ 2012-07-27 9:02 ` Luca Tettamanti 2012-07-27 13:21 ` Alex Deucher 1 sibling, 0 replies; 47+ messages in thread From: Luca Tettamanti @ 2012-07-27 9:02 UTC (permalink / raw) To: joeyli; +Cc: linux-acpi, dri-devel, Alex Deucher, Zhang Rui On Fri, Jul 27, 2012 at 12:46:50PM +0800, joeyli wrote: > 於 四，2012-07-26 於 23:31 -0400，Alex Deucher 提到： > > On Thu, Jul 26, 2012 at 10:50 PM, joeyli <jlee@suse.com> wrote: > > > 於 四，2012-07-26 於 14:58 +0200，Luca Tettamanti 提到： > > >> - again ACPI video module gets the nodification (in this case > > >> ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE > > >> - KDE seems this and muck with the screen configuration :( > > >> - meanwhile the brightness notification is propagated, the > > >> hypothetical > > >> radeon driver does its magic to adjust the screen. > > >> > > >> My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to > > >> the > > >> acpi notifier chain, and allow the handlers to veto the key press > > >> (like > > >> it's done for ACPI_VIDEO_NOTIFY_SWITCH). > > > > > > I welcome this approach! > > > > > > On some ATI machine's DSDT also issue ACPI_VIDEO_NOTIFY_PROBE when > > > AC-power unplug, because BIOS want to nodify video driver do some power > > > saving stuff. > > > It causes KEY_SWITCHVIDEOMODE issued by acpi/video driver when AC-power > > > unplug. > > > > > > At least acpi/video driver need to know this 0x81 event is filed by BIOS > > > to radeon-acpi for notify but not do video mode switch. That means the > > > radeon drm need take the video switch responsibility. > > > > Probably we'd just want the radeon acpi handler to just forward the > > events to userspace so that the user can choose what to do with it > > (xrandr command, etc.), otherwise we'll need to define policy in the > > driver. > > Any kernel module can issue KEY_SWITCHVIDEOMODE to user space, then > gnome-settings-daemon(on Gnome) and krandr(on KDE) will call xrandr > library to switch video mode. Exacly, and if we have pending system bios requests then the event is not a real ACPI_VIDEO_NOTIFY_PROBE and (IMHO) it should not be forwarded to the userspace as such. The "vanilla" ACPI_VIDEO_NOTIFY_PROBE is already forwared to userspace. > The 0x81 ACPI event for acpi/video driver is ACPI_VIDEO_NOTIFY_PROBE, > means need issue KEY_SWITCHVIDEOMODE. But, 0x81 for radeon is a general > notification event. > I didn't see probe state in GET_SYSTEM_BIOS_REQUESTS, how can we > distinguish this 0x81 is a ACPI_VIDEO_NOTIFY_PROBE or a ATI general > notification event? +#define ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS 0x2 +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_BIOS_REQUESTS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - pending sbios requests + * BYTE - panel expansion mode + * BYTE - thermal state: target gfx controller + * BYTE - thermal state: state id (0: exit state, non-0: state) + * BYTE - forced power state: target gfx controller + * BYTE - forced power state: state id + * BYTE - system power source + * BYTE - panel backlight level (0-255) I guess that if "pending sbios requests" == 0 then the event is the general purpose one, and is not handled by radeon driver. Luca _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 4:46 ` joeyli 2012-07-27 9:02 ` Luca Tettamanti @ 2012-07-27 13:21 ` Alex Deucher 2012-07-27 15:32 ` joeyli 1 sibling, 1 reply; 47+ messages in thread From: Alex Deucher @ 2012-07-27 13:21 UTC (permalink / raw) To: joeyli Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi On Fri, Jul 27, 2012 at 12:46 AM, joeyli <jlee@suse.com> wrote: > 於 四，2012-07-26 於 23:31 -0400，Alex Deucher 提到： >> On Thu, Jul 26, 2012 at 10:50 PM, joeyli <jlee@suse.com> wrote: >> > 於 四，2012-07-26 於 14:58 +0200，Luca Tettamanti 提到： >> >> - again ACPI video module gets the nodification (in this case >> >> ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE >> >> - KDE seems this and muck with the screen configuration :( >> >> - meanwhile the brightness notification is propagated, the >> >> hypothetical >> >> radeon driver does its magic to adjust the screen. >> >> >> >> My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to >> >> the >> >> acpi notifier chain, and allow the handlers to veto the key press >> >> (like >> >> it's done for ACPI_VIDEO_NOTIFY_SWITCH). >> > >> > I welcome this approach! >> > >> > On some ATI machine's DSDT also issue ACPI_VIDEO_NOTIFY_PROBE when >> > AC-power unplug, because BIOS want to nodify video driver do some power >> > saving stuff. >> > It causes KEY_SWITCHVIDEOMODE issued by acpi/video driver when AC-power >> > unplug. >> > >> > At least acpi/video driver need to know this 0x81 event is filed by BIOS >> > to radeon-acpi for notify but not do video mode switch. That means the >> > radeon drm need take the video switch responsibility. >> >> Probably we'd just want the radeon acpi handler to just forward the >> events to userspace so that the user can choose what to do with it >> (xrandr command, etc.), otherwise we'll need to define policy in the >> driver. >> >> Alex >> > > Any kernel module can issue KEY_SWITCHVIDEOMODE to user space, then > gnome-settings-daemon(on Gnome) and krandr(on KDE) will call xrandr > library to switch video mode. > > > The 0x81 ACPI event for acpi/video driver is ACPI_VIDEO_NOTIFY_PROBE, > means need issue KEY_SWITCHVIDEOMODE. But, 0x81 for radeon is a general > notification event. > I didn't see probe state in GET_SYSTEM_BIOS_REQUESTS, how can we > distinguish this 0x81 is a ACPI_VIDEO_NOTIFY_PROBE or a ATI general > notification event? > > + * flags > + * bits 1:0: > + * 0 - Notify(VGA, 0x81) is not used for notification > + * 1 - Notify(VGA, 0x81) is used for notification > > Per the above flags, when we detect bit set to 1, means 0x81 used for radeon-acpi > to be a general notification event. My question is: what's the event number for > ACPI_VIDEO_NOTIFY_PROBE on this AMD/ATI machine when 0x81 not available for acpi/video? > +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags + * + * OR + * + * WORD - structure size in bytes (includes size field) + * DWORD - valid flags mask + * DWORD - flags + * BYTE - notify command code + * + * flags + * bits 1:0: + * 0 - Notify(VGA, 0x81) is not used for notification + * 1 - Notify(VGA, 0x81) is used for notification + * 2 - Notify(VGA, n) is used for notification where + * n (0xd0-0xd9) is specified in notify command code. + * bit 2: + * 1 - lid changes not reported though int10 + */ if bits 1:0 == 0, there is no notify event for radeon. When bits 1:0 == 1, it uses 0x81; when bits 1:0 == 2 it uses the event number specified in the following byte (notify command code) which would be something in the 0xd0-0xd9 range. Alex -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 13:21 ` Alex Deucher @ 2012-07-27 15:32 ` joeyli 2012-07-27 15:36 ` joeyli 2012-07-27 16:31 ` Alex Deucher 0 siblings, 2 replies; 47+ messages in thread From: joeyli @ 2012-07-27 15:32 UTC (permalink / raw) To: Alex Deucher Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi 於 五，2012-07-27 於 09:21 -0400，Alex Deucher 提到： > On Fri, Jul 27, 2012 at 12:46 AM, joeyli <jlee@suse.com> wrote: > > > > + * flags > > + * bits 1:0: > > + * 0 - Notify(VGA, 0x81) is not used for notification > > + * 1 - Notify(VGA, 0x81) is used for notification > > > > Per the above flags, when we detect bit set to 1, means 0x81 used for radeon-acpi > > to be a general notification event. My question is: what's the event number for > > ACPI_VIDEO_NOTIFY_PROBE on this AMD/ATI machine when 0x81 not available for acpi/video? > > > > > +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS > + * ARG1: none > + * OUTPUT: > + * WORD - structure size in bytes (includes size field) > + * DWORD - valid flags mask > + * DWORD - flags > + * > + * OR > + * > + * WORD - structure size in bytes (includes size field) > + * DWORD - valid flags mask > + * DWORD - flags > + * BYTE - notify command code > + * > + * flags > + * bits 1:0: > + * 0 - Notify(VGA, 0x81) is not used for notification > + * 1 - Notify(VGA, 0x81) is used for notification > + * 2 - Notify(VGA, n) is used for notification where > + * n (0xd0-0xd9) is specified in notify command code. > + * bit 2: > + * 1 - lid changes not reported though int10 > + */ > > if bits 1:0 == 0, there is no notify event for radeon. When bits 1:0 > == 1, it uses 0x81; when bits 1:0 == 2 it uses the event number > specified in the following byte (notify command code) which would be > something in the 0xd0-0xd9 range. > > Alex Did you mean every time we received 0x81 event in kernel module, we need access GET_SYSTEM_PARAMETERS to get the flags for distinguish between ACPI_VIDEO_NOTIFY_PROBE? Or just need access ONE time when system boot? I have a machine the GET_SYSTEM_PARAMETERS looks like this: Method (AF01, 0, NotSerialized) /* ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 */ { CreateWordField (ATIB, Zero, SSZE) CreateDWordField (ATIB, 0x02, VMSK) CreateDWordField (ATIB, 0x06, FLGS) /* flags bits 1:0 */ Store (0x0A, SSZE) /* structure SIZE fixed */ Store (0x03, VMSK) /* valid flags mask fixed to 0x03 */ Store (One, FLGS) /* FLAGS always set to 1 */ Return (ATIB) } Looks like just need access ONE time when system boot because those return value of AF01 fixed in DSDT. On this machine doesn't support probe event, I didn't see any event issued when I plug D-Sub. Does that means those kind of ATI/AMD machines do NOT support probe notify? If YES, then we can just direct disable acpi/video driver by radeon-acpi when we detected FLAGS is 1. Thanks a lot! Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 15:32 ` joeyli @ 2012-07-27 15:36 ` joeyli 2012-07-27 16:31 ` Alex Deucher 1 sibling, 0 replies; 47+ messages in thread From: joeyli @ 2012-07-27 15:36 UTC (permalink / raw) To: Alex Deucher Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi 於 五，2012-07-27 於 23:32 +0800，joeyli 提到： > 於 五，2012-07-27 於 09:21 -0400，Alex Deucher 提到： > > On Fri, Jul 27, 2012 at 12:46 AM, joeyli <jlee@suse.com> wrote: > > > > > > + * flags > > > + * bits 1:0: > > > + * 0 - Notify(VGA, 0x81) is not used for notification > > > + * 1 - Notify(VGA, 0x81) is used for notification > > > > > > Per the above flags, when we detect bit set to 1, means 0x81 used for radeon-acpi > > > to be a general notification event. My question is: what's the event number for > > > ACPI_VIDEO_NOTIFY_PROBE on this AMD/ATI machine when 0x81 not available for acpi/video? > > > > > > > > > +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS > > + * ARG1: none > > + * OUTPUT: > > + * WORD - structure size in bytes (includes size field) > > + * DWORD - valid flags mask > > + * DWORD - flags > > + * > > + * OR > > + * > > + * WORD - structure size in bytes (includes size field) > > + * DWORD - valid flags mask > > + * DWORD - flags > > + * BYTE - notify command code > > + * > > + * flags > > + * bits 1:0: > > + * 0 - Notify(VGA, 0x81) is not used for notification > > + * 1 - Notify(VGA, 0x81) is used for notification > > + * 2 - Notify(VGA, n) is used for notification where > > + * n (0xd0-0xd9) is specified in notify command code. > > + * bit 2: > > + * 1 - lid changes not reported though int10 > > + */ > > > > if bits 1:0 == 0, there is no notify event for radeon. When bits 1:0 > > == 1, it uses 0x81; when bits 1:0 == 2 it uses the event number > > specified in the following byte (notify command code) which would be > > something in the 0xd0-0xd9 range. > > > > Alex > > Did you mean every time we received 0x81 event in kernel module, we need > access GET_SYSTEM_PARAMETERS to get the flags for distinguish between > ACPI_VIDEO_NOTIFY_PROBE? > > Or just need access ONE time when system boot? > > > I have a machine the GET_SYSTEM_PARAMETERS looks like this: > > Method (AF01, 0, NotSerialized) /* ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 */ > { > CreateWordField (ATIB, Zero, SSZE) > CreateDWordField (ATIB, 0x02, VMSK) > CreateDWordField (ATIB, 0x06, FLGS) /* flags bits 1:0 */ > Store (0x0A, SSZE) /* structure SIZE fixed */ > Store (0x03, VMSK) /* valid flags mask fixed to 0x03 */ > Store (One, FLGS) /* FLAGS always set to 1 */ > Return (ATIB) > } > > Looks like just need access ONE time when system boot because those > return value of AF01 fixed in DSDT. > > On this machine doesn't support probe event, I didn't see any event > issued when I plug D-Sub. Does that means those kind of ATI/AMD machines > do NOT support probe notify? > > If YES, then we can just direct disable acpi/video driver by radeon-acpi ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > when we detected FLAGS is 1. > I mean disable the ACPI_VIDEO_NOTIFY_PROBE related code in acpi/video driver. Thanks Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code 2012-07-27 15:32 ` joeyli 2012-07-27 15:36 ` joeyli @ 2012-07-27 16:31 ` Alex Deucher 1 sibling, 0 replies; 47+ messages in thread From: Alex Deucher @ 2012-07-27 16:31 UTC (permalink / raw) To: joeyli Cc: Luca Tettamanti, airlied, dri-devel, Alex Deucher, Zhang Rui, linux-acpi On Fri, Jul 27, 2012 at 11:32 AM, joeyli <jlee@suse.com> wrote: > 於 五，2012-07-27 於 09:21 -0400，Alex Deucher 提到： >> On Fri, Jul 27, 2012 at 12:46 AM, joeyli <jlee@suse.com> wrote: >> > >> > + * flags >> > + * bits 1:0: >> > + * 0 - Notify(VGA, 0x81) is not used for notification >> > + * 1 - Notify(VGA, 0x81) is used for notification >> > >> > Per the above flags, when we detect bit set to 1, means 0x81 used for radeon-acpi >> > to be a general notification event. My question is: what's the event number for >> > ACPI_VIDEO_NOTIFY_PROBE on this AMD/ATI machine when 0x81 not available for acpi/video? >> > >> >> >> +/* ARG0: ATIF_FUNCTION_GET_SYSTEM_PARAMETERS >> + * ARG1: none >> + * OUTPUT: >> + * WORD - structure size in bytes (includes size field) >> + * DWORD - valid flags mask >> + * DWORD - flags >> + * >> + * OR >> + * >> + * WORD - structure size in bytes (includes size field) >> + * DWORD - valid flags mask >> + * DWORD - flags >> + * BYTE - notify command code >> + * >> + * flags >> + * bits 1:0: >> + * 0 - Notify(VGA, 0x81) is not used for notification >> + * 1 - Notify(VGA, 0x81) is used for notification >> + * 2 - Notify(VGA, n) is used for notification where >> + * n (0xd0-0xd9) is specified in notify command code. >> + * bit 2: >> + * 1 - lid changes not reported though int10 >> + */ >> >> if bits 1:0 == 0, there is no notify event for radeon. When bits 1:0 >> == 1, it uses 0x81; when bits 1:0 == 2 it uses the event number >> specified in the following byte (notify command code) which would be >> something in the 0xd0-0xd9 range. >> >> Alex > > Did you mean every time we received 0x81 event in kernel module, we need > access GET_SYSTEM_PARAMETERS to get the flags for distinguish between > ACPI_VIDEO_NOTIFY_PROBE? > > Or just need access ONE time when system boot? Just once at start up to see what event, if any, the sbios will use to signal the driver. > > > I have a machine the GET_SYSTEM_PARAMETERS looks like this: > > Method (AF01, 0, NotSerialized) /* ATIF_FUNCTION_GET_SYSTEM_PARAMETERS 0x1 */ > { > CreateWordField (ATIB, Zero, SSZE) > CreateDWordField (ATIB, 0x02, VMSK) > CreateDWordField (ATIB, 0x06, FLGS) /* flags bits 1:0 */ > Store (0x0A, SSZE) /* structure SIZE fixed */ > Store (0x03, VMSK) /* valid flags mask fixed to 0x03 */ > Store (One, FLGS) /* FLAGS always set to 1 */ > Return (ATIB) > } > > Looks like just need access ONE time when system boot because those > return value of AF01 fixed in DSDT. > > On this machine doesn't support probe event, I didn't see any event > issued when I plug D-Sub. Does that means those kind of ATI/AMD machines > do NOT support probe notify? Analog connectors don't support hotplug at all. And hotplug on digital connectors is already handled by the driver. As far as I know ACPI doesn't provide any events for these. The driver gets interrupts directly from the hw. The only events the sbios uses the notify event for are the ones listed in the supported notifications mask from ATIF_FUNCTION_VERIFY_INTERFACE: +#define ATIF_FUNCTION_VERIFY_INTERFACE 0x0 +/* ARG0: ATIF_FUNCTION_VERIFY_INTERFACE + * ARG1: none + * OUTPUT: + * WORD - structure size in bytes (includes size field) + * WORD - version + * DWORD - supported notifications mask + * DWORD - supported functions bit vector + */ +/* Notifications mask */ +# define ATIF_DISPLAY_SWITCH_REQUEST_SUPPORTED (1 << 0) +# define ATIF_EXPANSION_MODE_CHANGE_REQUEST_SUPPORTED (1 << 1) +# define ATIF_THERMAL_STATE_CHANGE_REQUEST_SUPPORTED (1 << 2) +# define ATIF_FORCED_POWER_STATE_CHANGE_REQUEST_SUPPORTED (1 << 3) +# define ATIF_SYSTEM_POWER_SOURCE_CHANGE_REQUEST_SUPPORTED (1 << 4) +# define ATIF_DISPLAY_CONF_CHANGE_REQUEST_SUPPORTED (1 << 5) +# define ATIF_PX_GFX_SWITCH_REQUEST_SUPPORTED (1 << 6) +# define ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST_SUPPORTED (1 << 7) +# define ATIF_DGPU_DISPLAY_EVENT_SUPPORTED (1 << 8) They are things like the user presses the display switch hotkey or presses the brightness up hotkey. Alex -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 47+ messages in thread
end of thread, other threads:[~2012-08-03 2:04 UTC | newest] Thread overview: 47+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-25 17:38 [PATCH] drm/radeon: add new AMD ACPI header and update relevant code alexdeucher 2012-07-26 12:58 ` Luca Tettamanti 2012-07-26 15:35 ` Alex Deucher 2012-07-26 19:33 ` Luca Tettamanti 2012-07-26 19:42 ` Alex Deucher 2012-07-26 19:58 ` Alex Deucher 2012-07-28 14:56 ` Luca Tettamanti 2012-07-28 21:29 ` Alex Deucher 2012-07-29 13:06 ` Luca Tettamanti 2012-07-30 14:20 ` Alex Deucher 2012-07-30 20:24 ` Luca Tettamanti 2012-07-30 20:30 ` Alex Deucher 2012-07-30 20:36 ` Luca Tettamanti 2012-07-30 20:45 ` Alex Deucher 2012-07-31 9:16 ` Luca Tettamanti 2012-07-31 13:58 ` Alex Deucher 2012-07-31 20:05 ` Luca Tettamanti 2012-07-31 21:33 ` Alex Deucher 2012-08-01 8:57 ` Luca Tettamanti 2012-08-01 13:56 ` Alex Deucher 2012-08-02 15:03 ` Alex Deucher 2012-08-02 16:31 ` Luca Tettamanti 2012-08-02 16:33 ` Alex Deucher 2012-08-02 20:54 ` Alex Deucher 2012-08-01 13:49 ` [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events Luca Tettamanti 2012-08-01 14:02 ` Alex Deucher 2012-08-01 14:50 ` joeyli 2012-08-02 0:45 ` Zhang Rui 2012-08-02 13:46 ` Luca Tettamanti 2012-08-03 1:40 ` Zhang Rui 2012-08-03 1:45 ` Alex Deucher 2012-08-03 2:06 ` Zhang Rui 2012-07-29 19:33 ` [PATCH] drm/radeon: add new AMD ACPI header and update relevant code Luca Tettamanti 2012-07-30 14:29 ` Alex Deucher 2012-07-29 3:51 ` joeyli 2012-07-29 13:10 ` Luca Tettamanti 2012-07-30 8:32 ` joeyli 2012-07-30 14:16 ` Luca Tettamanti 2012-07-28 14:39 ` Pali Rohár 2012-07-27 2:50 ` joeyli 2012-07-27 3:31 ` Alex Deucher 2012-07-27 4:46 ` joeyli 2012-07-27 9:02 ` Luca Tettamanti 2012-07-27 13:21 ` Alex Deucher 2012-07-27 15:32 ` joeyli 2012-07-27 15:36 ` joeyli 2012-07-27 16:31 ` Alex Deucher
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.