[01/15] vga_switcheroo: Document _ALL_ the things!
diff mbox

Message ID 05cc4f74df634d84d4f461ee9f0e9d9b2908cf10.1442497843.git.lukas@wunner.de
State New
Headers show

Commit Message

Lukas Wunner Aug. 23, 2015, 1:18 p.m. UTC
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(-)

Comments

Danilo Cesar Lemes de Paula Sept. 17, 2015, 4:34 p.m. UTC | #1
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
Lukas Wunner Sept. 17, 2015, 5:31 p.m. UTC | #2
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
Danilo Cesar Lemes de Paula Sept. 17, 2015, 5:56 p.m. UTC | #3
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

Patch
diff mbox

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);