On Wed, Oct 02, 2019 at 04:17:58PM +0300, Jani Nikula wrote:
> Split out code related to vga switcheroo register/unregister and state
> handling from i915_drv.c into new i915_switcheroo.[ch] files.
>
> It's a bit difficult to draw the line how much to move to the new file
> from i915_drv.c, but it seemed to me keeping i915_suspend_switcheroo()
> and i915_resume_switcheroo() in place was the cleanest.
>
> No functional changes.
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 63 +++---------------------
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/i915_switcheroo.c | 67 ++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_switcheroo.h | 14 ++++++
> 5 files changed, 92 insertions(+), 56 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_switcheroo.c
> create mode 100644 drivers/gpu/drm/i915/i915_switcheroo.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index d2b53b5add81..136a1a51b6e2 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -46,6 +46,7 @@ i915-y += i915_drv.o \
> i915_pci.o \
> i915_scatterlist.o \
> i915_suspend.o \
> + i915_switcheroo.o \
> i915_sysfs.o \
> i915_utils.o \
> intel_csr.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3306c6bb515a..7349fd8c9796 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -72,6 +72,7 @@
> #include "i915_perf.h"
> #include "i915_query.h"
> #include "i915_suspend.h"
> +#include "i915_switcheroo.h"
> #include "i915_sysfs.h"
> #include "i915_trace.h"
> #include "i915_vgpu.h"
> @@ -269,56 +270,8 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv)
> release_resource(&dev_priv->mch_res);
> }
>
> -static int i915_resume_switcheroo(struct drm_i915_private *i915);
> -static int i915_suspend_switcheroo(struct drm_i915_private *i915,
> - pm_message_t state);
> -
> -static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
> -{
> - struct drm_i915_private *i915 = pdev_to_i915(pdev);
> - pm_message_t pmm = { .event = PM_EVENT_SUSPEND };
> -
> - if (!i915) {
> - dev_err(&pdev->dev, "DRM not initialized, aborting switch.\n");
> - return;
> - }
> -
> - if (state == VGA_SWITCHEROO_ON) {
> - pr_info("switched on\n");
> - i915->drm.switch_power_state = DRM_SWITCH_POWER_CHANGING;
> - /* i915 resume handler doesn't set to D0 */
> - pci_set_power_state(pdev, PCI_D0);
> - i915_resume_switcheroo(i915);
> - i915->drm.switch_power_state = DRM_SWITCH_POWER_ON;
> - } else {
> - pr_info("switched off\n");
> - i915->drm.switch_power_state = DRM_SWITCH_POWER_CHANGING;
> - i915_suspend_switcheroo(i915, pmm);
> - i915->drm.switch_power_state = DRM_SWITCH_POWER_OFF;
> - }
> -}
> -
> -static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
> -{
> - struct drm_i915_private *i915 = pdev_to_i915(pdev);
> -
> - /*
> - * FIXME: open_count is protected by drm_global_mutex but that would lead to
> - * locking inversion with the driver load path. And the access here is
> - * completely racy anyway. So don't bother with locking for now.
> - */
> - return i915 && i915->drm.open_count == 0;
> -}
> -
> -static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
> - .set_gpu_state = i915_switcheroo_set_state,
> - .reprobe = NULL,
> - .can_switch = i915_switcheroo_can_switch,
> -};
> -
> static int i915_driver_modeset_probe(struct drm_i915_private *i915)
> {
> - struct pci_dev *pdev = i915->drm.pdev;
> int ret;
>
> if (i915_inject_probe_failure(i915))
> @@ -339,7 +292,7 @@ static int i915_driver_modeset_probe(struct drm_i915_private *i915)
>
> intel_register_dsm_handler();
>
> - ret = vga_switcheroo_register_client(pdev, &i915_switcheroo_ops, false);
> + ret = i915_switcheroo_register(i915);
> if (ret)
> goto cleanup_vga_client;
>
> @@ -394,7 +347,7 @@ static int i915_driver_modeset_probe(struct drm_i915_private *i915)
> cleanup_csr:
> intel_csr_ucode_fini(i915);
> intel_power_domains_driver_remove(i915);
> - vga_switcheroo_unregister_client(pdev);
> + i915_switcheroo_unregister(i915);
> cleanup_vga_client:
> intel_vga_unregister(i915);
> out:
> @@ -403,13 +356,12 @@ static int i915_driver_modeset_probe(struct drm_i915_private *i915)
>
> static void i915_driver_modeset_remove(struct drm_i915_private *i915)
> {
> - struct pci_dev *pdev = i915->drm.pdev;
> -
> intel_modeset_driver_remove(i915);
>
> intel_bios_driver_remove(i915);
>
> - vga_switcheroo_unregister_client(pdev);
> + i915_switcheroo_unregister(i915);
> +
> intel_vga_unregister(i915);
>
> intel_csr_ucode_fini(i915);
> @@ -1825,8 +1777,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> return ret;
> }
>
> -static int
> -i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state)
> +int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state)
> {
> int error;
>
> @@ -1994,7 +1945,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> return ret;
> }
>
> -static int i915_resume_switcheroo(struct drm_i915_private *i915)
> +int i915_resume_switcheroo(struct drm_i915_private *i915)
> {
> int ret;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 337d8306416a..0ca4d90dfa46 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2223,6 +2223,9 @@ extern const struct dev_pm_ops i915_pm_ops;
> int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> void i915_driver_remove(struct drm_i915_private *i915);
>
> +int i915_resume_switcheroo(struct drm_i915_private *i915);
> +int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
> +
> void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
> int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
>
> diff --git a/drivers/gpu/drm/i915/i915_switcheroo.c b/drivers/gpu/drm/i915/i915_switcheroo.c
> new file mode 100644
> index 000000000000..39c79e1c5b52
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_switcheroo.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include <linux/vga_switcheroo.h>
> +
> +#include "i915_drv.h"
> +#include "i915_switcheroo.h"
> +
> +static void i915_switcheroo_set_state(struct pci_dev *pdev,
> + enum vga_switcheroo_state state)
> +{
> + struct drm_i915_private *i915 = pdev_to_i915(pdev);
> + pm_message_t pmm = { .event = PM_EVENT_SUSPEND };
> +
> + if (!i915) {
> + dev_err(&pdev->dev, "DRM not initialized, aborting switch.\n");
> + return;
> + }
Is that dead code?
Patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> +
> + if (state == VGA_SWITCHEROO_ON) {
> + pr_info("switched on\n");
> + i915->drm.switch_power_state = DRM_SWITCH_POWER_CHANGING;
> + /* i915 resume handler doesn't set to D0 */
> + pci_set_power_state(pdev, PCI_D0);
> + i915_resume_switcheroo(i915);
> + i915->drm.switch_power_state = DRM_SWITCH_POWER_ON;
> + } else {
> + pr_info("switched off\n");
> + i915->drm.switch_power_state = DRM_SWITCH_POWER_CHANGING;
> + i915_suspend_switcheroo(i915, pmm);
> + i915->drm.switch_power_state = DRM_SWITCH_POWER_OFF;
> + }
> +}
> +
> +static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
> +{
> + struct drm_i915_private *i915 = pdev_to_i915(pdev);
> +
> + /*
> + * FIXME: open_count is protected by drm_global_mutex but that would lead to
> + * locking inversion with the driver load path. And the access here is
> + * completely racy anyway. So don't bother with locking for now.
> + */
> + return i915 && i915->drm.open_count == 0;
> +}
> +
> +static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
> + .set_gpu_state = i915_switcheroo_set_state,
> + .reprobe = NULL,
> + .can_switch = i915_switcheroo_can_switch,
> +};
> +
> +int i915_switcheroo_register(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = i915->drm.pdev;
> +
> + return vga_switcheroo_register_client(pdev, &i915_switcheroo_ops, false);
> +}
> +
> +void i915_switcheroo_unregister(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = i915->drm.pdev;
> +
> + vga_switcheroo_unregister_client(pdev);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_switcheroo.h b/drivers/gpu/drm/i915/i915_switcheroo.h
> new file mode 100644
> index 000000000000..59b6c1e07d75
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_switcheroo.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#ifndef __I915_SWITCHEROO__
> +#define __I915_SWITCHEROO__
> +
> +struct drm_i915_private;
> +
> +int i915_switcheroo_register(struct drm_i915_private *i915);
> +void i915_switcheroo_unregister(struct drm_i915_private *i915);
> +
> +#endif /* __I915_SWITCHEROO__ */
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
