* [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr
@ 2024-09-12 17:45 Rodrigo Vivi
2024-09-12 21:50 ` ✗ Fi.CI.BUILD: failure for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2024-09-12 17:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Jesse Barnes, Jani Nikula
These save & restore functions inside i915_suspend are old display
functions to save and restore a bunch of display related registers.
Move it under display and rename accordantly. Just don't move it
entirely towards intel_display struct yet because it depends
on drm_i915_private for the IS_MOBILE.
While doing this conversion also update the MIT header using
the new SPDX ones.
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/Makefile | 2 +-
.../gpu/drm/i915/display/i9xx_display_sr.c | 119 +++++++++++++++
.../gpu/drm/i915/display/i9xx_display_sr.h | 14 ++
drivers/gpu/drm/i915/i915_driver.c | 6 +-
drivers/gpu/drm/i915/i915_suspend.c | 141 ------------------
drivers/gpu/drm/i915/i915_suspend.h | 14 --
6 files changed, 137 insertions(+), 159 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.c
create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.h
delete mode 100644 drivers/gpu/drm/i915/i915_suspend.c
delete mode 100644 drivers/gpu/drm/i915/i915_suspend.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c63fa2133ccb..89f04bdbc27f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -30,7 +30,6 @@ i915-y += \
i915_params.o \
i915_pci.o \
i915_scatterlist.o \
- i915_suspend.o \
i915_switcheroo.o \
i915_sysfs.o \
i915_utils.o \
@@ -219,6 +218,7 @@ i915-$(CONFIG_HWMON) += \
i915-y += \
display/hsw_ips.o \
display/i9xx_plane.o \
+ display/i9xx_suspend.o \
display/i9xx_wm.o \
display/intel_alpm.o \
display/intel_atomic.o \
diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.c b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
new file mode 100644
index 000000000000..211cf41119ad
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#include "i915_drv.h"
+#include "i915_reg.h"
+#include "i9xx_suspend.h"
+#include "intel_de.h"
+#include "intel_gmbus.h"
+#include "intel_vga.h"
+#include "intel_pci_config.h"
+
+static void intel_save_swf(struct drm_i915_private *i915)
+{
+ int i;
+
+ /* Scratch space */
+ if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
+ for (i = 0; i < 7; i++) {
+ i915->regfile.saveSWF0[i] = intel_de_read(i915,
+ SWF0(i915, i));
+ i915->regfile.saveSWF1[i] = intel_de_read(i915,
+ SWF1(i915, i));
+ }
+ for (i = 0; i < 3; i++)
+ i915->regfile.saveSWF3[i] = intel_de_read(i915,
+ SWF3(i915, i));
+ } else if (DISPLAY_VER(i915) == 2) {
+ for (i = 0; i < 7; i++)
+ i915->regfile.saveSWF1[i] = intel_de_read(i915,
+ SWF1(i915, i));
+ } else if (HAS_GMCH(i915)) {
+ for (i = 0; i < 16; i++) {
+ i915->regfile.saveSWF0[i] = intel_de_read(i915,
+ SWF0(i915, i));
+ i915->regfile.saveSWF1[i] = intel_de_read(i915,
+ SWF1(i915, i));
+ }
+ for (i = 0; i < 3; i++)
+ i915->regfile.saveSWF3[i] = intel_de_read(i915,
+ SWF3(i915, i));
+ }
+}
+
+static void intel_restore_swf(struct drm_i915_private *i915)
+{
+ int i;
+
+ /* Scratch space */
+ if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
+ for (i = 0; i < 7; i++) {
+ intel_de_write(i915, SWF0(i915, i),
+ i915->regfile.saveSWF0[i]);
+ intel_de_write(i915, SWF1(i915, i),
+ i915->regfile.saveSWF1[i]);
+ }
+ for (i = 0; i < 3; i++)
+ intel_de_write(i915, SWF3(i915, i),
+ i915->regfile.saveSWF3[i]);
+ } else if (DISPLAY_VER(i915) == 2) {
+ for (i = 0; i < 7; i++)
+ intel_de_write(i915, SWF1(i915, i),
+ i915->regfile.saveSWF1[i]);
+ } else if (HAS_GMCH(i915)) {
+ for (i = 0; i < 16; i++) {
+ intel_de_write(i915, SWF0(i915, i),
+ i915->regfile.saveSWF0[i]);
+ intel_de_write(i915, SWF1(i915, i),
+ i915->regfile.saveSWF1[i]);
+ }
+ for (i = 0; i < 3; i++)
+ intel_de_write(i915, SWF3(i915, i),
+ i915->regfile.saveSWF3[i]);
+ }
+}
+
+void i9xx_display_sr_save(struct drm_i915_private *i915)
+{
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+
+ if (!HAS_DISPLAY(i915))
+ return;
+
+ /* Display arbitration control */
+ if (DISPLAY_VER(i915) <= 4)
+ i915->regfile.saveDSPARB = intel_de_read(i915,
+ DSPARB(i915));
+
+ if (DISPLAY_VER(i915) == 4)
+ pci_read_config_word(pdev, GCDGMBUS,
+ &i915->regfile.saveGCDGMBUS);
+
+ intel_save_swf(i915);
+}
+
+void i9xx_display_sr_restore(struct drm_i915_private *i915)
+{
+ struct intel_display *display = &i915->display;
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+
+ if (!HAS_DISPLAY(i915))
+ return;
+
+ intel_restore_swf(i915);
+
+ if (DISPLAY_VER(i915) == 4)
+ pci_write_config_word(pdev, GCDGMBUS,
+ i915->regfile.saveGCDGMBUS);
+
+ /* Display arbitration */
+ if (DISPLAY_VER(i915) <= 4)
+ intel_de_write(i915, DSPARB(i915),
+ i915->regfile.saveDSPARB);
+
+ intel_vga_redisable(display);
+
+ intel_gmbus_reset(i915);
+}
diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.h b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
new file mode 100644
index 000000000000..d3598c729137
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2014 Intel Corporation
+ */
+
+#ifndef __I9XX_DISPLAY_SR_H__
+#define __I9XX_DISPLAY_SR_H__
+
+struct drm_i915_private;
+
+void i9xx_display_sr_save(struct drm_i915_private *i915);
+void i9xx_display_sr_restore(struct drm_i915_private *i915);
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index f82aa313f854..e15bf3aa44f9 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -45,6 +45,7 @@
#include <drm/drm_managed.h>
#include <drm/drm_probe_helper.h>
+#include "display/i9xx_display_sr.h"
#include "display/intel_acpi.h"
#include "display/intel_bw.h"
#include "display/intel_cdclk.h"
@@ -93,7 +94,6 @@
#include "i915_memcpy.h"
#include "i915_perf.h"
#include "i915_query.h"
-#include "i915_suspend.h"
#include "i915_switcheroo.h"
#include "i915_sysfs.h"
#include "i915_utils.h"
@@ -1047,7 +1047,7 @@ static int i915_drm_suspend(struct drm_device *dev)
intel_dpt_suspend(dev_priv);
i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
- i915_save_display(dev_priv);
+ i9xx_display_sr_save(dev_priv);
opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
intel_opregion_suspend(display, opregion_target_state);
@@ -1166,7 +1166,7 @@ static int i915_drm_resume(struct drm_device *dev)
intel_dmc_resume(display);
- i915_restore_display(dev_priv);
+ i9xx_display_sr_restore(dev_priv);
intel_pps_unlock_regs_wa(display);
intel_init_pch_refclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
deleted file mode 100644
index 9d3d9b983032..000000000000
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- *
- * Copyright 2008 (c) Intel Corporation
- * Jesse Barnes <jbarnes@virtuousgeek.org>
- *
- * 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, sub license, 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 NON-INFRINGEMENT.
- * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS 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.
- */
-
-#include "display/intel_de.h"
-#include "display/intel_gmbus.h"
-#include "display/intel_vga.h"
-
-#include "i915_drv.h"
-#include "i915_reg.h"
-#include "i915_suspend.h"
-#include "intel_pci_config.h"
-
-static void intel_save_swf(struct drm_i915_private *dev_priv)
-{
- int i;
-
- /* Scratch space */
- if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
- for (i = 0; i < 7; i++) {
- dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
- SWF0(dev_priv, i));
- dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
- SWF1(dev_priv, i));
- }
- for (i = 0; i < 3; i++)
- dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
- SWF3(dev_priv, i));
- } else if (GRAPHICS_VER(dev_priv) == 2) {
- for (i = 0; i < 7; i++)
- dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
- SWF1(dev_priv, i));
- } else if (HAS_GMCH(dev_priv)) {
- for (i = 0; i < 16; i++) {
- dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
- SWF0(dev_priv, i));
- dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
- SWF1(dev_priv, i));
- }
- for (i = 0; i < 3; i++)
- dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
- SWF3(dev_priv, i));
- }
-}
-
-static void intel_restore_swf(struct drm_i915_private *dev_priv)
-{
- int i;
-
- /* Scratch space */
- if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
- for (i = 0; i < 7; i++) {
- intel_de_write(dev_priv, SWF0(dev_priv, i),
- dev_priv->regfile.saveSWF0[i]);
- intel_de_write(dev_priv, SWF1(dev_priv, i),
- dev_priv->regfile.saveSWF1[i]);
- }
- for (i = 0; i < 3; i++)
- intel_de_write(dev_priv, SWF3(dev_priv, i),
- dev_priv->regfile.saveSWF3[i]);
- } else if (GRAPHICS_VER(dev_priv) == 2) {
- for (i = 0; i < 7; i++)
- intel_de_write(dev_priv, SWF1(dev_priv, i),
- dev_priv->regfile.saveSWF1[i]);
- } else if (HAS_GMCH(dev_priv)) {
- for (i = 0; i < 16; i++) {
- intel_de_write(dev_priv, SWF0(dev_priv, i),
- dev_priv->regfile.saveSWF0[i]);
- intel_de_write(dev_priv, SWF1(dev_priv, i),
- dev_priv->regfile.saveSWF1[i]);
- }
- for (i = 0; i < 3; i++)
- intel_de_write(dev_priv, SWF3(dev_priv, i),
- dev_priv->regfile.saveSWF3[i]);
- }
-}
-
-void i915_save_display(struct drm_i915_private *dev_priv)
-{
- struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
-
- if (!HAS_DISPLAY(dev_priv))
- return;
-
- /* Display arbitration control */
- if (GRAPHICS_VER(dev_priv) <= 4)
- dev_priv->regfile.saveDSPARB = intel_de_read(dev_priv,
- DSPARB(dev_priv));
-
- if (GRAPHICS_VER(dev_priv) == 4)
- pci_read_config_word(pdev, GCDGMBUS,
- &dev_priv->regfile.saveGCDGMBUS);
-
- intel_save_swf(dev_priv);
-}
-
-void i915_restore_display(struct drm_i915_private *dev_priv)
-{
- struct intel_display *display = &dev_priv->display;
- struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
-
- if (!HAS_DISPLAY(dev_priv))
- return;
-
- intel_restore_swf(dev_priv);
-
- if (GRAPHICS_VER(dev_priv) == 4)
- pci_write_config_word(pdev, GCDGMBUS,
- dev_priv->regfile.saveGCDGMBUS);
-
- /* Display arbitration */
- if (GRAPHICS_VER(dev_priv) <= 4)
- intel_de_write(dev_priv, DSPARB(dev_priv),
- dev_priv->regfile.saveDSPARB);
-
- intel_vga_redisable(display);
-
- intel_gmbus_reset(dev_priv);
-}
diff --git a/drivers/gpu/drm/i915/i915_suspend.h b/drivers/gpu/drm/i915/i915_suspend.h
deleted file mode 100644
index e5a611ee3d15..000000000000
--- a/drivers/gpu/drm/i915/i915_suspend.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: MIT */
-/*
- * Copyright © 2019 Intel Corporation
- */
-
-#ifndef __I915_SUSPEND_H__
-#define __I915_SUSPEND_H__
-
-struct drm_i915_private;
-
-void i915_save_display(struct drm_i915_private *i915);
-void i915_restore_display(struct drm_i915_private *i915);
-
-#endif /* __I915_SUSPEND_H__ */
--
2.46.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BUILD: failure for drm/i915/display: Convert i915_suspend into i9xx_display_sr
2024-09-12 17:45 [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr Rodrigo Vivi
@ 2024-09-12 21:50 ` Patchwork
2024-09-13 10:14 ` [PATCH] " Jani Nikula
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2024-09-12 21:50 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/display: Convert i915_suspend into i9xx_display_sr
URL : https://patchwork.freedesktop.org/series/138603/
State : failure
== Summary ==
Error: make failed
CALL scripts/checksyscalls.sh
DESCEND objtool
INSTALL libsubcmd_headers
make[6]: *** No rule to make target 'drivers/gpu/drm/i915/display/i9xx_suspend.o', needed by 'drivers/gpu/drm/i915/i915.o'. Stop.
make[5]: *** [scripts/Makefile.build:485: drivers/gpu/drm/i915] Error 2
make[4]: *** [scripts/Makefile.build:485: drivers/gpu/drm] Error 2
make[3]: *** [scripts/Makefile.build:485: drivers/gpu] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/home/kbuild/kernel/Makefile:1926: .] Error 2
make: *** [Makefile:224: __sub-make] Error 2
Build failed, no error log produced
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr
2024-09-12 17:45 [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr Rodrigo Vivi
2024-09-12 21:50 ` ✗ Fi.CI.BUILD: failure for " Patchwork
@ 2024-09-13 10:14 ` Jani Nikula
2024-09-13 11:22 ` Ville Syrjälä
2024-09-13 13:04 ` Jani Nikula
3 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2024-09-13 10:14 UTC (permalink / raw)
To: Rodrigo Vivi, intel-gfx; +Cc: Rodrigo Vivi, Jesse Barnes
On Thu, 12 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> These save & restore functions inside i915_suspend are old display
> functions to save and restore a bunch of display related registers.
>
> Move it under display and rename accordantly. Just don't move it
> entirely towards intel_display struct yet because it depends
> on drm_i915_private for the IS_MOBILE.
>
> While doing this conversion also update the MIT header using
> the new SPDX ones.
>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 2 +-
> .../gpu/drm/i915/display/i9xx_display_sr.c | 119 +++++++++++++++
> .../gpu/drm/i915/display/i9xx_display_sr.h | 14 ++
> drivers/gpu/drm/i915/i915_driver.c | 6 +-
> drivers/gpu/drm/i915/i915_suspend.c | 141 ------------------
> drivers/gpu/drm/i915/i915_suspend.h | 14 --
> 6 files changed, 137 insertions(+), 159 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.c
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.h
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.c
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..89f04bdbc27f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -30,7 +30,6 @@ i915-y += \
> i915_params.o \
> i915_pci.o \
> i915_scatterlist.o \
> - i915_suspend.o \
> i915_switcheroo.o \
> i915_sysfs.o \
> i915_utils.o \
> @@ -219,6 +218,7 @@ i915-$(CONFIG_HWMON) += \
> i915-y += \
> display/hsw_ips.o \
> display/i9xx_plane.o \
> + display/i9xx_suspend.o \
Maybe forgot to git add after settling on the file name? ;)
> display/i9xx_wm.o \
> display/intel_alpm.o \
> display/intel_atomic.o \
> diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.c b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> new file mode 100644
> index 000000000000..211cf41119ad
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_reg.h"
> +#include "i9xx_suspend.h"
> +#include "intel_de.h"
> +#include "intel_gmbus.h"
> +#include "intel_vga.h"
> +#include "intel_pci_config.h"
> +
> +static void intel_save_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + }
> +}
> +
> +static void intel_restore_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + }
> +}
> +
> +void i9xx_display_sr_save(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + /* Display arbitration control */
> + if (DISPLAY_VER(i915) <= 4)
> + i915->regfile.saveDSPARB = intel_de_read(i915,
> + DSPARB(i915));
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_read_config_word(pdev, GCDGMBUS,
> + &i915->regfile.saveGCDGMBUS);
> +
> + intel_save_swf(i915);
> +}
> +
> +void i9xx_display_sr_restore(struct drm_i915_private *i915)
> +{
> + struct intel_display *display = &i915->display;
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + intel_restore_swf(i915);
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_write_config_word(pdev, GCDGMBUS,
> + i915->regfile.saveGCDGMBUS);
> +
> + /* Display arbitration */
> + if (DISPLAY_VER(i915) <= 4)
> + intel_de_write(i915, DSPARB(i915),
> + i915->regfile.saveDSPARB);
> +
> + intel_vga_redisable(display);
> +
> + intel_gmbus_reset(i915);
These two aren't strictly i9xx, and neither are they about
saving/restoring stuff the same way the rest of this file is. So maybe
move them out of here first?
We also do neither on xe. Should we?
> +}
> diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.h b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
> new file mode 100644
> index 000000000000..d3598c729137
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2014 Intel Corporation
> + */
> +
> +#ifndef __I9XX_DISPLAY_SR_H__
> +#define __I9XX_DISPLAY_SR_H__
> +
> +struct drm_i915_private;
> +
> +void i9xx_display_sr_save(struct drm_i915_private *i915);
> +void i9xx_display_sr_restore(struct drm_i915_private *i915);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index f82aa313f854..e15bf3aa44f9 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -45,6 +45,7 @@
> #include <drm/drm_managed.h>
> #include <drm/drm_probe_helper.h>
>
> +#include "display/i9xx_display_sr.h"
> #include "display/intel_acpi.h"
> #include "display/intel_bw.h"
> #include "display/intel_cdclk.h"
> @@ -93,7 +94,6 @@
> #include "i915_memcpy.h"
> #include "i915_perf.h"
> #include "i915_query.h"
> -#include "i915_suspend.h"
> #include "i915_switcheroo.h"
> #include "i915_sysfs.h"
> #include "i915_utils.h"
> @@ -1047,7 +1047,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> intel_dpt_suspend(dev_priv);
> i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
>
> - i915_save_display(dev_priv);
> + i9xx_display_sr_save(dev_priv);
>
> opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> intel_opregion_suspend(display, opregion_target_state);
> @@ -1166,7 +1166,7 @@ static int i915_drm_resume(struct drm_device *dev)
>
> intel_dmc_resume(display);
>
> - i915_restore_display(dev_priv);
> + i9xx_display_sr_restore(dev_priv);
It's just painful that we do this from i915_driver.c. :/
Of course, unrelated to your patch.
BR,
Jani.
> intel_pps_unlock_regs_wa(display);
>
> intel_init_pch_refclk(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> deleted file mode 100644
> index 9d3d9b983032..000000000000
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ /dev/null
> @@ -1,141 +0,0 @@
> -/*
> - *
> - * Copyright 2008 (c) Intel Corporation
> - * Jesse Barnes <jbarnes@virtuousgeek.org>
> - *
> - * 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, sub license, 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 NON-INFRINGEMENT.
> - * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS 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.
> - */
> -
> -#include "display/intel_de.h"
> -#include "display/intel_gmbus.h"
> -#include "display/intel_vga.h"
> -
> -#include "i915_drv.h"
> -#include "i915_reg.h"
> -#include "i915_suspend.h"
> -#include "intel_pci_config.h"
> -
> -static void intel_save_swf(struct drm_i915_private *dev_priv)
> -{
> - int i;
> -
> - /* Scratch space */
> - if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
> - for (i = 0; i < 7; i++) {
> - dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
> - SWF0(dev_priv, i));
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
> - SWF3(dev_priv, i));
> - } else if (GRAPHICS_VER(dev_priv) == 2) {
> - for (i = 0; i < 7; i++)
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - } else if (HAS_GMCH(dev_priv)) {
> - for (i = 0; i < 16; i++) {
> - dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
> - SWF0(dev_priv, i));
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
> - SWF3(dev_priv, i));
> - }
> -}
> -
> -static void intel_restore_swf(struct drm_i915_private *dev_priv)
> -{
> - int i;
> -
> - /* Scratch space */
> - if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
> - for (i = 0; i < 7; i++) {
> - intel_de_write(dev_priv, SWF0(dev_priv, i),
> - dev_priv->regfile.saveSWF0[i]);
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - }
> - for (i = 0; i < 3; i++)
> - intel_de_write(dev_priv, SWF3(dev_priv, i),
> - dev_priv->regfile.saveSWF3[i]);
> - } else if (GRAPHICS_VER(dev_priv) == 2) {
> - for (i = 0; i < 7; i++)
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - } else if (HAS_GMCH(dev_priv)) {
> - for (i = 0; i < 16; i++) {
> - intel_de_write(dev_priv, SWF0(dev_priv, i),
> - dev_priv->regfile.saveSWF0[i]);
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - }
> - for (i = 0; i < 3; i++)
> - intel_de_write(dev_priv, SWF3(dev_priv, i),
> - dev_priv->regfile.saveSWF3[i]);
> - }
> -}
> -
> -void i915_save_display(struct drm_i915_private *dev_priv)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> -
> - if (!HAS_DISPLAY(dev_priv))
> - return;
> -
> - /* Display arbitration control */
> - if (GRAPHICS_VER(dev_priv) <= 4)
> - dev_priv->regfile.saveDSPARB = intel_de_read(dev_priv,
> - DSPARB(dev_priv));
> -
> - if (GRAPHICS_VER(dev_priv) == 4)
> - pci_read_config_word(pdev, GCDGMBUS,
> - &dev_priv->regfile.saveGCDGMBUS);
> -
> - intel_save_swf(dev_priv);
> -}
> -
> -void i915_restore_display(struct drm_i915_private *dev_priv)
> -{
> - struct intel_display *display = &dev_priv->display;
> - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> -
> - if (!HAS_DISPLAY(dev_priv))
> - return;
> -
> - intel_restore_swf(dev_priv);
> -
> - if (GRAPHICS_VER(dev_priv) == 4)
> - pci_write_config_word(pdev, GCDGMBUS,
> - dev_priv->regfile.saveGCDGMBUS);
> -
> - /* Display arbitration */
> - if (GRAPHICS_VER(dev_priv) <= 4)
> - intel_de_write(dev_priv, DSPARB(dev_priv),
> - dev_priv->regfile.saveDSPARB);
> -
> - intel_vga_redisable(display);
> -
> - intel_gmbus_reset(dev_priv);
> -}
> diff --git a/drivers/gpu/drm/i915/i915_suspend.h b/drivers/gpu/drm/i915/i915_suspend.h
> deleted file mode 100644
> index e5a611ee3d15..000000000000
> --- a/drivers/gpu/drm/i915/i915_suspend.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: MIT */
> -/*
> - * Copyright © 2019 Intel Corporation
> - */
> -
> -#ifndef __I915_SUSPEND_H__
> -#define __I915_SUSPEND_H__
> -
> -struct drm_i915_private;
> -
> -void i915_save_display(struct drm_i915_private *i915);
> -void i915_restore_display(struct drm_i915_private *i915);
> -
> -#endif /* __I915_SUSPEND_H__ */
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr
2024-09-12 17:45 [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr Rodrigo Vivi
2024-09-12 21:50 ` ✗ Fi.CI.BUILD: failure for " Patchwork
2024-09-13 10:14 ` [PATCH] " Jani Nikula
@ 2024-09-13 11:22 ` Ville Syrjälä
2024-09-16 21:25 ` Rodrigo Vivi
2024-09-13 13:04 ` Jani Nikula
3 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2024-09-13 11:22 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Jesse Barnes, Jani Nikula
On Thu, Sep 12, 2024 at 01:45:34PM -0400, Rodrigo Vivi wrote:
> These save & restore functions inside i915_suspend are old display
> functions to save and restore a bunch of display related registers.
>
> Move it under display and rename accordantly. Just don't move it
> entirely towards intel_display struct yet because it depends
> on drm_i915_private for the IS_MOBILE.
>
> While doing this conversion also update the MIT header using
> the new SPDX ones.
>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 2 +-
> .../gpu/drm/i915/display/i9xx_display_sr.c | 119 +++++++++++++++
> .../gpu/drm/i915/display/i9xx_display_sr.h | 14 ++
> drivers/gpu/drm/i915/i915_driver.c | 6 +-
> drivers/gpu/drm/i915/i915_suspend.c | 141 ------------------
> drivers/gpu/drm/i915/i915_suspend.h | 14 --
> 6 files changed, 137 insertions(+), 159 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.c
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.h
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.c
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..89f04bdbc27f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -30,7 +30,6 @@ i915-y += \
> i915_params.o \
> i915_pci.o \
> i915_scatterlist.o \
> - i915_suspend.o \
> i915_switcheroo.o \
> i915_sysfs.o \
> i915_utils.o \
> @@ -219,6 +218,7 @@ i915-$(CONFIG_HWMON) += \
> i915-y += \
> display/hsw_ips.o \
> display/i9xx_plane.o \
> + display/i9xx_suspend.o \
> display/i9xx_wm.o \
> display/intel_alpm.o \
> display/intel_atomic.o \
> diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.c b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> new file mode 100644
> index 000000000000..211cf41119ad
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_reg.h"
> +#include "i9xx_suspend.h"
> +#include "intel_de.h"
> +#include "intel_gmbus.h"
> +#include "intel_vga.h"
> +#include "intel_pci_config.h"
> +
> +static void intel_save_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + }
> +}
> +
> +static void intel_restore_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + }
> +}
> +
> +void i9xx_display_sr_save(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + /* Display arbitration control */
> + if (DISPLAY_VER(i915) <= 4)
> + i915->regfile.saveDSPARB = intel_de_read(i915,
> + DSPARB(i915));
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_read_config_word(pdev, GCDGMBUS,
> + &i915->regfile.saveGCDGMBUS);
> +
> + intel_save_swf(i915);
> +}
> +
> +void i9xx_display_sr_restore(struct drm_i915_private *i915)
> +{
> + struct intel_display *display = &i915->display;
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + intel_restore_swf(i915);
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_write_config_word(pdev, GCDGMBUS,
> + i915->regfile.saveGCDGMBUS);
> +
> + /* Display arbitration */
> + if (DISPLAY_VER(i915) <= 4)
> + intel_de_write(i915, DSPARB(i915),
> + i915->regfile.saveDSPARB);
> +
> + intel_vga_redisable(display);
> +
> + intel_gmbus_reset(i915);
The last two are for all platforms, so the function name is a bit
misleading now.
Also we might want to do the SWF save/restore for all platforms
as well, because technically we should be reading one of those
to determine the maximum CDCLK for the system (if the BIOS
has chosen a limit other than the platform default). We could
get into trouble there if the driver is reloaded after S3,
assuming S3 clobbers the SWF registers (which I'm not 100%
sure it does, would need to confirm it).
I have an old branch that does the SWF read for BDW only
'https://github.com/vsyrjala/linux.git bdw_vbios_cdclk_limit'
but last I looked Windows was still doing this for all
platforms, so we should probably we doing the same.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr
2024-09-12 17:45 [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr Rodrigo Vivi
` (2 preceding siblings ...)
2024-09-13 11:22 ` Ville Syrjälä
@ 2024-09-13 13:04 ` Jani Nikula
3 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2024-09-13 13:04 UTC (permalink / raw)
To: Rodrigo Vivi, intel-gfx; +Cc: Rodrigo Vivi, Jesse Barnes
On Thu, 12 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> These save & restore functions inside i915_suspend are old display
> functions to save and restore a bunch of display related registers.
>
> Move it under display and rename accordantly. Just don't move it
> entirely towards intel_display struct yet because it depends
> on drm_i915_private for the IS_MOBILE.
>
> While doing this conversion also update the MIT header using
> the new SPDX ones.
>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 2 +-
> .../gpu/drm/i915/display/i9xx_display_sr.c | 119 +++++++++++++++
> .../gpu/drm/i915/display/i9xx_display_sr.h | 14 ++
> drivers/gpu/drm/i915/i915_driver.c | 6 +-
> drivers/gpu/drm/i915/i915_suspend.c | 141 ------------------
> drivers/gpu/drm/i915/i915_suspend.h | 14 --
> 6 files changed, 137 insertions(+), 159 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.c
> create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.h
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.c
> delete mode 100644 drivers/gpu/drm/i915/i915_suspend.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..89f04bdbc27f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -30,7 +30,6 @@ i915-y += \
> i915_params.o \
> i915_pci.o \
> i915_scatterlist.o \
> - i915_suspend.o \
> i915_switcheroo.o \
> i915_sysfs.o \
> i915_utils.o \
> @@ -219,6 +218,7 @@ i915-$(CONFIG_HWMON) += \
> i915-y += \
> display/hsw_ips.o \
> display/i9xx_plane.o \
> + display/i9xx_suspend.o \
> display/i9xx_wm.o \
> display/intel_alpm.o \
> display/intel_atomic.o \
> diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.c b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> new file mode 100644
> index 000000000000..211cf41119ad
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_reg.h"
> +#include "i9xx_suspend.h"
> +#include "intel_de.h"
> +#include "intel_gmbus.h"
> +#include "intel_vga.h"
> +#include "intel_pci_config.h"
> +
> +static void intel_save_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
As follow-up, i915->regfile needs to be moved to display.
BR,
Jani.
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> + SWF0(i915, i));
> + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> + SWF1(i915, i));
> + }
> + for (i = 0; i < 3; i++)
> + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> + SWF3(i915, i));
> + }
> +}
> +
> +static void intel_restore_swf(struct drm_i915_private *i915)
> +{
> + int i;
> +
> + /* Scratch space */
> + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> + for (i = 0; i < 7; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + } else if (DISPLAY_VER(i915) == 2) {
> + for (i = 0; i < 7; i++)
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + } else if (HAS_GMCH(i915)) {
> + for (i = 0; i < 16; i++) {
> + intel_de_write(i915, SWF0(i915, i),
> + i915->regfile.saveSWF0[i]);
> + intel_de_write(i915, SWF1(i915, i),
> + i915->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + intel_de_write(i915, SWF3(i915, i),
> + i915->regfile.saveSWF3[i]);
> + }
> +}
> +
> +void i9xx_display_sr_save(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + /* Display arbitration control */
> + if (DISPLAY_VER(i915) <= 4)
> + i915->regfile.saveDSPARB = intel_de_read(i915,
> + DSPARB(i915));
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_read_config_word(pdev, GCDGMBUS,
> + &i915->regfile.saveGCDGMBUS);
> +
> + intel_save_swf(i915);
> +}
> +
> +void i9xx_display_sr_restore(struct drm_i915_private *i915)
> +{
> + struct intel_display *display = &i915->display;
> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +
> + if (!HAS_DISPLAY(i915))
> + return;
> +
> + intel_restore_swf(i915);
> +
> + if (DISPLAY_VER(i915) == 4)
> + pci_write_config_word(pdev, GCDGMBUS,
> + i915->regfile.saveGCDGMBUS);
> +
> + /* Display arbitration */
> + if (DISPLAY_VER(i915) <= 4)
> + intel_de_write(i915, DSPARB(i915),
> + i915->regfile.saveDSPARB);
> +
> + intel_vga_redisable(display);
> +
> + intel_gmbus_reset(i915);
> +}
> diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.h b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
> new file mode 100644
> index 000000000000..d3598c729137
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2014 Intel Corporation
> + */
> +
> +#ifndef __I9XX_DISPLAY_SR_H__
> +#define __I9XX_DISPLAY_SR_H__
> +
> +struct drm_i915_private;
> +
> +void i9xx_display_sr_save(struct drm_i915_private *i915);
> +void i9xx_display_sr_restore(struct drm_i915_private *i915);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index f82aa313f854..e15bf3aa44f9 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -45,6 +45,7 @@
> #include <drm/drm_managed.h>
> #include <drm/drm_probe_helper.h>
>
> +#include "display/i9xx_display_sr.h"
> #include "display/intel_acpi.h"
> #include "display/intel_bw.h"
> #include "display/intel_cdclk.h"
> @@ -93,7 +94,6 @@
> #include "i915_memcpy.h"
> #include "i915_perf.h"
> #include "i915_query.h"
> -#include "i915_suspend.h"
> #include "i915_switcheroo.h"
> #include "i915_sysfs.h"
> #include "i915_utils.h"
> @@ -1047,7 +1047,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> intel_dpt_suspend(dev_priv);
> i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
>
> - i915_save_display(dev_priv);
> + i9xx_display_sr_save(dev_priv);
>
> opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> intel_opregion_suspend(display, opregion_target_state);
> @@ -1166,7 +1166,7 @@ static int i915_drm_resume(struct drm_device *dev)
>
> intel_dmc_resume(display);
>
> - i915_restore_display(dev_priv);
> + i9xx_display_sr_restore(dev_priv);
> intel_pps_unlock_regs_wa(display);
>
> intel_init_pch_refclk(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> deleted file mode 100644
> index 9d3d9b983032..000000000000
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ /dev/null
> @@ -1,141 +0,0 @@
> -/*
> - *
> - * Copyright 2008 (c) Intel Corporation
> - * Jesse Barnes <jbarnes@virtuousgeek.org>
> - *
> - * 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, sub license, 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 NON-INFRINGEMENT.
> - * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS 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.
> - */
> -
> -#include "display/intel_de.h"
> -#include "display/intel_gmbus.h"
> -#include "display/intel_vga.h"
> -
> -#include "i915_drv.h"
> -#include "i915_reg.h"
> -#include "i915_suspend.h"
> -#include "intel_pci_config.h"
> -
> -static void intel_save_swf(struct drm_i915_private *dev_priv)
> -{
> - int i;
> -
> - /* Scratch space */
> - if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
> - for (i = 0; i < 7; i++) {
> - dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
> - SWF0(dev_priv, i));
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
> - SWF3(dev_priv, i));
> - } else if (GRAPHICS_VER(dev_priv) == 2) {
> - for (i = 0; i < 7; i++)
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - } else if (HAS_GMCH(dev_priv)) {
> - for (i = 0; i < 16; i++) {
> - dev_priv->regfile.saveSWF0[i] = intel_de_read(dev_priv,
> - SWF0(dev_priv, i));
> - dev_priv->regfile.saveSWF1[i] = intel_de_read(dev_priv,
> - SWF1(dev_priv, i));
> - }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF3[i] = intel_de_read(dev_priv,
> - SWF3(dev_priv, i));
> - }
> -}
> -
> -static void intel_restore_swf(struct drm_i915_private *dev_priv)
> -{
> - int i;
> -
> - /* Scratch space */
> - if (GRAPHICS_VER(dev_priv) == 2 && IS_MOBILE(dev_priv)) {
> - for (i = 0; i < 7; i++) {
> - intel_de_write(dev_priv, SWF0(dev_priv, i),
> - dev_priv->regfile.saveSWF0[i]);
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - }
> - for (i = 0; i < 3; i++)
> - intel_de_write(dev_priv, SWF3(dev_priv, i),
> - dev_priv->regfile.saveSWF3[i]);
> - } else if (GRAPHICS_VER(dev_priv) == 2) {
> - for (i = 0; i < 7; i++)
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - } else if (HAS_GMCH(dev_priv)) {
> - for (i = 0; i < 16; i++) {
> - intel_de_write(dev_priv, SWF0(dev_priv, i),
> - dev_priv->regfile.saveSWF0[i]);
> - intel_de_write(dev_priv, SWF1(dev_priv, i),
> - dev_priv->regfile.saveSWF1[i]);
> - }
> - for (i = 0; i < 3; i++)
> - intel_de_write(dev_priv, SWF3(dev_priv, i),
> - dev_priv->regfile.saveSWF3[i]);
> - }
> -}
> -
> -void i915_save_display(struct drm_i915_private *dev_priv)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> -
> - if (!HAS_DISPLAY(dev_priv))
> - return;
> -
> - /* Display arbitration control */
> - if (GRAPHICS_VER(dev_priv) <= 4)
> - dev_priv->regfile.saveDSPARB = intel_de_read(dev_priv,
> - DSPARB(dev_priv));
> -
> - if (GRAPHICS_VER(dev_priv) == 4)
> - pci_read_config_word(pdev, GCDGMBUS,
> - &dev_priv->regfile.saveGCDGMBUS);
> -
> - intel_save_swf(dev_priv);
> -}
> -
> -void i915_restore_display(struct drm_i915_private *dev_priv)
> -{
> - struct intel_display *display = &dev_priv->display;
> - struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> -
> - if (!HAS_DISPLAY(dev_priv))
> - return;
> -
> - intel_restore_swf(dev_priv);
> -
> - if (GRAPHICS_VER(dev_priv) == 4)
> - pci_write_config_word(pdev, GCDGMBUS,
> - dev_priv->regfile.saveGCDGMBUS);
> -
> - /* Display arbitration */
> - if (GRAPHICS_VER(dev_priv) <= 4)
> - intel_de_write(dev_priv, DSPARB(dev_priv),
> - dev_priv->regfile.saveDSPARB);
> -
> - intel_vga_redisable(display);
> -
> - intel_gmbus_reset(dev_priv);
> -}
> diff --git a/drivers/gpu/drm/i915/i915_suspend.h b/drivers/gpu/drm/i915/i915_suspend.h
> deleted file mode 100644
> index e5a611ee3d15..000000000000
> --- a/drivers/gpu/drm/i915/i915_suspend.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: MIT */
> -/*
> - * Copyright © 2019 Intel Corporation
> - */
> -
> -#ifndef __I915_SUSPEND_H__
> -#define __I915_SUSPEND_H__
> -
> -struct drm_i915_private;
> -
> -void i915_save_display(struct drm_i915_private *i915);
> -void i915_restore_display(struct drm_i915_private *i915);
> -
> -#endif /* __I915_SUSPEND_H__ */
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr
2024-09-13 11:22 ` Ville Syrjälä
@ 2024-09-16 21:25 ` Rodrigo Vivi
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2024-09-16 21:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Jesse Barnes, Jani Nikula
On Fri, Sep 13, 2024 at 02:22:34PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 12, 2024 at 01:45:34PM -0400, Rodrigo Vivi wrote:
> > These save & restore functions inside i915_suspend are old display
> > functions to save and restore a bunch of display related registers.
> >
> > Move it under display and rename accordantly. Just don't move it
> > entirely towards intel_display struct yet because it depends
> > on drm_i915_private for the IS_MOBILE.
> >
> > While doing this conversion also update the MIT header using
> > the new SPDX ones.
> >
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/Makefile | 2 +-
> > .../gpu/drm/i915/display/i9xx_display_sr.c | 119 +++++++++++++++
> > .../gpu/drm/i915/display/i9xx_display_sr.h | 14 ++
> > drivers/gpu/drm/i915/i915_driver.c | 6 +-
> > drivers/gpu/drm/i915/i915_suspend.c | 141 ------------------
> > drivers/gpu/drm/i915/i915_suspend.h | 14 --
> > 6 files changed, 137 insertions(+), 159 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.c
> > create mode 100644 drivers/gpu/drm/i915/display/i9xx_display_sr.h
> > delete mode 100644 drivers/gpu/drm/i915/i915_suspend.c
> > delete mode 100644 drivers/gpu/drm/i915/i915_suspend.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index c63fa2133ccb..89f04bdbc27f 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -30,7 +30,6 @@ i915-y += \
> > i915_params.o \
> > i915_pci.o \
> > i915_scatterlist.o \
> > - i915_suspend.o \
> > i915_switcheroo.o \
> > i915_sysfs.o \
> > i915_utils.o \
> > @@ -219,6 +218,7 @@ i915-$(CONFIG_HWMON) += \
> > i915-y += \
> > display/hsw_ips.o \
> > display/i9xx_plane.o \
> > + display/i9xx_suspend.o \
> > display/i9xx_wm.o \
> > display/intel_alpm.o \
> > display/intel_atomic.o \
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_display_sr.c b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> > new file mode 100644
> > index 000000000000..211cf41119ad
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/i9xx_display_sr.c
> > @@ -0,0 +1,119 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#include "i915_drv.h"
> > +#include "i915_reg.h"
> > +#include "i9xx_suspend.h"
> > +#include "intel_de.h"
> > +#include "intel_gmbus.h"
> > +#include "intel_vga.h"
> > +#include "intel_pci_config.h"
> > +
> > +static void intel_save_swf(struct drm_i915_private *i915)
> > +{
> > + int i;
> > +
> > + /* Scratch space */
> > + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> > + for (i = 0; i < 7; i++) {
> > + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> > + SWF0(i915, i));
> > + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> > + SWF1(i915, i));
> > + }
> > + for (i = 0; i < 3; i++)
> > + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> > + SWF3(i915, i));
> > + } else if (DISPLAY_VER(i915) == 2) {
> > + for (i = 0; i < 7; i++)
> > + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> > + SWF1(i915, i));
> > + } else if (HAS_GMCH(i915)) {
> > + for (i = 0; i < 16; i++) {
> > + i915->regfile.saveSWF0[i] = intel_de_read(i915,
> > + SWF0(i915, i));
> > + i915->regfile.saveSWF1[i] = intel_de_read(i915,
> > + SWF1(i915, i));
> > + }
> > + for (i = 0; i < 3; i++)
> > + i915->regfile.saveSWF3[i] = intel_de_read(i915,
> > + SWF3(i915, i));
> > + }
> > +}
> > +
> > +static void intel_restore_swf(struct drm_i915_private *i915)
> > +{
> > + int i;
> > +
> > + /* Scratch space */
> > + if (DISPLAY_VER(i915) == 2 && IS_MOBILE(i915)) {
> > + for (i = 0; i < 7; i++) {
> > + intel_de_write(i915, SWF0(i915, i),
> > + i915->regfile.saveSWF0[i]);
> > + intel_de_write(i915, SWF1(i915, i),
> > + i915->regfile.saveSWF1[i]);
> > + }
> > + for (i = 0; i < 3; i++)
> > + intel_de_write(i915, SWF3(i915, i),
> > + i915->regfile.saveSWF3[i]);
> > + } else if (DISPLAY_VER(i915) == 2) {
> > + for (i = 0; i < 7; i++)
> > + intel_de_write(i915, SWF1(i915, i),
> > + i915->regfile.saveSWF1[i]);
> > + } else if (HAS_GMCH(i915)) {
> > + for (i = 0; i < 16; i++) {
> > + intel_de_write(i915, SWF0(i915, i),
> > + i915->regfile.saveSWF0[i]);
> > + intel_de_write(i915, SWF1(i915, i),
> > + i915->regfile.saveSWF1[i]);
> > + }
> > + for (i = 0; i < 3; i++)
> > + intel_de_write(i915, SWF3(i915, i),
> > + i915->regfile.saveSWF3[i]);
> > + }
> > +}
> > +
> > +void i9xx_display_sr_save(struct drm_i915_private *i915)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > +
> > + if (!HAS_DISPLAY(i915))
> > + return;
> > +
> > + /* Display arbitration control */
> > + if (DISPLAY_VER(i915) <= 4)
> > + i915->regfile.saveDSPARB = intel_de_read(i915,
> > + DSPARB(i915));
> > +
> > + if (DISPLAY_VER(i915) == 4)
> > + pci_read_config_word(pdev, GCDGMBUS,
> > + &i915->regfile.saveGCDGMBUS);
> > +
> > + intel_save_swf(i915);
> > +}
> > +
> > +void i9xx_display_sr_restore(struct drm_i915_private *i915)
> > +{
> > + struct intel_display *display = &i915->display;
> > + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > +
> > + if (!HAS_DISPLAY(i915))
> > + return;
> > +
> > + intel_restore_swf(i915);
> > +
> > + if (DISPLAY_VER(i915) == 4)
> > + pci_write_config_word(pdev, GCDGMBUS,
> > + i915->regfile.saveGCDGMBUS);
> > +
> > + /* Display arbitration */
> > + if (DISPLAY_VER(i915) <= 4)
> > + intel_de_write(i915, DSPARB(i915),
> > + i915->regfile.saveDSPARB);
> > +
> > + intel_vga_redisable(display);
> > +
> > + intel_gmbus_reset(i915);
>
> The last two are for all platforms, so the function name is a bit
> misleading now.
Indeed... having another patch to remove this from this function
before the name conversion.
>
> Also we might want to do the SWF save/restore for all platforms
> as well, because technically we should be reading one of those
> to determine the maximum CDCLK for the system (if the BIOS
> has chosen a limit other than the platform default). We could
> get into trouble there if the driver is reloaded after S3,
> assuming S3 clobbers the SWF registers (which I'm not 100%
> sure it does, would need to confirm it).
Well, if we end up needing that we can always do a s/i9xx/i915
later in the name. But I really don't believe that we should
be the ones taking care of the registers save/restore. Some
one else should be taking care of this save restore, or we should
simply stash the boot value.
>
> I have an old branch that does the SWF read for BDW only
> 'https://github.com/vsyrjala/linux.git bdw_vbios_cdclk_limit'
> but last I looked Windows was still doing this for all
> platforms, so we should probably we doing the same.
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-16 21:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-12 17:45 [PATCH] drm/i915/display: Convert i915_suspend into i9xx_display_sr Rodrigo Vivi
2024-09-12 21:50 ` ✗ Fi.CI.BUILD: failure for " Patchwork
2024-09-13 10:14 ` [PATCH] " Jani Nikula
2024-09-13 11:22 ` Ville Syrjälä
2024-09-16 21:25 ` Rodrigo Vivi
2024-09-13 13:04 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox