[PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
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 > 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
[PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
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
---
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
*
+ * Copyright (c) 2015 Lukas Wunner
*
- * 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
+/**
+ * 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
[PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
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 >> 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=markdown=docbook > > Otherwise the markdown support works great (though I've only used it for > unsorted lists in this documentation). > > Thanks, > > Lukas > Danilo Cesar
[PATCH 01/15] vga_switcheroo: Document _ALL_ the things!
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
> ---
> 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
> *
> + * Copyright (c) 2015 Lukas Wunner
> *
> - * 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
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
>
> +/**
> + * 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
