13 Sep, 2018

1 commit

  • The recent change of vga_switcheroo allowed the runtime PM for
    HD-audio on AMD GPUs, but this also resulted in a regression. When
    the HD-audio controller driver gets runtime-suspended, HD-audio link
    is turned off, and the hotplug notification is ignored. This leads to
    the inconsistent audio state (the connection isn't notified and ELD is
    ignored).

    The best fix would be to implement the proper ELD notification via the
    audio component, but it's still not ready. As a quick workaround,
    this patch adds the check of runtime_idle and allows the runtime
    suspend only when the vga_switcheroo is bound with discrete GPU.
    That is, a system with a single GPU and APU would be again without
    runtime PM to keep the HD-audio link for the hotplug notification and
    ELD read out.

    Also, the codec->auto_runtime_pm flag is set only for the discrete GPU
    at the time GPU gets bound via vga_switcheroo (i.e. only dGPU is
    forcibly runtime-PM enabled), so that APU can still get the ELD
    notification.

    For identifying which GPU is bound, a new vga_switcheroo client
    callback, gpu_bound, is implemented. The vga_switcheroo simply calls
    this when GPU is bound, and tells whether it's dGPU or APU.

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200945
    Fixes: 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
    Reported-by: Jian-Hong Pan
    Tested-by: Jian-Hong Pan
    Acked-by: Lukas Wunner
    Signed-off-by: Takashi Iwai

    Takashi Iwai
     

17 Jul, 2018

1 commit

  • On modern laptop, there are more and more platforms
    have two GPUs, and each of them maybe have audio codec
    for HDMP/DP output. For some dGPU which is no output,
    audio codec usually is disabled.

    In currect HDA audio driver, it will set all codec as
    VGA_SWITCHEROO_DIS, the audio which is binded to UMA
    will be suspended if user use debugfs to contorl power

    In HDA driver side, it is difficult to know which GPU
    the audio has binded to. So set the bound gpu pci dev
    to vga_switcheroo.

    if the audio client is not the third registration, audio
    id will set in vga_switcheroo enable function. if the
    audio client is the last registration when vga_switcheroo
    _ready() get true, we should get audio client id from bound
    GPU directly.

    Signed-off-by: Jim Qu
    Reviewed-by: Lukas Wunner
    Signed-off-by: Takashi Iwai

    Jim Qu
     

14 Mar, 2018

