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 at suse.de>
> Signed-off-by: Lukas Wunner <lukas at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
