* [PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
@ 2015-08-23 13:18 Lukas Wunner
2015-08-29 12:29 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Lukas Wunner
2015-09-17 16:34 ` [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Danilo Cesar Lemes de Paula
0 siblings, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-08-23 13:18 UTC (permalink / raw)
To: dri-devel; +Cc: Danilo Cesar Lemes de Paula, Jonathan Corbet
This adds an "Overview" DOC section plus two DOC sections for the modes
of use ("Manual switching and manual power control" and "Driver power
control").
Also included is kernel-doc for all public functions, structs and enums.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 285 +++++++++++++++++++++++++++++++++++++--
include/linux/vga_switcheroo.h | 85 +++++++++++-
2 files changed, 353 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 2106066..b19a72f 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -1,20 +1,31 @@
/*
+ * vga_switcheroo.c - Support for laptop with dual GPU using one set of outputs
+ *
* Copyright (c) 2010 Red Hat Inc.
* Author : Dave Airlie <airlied@redhat.com>
*
+ * Copyright (c) 2015 Lukas Wunner <lukas@wunner.de>
*
- * Licensed under GPLv2
+ * 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:
*
- * vga_switcheroo.c - Support for laptop with dual GPU using one set of outputs
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
*
- * Switcher interface - methods require for ATPX and DCM
- * - switchto - this throws the output MUX switch
- * - discrete_set_power - sets the power state for the discrete card
+ * 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 AUTHORS OR COPYRIGHT HOLDERS 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.
*
- * GPU driver interface
- * - set_gpu_state - this should do the equiv of s/r for the card
- * - this should *not* set the discrete power state
- * - switch_check - check if the device is in a position to switch now
*/
#define pr_fmt(fmt) "vga_switcheroo: " fmt
@@ -33,6 +44,61 @@
#include <linux/vgaarb.h>
+/**
+ * DOC: Overview
+ *
+ * vga_switcheroo is the Linux subsystem for laptop hybrid graphics.
+ * These come in two flavors:
+ *
+ * * muxed: Dual GPUs with a multiplexer chip to switch outputs between GPUs.
+ * * muxless: Dual GPUs but only one of them is connected to outputs.
+ * The other one is merely used to offload rendering, its results
+ * are copied over PCIe into the framebuffer. On Linux this is
+ * supported with DRI PRIME.
+ *
+ * Hybrid graphics started to appear in the late Naughties and were initially
+ * all muxed. Newer laptops moved to a muxless architecture for cost reasons.
+ * A notable exception is the MacBook Pro which continues to use a mux.
+ * Muxes come with varying capabilities: Some switch only the panel, others
+ * can also switch external displays. Some switch all display pins at once
+ * while others can switch just the DDC lines. (To allow EDID probing
+ * for the inactive GPU.) Also, muxes are often used to cut power to the
+ * discrete GPU while it is not used.
+ *
+ * DRM drivers register GPUs with vga_switcheroo, these are heretoforth called
+ * clients. The mux is called the handler. Muxless machines also register a
+ * handler to control the power state of the discrete GPU, its ->switchto
+ * callback is a no-op for obvious reasons. The discrete GPU is often equipped
+ * with an HDA controller for the HDMI/DP audio signal, this will also
+ * register as a client so that vga_switcheroo can take care of the correct
+ * suspend/resume order when changing the discrete GPU's power state. In total
+ * there can thus be up to three clients: Two vga clients (GPUs) and one audio
+ * client (on the discrete GPU). The code is mostly prepared to support
+ * machines with more than two GPUs should they become available.
+ * The GPU to which the outputs are currently switched is called the
+ * active client in vga_switcheroo parlance. The GPU not in use is the
+ * inactive client.
+ */
+
+/**
+ * struct vga_switcheroo_client - registered client
+ * @pdev: client pci device
+ * @fb_info: framebuffer to which console is remapped on switching
+ * @pwr_state: current power state
+ * @ops: client callbacks
+ * @id: client identifier, see enum vga_switcheroo_client_id.
+ * Determining the id requires the handler, so GPUs are initially
+ * assigned -1 and later given their true id in vga_switcheroo_enable()
+ * @active: whether the outputs are currently switched to this client
+ * @driver_power_control: whether power state is controlled by the driver's
+ * runtime pm. If true, writing ON and OFF to the vga_switcheroo debugfs
+ * interface is a no-op so as not to interfere with runtime pm
+ * @list: client list
+ *
+ * Registered client. A client can be either a GPU or an audio device on a GPU.
+ * For audio clients, the @fb_info, @active and @driver_power_control members
+ * are bogus.
+ */
struct vga_switcheroo_client {
struct pci_dev *pdev;
struct fb_info *fb_info;
@@ -44,10 +110,28 @@ struct vga_switcheroo_client {
struct list_head list;
};
+/*
+ * protects access to struct vgasr_priv
+ */
static DEFINE_MUTEX(vgasr_mutex);
+/**
+ * struct vgasr_priv - vga_switcheroo private data
+ * @active: whether vga_switcheroo is enabled.
+ * Prerequisite is the registration of two GPUs and a handler
+ * @delayed_switch_active: whether a delayed switch is pending
+ * @delayed_client_id: client to which a delayed switch is pending
+ * @debugfs_root: directory for vga_switcheroo debugfs interface
+ * @switch_file: file for vga_switcheroo debugfs interface
+ * @registered_clients: number of registered GPUs
+ * (counting only vga clients, not audio clients)
+ * @clients: list of registered clients
+ * @handler: registered handler
+ *
+ * vga_switcheroo private data. Currently only one vga_switcheroo instance
+ * per system is supported.
+ */
struct vgasr_priv {
-
bool active;
bool delayed_switch_active;
enum vga_switcheroo_client_id delayed_client_id;
@@ -103,6 +187,15 @@ static void vga_switcheroo_enable(void)
vgasr_priv.active = true;
}
+/**
+ * vga_switcheroo_register_handler() - register handler
+ * @handler: handler callbacks
+ *
+ * Register handler. Enable vga_switcheroo if two vga clients have already
+ * registered.
+ *
+ * Return: 0 on success, -EINVAL if a handler was already registered.
+ */
int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
{
mutex_lock(&vgasr_mutex);
@@ -121,6 +214,11 @@ int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
}
EXPORT_SYMBOL(vga_switcheroo_register_handler);
+/**
+ * vga_switcheroo_unregister_handler() - unregister handler
+ *
+ * Unregister handler. Disable vga_switcheroo.
+ */
void vga_switcheroo_unregister_handler(void)
{
mutex_lock(&vgasr_mutex);
@@ -164,6 +262,19 @@ static int register_client(struct pci_dev *pdev,
return 0;
}
+/**
+ * vga_switcheroo_register_client - register vga client
+ * @pdev: client pci device
+ * @ops: client callbacks
+ * @driver_power_control: whether power state is controlled by the driver's
+ * runtime pm
+ *
+ * Register vga client (GPU). Enable vga_switcheroo if another GPU and a
+ * handler have already registered. The power state of the client is assumed
+ * to be ON.
+ *
+ * Return: 0 on success, -ENOMEM on memory allocation error.
+ */
int vga_switcheroo_register_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
bool driver_power_control)
@@ -174,6 +285,18 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
}
EXPORT_SYMBOL(vga_switcheroo_register_client);
+/**
+ * vga_switcheroo_register_audio_client - register audio client
+ * @pdev: client pci device
+ * @ops: client callbacks
+ * @id: client identifier, see enum vga_switcheroo_client_id
+ * @active: whether the audio device is fully initialized
+ *
+ * Register audio client (audio device on a GPU). The power state of the
+ * client is assumed to be ON.
+ *
+ * Return: 0 on success, -ENOMEM on memory allocation error.
+ */
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
int id, bool active)
@@ -215,6 +338,15 @@ find_active_client(struct list_head *head)
return NULL;
}
+/**
+ * vga_switcheroo_get_client_state() - obtain power state of a given client
+ * @pdev: client pci device
+ *
+ * Obtain power state of a given client as seen from vga_switcheroo.
+ * The function is only called from hda_intel.c.
+ *
+ * Return: Power state.
+ */
int vga_switcheroo_get_client_state(struct pci_dev *pdev)
{
struct vga_switcheroo_client *client;
@@ -228,6 +360,12 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
}
EXPORT_SYMBOL(vga_switcheroo_get_client_state);
+/**
+ * vga_switcheroo_unregister_client() - unregister client
+ * @pdev: client pci device
+ *
+ * Unregister client. Disable vga_switcheroo if this is a vga client (GPU).
+ */
void vga_switcheroo_unregister_client(struct pci_dev *pdev)
{
struct vga_switcheroo_client *client;
@@ -249,6 +387,14 @@ void vga_switcheroo_unregister_client(struct pci_dev *pdev)
}
EXPORT_SYMBOL(vga_switcheroo_unregister_client);
+/**
+ * vga_switcheroo_client_fb_set() - set framebuffer of a given client
+ * @pdev: client pci device
+ * @info: framebuffer
+ *
+ * Set framebuffer of a given client. The console will be remapped to this
+ * on switching.
+ */
void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
struct fb_info *info)
{
@@ -262,6 +408,42 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
}
EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
+/**
+ * DOC: Manual switching and manual power control
+ *
+ * In this mode of use, the file /sys/kernel/debug/vgaswitcheroo/switch
+ * can be read to retrieve the current vga_switcheroo state and commands
+ * can be written to it to change the state. The file appears as soon as
+ * two GPU drivers and one handler have registered with vga_switcheroo.
+ * The following commands are understood:
+ *
+ * * OFF: Power off the device not in use.
+ * * ON: Power on the device not in use.
+ * * IGD: Switch to the integrated graphics device.
+ * Power on the integrated GPU if necessary, power off the discrete GPU.
+ * Prerequisite is that no user space processes (e.g. Xorg, alsactl)
+ * have opened device files of the GPUs or the audio client. If the
+ * switch fails, the user may invoke lsof(8) or fuser(1) on /dev/dri/
+ * and /dev/snd/controlC1 to identify processes blocking the switch.
+ * * DIS: Switch to the discrete graphics device.
+ * * DIGD: Delayed switch to the integrated graphics device.
+ * This will perform the switch once the last user space process has
+ * closed the device files of the GPUs and the audio client.
+ * * DDIS: Delayed switch to the discrete graphics device.
+ * * MIGD: Mux-only switch to the integrated graphics device.
+ * Does not remap console or change the power state of either gpu.
+ * If the integrated GPU is currently off, the screen will turn black.
+ * If it is on, the screen will show whatever happens to be in VRAM.
+ * Either way, the user has to blindly enter the command to switch back.
+ * * MDIS: Mux-only switch to the discrete graphics device.
+ *
+ * For GPUs whose power state is controlled by the driver's runtime pm,
+ * the ON and OFF commands are a no-op (see next section).
+ *
+ * For muxless machines, the IGD/DIS, DIGD/DDIS and MIGD/MDIS commands
+ * should not be used.
+ */
+
static int vga_switcheroo_show(struct seq_file *m, void *v)
{
struct vga_switcheroo_client *client;
@@ -559,6 +741,16 @@ fail:
return -1;
}
+/**
+ * vga_switcheroo_process_delayed_switch() - helper for delayed switching
+ *
+ * Process a delayed switch if one is pending. DRM drivers should call this
+ * from their ->lastclose callback.
+ *
+ * Return: 0 on success. -EINVAL if no delayed switch is pending, if the client
+ * has unregistered in the meantime or if there are other clients blocking the
+ * switch. If the actual switch fails, an error is reported and 0 is returned.
+ */
int vga_switcheroo_process_delayed_switch(void)
{
struct vga_switcheroo_client *client;
@@ -589,6 +781,39 @@ err:
}
EXPORT_SYMBOL(vga_switcheroo_process_delayed_switch);
+/**
+ * DOC: Driver power control
+ *
+ * In this mode of use, the discrete GPU automatically powers up and down at
+ * the discretion of the driver's runtime pm. On muxed machines, the user may
+ * still influence the muxer state by way of the debugfs interface, however
+ * the ON and OFF commands become a no-op for the discrete GPU.
+ *
+ * This mode is the default on Nvidia HybridPower/Optimus and ATI PowerXpress.
+ * Specifying nouveau.runpm=0, radeon.runpm=0 or amdgpu.runpm=0 on the kernel
+ * command line disables it.
+ *
+ * When the driver decides to power up or down, it notifies vga_switcheroo
+ * thereof so that it can (a) power the audio device on the GPU up or down,
+ * and (b) update its internal power state representation for the device.
+ * This is achieved by vga_switcheroo_set_dynamic_switch().
+ *
+ * After the GPU has been suspended, the handler needs to be called to cut
+ * power to the GPU. Likewise it needs to reinstate power before the GPU
+ * can resume. This is achieved by vga_switcheroo_init_domain_pm_ops(),
+ * which augments the GPU's suspend/resume functions by the requisite
+ * calls to the handler.
+ *
+ * When the audio device resumes, the GPU needs to be woken. This is achieved
+ * by vga_switcheroo_init_domain_pm_optimus_hdmi_audio(), which augments the
+ * audio device's resume function.
+ *
+ * On muxed machines, if the mux is initially switched to the discrete GPU,
+ * the user ends up with a black screen when the GPU powers down after boot.
+ * As a workaround, the mux is forced to the integrated GPU on runtime suspend,
+ * cf. https://bugs.freedesktop.org/show_bug.cgi?id=75917
+ */
+
static void vga_switcheroo_power_switch(struct pci_dev *pdev,
enum vga_switcheroo_state state)
{
@@ -607,8 +832,17 @@ static void vga_switcheroo_power_switch(struct pci_dev *pdev,
vgasr_priv.handler->power_state(client->id, state);
}
-/* force a PCI device to a certain state - mainly to turn off audio clients */
-
+/**
+ * vga_switcheroo_set_dynamic_switch() - helper for driver power control
+ * @pdev: client pci device
+ * @dynamic: new power state
+ *
+ * Helper for GPUs whose power state is controlled by the driver's runtime pm.
+ * When the driver decides to power up or down, it notifies vga_switcheroo
+ * thereof using this helper so that it can (a) power the audio device on
+ * the GPU up or down, and (b) update its internal power state representation
+ * for the device.
+ */
void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev,
enum vga_switcheroo_state dynamic)
{
@@ -654,8 +888,18 @@ static int vga_switcheroo_runtime_resume(struct device *dev)
return 0;
}
-/* this version is for the case where the power switch is separate
- to the device being powered down. */
+/**
+ * vga_switcheroo_init_domain_pm_ops() - helper for driver power control
+ * @dev: vga client device
+ * @domain: power domain
+ *
+ * Helper for GPUs whose power state is controlled by the driver's runtime pm.
+ * After the GPU has been suspended, the handler needs to be called to cut
+ * power to the GPU. Likewise it needs to reinstate power before the GPU
+ * can resume. To this end, this helper augments the suspend/resume functions
+ * by the requisite calls to the handler. It needs only be called on platforms
+ * where the power switch is separate to the device being powered down.
+ */
int vga_switcheroo_init_domain_pm_ops(struct device *dev,
struct dev_pm_domain *domain)
{
@@ -709,6 +953,19 @@ static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
return ret;
}
+/**
+ * vga_switcheroo_init_domain_pm_optimus_hdmi_audio() - helper for driver
+ * power control
+ * @dev: audio client device
+ * @domain: power domain
+ *
+ * Helper for GPUs whose power state is controlled by the driver's runtime pm.
+ * When the audio device resumes, the GPU needs to be woken. This helper
+ * augments the audio device's resume function to do that.
+ *
+ * Return: 0 on success, -EINVAL if no power management operations are
+ * defined for this device.
+ */
int
vga_switcheroo_init_domain_pm_optimus_hdmi_audio(struct device *dev,
struct dev_pm_domain *domain)
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index b483abd..fe90bfc 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -1,10 +1,31 @@
/*
+ * vga_switcheroo.h - Support for laptop with dual GPU using one set of outputs
+ *
* Copyright (c) 2010 Red Hat Inc.
* Author : Dave Airlie <airlied@redhat.com>
*
- * Licensed under GPLv2
+ * Copyright (c) 2015 Lukas Wunner <lukas@wunner.de>
+ *
+ * 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 (including the next
+ * paragraph) 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 AUTHORS OR COPYRIGHT HOLDERS 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.
*
- * vga_switcheroo.h - Support for laptop with dual GPU using one set of outputs
*/
#ifndef _LINUX_VGA_SWITCHEROO_H_
@@ -14,6 +35,20 @@
struct pci_dev;
+/**
+ * enum vga_switcheroo_state - client power state
+ * @VGA_SWITCHEROO_OFF: off
+ * @VGA_SWITCHEROO_ON: on
+ * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
+ * vga_switcheroo is not enabled, i.e. no second client or no handler
+ * has registered. Only used in vga_switcheroo_get_client_state() which
+ * in turn is only called from hda_intel.c
+ * @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
+ * Only used in vga_switcheroo_get_client_state() which in turn is only
+ * called from hda_intel.c
+ *
+ * Client power state.
+ */
enum vga_switcheroo_state {
VGA_SWITCHEROO_OFF,
VGA_SWITCHEROO_ON,
@@ -22,20 +57,64 @@ enum vga_switcheroo_state {
VGA_SWITCHEROO_NOT_FOUND,
};
+/**
+ * enum vga_switcheroo_client_id - client identifier
+ * @VGA_SWITCHEROO_IGD: integrated graphics device
+ * @VGA_SWITCHEROO_DIS: discrete graphics device
+ * @VGA_SWITCHEROO_MAX_CLIENTS: currently no more than two GPUs are supported
+ *
+ * Client identifier. Audio clients use the same identifier & 0x100.
+ */
enum vga_switcheroo_client_id {
VGA_SWITCHEROO_IGD,
VGA_SWITCHEROO_DIS,
VGA_SWITCHEROO_MAX_CLIENTS,
};
+/**
+ * struct vga_switcheroo_handler - handler callbacks
+ * @init: initialize handler.
+ * Optional. This gets called when vga_switcheroo is enabled, i.e. when
+ * two vga clients have registered. It allows the handler to perform
+ * some delayed initialization that depends on the existence of the
+ * vga clients. Currently only the radeon and amdgpu drivers use this.
+ * The return value is ignored
+ * @switchto: switch outputs to given client.
+ * Mandatory. For muxless machines this should be a no-op. Returning 0
+ * denotes success, anything else failure (in which case the switch is
+ * aborted)
+ * @power_state: cut or reinstate power of given client.
+ * Optional. The return value is ignored
+ * @get_client_id: determine if given pci device is integrated or discrete GPU.
+ * Mandatory
+ *
+ * Handler callbacks. The multiplexer itself. The @switchto and @get_client_id
+ * methods are mandatory, all others may be set to NULL.
+ */
struct vga_switcheroo_handler {
+ int (*init)(void);
int (*switchto)(enum vga_switcheroo_client_id id);
int (*power_state)(enum vga_switcheroo_client_id id,
enum vga_switcheroo_state state);
- int (*init)(void);
int (*get_client_id)(struct pci_dev *pdev);
};
+/**
+ * struct vga_switcheroo_client_ops - client callbacks
+ * @set_gpu_state: do the equivalent of suspend/resume for the card.
+ * Mandatory. This should not cut power to the discrete GPU,
+ * which is the job of the handler
+ * @reprobe: poll outputs.
+ * Optional. This gets called after waking the GPU and switching
+ * the outputs to it
+ * @can_switch: check if the device is in a position to switch now.
+ * Mandatory. The client should return false if a user space process
+ * has one of its device files open
+ *
+ * Client callbacks. A client can be either a GPU or an audio device on a GPU.
+ * The @set_gpu_state and @can_switch methods are mandatory, @reprobe may be
+ * set to NULL. For audio clients, the @reprobe member is bogus.
+ */
struct vga_switcheroo_client_ops {
void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state);
void (*reprobe)(struct pci_dev *dev);
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 04/15] vga_switcheroo: Add missing locking
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
@ 2015-08-23 21:23 ` Lukas Wunner
2015-09-08 12:17 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Lukas Wunner
2015-08-27 14:43 ` [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
` (2 subsequent siblings)
3 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-08-23 21:23 UTC (permalink / raw)
To: dri-devel
The following functions iterate over the client list, invoke client
callbacks or invoke handler callbacks without locking anything at all:
- Introduced by c8e9cf7bb240 ("vga_switcheroo: Add a helper function to
get the client state"):
vga_switcheroo_get_client_state()
- Introduced by 0d69704ae348 ("gpu/vga_switcheroo: add driver control
power feature. (v3)"):
vga_switcheroo_set_dynamic_switch()
vga_switcheroo_runtime_suspend()
vga_switcheroo_runtime_resume()
vga_switcheroo_runtime_resume_hdmi_audio()
Refactor vga_switcheroo_runtime_resume_hdmi_audio() a bit to be able to
release vgasr_mutex immediately after iterating over the client list.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 50 +++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index fe32536..2138162 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -349,13 +349,18 @@ find_active_client(struct list_head *head)
int vga_switcheroo_get_client_state(struct pci_dev *pdev)
{
struct vga_switcheroo_client *client;
+ enum vga_switcheroo_state ret;
+ mutex_lock(&vgasr_mutex);
client = find_client_from_pci(&vgasr_priv.clients, pdev);
if (!client)
- return VGA_SWITCHEROO_NOT_FOUND;
- if (!vgasr_priv.active)
- return VGA_SWITCHEROO_INIT;
- return client->pwr_state;
+ ret = VGA_SWITCHEROO_NOT_FOUND;
+ else if (!vgasr_priv.active)
+ ret = VGA_SWITCHEROO_INIT;
+ else
+ ret = client->pwr_state;
+ mutex_unlock(&vgasr_mutex);
+ return ret;
}
EXPORT_SYMBOL(vga_switcheroo_get_client_state);
@@ -847,15 +852,16 @@ void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev,
{
struct vga_switcheroo_client *client;
+ mutex_lock(&vgasr_mutex);
client = find_client_from_pci(&vgasr_priv.clients, pdev);
- if (!client)
- return;
-
- if (!client->driver_power_control)
+ if (!client || !client->driver_power_control) {
+ mutex_unlock(&vgasr_mutex);
return;
+ }
client->pwr_state = dynamic;
set_audio_state(client->id, dynamic);
+ mutex_unlock(&vgasr_mutex);
}
EXPORT_SYMBOL(vga_switcheroo_set_dynamic_switch);
@@ -868,9 +874,11 @@ static int vga_switcheroo_runtime_suspend(struct device *dev)
ret = dev->bus->pm->runtime_suspend(dev);
if (ret)
return ret;
+ mutex_lock(&vgasr_mutex);
if (vgasr_priv.handler->switchto)
vgasr_priv.handler->switchto(VGA_SWITCHEROO_IGD);
vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_OFF);
+ mutex_unlock(&vgasr_mutex);
return 0;
}
@@ -879,7 +887,9 @@ static int vga_switcheroo_runtime_resume(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
int ret;
+ mutex_lock(&vgasr_mutex);
vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_ON);
+ mutex_unlock(&vgasr_mutex);
ret = dev->bus->pm->runtime_resume(dev);
if (ret)
return ret;
@@ -925,29 +935,33 @@ EXPORT_SYMBOL(vga_switcheroo_fini_domain_pm_ops);
static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct vga_switcheroo_client *client;
+ struct device *video_dev = NULL;
int ret;
- struct vga_switcheroo_client *client, *found = NULL;
/* we need to check if we have to switch back on the video
device so the audio device can come back */
+ mutex_lock(&vgasr_mutex);
list_for_each_entry(client, &vgasr_priv.clients, list) {
if (PCI_SLOT(client->pdev->devfn) == PCI_SLOT(pdev->devfn) &&
client_is_vga(client)) {
- found = client;
- ret = pm_runtime_get_sync(&client->pdev->dev);
- if (ret) {
- if (ret != 1)
- return ret;
- }
+ video_dev = &client->pdev->dev;
break;
}
}
+ mutex_unlock(&vgasr_mutex);
+
+ if (video_dev) {
+ ret = pm_runtime_get_sync(video_dev);
+ if (ret && ret != 1)
+ return ret;
+ }
ret = dev->bus->pm->runtime_resume(dev);
/* put the reference for the gpu */
- if (found) {
- pm_runtime_mark_last_busy(&found->pdev->dev);
- pm_runtime_put_autosuspend(&found->pdev->dev);
+ if (video_dev) {
+ pm_runtime_mark_last_busy(video_dev);
+ pm_runtime_put_autosuspend(video_dev);
}
return ret;
}
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-29 12:29 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Lukas Wunner
@ 2015-08-27 14:43 ` Lukas Wunner
2015-08-23 21:23 ` [PATCH 04/15] vga_switcheroo: Add missing locking Lukas Wunner
` (3 more replies)
2015-09-22 8:38 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Daniel Vetter
1 sibling, 4 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-08-27 14:43 UTC (permalink / raw)
To: dri-devel
The active attribute in struct vga_switcheroo_client denotes whether
the outputs are currently switched to this client. The attribute is
only meaningful for vga clients. It is never used for audio clients.
The function vga_switcheroo_register_audio_client() misuses this
attribute to store whether the audio device is fully initialized.
Most likely there was a misunderstanding about the meaning of
"active" when this was added.
Set the active attribute to false for audio clients. Remove the
active parameter from vga_switcheroo_register_audio_client() and
its sole caller, hda_intel.c:register_vga_switcheroo().
vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
("vga_switcheroo: Add the support for audio clients"). Its use in
hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
VGA-switcheroo").
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 5 ++---
include/linux/vga_switcheroo.h | 4 ++--
sound/pci/hda/hda_intel.c | 3 +--
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index b19a72f..fe32536 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
* @pdev: client pci device
* @ops: client callbacks
* @id: client identifier, see enum vga_switcheroo_client_id
- * @active: whether the audio device is fully initialized
*
* Register audio client (audio device on a GPU). The power state of the
* client is assumed to be ON.
@@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
*/
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active)
+ int id)
{
- return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
+ return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
}
EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index fe90bfc..3764991 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
bool driver_power_control);
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active);
+ int id);
void vga_switcheroo_client_fb_set(struct pci_dev *dev,
struct fb_info *info);
@@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active) { return 0; }
+ int id) { return 0; }
static inline void vga_switcheroo_unregister_handler(void) {}
static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index c38c68f..e819013 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
* is there any machine with two switchable HDMI audio controllers?
*/
err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
- VGA_SWITCHEROO_DIS,
- hda->probe_continued);
+ VGA_SWITCHEROO_DIS);
if (err < 0)
return err;
hda->vga_switcheroo_registered = 1;
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-08-23 21:23 ` [PATCH 04/15] vga_switcheroo: Add missing locking Lukas Wunner
@ 2015-08-27 14:43 ` Lukas Wunner
2015-09-23 7:13 ` Daniel Vetter
2015-09-24 9:37 ` Takashi Iwai
2015-09-17 17:15 ` [PATCH " Lukas Wunner
2015-09-19 16:44 ` Takashi Iwai
3 siblings, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-08-27 14:43 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
The active attribute in struct vga_switcheroo_client denotes whether
the outputs are currently switched to this client. The attribute is
only meaningful for vga clients. It is never used for audio clients.
The function vga_switcheroo_register_audio_client() misuses this
attribute to store whether the audio device is fully initialized.
Most likely there was a misunderstanding about the meaning of
"active" when this was added.
Set the active attribute to false for audio clients. Remove the
active parameter from vga_switcheroo_register_audio_client() and
its sole caller, hda_intel.c:register_vga_switcheroo().
vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
("vga_switcheroo: Add the support for audio clients"). Its use in
hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
VGA-switcheroo").
v1.1: The changes above imply that in find_active_client() the call
to client_is_vga() is now superfluous. Drop it.
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 7 +++----
include/linux/vga_switcheroo.h | 4 ++--
sound/pci/hda/hda_intel.c | 3 +--
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 2e7e2f8e..563b82f 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
* @pdev: client pci device
* @ops: client callbacks
* @id: client identifier, see enum vga_switcheroo_client_id
- * @active: whether the audio device is fully initialized
*
* Register audio client (audio device on a GPU). The power state of the
* client is assumed to be ON.
@@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
*/
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active)
+ int id)
{
- return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
+ return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
}
EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
@@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
struct vga_switcheroo_client *client;
list_for_each_entry(client, head, list)
- if (client->active && client_is_vga(client))
+ if (client->active)
return client;
return NULL;
}
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index fe90bfc..3764991 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
bool driver_power_control);
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active);
+ int id);
void vga_switcheroo_client_fb_set(struct pci_dev *dev,
struct fb_info *info);
@@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active) { return 0; }
+ int id) { return 0; }
static inline void vga_switcheroo_unregister_handler(void) {}
static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index c38c68f..e819013 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
* is there any machine with two switchable HDMI audio controllers?
*/
err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
- VGA_SWITCHEROO_DIS,
- hda->probe_continued);
+ VGA_SWITCHEROO_DIS);
if (err < 0)
return err;
hda->vga_switcheroo_registered = 1;
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int
2015-09-08 12:17 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Lukas Wunner
@ 2015-08-28 9:56 ` Lukas Wunner
2015-08-28 11:30 ` [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1 Lukas Wunner
2015-10-02 7:35 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Jani Nikula
2015-10-02 8:22 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Daniel Vetter
1 sibling, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-08-28 9:56 UTC (permalink / raw)
To: dri-devel
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 6 +++---
include/linux/vga_switcheroo.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 845e47d..7b53246 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -102,7 +102,7 @@
struct vga_switcheroo_client {
struct pci_dev *pdev;
struct fb_info *fb_info;
- int pwr_state;
+ enum vga_switcheroo_state pwr_state;
const struct vga_switcheroo_client_ops *ops;
int id;
bool active;
@@ -346,7 +346,7 @@ find_active_client(struct list_head *head)
*
* Return: Power state.
*/
-int vga_switcheroo_get_client_state(struct pci_dev *pdev)
+enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *pdev)
{
struct vga_switcheroo_client *client;
enum vga_switcheroo_state ret;
@@ -496,7 +496,7 @@ static int vga_switchoff(struct vga_switcheroo_client *client)
return 0;
}
-static void set_audio_state(int id, int state)
+static void set_audio_state(int id, enum vga_switcheroo_state state)
{
struct vga_switcheroo_client *client;
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 95bfbeb0..219876e 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -133,7 +133,7 @@ void vga_switcheroo_unregister_handler(void);
int vga_switcheroo_process_delayed_switch(void);
-int vga_switcheroo_get_client_state(struct pci_dev *dev);
+enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev);
void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic);
@@ -152,7 +152,7 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
int id) { return 0; }
static inline void vga_switcheroo_unregister_handler(void) {}
static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
-static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
+static inline enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id instead of int
2015-08-28 11:30 ` [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1 Lukas Wunner
@ 2015-08-28 10:54 ` Lukas Wunner
2015-09-05 11:40 ` [PATCH 09/15] vga_switcheroo: Sort headers alphabetically Lukas Wunner
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-08-28 10:54 UTC (permalink / raw)
To: dri-devel
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 17 ++++++++++-------
include/linux/vga_switcheroo.h | 6 +++---
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index bbd0a8e..6e0e705 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -104,7 +104,7 @@ struct vga_switcheroo_client {
struct fb_info *fb_info;
enum vga_switcheroo_state pwr_state;
const struct vga_switcheroo_client_ops *ops;
- int id;
+ enum vga_switcheroo_client_id id;
bool active;
bool driver_power_control;
struct list_head list;
@@ -235,7 +235,8 @@ EXPORT_SYMBOL(vga_switcheroo_unregister_handler);
static int register_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id, bool active, bool driver_power_control)
+ enum vga_switcheroo_client_id id, bool active,
+ bool driver_power_control)
{
struct vga_switcheroo_client *client;
@@ -290,7 +291,7 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
* vga_switcheroo_register_audio_client - register audio client
* @pdev: client pci device
* @ops: client callbacks
- * @id: client identifier, see enum vga_switcheroo_client_id
+ * @id: client identifier
*
* Register audio client (audio device on a GPU). The power state of the
* client is assumed to be ON.
@@ -299,7 +300,7 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
*/
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id)
+ enum vga_switcheroo_client_id id)
{
return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
}
@@ -317,7 +318,8 @@ find_client_from_pci(struct list_head *head, struct pci_dev *pdev)
}
static struct vga_switcheroo_client *
-find_client_from_id(struct list_head *head, int client_id)
+find_client_from_id(struct list_head *head,
+ enum vga_switcheroo_client_id client_id)
{
struct vga_switcheroo_client *client;
@@ -497,7 +499,8 @@ static int vga_switchoff(struct vga_switcheroo_client *client)
return 0;
}
-static void set_audio_state(int id, enum vga_switcheroo_state state)
+static void set_audio_state(enum vga_switcheroo_client_id id,
+ enum vga_switcheroo_state state)
{
struct vga_switcheroo_client *client;
@@ -584,7 +587,7 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
int ret;
bool delay = false, can_switch;
bool just_mux = false;
- int client_id = VGA_SWITCHEROO_UNKNOWN_ID;
+ enum vga_switcheroo_client_id client_id = VGA_SWITCHEROO_UNKNOWN_ID;
struct vga_switcheroo_client *client = NULL;
if (cnt > 63)
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 664dca5..b2a3dda 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -95,7 +95,7 @@ struct vga_switcheroo_handler {
int (*switchto)(enum vga_switcheroo_client_id id);
int (*power_state)(enum vga_switcheroo_client_id id,
enum vga_switcheroo_state state);
- int (*get_client_id)(struct pci_dev *pdev);
+ enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);
};
/**
@@ -127,7 +127,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
bool driver_power_control);
int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id);
+ enum vga_switcheroo_client_id id);
void vga_switcheroo_client_fb_set(struct pci_dev *dev,
struct fb_info *info);
@@ -153,7 +153,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
- int id) { return 0; }
+ enum vga_switcheroo_client_id id) { return 0; }
static inline void vga_switcheroo_unregister_handler(void) {}
static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
static inline enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1
2015-08-28 9:56 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Lukas Wunner
@ 2015-08-28 11:30 ` Lukas Wunner
2015-08-28 10:54 ` [PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id instead of int Lukas Wunner
2015-10-02 7:35 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Jani Nikula
1 sibling, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-08-28 11:30 UTC (permalink / raw)
To: dri-devel
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 17 +++++++++--------
include/linux/vga_switcheroo.h | 4 ++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 7b53246..bbd0a8e 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -86,9 +86,9 @@
* @fb_info: framebuffer to which console is remapped on switching
* @pwr_state: current power state
* @ops: client callbacks
- * @id: client identifier, see enum vga_switcheroo_client_id.
- * Determining the id requires the handler, so GPUs are initially
- * assigned -1 and later given their true id in vga_switcheroo_enable()
+ * @id: client identifier. Determining the id requires the handler,
+ * so gpus are initially assigned VGA_SWITCHEROO_UNKNOWN_ID
+ * and later given their true id in vga_switcheroo_enable()
* @active: whether the outputs are currently switched to this client
* @driver_power_control: whether power state is controlled by the driver's
* runtime pm. If true, writing ON and OFF to the vga_switcheroo debugfs
@@ -147,7 +147,8 @@ struct vgasr_priv {
#define ID_BIT_AUDIO 0x100
#define client_is_audio(c) ((c)->id & ID_BIT_AUDIO)
-#define client_is_vga(c) ((c)->id == -1 || !client_is_audio(c))
+#define client_is_vga(c) ((c)->id == VGA_SWITCHEROO_UNKNOWN_ID || \
+ !client_is_audio(c))
#define client_id(c) ((c)->id & ~ID_BIT_AUDIO)
static int vga_switcheroo_debugfs_init(struct vgasr_priv *priv);
@@ -175,7 +176,7 @@ static void vga_switcheroo_enable(void)
vgasr_priv.handler->init();
list_for_each_entry(client, &vgasr_priv.clients, list) {
- if (client->id != -1)
+ if (client->id != VGA_SWITCHEROO_UNKNOWN_ID)
continue;
ret = vgasr_priv.handler->get_client_id(client->pdev);
if (ret < 0)
@@ -279,7 +280,7 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
const struct vga_switcheroo_client_ops *ops,
bool driver_power_control)
{
- return register_client(pdev, ops, -1,
+ return register_client(pdev, ops, VGA_SWITCHEROO_UNKNOWN_ID,
pdev == vga_default_device(),
driver_power_control);
}
@@ -583,7 +584,7 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
int ret;
bool delay = false, can_switch;
bool just_mux = false;
- int client_id = -1;
+ int client_id = VGA_SWITCHEROO_UNKNOWN_ID;
struct vga_switcheroo_client *client = NULL;
if (cnt > 63)
@@ -652,7 +653,7 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
client_id = VGA_SWITCHEROO_DIS;
}
- if (client_id == -1)
+ if (client_id == VGA_SWITCHEROO_UNKNOWN_ID)
goto out;
client = find_client_from_id(&vgasr_priv.clients, client_id);
if (!client)
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 219876e..664dca5 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -54,6 +54,9 @@ enum vga_switcheroo_state {
/**
* enum vga_switcheroo_client_id - client identifier
+ * @VGA_SWITCHEROO_UNKNOWN_ID: initial identifier assigned to vga clients.
+ * Determining the id requires the handler, so GPUs are given their
+ * true id in a delayed fashion in vga_switcheroo_enable()
* @VGA_SWITCHEROO_IGD: integrated graphics device
* @VGA_SWITCHEROO_DIS: discrete graphics device
* @VGA_SWITCHEROO_MAX_CLIENTS: currently no more than two GPUs are supported
@@ -61,6 +64,7 @@ enum vga_switcheroo_state {
* Client identifier. Audio clients use the same identifier & 0x100.
*/
enum vga_switcheroo_client_id {
+ VGA_SWITCHEROO_UNKNOWN_ID = -1,
VGA_SWITCHEROO_IGD,
VGA_SWITCHEROO_DIS,
VGA_SWITCHEROO_MAX_CLIENTS,
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide
2015-08-23 13:18 [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Lukas Wunner
@ 2015-08-29 12:29 ` Lukas Wunner
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-09-22 8:38 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Daniel Vetter
2015-09-17 16:34 ` [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Danilo Cesar Lemes de Paula
1 sibling, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-08-29 12:29 UTC (permalink / raw)
To: dri-devel; +Cc: Danilo Cesar Lemes de Paula, Jonathan Corbet
This is not part of drm.tmpl as vga_switcheroo is a subsystem of its own
which interfaces not just with DRM but also with multiplexer drivers,
ALSA and power management.
Requires Markdown support.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Documentation/DocBook/Makefile | 5 +-
Documentation/DocBook/vga_switcheroo.tmpl | 92 +++++++++++++++++++++++++++++++
2 files changed, 95 insertions(+), 2 deletions(-)
create mode 100644 Documentation/DocBook/vga_switcheroo.tmpl
diff --git a/Documentation/DocBook/Makefile b/Documentation/DocBook/Makefile
index 8276944..4495b37 100644
--- a/Documentation/DocBook/Makefile
+++ b/Documentation/DocBook/Makefile
@@ -15,9 +15,10 @@ DOCBOOKS := z8530book.xml device-drivers.xml \
80211.xml debugobjects.xml sh.xml regulator.xml \
alsa-driver-api.xml writing-an-alsa-driver.xml \
tracepoint.xml drm.xml media_api.xml w1.xml \
- writing_musb_glue_layer.xml crypto-API.xml
+ writing_musb_glue_layer.xml crypto-API.xml \
+ vga_switcheroo.xml
-MARKDOWNREADY :=
+MARKDOWNREADY := vga_switcheroo.xml
include Documentation/DocBook/media/Makefile
diff --git a/Documentation/DocBook/vga_switcheroo.tmpl b/Documentation/DocBook/vga_switcheroo.tmpl
new file mode 100644
index 0000000..e6128e7
--- /dev/null
+++ b/Documentation/DocBook/vga_switcheroo.tmpl
@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
+ "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
+
+<book id="vga_switcheroo">
+ <bookinfo>
+ <title>vga_switcheroo Subsystem Guide</title>
+
+ <authorgroup>
+ <author>
+ <firstname>Lukas</firstname>
+ <surname>Wunner</surname>
+ <contrib>Initial version</contrib>
+ <affiliation>
+ <address>
+ <email>lukas@wunner.de</email>
+ </address>
+ </affiliation>
+ </author>
+ </authorgroup>
+
+ <copyright>
+ <year>2015</year>
+ <holder>Lukas Wunner</holder>
+ </copyright>
+
+ <legalnotice>
+ <para>
+ The contents of this file may be used under the terms of the GNU
+ General Public License version 2 (the "GPL") as distributed in
+ the kernel source COPYING file.
+ </para>
+ </legalnotice>
+
+ <revhistory>
+ <!-- Put document revisions here, newest first. -->
+ <revision>
+ <revnumber>1.0</revnumber>
+ <date>2015-08-29</date>
+ <authorinitials>LW</authorinitials>
+ <revremark>Initial version
+ </revremark>
+ </revision>
+ </revhistory>
+ </bookinfo>
+
+<toc></toc>
+
+ <chapter id="overview">
+ <title>Overview</title>
+!Pdrivers/gpu/vga/vga_switcheroo.c Overview
+ </chapter>
+
+ <chapter id="modes_of_use">
+ <title>Modes of Use</title>
+ <sect1>
+ <title>Manual switching and manual power control</title>
+!Pdrivers/gpu/vga/vga_switcheroo.c Manual switching and manual power control
+ </sect1>
+ <sect1>
+ <title>Driver power control</title>
+!Pdrivers/gpu/vga/vga_switcheroo.c Driver power control
+ </sect1>
+ </chapter>
+
+ <chapter id="pubfunctions">
+ <title>Public functions</title>
+!Edrivers/gpu/vga/vga_switcheroo.c
+ </chapter>
+
+ <chapter id="pubstructures">
+ <title>Public structures</title>
+!Finclude/linux/vga_switcheroo.h vga_switcheroo_handler
+!Finclude/linux/vga_switcheroo.h vga_switcheroo_client_ops
+ </chapter>
+
+ <chapter id="pubconstants">
+ <title>Public constants</title>
+!Finclude/linux/vga_switcheroo.h vga_switcheroo_client_id
+!Finclude/linux/vga_switcheroo.h vga_switcheroo_state
+ </chapter>
+
+ <chapter id="privstructures">
+ <title>Private structures</title>
+!Fdrivers/gpu/vga/vga_switcheroo.c vgasr_priv
+!Fdrivers/gpu/vga/vga_switcheroo.c vga_switcheroo_client
+ </chapter>
+
+!Cdrivers/gpu/vga/vga_switcheroo.c
+!Cinclude/linux/vga_switcheroo.h
+
+</book>
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently
2015-09-05 11:40 ` [PATCH 09/15] vga_switcheroo: Sort headers alphabetically Lukas Wunner
@ 2015-09-04 18:49 ` Lukas Wunner
2015-09-04 19:06 ` [PATCH 11/15] drm/i915: " Lukas Wunner
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-04 18:49 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
sound/pci/hda/hda_controller.h | 2 +-
sound/pci/hda/hda_intel.c | 12 ++++++------
sound/pci/hda/hda_intel.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index 314105c..7b635d6 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -153,7 +153,7 @@ struct azx {
unsigned int snoop:1;
unsigned int align_buffer_size:1;
unsigned int region_requested:1;
- unsigned int disabled:1; /* disabled by VGA-switcher */
+ unsigned int disabled:1; /* disabled by vga_switcheroo */
#ifdef CONFIG_SND_HDA_DSP_LOADER
struct azx_dev saved_azx_dev;
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e819013..45233731 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -337,7 +337,7 @@ enum {
AZX_DCAPS_4K_BDLE_BOUNDARY | AZX_DCAPS_SNOOP_OFF)
/*
- * VGA-switcher support
+ * vga_switcheroo support
*/
#ifdef SUPPORT_VGA_SWITCHEROO
#define use_vga_switcheroo(chip) ((chip)->use_vga_switcheroo)
@@ -1076,12 +1076,12 @@ static void azx_vs_set_state(struct pci_dev *pci,
}
}
} else {
- dev_info(chip->card->dev, "%s via VGA-switcheroo\n",
+ dev_info(chip->card->dev, "%s via vga_switcheroo\n",
disabled ? "Disabling" : "Enabling");
if (disabled) {
pm_runtime_put_sync_suspend(card->dev);
azx_suspend(card->dev);
- /* when we get suspended by vga switcheroo we end up in D3cold,
+ /* when we get suspended by vga_switcheroo we end up in D3cold,
* however we have no ACPI handle, so pci/acpi can't put us there,
* put ourselves there */
pci->current_state = PCI_D3cold;
@@ -1121,7 +1121,7 @@ static void init_vga_switcheroo(struct azx *chip)
struct pci_dev *p = get_bound_vga(chip->pci);
if (p) {
dev_info(chip->card->dev,
- "Handle VGA-switcheroo audio client\n");
+ "Handle vga_switcheroo audio client\n");
hda->use_vga_switcheroo = 1;
pci_dev_put(p);
}
@@ -1232,7 +1232,7 @@ static int azx_dev_free(struct snd_device *device)
#ifdef SUPPORT_VGA_SWITCHEROO
/*
- * Check of disabled HDMI controller by vga-switcheroo
+ * Check of disabled HDMI controller by vga_switcheroo
*/
static struct pci_dev *get_bound_vga(struct pci_dev *pci)
{
@@ -1917,7 +1917,7 @@ static int azx_probe(struct pci_dev *pci,
err = register_vga_switcheroo(chip);
if (err < 0) {
- dev_err(card->dev, "Error registering VGA-switcheroo client\n");
+ dev_err(card->dev, "Error registering vga_switcheroo client\n");
goto out_free;
}
diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
index 354f0bb..ff0c4d6 100644
--- a/sound/pci/hda/hda_intel.h
+++ b/sound/pci/hda/hda_intel.h
@@ -35,7 +35,7 @@ struct hda_intel {
unsigned int irq_pending_warned:1;
unsigned int probe_continued:1;
- /* VGA-switcheroo setup */
+ /* vga_switcheroo setup */
unsigned int use_vga_switcheroo:1;
unsigned int vga_switcheroo_registered:1;
unsigned int init_failed:1; /* delayed init failed */
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 11/15] drm/i915: Spell vga_switcheroo consistently
2015-09-04 18:49 ` [PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently Lukas Wunner
@ 2015-09-04 19:06 ` Lukas Wunner
2015-09-05 9:09 ` [PATCH 12/15] drm/nouveau: " Lukas Wunner
2015-09-22 9:20 ` [PATCH 11/15] drm/i915: " Daniel Vetter
0 siblings, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-09-04 19:06 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/i915/intel_acpi.c | 2 +-
drivers/gpu/drm/i915/intel_panel.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index d96eee1..8b13b9d 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -146,7 +146,7 @@ static bool intel_dsm_detect(void)
if (vga_count == 2 && has_dsm) {
acpi_get_name(intel_dsm_priv.dhandle, ACPI_FULL_PATHNAME, &buffer);
- DRM_DEBUG_DRIVER("VGA switcheroo: detected DSM switching method %s handle\n",
+ DRM_DEBUG_DRIVER("vga_switcheroo: detected DSM switching method %s handle\n",
acpi_method_name);
return true;
}
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e2ab3f6..04f7d8e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -809,7 +809,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
return;
/*
- * Do not disable backlight on the vgaswitcheroo path. When switching
+ * Do not disable backlight on the vga_switcheroo path. When switching
* away from i915, the other client may depend on i915 to handle the
* backlight. This will leave the backlight on unnecessarily when
* another client is not activated.
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 12/15] drm/nouveau: Spell vga_switcheroo consistently
2015-09-04 19:06 ` [PATCH 11/15] drm/i915: " Lukas Wunner
@ 2015-09-05 9:09 ` Lukas Wunner
2015-09-05 9:14 ` [PATCH 13/15] drm/radeon: " Lukas Wunner
2015-09-22 9:20 ` [PATCH 11/15] drm/i915: " Daniel Vetter
1 sibling, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-05 9:09 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/nouveau/nouveau_acpi.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_vga.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index df2d981..af20dd2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -289,14 +289,14 @@ static bool nouveau_dsm_detect(void)
if (has_optimus == 1) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
&buffer);
- printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method %s handle\n",
+ printk(KERN_INFO "vga_switcheroo: detected Optimus DSM method %s handle\n",
acpi_method_name);
nouveau_dsm_priv.optimus_detected = true;
ret = true;
} else if (vga_count == 2 && has_dsm && guid_valid) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
&buffer);
- printk(KERN_INFO "VGA switcheroo: detected DSM switching method %s handle\n",
+ printk(KERN_INFO "vga_switcheroo: detected DSM switching method %s handle\n",
acpi_method_name);
nouveau_dsm_priv.dsm_detected = true;
ret = true;
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index af89c36..0cda8e1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -41,13 +41,13 @@ nouveau_switcheroo_set_state(struct pci_dev *pdev,
return;
if (state == VGA_SWITCHEROO_ON) {
- printk(KERN_ERR "VGA switcheroo: switched nouveau on\n");
+ printk(KERN_ERR "vga_switcheroo: switched nouveau on\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
nouveau_pmops_resume(&pdev->dev);
drm_kms_helper_poll_enable(dev);
dev->switch_power_state = DRM_SWITCH_POWER_ON;
} else {
- printk(KERN_ERR "VGA switcheroo: switched nouveau off\n");
+ printk(KERN_ERR "vga_switcheroo: switched nouveau off\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
drm_kms_helper_poll_disable(dev);
nouveau_switcheroo_optimus_dsm();
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 13/15] drm/radeon: Spell vga_switcheroo consistently
2015-09-05 9:09 ` [PATCH 12/15] drm/nouveau: " Lukas Wunner
@ 2015-09-05 9:14 ` Lukas Wunner
2015-09-05 9:17 ` [PATCH 14/15] drm/amdgpu: " Lukas Wunner
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-05 9:14 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/radeon/radeon_atpx_handler.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/gpu/drm/radeon/radeon_kms.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 8bc7d0b..a771b9f 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -535,7 +535,7 @@ static bool radeon_atpx_detect(void)
if (has_atpx && vga_count == 2) {
acpi_get_name(radeon_atpx_priv.atpx.handle, ACPI_FULL_PATHNAME, &buffer);
- printk(KERN_INFO "VGA switcheroo: detected switching method %s handle\n",
+ printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
acpi_method_name);
radeon_atpx_priv.atpx_detected = true;
return true;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index d8319da..38d730c 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1197,7 +1197,7 @@ static void radeon_check_arguments(struct radeon_device *rdev)
* radeon_switcheroo_set_state - set switcheroo state
*
* @pdev: pci dev pointer
- * @state: vga switcheroo state
+ * @state: vga_switcheroo state
*
* Callback for the switcheroo driver. Suspends or resumes the
* the asics before or after it is powered up using ACPI methods.
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 4a119c2..021abcc 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -602,7 +602,7 @@ static int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file
*
* @dev: drm dev pointer
*
- * Switch vga switcheroo state after last close (all asics).
+ * Switch vga_switcheroo state after last close (all asics).
*/
void radeon_driver_lastclose_kms(struct drm_device *dev)
{
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 14/15] drm/amdgpu: Spell vga_switcheroo consistently
2015-09-05 9:14 ` [PATCH 13/15] drm/radeon: " Lukas Wunner
@ 2015-09-05 9:17 ` Lukas Wunner
2015-09-05 9:22 ` [PATCH 15/15] drm: " Lukas Wunner
2015-09-23 21:45 ` [PATCH 14/15] drm/amdgpu: " Alex Deucher
0 siblings, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-09-05 9:17 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 3f7aaa4..1a6b239 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -536,7 +536,7 @@ static bool amdgpu_atpx_detect(void)
if (has_atpx && vga_count == 2) {
acpi_get_name(amdgpu_atpx_priv.atpx.handle, ACPI_FULL_PATHNAME, &buffer);
- printk(KERN_INFO "VGA switcheroo: detected switching method %s handle\n",
+ printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
acpi_method_name);
amdgpu_atpx_priv.atpx_detected = true;
return true;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6ff6ae9..b1a08d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1021,7 +1021,7 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
* amdgpu_switcheroo_set_state - set switcheroo state
*
* @pdev: pci dev pointer
- * @state: vga switcheroo state
+ * @state: vga_switcheroo state
*
* Callback for the switcheroo driver. Suspends or resumes the
* the asics before or after it is powered up using ACPI methods.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 2236793..a60eb4b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -488,7 +488,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
*
* @dev: drm dev pointer
*
- * Switch vga switcheroo state after last close (all asics).
+ * Switch vga_switcheroo state after last close (all asics).
*/
void amdgpu_driver_lastclose_kms(struct drm_device *dev)
{
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 15/15] drm: Spell vga_switcheroo consistently
2015-09-05 9:17 ` [PATCH 14/15] drm/amdgpu: " Lukas Wunner
@ 2015-09-05 9:22 ` Lukas Wunner
2015-09-22 9:20 ` Daniel Vetter
2015-09-23 21:45 ` [PATCH 14/15] drm/amdgpu: " Alex Deucher
1 sibling, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-05 9:22 UTC (permalink / raw)
To: dri-devel
Currently everyone and their dog has their own favourite spelling
for vga_switcheroo. This makes it hard to grep dmesg for log entries
relating to vga_switcheroo. It also makes it hard to find related
source files in the tree.
vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Documentation/DocBook/drm.tmpl | 2 +-
drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
include/linux/fb.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 9ddf8c6..30401f9 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3646,7 +3646,7 @@ void (*postclose) (struct drm_device *, struct drm_file *);</synopsis>
plane properties to default value, so that a subsequent open of the
device will not inherit state from the previous user. It can also be
used to execute delayed power switching state changes, e.g. in
- conjunction with the vga-switcheroo infrastructure. Beyond that KMS
+ conjunction with the vga_switcheroo infrastructure. Beyond that KMS
drivers should not do any further cleanup. Only legacy UMS drivers might
need to clean up device state so that the vga console or an independent
fbdev driver could take over.
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index a5f9d8b..d685e23 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -753,7 +753,7 @@ static void dev_lastclose(struct drm_device *dev)
{
int i;
- /* we don't support vga-switcheroo.. so just make sure the fbdev
+ /* we don't support vga_switcheroo.. so just make sure the fbdev
* mode is active
*/
struct omap_drm_private *priv = dev->dev_private;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 043f328..36da20a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -156,7 +156,7 @@ struct fb_cursor_user {
#define FB_EVENT_GET_REQ 0x0D
/* Unbind from the console if possible */
#define FB_EVENT_FB_UNBIND 0x0E
-/* CONSOLE-SPECIFIC: remap all consoles to new fb - for vga switcheroo */
+/* CONSOLE-SPECIFIC: remap all consoles to new fb - for vga_switcheroo */
#define FB_EVENT_REMAP_ALL_CONSOLE 0x0F
/* A hardware display blank early change occured */
#define FB_EARLY_EVENT_BLANK 0x10
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 09/15] vga_switcheroo: Sort headers alphabetically
2015-08-28 10:54 ` [PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id instead of int Lukas Wunner
@ 2015-09-05 11:40 ` Lukas Wunner
2015-09-04 18:49 ` [PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently Lukas Wunner
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-05 11:40 UTC (permalink / raw)
To: dri-devel
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 6e0e705..578aa47 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -30,19 +30,17 @@
#define pr_fmt(fmt) "vga_switcheroo: " fmt
-#include <linux/module.h>
-#include <linux/seq_file.h>
-#include <linux/uaccess.h>
-#include <linux/fs.h>
+#include <linux/console.h>
#include <linux/debugfs.h>
#include <linux/fb.h>
-
+#include <linux/fs.h>
+#include <linux/module.h>
#include <linux/pci.h>
-#include <linux/console.h>
-#include <linux/vga_switcheroo.h>
#include <linux/pm_runtime.h>
-
+#include <linux/seq_file.h>
+#include <linux/uaccess.h>
#include <linux/vgaarb.h>
+#include <linux/vga_switcheroo.h>
/**
* DOC: Overview
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT
2015-08-23 21:23 ` [PATCH 04/15] vga_switcheroo: Add missing locking Lukas Wunner
@ 2015-09-08 12:17 ` Lukas Wunner
2015-08-28 9:56 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Lukas Wunner
2015-10-02 8:22 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Daniel Vetter
0 siblings, 2 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-09-08 12:17 UTC (permalink / raw)
To: dri-devel
hda_intel.c:azx_probe() defers initialization of an audio controller
on the discrete GPU if the GPU is powered off. The power state of the
GPU is determined by calling vga_switcheroo_get_client_state().
vga_switcheroo_get_client_state() returns VGA_SWITCHEROO_INIT if
vga_switcheroo is not enabled, i.e. if no second GPU or no handler
has registered.
This can go wrong in the following scenario:
- Driver for the integrated GPU is not loaded.
- Driver for the discrete GPU registers with vga_switcheroo, uses driver
power control to power down the GPU, handler cuts power to the GPU.
- Driver for the audio controller gets loaded after the GPU was powered
down, calls vga_switcheroo_get_client_state() which returns
VGA_SWITCHEROO_INIT instead of VGA_SWITCHEROO_OFF.
- Consequence: azx_probe() tries to initialize the audio controller even
though the GPU is powered down.
The power state VGA_SWITCHEROO_INIT was introduced by c8e9cf7bb240
("vga_switcheroo: Add a helper function to get the client state").
It is not apparent what its benefit might be. The idea seems to
be to initialize the audio controller even if the power state is
VGA_SWITCHEROO_OFF (were vga_switcheroo enabled), but as shown
above this can fail.
Drop VGA_SWITCHEROO_INIT to solve this.
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/vga/vga_switcheroo.c | 2 --
include/linux/vga_switcheroo.h | 5 -----
2 files changed, 7 deletions(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 2138162..845e47d 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -355,8 +355,6 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
client = find_client_from_pci(&vgasr_priv.clients, pdev);
if (!client)
ret = VGA_SWITCHEROO_NOT_FOUND;
- else if (!vgasr_priv.active)
- ret = VGA_SWITCHEROO_INIT;
else
ret = client->pwr_state;
mutex_unlock(&vgasr_mutex);
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 3764991..95bfbeb0 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -39,10 +39,6 @@ struct pci_dev;
* enum vga_switcheroo_state - client power state
* @VGA_SWITCHEROO_OFF: off
* @VGA_SWITCHEROO_ON: on
- * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
- * vga_switcheroo is not enabled, i.e. no second client or no handler
- * has registered. Only used in vga_switcheroo_get_client_state() which
- * in turn is only called from hda_intel.c
* @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
* Only used in vga_switcheroo_get_client_state() which in turn is only
* called from hda_intel.c
@@ -53,7 +49,6 @@ enum vga_switcheroo_state {
VGA_SWITCHEROO_OFF,
VGA_SWITCHEROO_ON,
/* below are referred only from vga_switcheroo_get_client_state() */
- VGA_SWITCHEROO_INIT,
VGA_SWITCHEROO_NOT_FOUND,
};
--
1.8.5.2 (Apple Git-48)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
2015-08-23 13:18 [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Lukas Wunner
2015-08-29 12:29 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Lukas Wunner
@ 2015-09-17 16:34 ` Danilo Cesar Lemes de Paula
2015-09-17 17:31 ` Lukas Wunner
1 sibling, 1 reply; 37+ messages in thread
From: Danilo Cesar Lemes de Paula @ 2015-09-17 16:34 UTC (permalink / raw)
To: Lukas Wunner, dri-devel; +Cc: Jonathan Corbet
On 08/23/2015 10:18 AM, Lukas Wunner wrote:
> This adds an "Overview" DOC section plus two DOC sections for the modes
> of use ("Manual switching and manual power control" and "Driver power
> control").
>
> Also included is kernel-doc for all public functions, structs and enums.
Regarding the markdown support, it does makes sense.
Just a small comment in the middle to be sure that required effect is
achieved.
Danilo Cesar
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> drivers/gpu/vga/vga_switcheroo.c | 285 +++++++++++++++++++++++++++++++++++++--
> include/linux/vga_switcheroo.h | 85 +++++++++++-
> 2 files changed, 353 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 2106066..b19a72f 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -1,20 +1,31 @@
> /*
> + * vga_switcheroo.c - Support for laptop with dual GPU using one set of outputs
> + *
> * Copyright (c) 2010 Red Hat Inc.
> * Author : Dave Airlie <airlied@redhat.com>
> *
> + * Copyright (c) 2015 Lukas Wunner <lukas@wunner.de>
> *
> - * Licensed under GPLv2
> + * 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:
> *
> - * vga_switcheroo.c - Support for laptop with dual GPU using one set of outputs
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> *
> - * Switcher interface - methods require for ATPX and DCM
> - * - switchto - this throws the output MUX switch
> - * - discrete_set_power - sets the power state for the discrete card
> + * 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 AUTHORS OR COPYRIGHT HOLDERS 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.
> *
> - * GPU driver interface
> - * - set_gpu_state - this should do the equiv of s/r for the card
> - * - this should *not* set the discrete power state
Did you check if this creates the desired format? (ie: Doesn't create a
<code> block).
> - * - switch_check - check if the device is in a position to switch now
> */
>
> #define pr_fmt(fmt) "vga_switcheroo: " fmt
> @@ -33,6 +44,61 @@
>
> #include <linux/vgaarb.h>
>
> +/**
> + * DOC: Overview
> + *
> + * vga_switcheroo is the Linux subsystem for laptop hybrid graphics.
> + * These come in two flavors:
> + *
> + * * muxed: Dual GPUs with a multiplexer chip to switch outputs between GPUs.
> + * * muxless: Dual GPUs but only one of them is connected to outputs.
> + * The other one is merely used to offload rendering, its results
> + * are copied over PCIe into the framebuffer. On Linux this is
> + * supported with DRI PRIME.
> + *
> + * Hybrid graphics started to appear in the late Naughties and were initially
> + * all muxed. Newer laptops moved to a muxless architecture for cost reasons.
> + * A notable exception is the MacBook Pro which continues to use a mux.
> + * Muxes come with varying capabilities: Some switch only the panel, others
> + * can also switch external displays. Some switch all display pins at once
> + * while others can switch just the DDC lines. (To allow EDID probing
> + * for the inactive GPU.) Also, muxes are often used to cut power to the
> + * discrete GPU while it is not used.
> + *
> + * DRM drivers register GPUs with vga_switcheroo, these are heretoforth called
> + * clients. The mux is called the handler. Muxless machines also register a
> + * handler to control the power state of the discrete GPU, its ->switchto
> + * callback is a no-op for obvious reasons. The discrete GPU is often equipped
> + * with an HDA controller for the HDMI/DP audio signal, this will also
> + * register as a client so that vga_switcheroo can take care of the correct
> + * suspend/resume order when changing the discrete GPU's power state. In total
> + * there can thus be up to three clients: Two vga clients (GPUs) and one audio
> + * client (on the discrete GPU). The code is mostly prepared to support
> + * machines with more than two GPUs should they become available.
> + * The GPU to which the outputs are currently switched is called the
> + * active client in vga_switcheroo parlance. The GPU not in use is the
> + * inactive client.
> + */
> +
> +/**
> + * struct vga_switcheroo_client - registered client
> + * @pdev: client pci device
> + * @fb_info: framebuffer to which console is remapped on switching
> + * @pwr_state: current power state
> + * @ops: client callbacks
> + * @id: client identifier, see enum vga_switcheroo_client_id.
> + * Determining the id requires the handler, so GPUs are initially
> + * assigned -1 and later given their true id in vga_switcheroo_enable()
> + * @active: whether the outputs are currently switched to this client
> + * @driver_power_control: whether power state is controlled by the driver's
> + * runtime pm. If true, writing ON and OFF to the vga_switcheroo debugfs
> + * interface is a no-op so as not to interfere with runtime pm
> + * @list: client list
> + *
> + * Registered client. A client can be either a GPU or an audio device on a GPU.
> + * For audio clients, the @fb_info, @active and @driver_power_control members
> + * are bogus.
> + */
> struct vga_switcheroo_client {
> struct pci_dev *pdev;
> struct fb_info *fb_info;
> @@ -44,10 +110,28 @@ struct vga_switcheroo_client {
> struct list_head list;
> };
>
> +/*
> + * protects access to struct vgasr_priv
> + */
> static DEFINE_MUTEX(vgasr_mutex);
>
> +/**
> + * struct vgasr_priv - vga_switcheroo private data
> + * @active: whether vga_switcheroo is enabled.
> + * Prerequisite is the registration of two GPUs and a handler
> + * @delayed_switch_active: whether a delayed switch is pending
> + * @delayed_client_id: client to which a delayed switch is pending
> + * @debugfs_root: directory for vga_switcheroo debugfs interface
> + * @switch_file: file for vga_switcheroo debugfs interface
> + * @registered_clients: number of registered GPUs
> + * (counting only vga clients, not audio clients)
> + * @clients: list of registered clients
> + * @handler: registered handler
> + *
> + * vga_switcheroo private data. Currently only one vga_switcheroo instance
> + * per system is supported.
> + */
> struct vgasr_priv {
> -
> bool active;
> bool delayed_switch_active;
> enum vga_switcheroo_client_id delayed_client_id;
> @@ -103,6 +187,15 @@ static void vga_switcheroo_enable(void)
> vgasr_priv.active = true;
> }
>
> +/**
> + * vga_switcheroo_register_handler() - register handler
> + * @handler: handler callbacks
> + *
> + * Register handler. Enable vga_switcheroo if two vga clients have already
> + * registered.
> + *
> + * Return: 0 on success, -EINVAL if a handler was already registered.
> + */
> int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
> {
> mutex_lock(&vgasr_mutex);
> @@ -121,6 +214,11 @@ int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
> }
> EXPORT_SYMBOL(vga_switcheroo_register_handler);
>
> +/**
> + * vga_switcheroo_unregister_handler() - unregister handler
> + *
> + * Unregister handler. Disable vga_switcheroo.
> + */
> void vga_switcheroo_unregister_handler(void)
> {
> mutex_lock(&vgasr_mutex);
> @@ -164,6 +262,19 @@ static int register_client(struct pci_dev *pdev,
> return 0;
> }
>
> +/**
> + * vga_switcheroo_register_client - register vga client
> + * @pdev: client pci device
> + * @ops: client callbacks
> + * @driver_power_control: whether power state is controlled by the driver's
> + * runtime pm
> + *
> + * Register vga client (GPU). Enable vga_switcheroo if another GPU and a
> + * handler have already registered. The power state of the client is assumed
> + * to be ON.
> + *
> + * Return: 0 on success, -ENOMEM on memory allocation error.
> + */
> int vga_switcheroo_register_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> bool driver_power_control)
> @@ -174,6 +285,18 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
> }
> EXPORT_SYMBOL(vga_switcheroo_register_client);
>
> +/**
> + * vga_switcheroo_register_audio_client - register audio client
> + * @pdev: client pci device
> + * @ops: client callbacks
> + * @id: client identifier, see enum vga_switcheroo_client_id
> + * @active: whether the audio device is fully initialized
> + *
> + * Register audio client (audio device on a GPU). The power state of the
> + * client is assumed to be ON.
> + *
> + * Return: 0 on success, -ENOMEM on memory allocation error.
> + */
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> int id, bool active)
> @@ -215,6 +338,15 @@ find_active_client(struct list_head *head)
> return NULL;
> }
>
> +/**
> + * vga_switcheroo_get_client_state() - obtain power state of a given client
> + * @pdev: client pci device
> + *
> + * Obtain power state of a given client as seen from vga_switcheroo.
> + * The function is only called from hda_intel.c.
> + *
> + * Return: Power state.
> + */
> int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> {
> struct vga_switcheroo_client *client;
> @@ -228,6 +360,12 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> }
> EXPORT_SYMBOL(vga_switcheroo_get_client_state);
>
> +/**
> + * vga_switcheroo_unregister_client() - unregister client
> + * @pdev: client pci device
> + *
> + * Unregister client. Disable vga_switcheroo if this is a vga client (GPU).
> + */
> void vga_switcheroo_unregister_client(struct pci_dev *pdev)
> {
> struct vga_switcheroo_client *client;
> @@ -249,6 +387,14 @@ void vga_switcheroo_unregister_client(struct pci_dev *pdev)
> }
> EXPORT_SYMBOL(vga_switcheroo_unregister_client);
>
> +/**
> + * vga_switcheroo_client_fb_set() - set framebuffer of a given client
> + * @pdev: client pci device
> + * @info: framebuffer
> + *
> + * Set framebuffer of a given client. The console will be remapped to this
> + * on switching.
> + */
> void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
> struct fb_info *info)
> {
> @@ -262,6 +408,42 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
> }
> EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
>
> +/**
> + * DOC: Manual switching and manual power control
> + *
> + * In this mode of use, the file /sys/kernel/debug/vgaswitcheroo/switch
> + * can be read to retrieve the current vga_switcheroo state and commands
> + * can be written to it to change the state. The file appears as soon as
> + * two GPU drivers and one handler have registered with vga_switcheroo.
> + * The following commands are understood:
> + *
> + * * OFF: Power off the device not in use.
> + * * ON: Power on the device not in use.
> + * * IGD: Switch to the integrated graphics device.
> + * Power on the integrated GPU if necessary, power off the discrete GPU.
> + * Prerequisite is that no user space processes (e.g. Xorg, alsactl)
> + * have opened device files of the GPUs or the audio client. If the
> + * switch fails, the user may invoke lsof(8) or fuser(1) on /dev/dri/
> + * and /dev/snd/controlC1 to identify processes blocking the switch.
> + * * DIS: Switch to the discrete graphics device.
> + * * DIGD: Delayed switch to the integrated graphics device.
> + * This will perform the switch once the last user space process has
> + * closed the device files of the GPUs and the audio client.
> + * * DDIS: Delayed switch to the discrete graphics device.
> + * * MIGD: Mux-only switch to the integrated graphics device.
> + * Does not remap console or change the power state of either gpu.
> + * If the integrated GPU is currently off, the screen will turn black.
> + * If it is on, the screen will show whatever happens to be in VRAM.
> + * Either way, the user has to blindly enter the command to switch back.
> + * * MDIS: Mux-only switch to the discrete graphics device.
> + *
> + * For GPUs whose power state is controlled by the driver's runtime pm,
> + * the ON and OFF commands are a no-op (see next section).
> + *
> + * For muxless machines, the IGD/DIS, DIGD/DDIS and MIGD/MDIS commands
> + * should not be used.
> + */
> +
> static int vga_switcheroo_show(struct seq_file *m, void *v)
> {
> struct vga_switcheroo_client *client;
> @@ -559,6 +741,16 @@ fail:
> return -1;
> }
>
> +/**
> + * vga_switcheroo_process_delayed_switch() - helper for delayed switching
> + *
> + * Process a delayed switch if one is pending. DRM drivers should call this
> + * from their ->lastclose callback.
> + *
> + * Return: 0 on success. -EINVAL if no delayed switch is pending, if the client
> + * has unregistered in the meantime or if there are other clients blocking the
> + * switch. If the actual switch fails, an error is reported and 0 is returned.
> + */
> int vga_switcheroo_process_delayed_switch(void)
> {
> struct vga_switcheroo_client *client;
> @@ -589,6 +781,39 @@ err:
> }
> EXPORT_SYMBOL(vga_switcheroo_process_delayed_switch);
>
> +/**
> + * DOC: Driver power control
> + *
> + * In this mode of use, the discrete GPU automatically powers up and down at
> + * the discretion of the driver's runtime pm. On muxed machines, the user may
> + * still influence the muxer state by way of the debugfs interface, however
> + * the ON and OFF commands become a no-op for the discrete GPU.
> + *
> + * This mode is the default on Nvidia HybridPower/Optimus and ATI PowerXpress.
> + * Specifying nouveau.runpm=0, radeon.runpm=0 or amdgpu.runpm=0 on the kernel
> + * command line disables it.
> + *
> + * When the driver decides to power up or down, it notifies vga_switcheroo
> + * thereof so that it can (a) power the audio device on the GPU up or down,
> + * and (b) update its internal power state representation for the device.
> + * This is achieved by vga_switcheroo_set_dynamic_switch().
> + *
> + * After the GPU has been suspended, the handler needs to be called to cut
> + * power to the GPU. Likewise it needs to reinstate power before the GPU
> + * can resume. This is achieved by vga_switcheroo_init_domain_pm_ops(),
> + * which augments the GPU's suspend/resume functions by the requisite
> + * calls to the handler.
> + *
> + * When the audio device resumes, the GPU needs to be woken. This is achieved
> + * by vga_switcheroo_init_domain_pm_optimus_hdmi_audio(), which augments the
> + * audio device's resume function.
> + *
> + * On muxed machines, if the mux is initially switched to the discrete GPU,
> + * the user ends up with a black screen when the GPU powers down after boot.
> + * As a workaround, the mux is forced to the integrated GPU on runtime suspend,
> + * cf. https://bugs.freedesktop.org/show_bug.cgi?id=75917
> + */
> +
> static void vga_switcheroo_power_switch(struct pci_dev *pdev,
> enum vga_switcheroo_state state)
> {
> @@ -607,8 +832,17 @@ static void vga_switcheroo_power_switch(struct pci_dev *pdev,
> vgasr_priv.handler->power_state(client->id, state);
> }
>
> -/* force a PCI device to a certain state - mainly to turn off audio clients */
> -
> +/**
> + * vga_switcheroo_set_dynamic_switch() - helper for driver power control
> + * @pdev: client pci device
> + * @dynamic: new power state
> + *
> + * Helper for GPUs whose power state is controlled by the driver's runtime pm.
> + * When the driver decides to power up or down, it notifies vga_switcheroo
> + * thereof using this helper so that it can (a) power the audio device on
> + * the GPU up or down, and (b) update its internal power state representation
> + * for the device.
> + */
> void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev,
> enum vga_switcheroo_state dynamic)
> {
> @@ -654,8 +888,18 @@ static int vga_switcheroo_runtime_resume(struct device *dev)
> return 0;
> }
>
> -/* this version is for the case where the power switch is separate
> - to the device being powered down. */
> +/**
> + * vga_switcheroo_init_domain_pm_ops() - helper for driver power control
> + * @dev: vga client device
> + * @domain: power domain
> + *
> + * Helper for GPUs whose power state is controlled by the driver's runtime pm.
> + * After the GPU has been suspended, the handler needs to be called to cut
> + * power to the GPU. Likewise it needs to reinstate power before the GPU
> + * can resume. To this end, this helper augments the suspend/resume functions
> + * by the requisite calls to the handler. It needs only be called on platforms
> + * where the power switch is separate to the device being powered down.
> + */
> int vga_switcheroo_init_domain_pm_ops(struct device *dev,
> struct dev_pm_domain *domain)
> {
> @@ -709,6 +953,19 @@ static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
> return ret;
> }
>
> +/**
> + * vga_switcheroo_init_domain_pm_optimus_hdmi_audio() - helper for driver
> + * power control
> + * @dev: audio client device
> + * @domain: power domain
> + *
> + * Helper for GPUs whose power state is controlled by the driver's runtime pm.
> + * When the audio device resumes, the GPU needs to be woken. This helper
> + * augments the audio device's resume function to do that.
> + *
> + * Return: 0 on success, -EINVAL if no power management operations are
> + * defined for this device.
> + */
> int
> vga_switcheroo_init_domain_pm_optimus_hdmi_audio(struct device *dev,
> struct dev_pm_domain *domain)
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index b483abd..fe90bfc 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -1,10 +1,31 @@
> /*
> + * vga_switcheroo.h - Support for laptop with dual GPU using one set of outputs
> + *
> * Copyright (c) 2010 Red Hat Inc.
> * Author : Dave Airlie <airlied@redhat.com>
> *
> - * Licensed under GPLv2
> + * Copyright (c) 2015 Lukas Wunner <lukas@wunner.de>
> + *
> + * 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 (including the next
> + * paragraph) 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 AUTHORS OR COPYRIGHT HOLDERS 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.
> *
> - * vga_switcheroo.h - Support for laptop with dual GPU using one set of outputs
> */
>
> #ifndef _LINUX_VGA_SWITCHEROO_H_
> @@ -14,6 +35,20 @@
>
> struct pci_dev;
>
> +/**
> + * enum vga_switcheroo_state - client power state
> + * @VGA_SWITCHEROO_OFF: off
> + * @VGA_SWITCHEROO_ON: on
> + * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
> + * vga_switcheroo is not enabled, i.e. no second client or no handler
> + * has registered. Only used in vga_switcheroo_get_client_state() which
> + * in turn is only called from hda_intel.c
> + * @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
> + * Only used in vga_switcheroo_get_client_state() which in turn is only
> + * called from hda_intel.c
> + *
> + * Client power state.
> + */
> enum vga_switcheroo_state {
> VGA_SWITCHEROO_OFF,
> VGA_SWITCHEROO_ON,
> @@ -22,20 +57,64 @@ enum vga_switcheroo_state {
> VGA_SWITCHEROO_NOT_FOUND,
> };
>
> +/**
> + * enum vga_switcheroo_client_id - client identifier
> + * @VGA_SWITCHEROO_IGD: integrated graphics device
> + * @VGA_SWITCHEROO_DIS: discrete graphics device
> + * @VGA_SWITCHEROO_MAX_CLIENTS: currently no more than two GPUs are supported
> + *
> + * Client identifier. Audio clients use the same identifier & 0x100.
> + */
> enum vga_switcheroo_client_id {
> VGA_SWITCHEROO_IGD,
> VGA_SWITCHEROO_DIS,
> VGA_SWITCHEROO_MAX_CLIENTS,
> };
>
> +/**
> + * struct vga_switcheroo_handler - handler callbacks
> + * @init: initialize handler.
> + * Optional. This gets called when vga_switcheroo is enabled, i.e. when
> + * two vga clients have registered. It allows the handler to perform
> + * some delayed initialization that depends on the existence of the
> + * vga clients. Currently only the radeon and amdgpu drivers use this.
> + * The return value is ignored
> + * @switchto: switch outputs to given client.
> + * Mandatory. For muxless machines this should be a no-op. Returning 0
> + * denotes success, anything else failure (in which case the switch is
> + * aborted)
> + * @power_state: cut or reinstate power of given client.
> + * Optional. The return value is ignored
> + * @get_client_id: determine if given pci device is integrated or discrete GPU.
> + * Mandatory
> + *
> + * Handler callbacks. The multiplexer itself. The @switchto and @get_client_id
> + * methods are mandatory, all others may be set to NULL.
> + */
> struct vga_switcheroo_handler {
> + int (*init)(void);
> int (*switchto)(enum vga_switcheroo_client_id id);
> int (*power_state)(enum vga_switcheroo_client_id id,
> enum vga_switcheroo_state state);
> - int (*init)(void);
> int (*get_client_id)(struct pci_dev *pdev);
> };
>
> +/**
> + * struct vga_switcheroo_client_ops - client callbacks
> + * @set_gpu_state: do the equivalent of suspend/resume for the card.
> + * Mandatory. This should not cut power to the discrete GPU,
> + * which is the job of the handler
> + * @reprobe: poll outputs.
> + * Optional. This gets called after waking the GPU and switching
> + * the outputs to it
> + * @can_switch: check if the device is in a position to switch now.
> + * Mandatory. The client should return false if a user space process
> + * has one of its device files open
> + *
> + * Client callbacks. A client can be either a GPU or an audio device on a GPU.
> + * The @set_gpu_state and @can_switch methods are mandatory, @reprobe may be
> + * set to NULL. For audio clients, the @reprobe member is bogus.
> + */
> struct vga_switcheroo_client_ops {
> void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state);
> void (*reprobe)(struct pci_dev *dev);
>
--
Danilo Cesar Lemes de Paula
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-08-23 21:23 ` [PATCH 04/15] vga_switcheroo: Add missing locking Lukas Wunner
2015-08-27 14:43 ` [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
@ 2015-09-17 17:15 ` Lukas Wunner
2015-09-22 9:17 ` Daniel Vetter
2015-09-19 16:44 ` Takashi Iwai
3 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-17 17:15 UTC (permalink / raw)
To: dri-devel
After sending the below patch away I realized that this change allows
us to simplify the code a bit by dropping the client_is_vga() call
from find_active_client().
I force-pushed an amended version of the commit to my vga_switcheroo_docs
branch on GitHub:
https://github.com/l1k/linux/commit/85dc1f299dec5a52266d82e2f4c02adfb04f77e6
On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> The active attribute in struct vga_switcheroo_client denotes whether
> the outputs are currently switched to this client. The attribute is
> only meaningful for vga clients. It is never used for audio clients.
>
> The function vga_switcheroo_register_audio_client() misuses this
> attribute to store whether the audio device is fully initialized.
> Most likely there was a misunderstanding about the meaning of
> "active" when this was added.
>
> Set the active attribute to false for audio clients. Remove the
> active parameter from vga_switcheroo_register_audio_client() and
> its sole caller, hda_intel.c:register_vga_switcheroo().
>
> vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> ("vga_switcheroo: Add the support for audio clients"). Its use in
> hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> VGA-switcheroo").
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> drivers/gpu/vga/vga_switcheroo.c | 5 ++---
> include/linux/vga_switcheroo.h | 4 ++--
> sound/pci/hda/hda_intel.c | 3 +--
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index b19a72f..fe32536 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> * @pdev: client pci device
> * @ops: client callbacks
> * @id: client identifier, see enum vga_switcheroo_client_id
> - * @active: whether the audio device is fully initialized
> *
> * Register audio client (audio device on a GPU). The power state of the
> * client is assumed to be ON.
> @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> */
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active)
> + int id)
> {
> - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> }
> EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index fe90bfc..3764991 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> bool driver_power_control);
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active);
> + int id);
>
> void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> struct fb_info *info);
> @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active) { return 0; }
> + int id) { return 0; }
> static inline void vga_switcheroo_unregister_handler(void) {}
> static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c38c68f..e819013 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> * is there any machine with two switchable HDMI audio controllers?
> */
> err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> - VGA_SWITCHEROO_DIS,
> - hda->probe_continued);
> + VGA_SWITCHEROO_DIS);
> if (err < 0)
> return err;
> hda->vga_switcheroo_registered = 1;
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
2015-09-17 16:34 ` [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Danilo Cesar Lemes de Paula
@ 2015-09-17 17:31 ` Lukas Wunner
2015-09-17 17:56 ` Danilo Cesar Lemes de Paula
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-17 17:31 UTC (permalink / raw)
To: Danilo Cesar Lemes de Paula; +Cc: dri-devel, Jonathan Corbet
Hi Danilo,
On Thu, Sep 17, 2015 at 01:34:54PM -0300, Danilo Cesar Lemes de Paula wrote:
> > - * GPU driver interface
> > - * - set_gpu_state - this should do the equiv of s/r for the card
> > - * - this should *not* set the discrete power state
>
> Did you check if this creates the desired format? (ie: Doesn't create a
> <code> block).
These lines are deleted by the patch.
One oddity I did notice however is that in the HTML documentation for the
public function vga_switcheroo_process_delayed_switch(), instead of the
heading "Description" it says "Manual switching and manual power control"
(which is the identifier of the preceding DOC section). I guess it's a bug
in kernel-doc but I couldn't be bothered so far to look into it. No idea
if it's introduced by your patches or if it was there all along.
Another thing I noticed is that URLs are not clickable in the HTML output.
Otherwise the markdown support works great (though I've only used it for
unsorted lists in this documentation).
Thanks,
Lukas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
2015-09-17 17:31 ` Lukas Wunner
@ 2015-09-17 17:56 ` Danilo Cesar Lemes de Paula
0 siblings, 0 replies; 37+ messages in thread
From: Danilo Cesar Lemes de Paula @ 2015-09-17 17:56 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel, Jonathan Corbet
On 09/17/2015 02:31 PM, Lukas Wunner wrote:
> Hi Danilo,
>
> On Thu, Sep 17, 2015 at 01:34:54PM -0300, Danilo Cesar Lemes de Paula wrote:
>>> - * GPU driver interface
>>> - * - set_gpu_state - this should do the equiv of s/r for the card
>>> - * - this should *not* set the discrete power state
>>
>> Did you check if this creates the desired format? (ie: Doesn't create a
>> <code> block).
>
> These lines are deleted by the patch.
Sorry, you're totally right.
>
> One oddity I did notice however is that in the HTML documentation for the
> public function vga_switcheroo_process_delayed_switch(), instead of the
> heading "Description" it says "Manual switching and manual power control"
> (which is the identifier of the preceding DOC section). I guess it's a bug
> in kernel-doc but I couldn't be bothered so far to look into it. No idea
> if it's introduced by your patches or if it was there all along.
That's odd... But I can have a look later.
>
> Another thing I noticed is that URLs are not clickable in the HTML output.
Did you tried the correct format? I just saw a regular http string,
which won't be converted. Markdown has this not obvious format for
links: This is a [Google](http://google.com) link
http://pandoc.org/try/?text=sudo+give+me+a+[link]%28http%3A%2F%2Fkernel.org%29%0A&from=markdown&to=docbook
>
> Otherwise the markdown support works great (though I've only used it for
> unsorted lists in this documentation).
>
> Thanks,
>
> Lukas
>
Danilo Cesar
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
` (2 preceding siblings ...)
2015-09-17 17:15 ` [PATCH " Lukas Wunner
@ 2015-09-19 16:44 ` Takashi Iwai
2015-09-19 17:50 ` Lukas Wunner
3 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2015-09-19 16:44 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Thu, 27 Aug 2015 16:43:43 +0200,
Lukas Wunner wrote:
>
> The active attribute in struct vga_switcheroo_client denotes whether
> the outputs are currently switched to this client. The attribute is
> only meaningful for vga clients. It is never used for audio clients.
>
> The function vga_switcheroo_register_audio_client() misuses this
> attribute to store whether the audio device is fully initialized.
> Most likely there was a misunderstanding about the meaning of
> "active" when this was added.
>
> Set the active attribute to false for audio clients. Remove the
> active parameter from vga_switcheroo_register_audio_client() and
> its sole caller, hda_intel.c:register_vga_switcheroo().
>
> vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> ("vga_switcheroo: Add the support for audio clients"). Its use in
> hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> VGA-switcheroo").
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Without the cover letter I can't judge why this change is needed, so
at the next time, please put relevant people to Cc of the cover letter
(or the whole series).
In anyway, I'm traveling now and can't review in details, so I'll
have a closer look in the next week after back from vacation.
thanks,
Takashi
> ---
> drivers/gpu/vga/vga_switcheroo.c | 5 ++---
> include/linux/vga_switcheroo.h | 4 ++--
> sound/pci/hda/hda_intel.c | 3 +--
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index b19a72f..fe32536 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> * @pdev: client pci device
> * @ops: client callbacks
> * @id: client identifier, see enum vga_switcheroo_client_id
> - * @active: whether the audio device is fully initialized
> *
> * Register audio client (audio device on a GPU). The power state of the
> * client is assumed to be ON.
> @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> */
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active)
> + int id)
> {
> - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> }
> EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index fe90bfc..3764991 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> bool driver_power_control);
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active);
> + int id);
>
> void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> struct fb_info *info);
> @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active) { return 0; }
> + int id) { return 0; }
> static inline void vga_switcheroo_unregister_handler(void) {}
> static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c38c68f..e819013 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> * is there any machine with two switchable HDMI audio controllers?
> */
> err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> - VGA_SWITCHEROO_DIS,
> - hda->probe_continued);
> + VGA_SWITCHEROO_DIS);
> if (err < 0)
> return err;
> hda->vga_switcheroo_registered = 1;
> --
> 1.8.5.2 (Apple Git-48)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-19 16:44 ` Takashi Iwai
@ 2015-09-19 17:50 ` Lukas Wunner
2015-09-24 9:39 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-09-19 17:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: dri-devel
Hi Takashi,
On Sat, Sep 19, 2015 at 06:44:58PM +0200, Takashi Iwai wrote:
> Without the cover letter I can't judge why this change is needed, so
> at the next time, please put relevant people to Cc of the cover letter
> (or the whole series).
There was no cover letter, aside from this reply to Daniel Vetter:
http://lists.freedesktop.org/archives/dri-devel/2015-September/090510.html
Basically he had asked for vga_switcheroo docs, which I've now written,
and in course of doing that I fixed some bugs and cosmetic issues.
I cc'ed you on the docs patch and all the patches which touch ALSA code
so that you get a chance to raise objections (or ack the patches should
you have none). Full patch set is here:
https://github.com/l1k/linux/commits/vga_switcheroo_docs
The patch you've replied to (03/15 "vga_switcheroo: Set active attribute
to false for audio clients") fixes a misuse of the "active" attribute in
struct vga_switcheroo_client by audio clients. The attribute is never
used for audio clients, so there are no behavioral changes with this patch,
it only removes superfluous/confusing code.
Thanks,
Lukas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide
2015-08-29 12:29 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Lukas Wunner
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
@ 2015-09-22 8:38 ` Daniel Vetter
1 sibling, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-09-22 8:38 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Danilo Cesar Lemes de Paula, dri-devel, Jonathan Corbet
On Sat, Aug 29, 2015 at 02:29:03PM +0200, Lukas Wunner wrote:
> This is not part of drm.tmpl as vga_switcheroo is a subsystem of its own
> which interfaces not just with DRM but also with multiplexer drivers,
> ALSA and power management.
I still think this would be better served included in the DRM docbook,
simply because doing that will increase the changes it's read. And if the
audio folks want to, they can pull in the same text somewhere in the audio
docs too.
I merged patch 1 meanwhile, thanks.
-Daniel
>
> Requires Markdown support.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> Documentation/DocBook/Makefile | 5 +-
> Documentation/DocBook/vga_switcheroo.tmpl | 92 +++++++++++++++++++++++++++++++
> 2 files changed, 95 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/DocBook/vga_switcheroo.tmpl
>
> diff --git a/Documentation/DocBook/Makefile b/Documentation/DocBook/Makefile
> index 8276944..4495b37 100644
> --- a/Documentation/DocBook/Makefile
> +++ b/Documentation/DocBook/Makefile
> @@ -15,9 +15,10 @@ DOCBOOKS := z8530book.xml device-drivers.xml \
> 80211.xml debugobjects.xml sh.xml regulator.xml \
> alsa-driver-api.xml writing-an-alsa-driver.xml \
> tracepoint.xml drm.xml media_api.xml w1.xml \
> - writing_musb_glue_layer.xml crypto-API.xml
> + writing_musb_glue_layer.xml crypto-API.xml \
> + vga_switcheroo.xml
>
> -MARKDOWNREADY :=
> +MARKDOWNREADY := vga_switcheroo.xml
>
> include Documentation/DocBook/media/Makefile
>
> diff --git a/Documentation/DocBook/vga_switcheroo.tmpl b/Documentation/DocBook/vga_switcheroo.tmpl
> new file mode 100644
> index 0000000..e6128e7
> --- /dev/null
> +++ b/Documentation/DocBook/vga_switcheroo.tmpl
> @@ -0,0 +1,92 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
> + "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
> +
> +<book id="vga_switcheroo">
> + <bookinfo>
> + <title>vga_switcheroo Subsystem Guide</title>
> +
> + <authorgroup>
> + <author>
> + <firstname>Lukas</firstname>
> + <surname>Wunner</surname>
> + <contrib>Initial version</contrib>
> + <affiliation>
> + <address>
> + <email>lukas@wunner.de</email>
> + </address>
> + </affiliation>
> + </author>
> + </authorgroup>
> +
> + <copyright>
> + <year>2015</year>
> + <holder>Lukas Wunner</holder>
> + </copyright>
> +
> + <legalnotice>
> + <para>
> + The contents of this file may be used under the terms of the GNU
> + General Public License version 2 (the "GPL") as distributed in
> + the kernel source COPYING file.
> + </para>
> + </legalnotice>
> +
> + <revhistory>
> + <!-- Put document revisions here, newest first. -->
> + <revision>
> + <revnumber>1.0</revnumber>
> + <date>2015-08-29</date>
> + <authorinitials>LW</authorinitials>
> + <revremark>Initial version
> + </revremark>
> + </revision>
> + </revhistory>
> + </bookinfo>
> +
> +<toc></toc>
> +
> + <chapter id="overview">
> + <title>Overview</title>
> +!Pdrivers/gpu/vga/vga_switcheroo.c Overview
> + </chapter>
> +
> + <chapter id="modes_of_use">
> + <title>Modes of Use</title>
> + <sect1>
> + <title>Manual switching and manual power control</title>
> +!Pdrivers/gpu/vga/vga_switcheroo.c Manual switching and manual power control
> + </sect1>
> + <sect1>
> + <title>Driver power control</title>
> +!Pdrivers/gpu/vga/vga_switcheroo.c Driver power control
> + </sect1>
> + </chapter>
> +
> + <chapter id="pubfunctions">
> + <title>Public functions</title>
> +!Edrivers/gpu/vga/vga_switcheroo.c
> + </chapter>
> +
> + <chapter id="pubstructures">
> + <title>Public structures</title>
> +!Finclude/linux/vga_switcheroo.h vga_switcheroo_handler
> +!Finclude/linux/vga_switcheroo.h vga_switcheroo_client_ops
> + </chapter>
> +
> + <chapter id="pubconstants">
> + <title>Public constants</title>
> +!Finclude/linux/vga_switcheroo.h vga_switcheroo_client_id
> +!Finclude/linux/vga_switcheroo.h vga_switcheroo_state
> + </chapter>
> +
> + <chapter id="privstructures">
> + <title>Private structures</title>
> +!Fdrivers/gpu/vga/vga_switcheroo.c vgasr_priv
> +!Fdrivers/gpu/vga/vga_switcheroo.c vga_switcheroo_client
> + </chapter>
> +
> +!Cdrivers/gpu/vga/vga_switcheroo.c
> +!Cinclude/linux/vga_switcheroo.h
> +
> +</book>
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-17 17:15 ` [PATCH " Lukas Wunner
@ 2015-09-22 9:17 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-09-22 9:17 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Thu, Sep 17, 2015 at 07:15:59PM +0200, Lukas Wunner wrote:
> After sending the below patch away I realized that this change allows
> us to simplify the code a bit by dropping the client_is_vga() call
> from find_active_client().
>
> I force-pushed an amended version of the commit to my vga_switcheroo_docs
> branch on GitHub:
> https://github.com/l1k/linux/commit/85dc1f299dec5a52266d82e2f4c02adfb04f77e6
Please resubmit just the changed patche in-reply the previous version,
with an updated commit message.
Thanks, Daniel
>
>
> On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> > The active attribute in struct vga_switcheroo_client denotes whether
> > the outputs are currently switched to this client. The attribute is
> > only meaningful for vga clients. It is never used for audio clients.
> >
> > The function vga_switcheroo_register_audio_client() misuses this
> > attribute to store whether the audio device is fully initialized.
> > Most likely there was a misunderstanding about the meaning of
> > "active" when this was added.
> >
> > Set the active attribute to false for audio clients. Remove the
> > active parameter from vga_switcheroo_register_audio_client() and
> > its sole caller, hda_intel.c:register_vga_switcheroo().
> >
> > vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> > ("vga_switcheroo: Add the support for audio clients"). Its use in
> > hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> > VGA-switcheroo").
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> > drivers/gpu/vga/vga_switcheroo.c | 5 ++---
> > include/linux/vga_switcheroo.h | 4 ++--
> > sound/pci/hda/hda_intel.c | 3 +--
> > 3 files changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > index b19a72f..fe32536 100644
> > --- a/drivers/gpu/vga/vga_switcheroo.c
> > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > * @pdev: client pci device
> > * @ops: client callbacks
> > * @id: client identifier, see enum vga_switcheroo_client_id
> > - * @active: whether the audio device is fully initialized
> > *
> > * Register audio client (audio device on a GPU). The power state of the
> > * client is assumed to be ON.
> > @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > */
> > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active)
> > + int id)
> > {
> > - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> > + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> > }
> > EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
> >
> > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > index fe90bfc..3764991 100644
> > --- a/include/linux/vga_switcheroo.h
> > +++ b/include/linux/vga_switcheroo.h
> > @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> > bool driver_power_control);
> > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active);
> > + int id);
> >
> > void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> > struct fb_info *info);
> > @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> > static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> > static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active) { return 0; }
> > + int id) { return 0; }
> > static inline void vga_switcheroo_unregister_handler(void) {}
> > static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> > static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index c38c68f..e819013 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> > * is there any machine with two switchable HDMI audio controllers?
> > */
> > err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> > - VGA_SWITCHEROO_DIS,
> > - hda->probe_continued);
> > + VGA_SWITCHEROO_DIS);
> > if (err < 0)
> > return err;
> > hda->vga_switcheroo_registered = 1;
> > --
> > 1.8.5.2 (Apple Git-48)
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 11/15] drm/i915: Spell vga_switcheroo consistently
2015-09-04 19:06 ` [PATCH 11/15] drm/i915: " Lukas Wunner
2015-09-05 9:09 ` [PATCH 12/15] drm/nouveau: " Lukas Wunner
@ 2015-09-22 9:20 ` Daniel Vetter
1 sibling, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-09-22 9:20 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Fri, Sep 04, 2015 at 09:06:15PM +0200, Lukas Wunner wrote:
> Currently everyone and their dog has their own favourite spelling
> for vga_switcheroo. This makes it hard to grep dmesg for log entries
> relating to vga_switcheroo. It also makes it hard to find related
> source files in the tree.
>
> vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_acpi.c | 2 +-
> drivers/gpu/drm/i915/intel_panel.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index d96eee1..8b13b9d 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -146,7 +146,7 @@ static bool intel_dsm_detect(void)
>
> if (vga_count == 2 && has_dsm) {
> acpi_get_name(intel_dsm_priv.dhandle, ACPI_FULL_PATHNAME, &buffer);
> - DRM_DEBUG_DRIVER("VGA switcheroo: detected DSM switching method %s handle\n",
> + DRM_DEBUG_DRIVER("vga_switcheroo: detected DSM switching method %s handle\n",
> acpi_method_name);
> return true;
> }
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e2ab3f6..04f7d8e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -809,7 +809,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
> return;
>
> /*
> - * Do not disable backlight on the vgaswitcheroo path. When switching
> + * Do not disable backlight on the vga_switcheroo path. When switching
> * away from i915, the other client may depend on i915 to handle the
> * backlight. This will leave the backlight on unnecessarily when
> * another client is not activated.
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 15/15] drm: Spell vga_switcheroo consistently
2015-09-05 9:22 ` [PATCH 15/15] drm: " Lukas Wunner
@ 2015-09-22 9:20 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-09-22 9:20 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Sat, Sep 05, 2015 at 11:22:39AM +0200, Lukas Wunner wrote:
> Currently everyone and their dog has their own favourite spelling
> for vga_switcheroo. This makes it hard to grep dmesg for log entries
> relating to vga_switcheroo. It also makes it hard to find related
> source files in the tree.
>
> vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Applied to drm-misc, thanks.
-Daniel
> ---
> Documentation/DocBook/drm.tmpl | 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
> include/linux/fb.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 9ddf8c6..30401f9 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -3646,7 +3646,7 @@ void (*postclose) (struct drm_device *, struct drm_file *);</synopsis>
> plane properties to default value, so that a subsequent open of the
> device will not inherit state from the previous user. It can also be
> used to execute delayed power switching state changes, e.g. in
> - conjunction with the vga-switcheroo infrastructure. Beyond that KMS
> + conjunction with the vga_switcheroo infrastructure. Beyond that KMS
> drivers should not do any further cleanup. Only legacy UMS drivers might
> need to clean up device state so that the vga console or an independent
> fbdev driver could take over.
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index a5f9d8b..d685e23 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -753,7 +753,7 @@ static void dev_lastclose(struct drm_device *dev)
> {
> int i;
>
> - /* we don't support vga-switcheroo.. so just make sure the fbdev
> + /* we don't support vga_switcheroo.. so just make sure the fbdev
> * mode is active
> */
> struct omap_drm_private *priv = dev->dev_private;
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 043f328..36da20a 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -156,7 +156,7 @@ struct fb_cursor_user {
> #define FB_EVENT_GET_REQ 0x0D
> /* Unbind from the console if possible */
> #define FB_EVENT_FB_UNBIND 0x0E
> -/* CONSOLE-SPECIFIC: remap all consoles to new fb - for vga switcheroo */
> +/* CONSOLE-SPECIFIC: remap all consoles to new fb - for vga_switcheroo */
> #define FB_EVENT_REMAP_ALL_CONSOLE 0x0F
> /* A hardware display blank early change occured */
> #define FB_EARLY_EVENT_BLANK 0x10
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-27 14:43 ` [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
@ 2015-09-23 7:13 ` Daniel Vetter
2015-09-24 9:40 ` Takashi Iwai
2015-09-24 9:37 ` Takashi Iwai
1 sibling, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-09-23 7:13 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Daniel Vetter, dri-devel
On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> The active attribute in struct vga_switcheroo_client denotes whether
> the outputs are currently switched to this client. The attribute is
> only meaningful for vga clients. It is never used for audio clients.
>
> The function vga_switcheroo_register_audio_client() misuses this
> attribute to store whether the audio device is fully initialized.
> Most likely there was a misunderstanding about the meaning of
> "active" when this was added.
>
> Set the active attribute to false for audio clients. Remove the
> active parameter from vga_switcheroo_register_audio_client() and
> its sole caller, hda_intel.c:register_vga_switcheroo().
>
> vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> ("vga_switcheroo: Add the support for audio clients"). Its use in
> hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> VGA-switcheroo").
>
> v1.1: The changes above imply that in find_active_client() the call
> to client_is_vga() is now superfluous. Drop it.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Takashi, can you pls ack this for merging through drm-misc? Patch lgtm.
-Daniel
> ---
> drivers/gpu/vga/vga_switcheroo.c | 7 +++----
> include/linux/vga_switcheroo.h | 4 ++--
> sound/pci/hda/hda_intel.c | 3 +--
> 3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 2e7e2f8e..563b82f 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> * @pdev: client pci device
> * @ops: client callbacks
> * @id: client identifier, see enum vga_switcheroo_client_id
> - * @active: whether the audio device is fully initialized
> *
> * Register audio client (audio device on a GPU). The power state of the
> * client is assumed to be ON.
> @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> */
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active)
> + int id)
> {
> - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> }
> EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>
> @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
> struct vga_switcheroo_client *client;
>
> list_for_each_entry(client, head, list)
> - if (client->active && client_is_vga(client))
> + if (client->active)
> return client;
> return NULL;
> }
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index fe90bfc..3764991 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> bool driver_power_control);
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active);
> + int id);
>
> void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> struct fb_info *info);
> @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active) { return 0; }
> + int id) { return 0; }
> static inline void vga_switcheroo_unregister_handler(void) {}
> static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c38c68f..e819013 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> * is there any machine with two switchable HDMI audio controllers?
> */
> err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> - VGA_SWITCHEROO_DIS,
> - hda->probe_continued);
> + VGA_SWITCHEROO_DIS);
> if (err < 0)
> return err;
> hda->vga_switcheroo_registered = 1;
> --
> 1.8.5.2 (Apple Git-48)
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 14/15] drm/amdgpu: Spell vga_switcheroo consistently
2015-09-05 9:17 ` [PATCH 14/15] drm/amdgpu: " Lukas Wunner
2015-09-05 9:22 ` [PATCH 15/15] drm: " Lukas Wunner
@ 2015-09-23 21:45 ` Alex Deucher
1 sibling, 0 replies; 37+ messages in thread
From: Alex Deucher @ 2015-09-23 21:45 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Maling list - DRI developers
On Sat, Sep 5, 2015 at 5:17 AM, Lukas Wunner <lukas@wunner.de> wrote:
> Currently everyone and their dog has their own favourite spelling
> for vga_switcheroo. This makes it hard to grep dmesg for log entries
> relating to vga_switcheroo. It also makes it hard to find related
> source files in the tree.
>
> vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Applied patches 13 and 14.
Thanks,
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index 3f7aaa4..1a6b239 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -536,7 +536,7 @@ static bool amdgpu_atpx_detect(void)
>
> if (has_atpx && vga_count == 2) {
> acpi_get_name(amdgpu_atpx_priv.atpx.handle, ACPI_FULL_PATHNAME, &buffer);
> - printk(KERN_INFO "VGA switcheroo: detected switching method %s handle\n",
> + printk(KERN_INFO "vga_switcheroo: detected switching method %s handle\n",
> acpi_method_name);
> amdgpu_atpx_priv.atpx_detected = true;
> return true;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 6ff6ae9..b1a08d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1021,7 +1021,7 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
> * amdgpu_switcheroo_set_state - set switcheroo state
> *
> * @pdev: pci dev pointer
> - * @state: vga switcheroo state
> + * @state: vga_switcheroo state
> *
> * Callback for the switcheroo driver. Suspends or resumes the
> * the asics before or after it is powered up using ACPI methods.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 2236793..a60eb4b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -488,7 +488,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> *
> * @dev: drm dev pointer
> *
> - * Switch vga switcheroo state after last close (all asics).
> + * Switch vga_switcheroo state after last close (all asics).
> */
> void amdgpu_driver_lastclose_kms(struct drm_device *dev)
> {
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-08-27 14:43 ` [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-09-23 7:13 ` Daniel Vetter
@ 2015-09-24 9:37 ` Takashi Iwai
1 sibling, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2015-09-24 9:37 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Daniel Vetter, dri-devel
On Thu, 27 Aug 2015 16:43:43 +0200,
Lukas Wunner wrote:
>
> The active attribute in struct vga_switcheroo_client denotes whether
> the outputs are currently switched to this client. The attribute is
> only meaningful for vga clients. It is never used for audio clients.
>
> The function vga_switcheroo_register_audio_client() misuses this
> attribute to store whether the audio device is fully initialized.
> Most likely there was a misunderstanding about the meaning of
> "active" when this was added.
Not really. The full initialization of audio was meant that the audio
is active indeed. Admittedly, though, the active flag for each audio
client doesn't play any role because the audio always follows the gfx
state changes, and the value passed there doesn't reflect the actual
state due to the later change. So, I agree with the removal of the
flag itself -- or let the audio active flag following the
corresponding gfx flag. The latter will make the proc output more
consistent while the former is certainly more reduction of code.
thanks,
Takashi
> Set the active attribute to false for audio clients. Remove the
> active parameter from vga_switcheroo_register_audio_client() and
> its sole caller, hda_intel.c:register_vga_switcheroo().
>
> vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> ("vga_switcheroo: Add the support for audio clients"). Its use in
> hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> VGA-switcheroo").
>
> v1.1: The changes above imply that in find_active_client() the call
> to client_is_vga() is now superfluous. Drop it.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> drivers/gpu/vga/vga_switcheroo.c | 7 +++----
> include/linux/vga_switcheroo.h | 4 ++--
> sound/pci/hda/hda_intel.c | 3 +--
> 3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 2e7e2f8e..563b82f 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> * @pdev: client pci device
> * @ops: client callbacks
> * @id: client identifier, see enum vga_switcheroo_client_id
> - * @active: whether the audio device is fully initialized
> *
> * Register audio client (audio device on a GPU). The power state of the
> * client is assumed to be ON.
> @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> */
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active)
> + int id)
> {
> - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> }
> EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>
> @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
> struct vga_switcheroo_client *client;
>
> list_for_each_entry(client, head, list)
> - if (client->active && client_is_vga(client))
> + if (client->active)
> return client;
> return NULL;
> }
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index fe90bfc..3764991 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> bool driver_power_control);
> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active);
> + int id);
>
> void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> struct fb_info *info);
> @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> const struct vga_switcheroo_client_ops *ops,
> - int id, bool active) { return 0; }
> + int id) { return 0; }
> static inline void vga_switcheroo_unregister_handler(void) {}
> static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c38c68f..e819013 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> * is there any machine with two switchable HDMI audio controllers?
> */
> err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> - VGA_SWITCHEROO_DIS,
> - hda->probe_continued);
> + VGA_SWITCHEROO_DIS);
> if (err < 0)
> return err;
> hda->vga_switcheroo_registered = 1;
> --
> 1.8.5.2 (Apple Git-48)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-19 17:50 ` Lukas Wunner
@ 2015-09-24 9:39 ` Takashi Iwai
0 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2015-09-24 9:39 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Sat, 19 Sep 2015 19:50:12 +0200,
Lukas Wunner wrote:
>
> Hi Takashi,
>
> On Sat, Sep 19, 2015 at 06:44:58PM +0200, Takashi Iwai wrote:
> > Without the cover letter I can't judge why this change is needed, so
> > at the next time, please put relevant people to Cc of the cover letter
> > (or the whole series).
>
> There was no cover letter, aside from this reply to Daniel Vetter:
> http://lists.freedesktop.org/archives/dri-devel/2015-September/090510.html
Then you should have had it :) A big patch series like this should
always have a cover letter. Otherwise stray sheep like me gets lost
easily when Cc'ed out of sudden.
Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-23 7:13 ` Daniel Vetter
@ 2015-09-24 9:40 ` Takashi Iwai
2015-09-24 15:13 ` Daniel Vetter
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2015-09-24 9:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
On Wed, 23 Sep 2015 09:13:28 +0200,
Daniel Vetter wrote:
>
> On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> > The active attribute in struct vga_switcheroo_client denotes whether
> > the outputs are currently switched to this client. The attribute is
> > only meaningful for vga clients. It is never used for audio clients.
> >
> > The function vga_switcheroo_register_audio_client() misuses this
> > attribute to store whether the audio device is fully initialized.
> > Most likely there was a misunderstanding about the meaning of
> > "active" when this was added.
> >
> > Set the active attribute to false for audio clients. Remove the
> > active parameter from vga_switcheroo_register_audio_client() and
> > its sole caller, hda_intel.c:register_vga_switcheroo().
> >
> > vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> > ("vga_switcheroo: Add the support for audio clients"). Its use in
> > hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> > VGA-switcheroo").
> >
> > v1.1: The changes above imply that in find_active_client() the call
> > to client_is_vga() is now superfluous. Drop it.
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
>
> Takashi, can you pls ack this for merging through drm-misc? Patch lgtm.
For the code change, feel free to take my ack:
Reviewed-by: Takashi Iwai <tiwai@suse.de>
But it'd be better if the changelog can be rephrased a bit.
thanks,
Takashi
> -Daniel
>
> > ---
> > drivers/gpu/vga/vga_switcheroo.c | 7 +++----
> > include/linux/vga_switcheroo.h | 4 ++--
> > sound/pci/hda/hda_intel.c | 3 +--
> > 3 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > index 2e7e2f8e..563b82f 100644
> > --- a/drivers/gpu/vga/vga_switcheroo.c
> > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > * @pdev: client pci device
> > * @ops: client callbacks
> > * @id: client identifier, see enum vga_switcheroo_client_id
> > - * @active: whether the audio device is fully initialized
> > *
> > * Register audio client (audio device on a GPU). The power state of the
> > * client is assumed to be ON.
> > @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > */
> > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active)
> > + int id)
> > {
> > - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> > + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> > }
> > EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
> >
> > @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
> > struct vga_switcheroo_client *client;
> >
> > list_for_each_entry(client, head, list)
> > - if (client->active && client_is_vga(client))
> > + if (client->active)
> > return client;
> > return NULL;
> > }
> > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > index fe90bfc..3764991 100644
> > --- a/include/linux/vga_switcheroo.h
> > +++ b/include/linux/vga_switcheroo.h
> > @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> > bool driver_power_control);
> > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active);
> > + int id);
> >
> > void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> > struct fb_info *info);
> > @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> > static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> > static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > const struct vga_switcheroo_client_ops *ops,
> > - int id, bool active) { return 0; }
> > + int id) { return 0; }
> > static inline void vga_switcheroo_unregister_handler(void) {}
> > static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> > static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index c38c68f..e819013 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> > * is there any machine with two switchable HDMI audio controllers?
> > */
> > err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> > - VGA_SWITCHEROO_DIS,
> > - hda->probe_continued);
> > + VGA_SWITCHEROO_DIS);
> > if (err < 0)
> > return err;
> > hda->vga_switcheroo_registered = 1;
> > --
> > 1.8.5.2 (Apple Git-48)
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-24 9:40 ` Takashi Iwai
@ 2015-09-24 15:13 ` Daniel Vetter
2015-10-01 16:19 ` Lukas Wunner
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-09-24 15:13 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Daniel Vetter, dri-devel
On Thu, Sep 24, 2015 at 11:40:52AM +0200, Takashi Iwai wrote:
> On Wed, 23 Sep 2015 09:13:28 +0200,
> Daniel Vetter wrote:
> >
> > On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> > > The active attribute in struct vga_switcheroo_client denotes whether
> > > the outputs are currently switched to this client. The attribute is
> > > only meaningful for vga clients. It is never used for audio clients.
> > >
> > > The function vga_switcheroo_register_audio_client() misuses this
> > > attribute to store whether the audio device is fully initialized.
> > > Most likely there was a misunderstanding about the meaning of
> > > "active" when this was added.
> > >
> > > Set the active attribute to false for audio clients. Remove the
> > > active parameter from vga_switcheroo_register_audio_client() and
> > > its sole caller, hda_intel.c:register_vga_switcheroo().
> > >
> > > vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> > > ("vga_switcheroo: Add the support for audio clients"). Its use in
> > > hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> > > VGA-switcheroo").
> > >
> > > v1.1: The changes above imply that in find_active_client() the call
> > > to client_is_vga() is now superfluous. Drop it.
> > >
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> >
> > Takashi, can you pls ack this for merging through drm-misc? Patch lgtm.
>
> For the code change, feel free to take my ack:
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
>
> But it'd be better if the changelog can be rephrased a bit.
I added your clarification to the commit message and pulled in the patch.
Thanks, Daniel
>
>
> thanks,
>
> Takashi
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/vga/vga_switcheroo.c | 7 +++----
> > > include/linux/vga_switcheroo.h | 4 ++--
> > > sound/pci/hda/hda_intel.c | 3 +--
> > > 3 files changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > > index 2e7e2f8e..563b82f 100644
> > > --- a/drivers/gpu/vga/vga_switcheroo.c
> > > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > > @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > > * @pdev: client pci device
> > > * @ops: client callbacks
> > > * @id: client identifier, see enum vga_switcheroo_client_id
> > > - * @active: whether the audio device is fully initialized
> > > *
> > > * Register audio client (audio device on a GPU). The power state of the
> > > * client is assumed to be ON.
> > > @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > > */
> > > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active)
> > > + int id)
> > > {
> > > - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> > > + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> > > }
> > > EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
> > >
> > > @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
> > > struct vga_switcheroo_client *client;
> > >
> > > list_for_each_entry(client, head, list)
> > > - if (client->active && client_is_vga(client))
> > > + if (client->active)
> > > return client;
> > > return NULL;
> > > }
> > > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > > index fe90bfc..3764991 100644
> > > --- a/include/linux/vga_switcheroo.h
> > > +++ b/include/linux/vga_switcheroo.h
> > > @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> > > bool driver_power_control);
> > > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active);
> > > + int id);
> > >
> > > void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> > > struct fb_info *info);
> > > @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> > > static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> > > static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active) { return 0; }
> > > + int id) { return 0; }
> > > static inline void vga_switcheroo_unregister_handler(void) {}
> > > static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> > > static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index c38c68f..e819013 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> > > * is there any machine with two switchable HDMI audio controllers?
> > > */
> > > err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> > > - VGA_SWITCHEROO_DIS,
> > > - hda->probe_continued);
> > > + VGA_SWITCHEROO_DIS);
> > > if (err < 0)
> > > return err;
> > > hda->vga_switcheroo_registered = 1;
> > > --
> > > 1.8.5.2 (Apple Git-48)
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients
2015-09-24 15:13 ` Daniel Vetter
@ 2015-10-01 16:19 ` Lukas Wunner
0 siblings, 0 replies; 37+ messages in thread
From: Lukas Wunner @ 2015-10-01 16:19 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel
Hi Daniel,
On Thu, Sep 24, 2015 at 05:13:02PM +0200, Daniel Vetter wrote:
> I added your clarification to the commit message and pulled in the patch.
As requested, here's an on-list gentle reminder that a portion of
this series hasn't been merged yet, specifically:
[PATCH 04/15] vga_switcheroo: Add missing locking
[PATCH 05/15] vga_switcheroo: Drop client power state
[PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead
[PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead
[PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id
[PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently
[PATCH 12/15] drm/nouveau: Spell vga_switcheroo consistently
In the case of patch 10, Takashi Iwai has suggested off-list that it be
merged through drm-misc and has kindly offered his
Reviewed-by: Takashi Iwai <tiwai@suse.de>
In the case of patch 12, I guess it may be merged through darktama's repo.
The patches were submitted to dri-devel September 17 and are also
available on GitHub for easier reviewing:
https://github.com/l1k/linux/commits/vga_switcheroo_docs
If anyone has comments or objections please come forward.
Thank you,
Lukas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int
2015-08-28 9:56 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Lukas Wunner
2015-08-28 11:30 ` [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1 Lukas Wunner
@ 2015-10-02 7:35 ` Jani Nikula
1 sibling, 0 replies; 37+ messages in thread
From: Jani Nikula @ 2015-10-02 7:35 UTC (permalink / raw)
To: Lukas Wunner, dri-devel
On Fri, 28 Aug 2015, Lukas Wunner <lukas@wunner.de> wrote:
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/vga/vga_switcheroo.c | 6 +++---
> include/linux/vga_switcheroo.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 845e47d..7b53246 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -102,7 +102,7 @@
> struct vga_switcheroo_client {
> struct pci_dev *pdev;
> struct fb_info *fb_info;
> - int pwr_state;
> + enum vga_switcheroo_state pwr_state;
> const struct vga_switcheroo_client_ops *ops;
> int id;
> bool active;
> @@ -346,7 +346,7 @@ find_active_client(struct list_head *head)
> *
> * Return: Power state.
> */
> -int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> +enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *pdev)
> {
> struct vga_switcheroo_client *client;
> enum vga_switcheroo_state ret;
> @@ -496,7 +496,7 @@ static int vga_switchoff(struct vga_switcheroo_client *client)
> return 0;
> }
>
> -static void set_audio_state(int id, int state)
> +static void set_audio_state(int id, enum vga_switcheroo_state state)
> {
> struct vga_switcheroo_client *client;
>
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index 95bfbeb0..219876e 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -133,7 +133,7 @@ void vga_switcheroo_unregister_handler(void);
>
> int vga_switcheroo_process_delayed_switch(void);
>
> -int vga_switcheroo_get_client_state(struct pci_dev *dev);
> +enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev);
>
> void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic);
>
> @@ -152,7 +152,7 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> int id) { return 0; }
> static inline void vga_switcheroo_unregister_handler(void) {}
> static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> -static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> +static inline enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
>
> static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}
>
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT
2015-09-08 12:17 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Lukas Wunner
2015-08-28 9:56 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Lukas Wunner
@ 2015-10-02 8:22 ` Daniel Vetter
2015-10-25 18:19 ` Lukas Wunner
1 sibling, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-10-02 8:22 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Tue, Sep 08, 2015 at 02:17:47PM +0200, Lukas Wunner wrote:
> hda_intel.c:azx_probe() defers initialization of an audio controller
> on the discrete GPU if the GPU is powered off. The power state of the
> GPU is determined by calling vga_switcheroo_get_client_state().
>
> vga_switcheroo_get_client_state() returns VGA_SWITCHEROO_INIT if
> vga_switcheroo is not enabled, i.e. if no second GPU or no handler
> has registered.
>
> This can go wrong in the following scenario:
> - Driver for the integrated GPU is not loaded.
> - Driver for the discrete GPU registers with vga_switcheroo, uses driver
> power control to power down the GPU, handler cuts power to the GPU.
> - Driver for the audio controller gets loaded after the GPU was powered
> down, calls vga_switcheroo_get_client_state() which returns
> VGA_SWITCHEROO_INIT instead of VGA_SWITCHEROO_OFF.
> - Consequence: azx_probe() tries to initialize the audio controller even
> though the GPU is powered down.
>
> The power state VGA_SWITCHEROO_INIT was introduced by c8e9cf7bb240
> ("vga_switcheroo: Add a helper function to get the client state").
> It is not apparent what its benefit might be. The idea seems to
> be to initialize the audio controller even if the power state is
> VGA_SWITCHEROO_OFF (were vga_switcheroo enabled), but as shown
> above this can fail.
>
> Drop VGA_SWITCHEROO_INIT to solve this.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Takashi, does this look good to you? Ack for merging through drm-misc?
I pulled in patch 4 meanwhile.
-Daniel
> ---
> drivers/gpu/vga/vga_switcheroo.c | 2 --
> include/linux/vga_switcheroo.h | 5 -----
> 2 files changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 2138162..845e47d 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -355,8 +355,6 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> client = find_client_from_pci(&vgasr_priv.clients, pdev);
> if (!client)
> ret = VGA_SWITCHEROO_NOT_FOUND;
> - else if (!vgasr_priv.active)
> - ret = VGA_SWITCHEROO_INIT;
> else
> ret = client->pwr_state;
> mutex_unlock(&vgasr_mutex);
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index 3764991..95bfbeb0 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -39,10 +39,6 @@ struct pci_dev;
> * enum vga_switcheroo_state - client power state
> * @VGA_SWITCHEROO_OFF: off
> * @VGA_SWITCHEROO_ON: on
> - * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
> - * vga_switcheroo is not enabled, i.e. no second client or no handler
> - * has registered. Only used in vga_switcheroo_get_client_state() which
> - * in turn is only called from hda_intel.c
> * @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
> * Only used in vga_switcheroo_get_client_state() which in turn is only
> * called from hda_intel.c
> @@ -53,7 +49,6 @@ enum vga_switcheroo_state {
> VGA_SWITCHEROO_OFF,
> VGA_SWITCHEROO_ON,
> /* below are referred only from vga_switcheroo_get_client_state() */
> - VGA_SWITCHEROO_INIT,
> VGA_SWITCHEROO_NOT_FOUND,
> };
>
> --
> 1.8.5.2 (Apple Git-48)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT
2015-10-02 8:22 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Daniel Vetter
@ 2015-10-25 18:19 ` Lukas Wunner
2015-10-26 7:24 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: Lukas Wunner @ 2015-10-25 18:19 UTC (permalink / raw)
To: Takashi Iwai; +Cc: dri-devel
Hi Takashi,
gentle ping -- could you take a look at the patch below,
would this change be okay with you?
hda_intel.c:check_hdmi_disabled() is the sole caller of
vga_switcheroo_get_client_state() and only checks for the
return value VGA_SWITCHEROO_OFF, not for VGA_SWITCHEROO_INIT.
So why was VGA_SWITCHEROO_INIT introduced in the first place?
The only explanation I can think of is that for some reason you
wanted to *avoid* returning VGA_SWITCHEROO_OFF (even if the card
is in fact asleep) if no second GPU or no handler have registered
with vga_switcheroo. But as shown in the scenario below, this can
actually go awry.
With VGA_SWITCHEROO_INIT apparently not having a purpose but on the
other hand being harmful, I suggest to drop it with the patch below.
For the meaning of the vga_switcheroo power states, refer to
vga_switcheroo.h in drm-next:
http://cgit.freedesktop.org/~airlied/linux/tree/include/linux/vga_switcheroo.h?h=drm-next#n38
Thanks & best regards,
Lukas
On Fri, Oct 02, 2015 at 10:22:48AM +0200, Daniel Vetter wrote:
> On Tue, Sep 08, 2015 at 02:17:47PM +0200, Lukas Wunner wrote:
> > hda_intel.c:azx_probe() defers initialization of an audio controller
> > on the discrete GPU if the GPU is powered off. The power state of the
> > GPU is determined by calling vga_switcheroo_get_client_state().
> >
> > vga_switcheroo_get_client_state() returns VGA_SWITCHEROO_INIT if
> > vga_switcheroo is not enabled, i.e. if no second GPU or no handler
> > has registered.
> >
> > This can go wrong in the following scenario:
> > - Driver for the integrated GPU is not loaded.
> > - Driver for the discrete GPU registers with vga_switcheroo, uses driver
> > power control to power down the GPU, handler cuts power to the GPU.
> > - Driver for the audio controller gets loaded after the GPU was powered
> > down, calls vga_switcheroo_get_client_state() which returns
> > VGA_SWITCHEROO_INIT instead of VGA_SWITCHEROO_OFF.
> > - Consequence: azx_probe() tries to initialize the audio controller even
> > though the GPU is powered down.
> >
> > The power state VGA_SWITCHEROO_INIT was introduced by c8e9cf7bb240
> > ("vga_switcheroo: Add a helper function to get the client state").
> > It is not apparent what its benefit might be. The idea seems to
> > be to initialize the audio controller even if the power state is
> > VGA_SWITCHEROO_OFF (were vga_switcheroo enabled), but as shown
> > above this can fail.
> >
> > Drop VGA_SWITCHEROO_INIT to solve this.
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
>
> Takashi, does this look good to you? Ack for merging through drm-misc?
>
> I pulled in patch 4 meanwhile.
> -Daniel
>
> > ---
> > drivers/gpu/vga/vga_switcheroo.c | 2 --
> > include/linux/vga_switcheroo.h | 5 -----
> > 2 files changed, 7 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > index 2138162..845e47d 100644
> > --- a/drivers/gpu/vga/vga_switcheroo.c
> > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > @@ -355,8 +355,6 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> > client = find_client_from_pci(&vgasr_priv.clients, pdev);
> > if (!client)
> > ret = VGA_SWITCHEROO_NOT_FOUND;
> > - else if (!vgasr_priv.active)
> > - ret = VGA_SWITCHEROO_INIT;
> > else
> > ret = client->pwr_state;
> > mutex_unlock(&vgasr_mutex);
> > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > index 3764991..95bfbeb0 100644
> > --- a/include/linux/vga_switcheroo.h
> > +++ b/include/linux/vga_switcheroo.h
> > @@ -39,10 +39,6 @@ struct pci_dev;
> > * enum vga_switcheroo_state - client power state
> > * @VGA_SWITCHEROO_OFF: off
> > * @VGA_SWITCHEROO_ON: on
> > - * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
> > - * vga_switcheroo is not enabled, i.e. no second client or no handler
> > - * has registered. Only used in vga_switcheroo_get_client_state() which
> > - * in turn is only called from hda_intel.c
> > * @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
> > * Only used in vga_switcheroo_get_client_state() which in turn is only
> > * called from hda_intel.c
> > @@ -53,7 +49,6 @@ enum vga_switcheroo_state {
> > VGA_SWITCHEROO_OFF,
> > VGA_SWITCHEROO_ON,
> > /* below are referred only from vga_switcheroo_get_client_state() */
> > - VGA_SWITCHEROO_INIT,
> > VGA_SWITCHEROO_NOT_FOUND,
> > };
> >
> > --
> > 1.8.5.2 (Apple Git-48)
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT
2015-10-25 18:19 ` Lukas Wunner
@ 2015-10-26 7:24 ` Takashi Iwai
0 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2015-10-26 7:24 UTC (permalink / raw)
To: Lukas Wunner; +Cc: dri-devel
On Sun, 25 Oct 2015 19:19:53 +0100,
Lukas Wunner wrote:
>
> Hi Takashi,
>
> gentle ping -- could you take a look at the patch below,
> would this change be okay with you?
I thought I gave already my ack? Hmm.. The series is unnecessarily
confusing...
In anyway, feel free to take my ack if not pushed yet.
> hda_intel.c:check_hdmi_disabled() is the sole caller of
> vga_switcheroo_get_client_state() and only checks for the
> return value VGA_SWITCHEROO_OFF, not for VGA_SWITCHEROO_INIT.
>
> So why was VGA_SWITCHEROO_INIT introduced in the first place?
> The only explanation I can think of is that for some reason you
> wanted to *avoid* returning VGA_SWITCHEROO_OFF (even if the card
> is in fact asleep) if no second GPU or no handler have registered
> with vga_switcheroo. But as shown in the scenario below, this can
> actually go awry.
>
> With VGA_SWITCHEROO_INIT apparently not having a purpose but on the
> other hand being harmful, I suggest to drop it with the patch below.
>
> For the meaning of the vga_switcheroo power states, refer to
> vga_switcheroo.h in drm-next:
> http://cgit.freedesktop.org/~airlied/linux/tree/include/linux/vga_switcheroo.h?h=drm-next#n38
The initial concept was that the audio component may be independent
from the GPU state. Indeed, this may happen in theory, as it's an
individual PCI entry that has nothing to do with the GPU per se.
The actual existing hardware for AMD and Nvidia, however, requires the
implicitly tight binding with the GPU state. Thus the hackish
function like check_hdmi_disabled() was introduced.
That said, conceptually we had to think of the different states, and
it was the reason that each client (even audio) has the individual
active field and passed by ops. In reality, though, we got only these
a few GPUs. So, it's fine to drop the initial concept, and adjust to
the existing code.
Takashi
>
> Thanks & best regards,
>
> Lukas
>
> On Fri, Oct 02, 2015 at 10:22:48AM +0200, Daniel Vetter wrote:
> > On Tue, Sep 08, 2015 at 02:17:47PM +0200, Lukas Wunner wrote:
> > > hda_intel.c:azx_probe() defers initialization of an audio controller
> > > on the discrete GPU if the GPU is powered off. The power state of the
> > > GPU is determined by calling vga_switcheroo_get_client_state().
> > >
> > > vga_switcheroo_get_client_state() returns VGA_SWITCHEROO_INIT if
> > > vga_switcheroo is not enabled, i.e. if no second GPU or no handler
> > > has registered.
> > >
> > > This can go wrong in the following scenario:
> > > - Driver for the integrated GPU is not loaded.
> > > - Driver for the discrete GPU registers with vga_switcheroo, uses driver
> > > power control to power down the GPU, handler cuts power to the GPU.
> > > - Driver for the audio controller gets loaded after the GPU was powered
> > > down, calls vga_switcheroo_get_client_state() which returns
> > > VGA_SWITCHEROO_INIT instead of VGA_SWITCHEROO_OFF.
> > > - Consequence: azx_probe() tries to initialize the audio controller even
> > > though the GPU is powered down.
> > >
> > > The power state VGA_SWITCHEROO_INIT was introduced by c8e9cf7bb240
> > > ("vga_switcheroo: Add a helper function to get the client state").
> > > It is not apparent what its benefit might be. The idea seems to
> > > be to initialize the audio controller even if the power state is
> > > VGA_SWITCHEROO_OFF (were vga_switcheroo enabled), but as shown
> > > above this can fail.
> > >
> > > Drop VGA_SWITCHEROO_INIT to solve this.
> > >
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> >
> > Takashi, does this look good to you? Ack for merging through drm-misc?
> >
> > I pulled in patch 4 meanwhile.
> > -Daniel
> >
> > > ---
> > > drivers/gpu/vga/vga_switcheroo.c | 2 --
> > > include/linux/vga_switcheroo.h | 5 -----
> > > 2 files changed, 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > > index 2138162..845e47d 100644
> > > --- a/drivers/gpu/vga/vga_switcheroo.c
> > > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > > @@ -355,8 +355,6 @@ int vga_switcheroo_get_client_state(struct pci_dev *pdev)
> > > client = find_client_from_pci(&vgasr_priv.clients, pdev);
> > > if (!client)
> > > ret = VGA_SWITCHEROO_NOT_FOUND;
> > > - else if (!vgasr_priv.active)
> > > - ret = VGA_SWITCHEROO_INIT;
> > > else
> > > ret = client->pwr_state;
> > > mutex_unlock(&vgasr_mutex);
> > > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > > index 3764991..95bfbeb0 100644
> > > --- a/include/linux/vga_switcheroo.h
> > > +++ b/include/linux/vga_switcheroo.h
> > > @@ -39,10 +39,6 @@ struct pci_dev;
> > > * enum vga_switcheroo_state - client power state
> > > * @VGA_SWITCHEROO_OFF: off
> > > * @VGA_SWITCHEROO_ON: on
> > > - * @VGA_SWITCHEROO_INIT: client has registered with vga_switcheroo but
> > > - * vga_switcheroo is not enabled, i.e. no second client or no handler
> > > - * has registered. Only used in vga_switcheroo_get_client_state() which
> > > - * in turn is only called from hda_intel.c
> > > * @VGA_SWITCHEROO_NOT_FOUND: client has not registered with vga_switcheroo.
> > > * Only used in vga_switcheroo_get_client_state() which in turn is only
> > > * called from hda_intel.c
> > > @@ -53,7 +49,6 @@ enum vga_switcheroo_state {
> > > VGA_SWITCHEROO_OFF,
> > > VGA_SWITCHEROO_ON,
> > > /* below are referred only from vga_switcheroo_get_client_state() */
> > > - VGA_SWITCHEROO_INIT,
> > > VGA_SWITCHEROO_NOT_FOUND,
> > > };
> > >
> > > --
> > > 1.8.5.2 (Apple Git-48)
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2015-10-26 7:24 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-23 13:18 [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Lukas Wunner
2015-08-29 12:29 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Lukas Wunner
2015-08-27 14:43 ` [PATCH 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-08-23 21:23 ` [PATCH 04/15] vga_switcheroo: Add missing locking Lukas Wunner
2015-09-08 12:17 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Lukas Wunner
2015-08-28 9:56 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Lukas Wunner
2015-08-28 11:30 ` [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1 Lukas Wunner
2015-08-28 10:54 ` [PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id instead of int Lukas Wunner
2015-09-05 11:40 ` [PATCH 09/15] vga_switcheroo: Sort headers alphabetically Lukas Wunner
2015-09-04 18:49 ` [PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently Lukas Wunner
2015-09-04 19:06 ` [PATCH 11/15] drm/i915: " Lukas Wunner
2015-09-05 9:09 ` [PATCH 12/15] drm/nouveau: " Lukas Wunner
2015-09-05 9:14 ` [PATCH 13/15] drm/radeon: " Lukas Wunner
2015-09-05 9:17 ` [PATCH 14/15] drm/amdgpu: " Lukas Wunner
2015-09-05 9:22 ` [PATCH 15/15] drm: " Lukas Wunner
2015-09-22 9:20 ` Daniel Vetter
2015-09-23 21:45 ` [PATCH 14/15] drm/amdgpu: " Alex Deucher
2015-09-22 9:20 ` [PATCH 11/15] drm/i915: " Daniel Vetter
2015-10-02 7:35 ` [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead of int Jani Nikula
2015-10-02 8:22 ` [PATCH 05/15] vga_switcheroo: Drop client power state VGA_SWITCHEROO_INIT Daniel Vetter
2015-10-25 18:19 ` Lukas Wunner
2015-10-26 7:24 ` Takashi Iwai
2015-08-27 14:43 ` [PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients Lukas Wunner
2015-09-23 7:13 ` Daniel Vetter
2015-09-24 9:40 ` Takashi Iwai
2015-09-24 15:13 ` Daniel Vetter
2015-10-01 16:19 ` Lukas Wunner
2015-09-24 9:37 ` Takashi Iwai
2015-09-17 17:15 ` [PATCH " Lukas Wunner
2015-09-22 9:17 ` Daniel Vetter
2015-09-19 16:44 ` Takashi Iwai
2015-09-19 17:50 ` Lukas Wunner
2015-09-24 9:39 ` Takashi Iwai
2015-09-22 8:38 ` [PATCH 02/15] DocBook: Add vga_switcheroo Subsystem Guide Daniel Vetter
2015-09-17 16:34 ` [PATCH 01/15] vga_switcheroo: Document _ALL_ the things! Danilo Cesar Lemes de Paula
2015-09-17 17:31 ` Lukas Wunner
2015-09-17 17:56 ` Danilo Cesar Lemes de Paula
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.