1 commit

  • Back in 2013, runtime PM for GPUs with integrated HDA controller was
    introduced with commits 0d69704ae348 ("gpu/vga_switcheroo: add driver
    control power feature. (v3)") and 246efa4a072f ("snd/hda: add runtime
    suspend/resume on optimus support (v4)").

    Briefly, the idea was that the HDA controller is forced on and off in
    unison with the GPU.

    The original code is mostly still in place even though it was never a
    100% perfect solution: E.g. on access to the HDA controller, the GPU
    is powered up via vga_switcheroo_runtime_resume_hdmi_audio() but there
    are no provisions to keep it resumed until access to the HDA controller
    has ceased: The GPU autosuspends after 5 seconds, rendering the HDA
    controller inaccessible.

    Additionally, a kludge is required when hda_intel.c probes: It has to
    check whether the GPU is powered down (check_hdmi_disabled()) and defer
    probing if so.

    However in the meantime (in v4.10) the driver core has gained a feature
    called device links which promises to solve such issues in a clean way:
    It allows us to declare a dependency from the HDA controller (consumer)
    to the GPU (supplier). The PM core then automagically ensures that the
    GPU is runtime resumed as long as the HDA controller's ->probe hook is
    executed and whenever the HDA controller is accessed.

    By default, the HDA controller has a dependency on its parent, a PCIe
    Root Port. Adding a device link creates another dependency on its
    sibling:

    PCIe Root Port
    ^ ^
    | |
    | |
    HDA ===> GPU

    The device link is not only used for runtime PM, it also guarantees that
    on system sleep, the HDA controller suspends before the GPU and resumes
    after the GPU, and on system shutdown the HDA controller's ->shutdown
    hook is executed before the one of the GPU. It is a complete solution.

    Using this functionality is as simple as calling device_link_add(),
    which results in a dmesg entry like this:

    pci 0000:01:00.1: Linked as a consumer to 0000:01:00.0

    The code for the GPU-governed audio power management can thus be removed
    (except where it's still needed for legacy manual power control).

    The device link is added in a PCI quirk rather than in hda_intel.c.
    It is therefore legal for the GPU to runtime suspend to D3cold even if
    the HDA controller is not bound to a driver or if CONFIG_SND_HDA_INTEL
    is not enabled, for accesses to the HDA controller will cause the GPU to
    wake up regardless if they're occurring outside of hda_intel.c (think
    config space readout via sysfs).

    Contrary to the previous implementation, the HDA controller's power
    state is now self-governed, rather than GPU-governed, whereas the GPU's
    power state is no longer fully self-governed. (The HDA controller needs
    to runtime suspend before the GPU can.)

    It is thus crucial that runtime PM is always activated on the HDA
    controller even if CONFIG_SND_HDA_POWER_SAVE_DEFAULT is set to 0 (which
    is the default), lest the GPU stays awake. This is achieved by setting
    the auto_runtime_pm flag on every codec and the AZX_DCAPS_PM_RUNTIME
    flag on the HDA controller.

    A side effect is that power consumption might be reduced if the GPU is
    in use but the HDA controller is not, because the HDA controller is now
    allowed to go to D3hot. Before, it was forced to stay in D0 as long as
    the GPU was in use. (There is no reduction in power consumption on my
    Nvidia GK107, but there might be on other chips.)

    The code paths for legacy manual power control are adjusted such that
    runtime PM is disabled during power off, thereby preventing the PM core
    from resuming the HDA controller.

    Note that the device link is not only added on vga_switcheroo capable
    systems, but for *any* GPU with integrated HDA controller. The idea is
    that the HDA controller streams audio via connectors located on the GPU,
    so the GPU needs to be on for the HDA controller to do anything useful.

    This commit implicitly fixes an unbalanced runtime PM ref upon unbind of
    hda_intel.c: On ->probe, a runtime PM ref was previously released under
    the condition "azx_has_pm_runtime(chip) || hda->use_vga_switcheroo", but
    on ->remove a runtime PM ref was only acquired under the first of those
    conditions. Thus, binding and unbinding the driver twice on a
    vga_switcheroo capable system caused the runtime PM refcount to drop
    below zero. The issue is resolved because the AZX_DCAPS_PM_RUNTIME flag
    is now always set if use_vga_switcheroo is true.

    For more information on device links please refer to:
    https://www.kernel.org/doc/html/latest/driver-api/device_link.html
    Documentation/driver-api/device_link.rst

    Cc: Dave Airlie
    Cc: Ben Skeggs
    Cc: Alex Deucher
    Cc: Rafael J. Wysocki
    Acked-by: Bjorn Helgaas
    Reviewed-by: Takashi Iwai
    Reviewed-by: Peter Wu
    Tested-by: Kai Heng Feng # AMD PowerXpress
    Tested-by: Mike Lothian # AMD PowerXpress
    Tested-by: Denis Lisov # Nvidia Optimus
    Tested-by: Peter Wu # Nvidia Optimus
    Tested-by: Lukas Wunner # MacBook Pro
    Signed-off-by: Lukas Wunner
    Link: https://patchwork.freedesktop.org/patch/msgid/51bd38360ff502a8c42b1ebf4405ee1d3f27118d.1520068884.git.lukas@wunner.de

    Lukas Wunner
     

31 May, 2016

1 commit

  • So far we've got one condition when DRM drivers need to defer probing
    on a dual GPU system and it's coded separately into each of the relevant
    drivers. As suggested by Daniel Vetter, deduplicate that code in the
    drivers and move it to a new vga_switcheroo helper. This yields better
    encapsulation of concepts and lets us add further checks in a central
    place. (The existing check pertains to pre-retina MacBook Pros and an
    additional check is expected to be needed for retinas.)

    One might be tempted to check deferred probing conditions in
    vga_switcheroo_register_client(), but this is usually called fairly late
    during driver load. The GPU is fully brought up and ready for switching
    at that point. On boot the ->probe hook is potentially called dozens of
    times until it finally succeeds, and each time we'd repeat bringup and
    teardown of the GPU, lengthening boot time considerably and cluttering
    logfiles. A separate helper is therefore needed which can be called
    right at the beginning of the ->probe hook.

    Note that amdgpu currently does not call this helper as the AMD GPUs
    built into MacBook Pros are only supported by radeon so far.

    v2: This helper could eventually be used by audio clients as well,
    so rephrase kerneldoc to refer to "client" instead of "GPU"
    and move the single existing check in an if block specific
    to PCI_CLASS_DISPLAY_VGA devices. Move documentation on
    that check from kerneldoc to a comment. (Daniel Vetter)

    v3: Mandate in kerneldoc that registration of client shall only
    happen after calling this helper. (Daniel Vetter)

    v4: Rebase on 412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when
    amdkfd not loaded")

    v5: Some Optimus GPUs use PCI_CLASS_DISPLAY_3D, make sure those are
    matched as well. (Emil Velikov)

    v6: The if-condition referring to PCI_BASE_CLASS_DISPLAY may be
    considered a functional change. Move to a separate commit to
    keep this a pure refactoring change. (Emil Velikov, Jani Nikula)

    Cc: Daniel Vetter
    Cc: Ben Skeggs
    Cc: Alex Deucher
    Signed-off-by: Lukas Wunner
    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/575885fd440c2b13c3f19ddf44360cfbbff35f50.1464685538.git.lukas@wunner.de

    Lukas Wunner
     

09 Feb, 2016

2 commits

  • Originally by Seth Forshee , 2012-10-04:
    During graphics driver initialization it's useful to be able to mux
    only the DDC to the inactive client in order to read the EDID. Add
    a switch_ddc callback to allow capable handlers to provide this
    functionality, and add vga_switcheroo_switch_ddc() to allow DRM
    to mux only the DDC.

    Modified by Dave Airlie , 2012-12-22:
    I can't figure out why I didn't like this, but I rewrote this [...]
    to lock/unlock the ddc lines [...]. I think I'd prefer something
    like that otherwise the interface got really ugly.

    Modified by Lukas Wunner , 2015-04 - 2015-10:
    Change semantics of ->switch_ddc handler callback to return previous
    DDC owner. Original version tried to determine previous DDC owner
    with find_active_client() but this fails if the inactive client
    registers before the active client.

    Don't lock vgasr_mutex in _lock_ddc() / _unlock_ddc(), it can cause
    deadlocks because (a) during switch (with vgasr_mutex already held),
    GPU is woken and probes its outputs, tries to re-acquire vgasr_mutex
    to lock DDC lines; (b) Likewise during switch, GPU is suspended and
    calls cancel_delayed_work_sync() to stop output polling, if poll
    task is running at this moment we may wait forever for it to finish.

    Instead, lock mux_hw_lock when unregistering the handler because
    the only reason why we'd want to lock vgasr_mutex in _lock_ddc() /
    _unlock_ddc() is to block the handler from disappearing while DDC
    lines are switched.

    Also acquire mux_hw_lock in stage2 to avoid race condition where
    reading the EDID and switching happens simultaneously. Likewise on
    MIGD / MDIS commands and on runtime suspend.

    v2.1: Overhaul locking, squash commits (Daniel Vetter)

    v2.2: Readability improvements (Thierry Reding)

    v2.3: Overhaul locking once more

    v2.4: Retain semantics of ->switchto handler callback to switch all
    pins, including DDC (Daniel Vetter)

    v5: Rename ddc_lock to mux_hw_lock: Since we acquire this both
    when calling ->switch_ddc and ->switchto, it protects not just
    access to the DDC lines but to the mux in general. This is in
    line with the DRM convention to use low-level locks to avoid
    concurrent hw access (e.g. i2c, dp_aux) which are often called
    hw_lock (Daniel Vetter)

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
    Tested-by: Lukas Wunner
    [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"]
    Cc: Seth Forshee
    Cc: Dave Airlie
    Signed-off-by: Lukas Wunner
    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/e81ae9722b84c5ed591805fee3ea6dbf5dc6c4b3.1452525860.git.lukas@wunner.de

    Lukas Wunner
     
  • Allow handlers to declare their capabilities and allow clients to
    obtain that information. So far we have these use cases:

    * If the handler is able to switch DDC separately, clients need to
    probe EDID with drm_get_edid_switcheroo(). We should allow them
    to detect a capable handler to ensure this function only gets
    called when needed.

    * Likewise if the handler is unable to switch AUX separately, the active
    client needs to communicate link training parameters to the inactive
    client, which may then skip the AUX handshake and set up its output
    with these pre-calibrated values (DisplayPort specification v1.1a,
    section 2.5.3.3). Clients need a way to recognize such a situation.

    The flags for the radeon_atpx_handler and amdgpu_atpx_handler are
    initially set to 0, this can later on be amended with
    handler_flags |= VGA_SWITCHEROO_CAN_SWITCH_DDC;
    when a ->switch_ddc callback is added.

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
    Tested-by: Lukas Wunner
    [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"]
    Signed-off-by: Lukas Wunner
    Reviewed-by: Darren Hart
    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/2b0d93ed6e511ca09e95e45e0b35627f330fabce.1452525860.git.lukas@wunner.de

    Lukas Wunner
     

05 Nov, 2015

1 commit

  • 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.

    Acked-by: Takashi Iwai
    Signed-off-by: Lukas Wunner
    Signed-off-by: Dave Airlie

    Lukas Wunner
     

21 Oct, 2015

1 commit

  • vga_switcheroo_client_ops has always been declared const since its
    introduction with 26ec685ff9d9 ("vga_switcheroo: Introduce struct
    vga_switcheroo_client_ops").

    Do so for vga_switcheroo_handler as well.

    drivers/gpu/drm/amd/amdgpu/amdgpu.ko:
    6 .rodata 00009888
    - 19 .data 00001f00
    + 19 .data 00001ee0
    drivers/gpu/drm/nouveau/nouveau.ko:
    6 .rodata 000460b8
    17 .data 00018fe0
    drivers/gpu/drm/radeon/radeon.ko:
    - 7 .rodata 00030944
    + 7 .rodata 00030964
    - 21 .data 0000d6a0
    + 21 .data 0000d678
    drivers/platform/x86/apple-gmux.ko:
    - 7 .rodata 00000140
    + 7 .rodata 00000160
    - 11 .data 000000e0
    + 11 .data 000000b8

    Cc: Ben Skeggs
    Cc: Darren Hart
    Cc: Alex Deucher
    Signed-off-by: Lukas Wunner
    Reviewed-by: Christian König .
    Signed-off-by: Daniel Vetter

    Lukas Wunner
     

16 Oct, 2015

3 commits


25 Sep, 2015

1 commit

  • 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.

    Comment from Takashi's review:

    "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."

    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
    Signed-off-by: Lukas Wunner
    [danvet: Add Takashi's clarification to the commit message.]
    Reviewed-by: Takashi Iwai
    Signed-off-by: Daniel Vetter

    Lukas Wunner
     

22 Sep, 2015

1 commit

  • 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
    Signed-off-by: Daniel Vetter

    Lukas Wunner
     

19 Sep, 2014

1 commit


29 Aug, 2013

1 commit

  • For optimus and powerxpress muxless we really want the GPU
    driver deciding when to power up/down the GPU, not userspace.

    This adds the ability for a driver to dynamically power up/down
    the GPU and remove the switcheroo from controlling it, the
    switcheroo reports the dynamic state to userspace also.

    It also adds 2 power domains, one for machine where the power
    switch is controlled outside the GPU D3 state, so the powerdown
    ordering is done correctly, and the second for the hdmi audio
    device to make sure it can resume for PCI config space accesses.

    v1.1: fix build with switcheroo off

    v2: add power domain support for radeon and v1 nvidia dsms
    v2.1: fix typo in off case

    v3: add audio power domain for hdmi audio + misc audio fixes

    v4: use PCI_SLOT macro, drop power reference on hdmi audio resume
    failure also.

    Signed-off-by: Dave Airlie

    Dave Airlie
     

21 Jun, 2012

1 commit


16 Jun, 2012

1 commit

  • Fix warnings on some architectures/configs (not on x86):

    include/linux/vga_switcheroo.h:28:30: warning: 'struct pci_dev' declared inside parameter list [enabled by default]
    include/linux/vga_switcheroo.h:28:30: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]

    Signed-off-by: Randy Dunlap
    Cc: Takashi Iwai
    Reported-by: Geert Uytterhoeven
    Signed-off-by: Dave Airlie

    Randy Dunlap
     

08 Jun, 2012

2 commits


13 May, 2012

2 commits

  • Add the support for audio clients to VGA-switcheroo for handling the
    HDMI audio controller together with VGA switching. The id of the
    audio controller should be given explicitly at registration time
    unlike the video controller.

    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43155

    Signed-off-by: Takashi Iwai

    Takashi Iwai
     
  • This changes the API as a clean-up. Instead of passing multiple
    function pointers at each time, introduce a new struct holding the
    whole callback functions and pass it to the registration.

    The same struct will be used for the upcoming audio client
    registration, too.

    Signed-off-by: Takashi Iwai

    Takashi Iwai
     

06 Jan, 2011

1 commit


05 Jan, 2011

1 commit


01 Mar, 2010

2 commits

  • radeon was always including the atpx code unnecessarily, also core
    switcheroo was including acpi headers.

    Signed-off-by: Dave Airlie

    Dave Airlie
     
  • Many new laptops now come with 2 gpus, one to be used for low power
    modes and one for gaming/on-ac applications. These GPUs are typically
    wired to the laptop panel and VGA ports via a multiplexer unit which
    is controlled via ACPI methods.

    4 combinations of systems typically exist - with 2 ACPI methods.
    Intel/ATI - Lenovo W500/T500 - use ATPX ACPI method
    ATI/ATI - some ASUS - use ATPX ACPI Method
    Intel/Nvidia - - use _DSM ACPI method
    Nvidia/Nvidia - - use _DSM ACPI method.

    TODO:
    This patch adds support for the ATPX method and initial bits
    for the _DSM methods that need to written by someone with
    access to the hardware.
    Add a proper non-debugfs interface - need to get some proper
    testing first.

    v2: add power up/down support for both devices
    on W500 puts i915/radeon into D3 and cuts power to radeon.

    v3: redo probing methods, no DMI list, drm devices call to
    register with switcheroo, it tries to find an ATPX method on
    any device and once there is two devices + ATPX it inits the
    switcher.

    v4: ATPX msg handling using buffers - should work on more machines

    v5: rearchitect after more mjg59 discussion - move ATPX handling to
    radeon driver.

    v6: add file headers + initial nouveau bits (to be filled out).

    v7: merge delayed switcher code.

    v8: avoid suspend/resume of gpu that is off

    v9: rearchitect - mjg59 is always right. - move all ATPX code to
    radeon, should allow simpler DSM also proper ATRM handling

    v10: add ATRM support for radeon BIOS, add mutex to lock vgasr_priv

    v11: fix bug in resuming Intel for 2nd time.

    v12: start fixing up nvidia code blindly.

    v13: blindly guess at finishing nvidia code

    v14: remove radeon audio hacks - fix up intel resume more like upstream

    v15: clean up printks + remove unnecessary igd/dis pointers

    mount debugfs

    /sys/kernel/debug/vgaswitcheroo/switch - should exist if ATPX detected
    + 2 cards.

    DIS - immediate change to discrete
    IGD - immediate change to IGD
    DDIS - delayed change to discrete
    DIGD - delayed change to IGD
    ON - turn on not in use
    OFF - turn off not in use

    Tested on W500 (Intel/ATI) and T500 (Intel/ATI)

    Signed-off-by: Dave Airlie

    Dave Airlie