linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
@ 2020-08-06  7:22 Vaibhav Gupta
  2020-08-06  7:26 ` Vaibhav Gupta
  2020-09-07  7:55 ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Vaibhav Gupta @ 2020-08-06  7:22 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, Vaibhav Gupta, linux-kernel, dri-devel,
	linux-kernel-mentees

Linux Kernel Mentee: Remove Legacy Power Management. 

The original goal of the patch series is to upgrade the power management
framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
and .resume() callbacks.

The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.

During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
redundant. This was introduced in the commit
42ddb453a0cd ("radeon: Conditionally compile PM code").

------------

Before 42ddb453a0cd:
$ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"

Based on output in terminal:

547:#ifdef CONFIG_PM
       |-- 959:#ifdef CONFIG_PPC_PMAC
       |-- 972:#endif
       |-- 1291:#ifdef CONFIG_PPC_OF
       |-- 1301:#endif /* CONFIG_PPC_OF */
       |-- 1943:#ifdef CONFIG_PPC_OF
                   |-- 2206:#if 0 /* Not ready yet */
                   |-- 2508:#endif /* 0 */
       |-- 2510:#endif /* CONFIG_PPC_OF */
       |-- 2648:#ifdef CONFIG_PPC_PMAC
       |-- 2654:#endif /* CONFIG_PPC_PMAC */
       |-- 2768:#ifdef CONFIG_PPC_PMAC
       |-- 2774:#endif /* CONFIG_PPC_PMAC */
       |-- 2791:#ifdef CONFIG_PPC_OF__disabled
       |-- 2801:#endif /* CONFIG_PPC_OF */
2803:#endif /* CONFIG_PM */

------------

After 42ddb453a0cd:
$ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"

Based on output in terminal:

547:#ifdef CONFIG_PM
       |-- 959:#ifdef CONFIG_PPC_PMAC
       |-- 972:#endif
       |-- 1291:#ifdef CONFIG_PPC_OF
       |-- 1301:#endif /* CONFIG_PPC_OF */
       |-- 1430:#if defined(CONFIG_PM)
                   |-- 1431:#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
                   |-- 1944:#endif
                   |-- 1946:#ifdef CONFIG_PPC_OF
                               |-- 1947:#ifdef CONFIG_PPC_PMAC
                               |-- 2208:#endif
                   |-- 2209:#endif
                   |-- 2211:#if 0 /* Not ready yet */
                   |-- 2513:#endif /* 0 */
       |-- 2515:#endif /* CONFIG_PPC_OF */
       |-- 2653:#ifdef CONFIG_PPC_PMAC
       |-- 2659:#endif /* CONFIG_PPC_PMAC */
       |-- 2773:#ifdef CONFIG_PPC_PMAC
       |-- 2779:#endif /* CONFIG_PPC_PMAC */
       |-- 2796:#ifdef CONFIG_PPC_OF__disabled
       |-- 2806:#endif /* CONFIG_PPC_OF */
2808:#endif /* CONFIG_PM */

------------

This also affected the CONFIG_PPC_OF container (line 1943 at commit 65122f7e80b5)

The patch-series fixes it along with PM upgrade.

All patches are compile-tested only.

Test tools:
    - Compiler: gcc (GCC) 10.1.0
    - allmodconfig build: make -j$(nproc) W=1 all

Vaibhav Gupta (2):
  video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  fbdev: radeonfb:use generic power management

 drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
 drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++-------
 drivers/video/fbdev/aty/radeonfb.h    |  3 +--
 3 files changed, 35 insertions(+), 16 deletions(-)

-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-08-06  7:22 [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
@ 2020-08-06  7:26 ` Vaibhav Gupta
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container Vaibhav Gupta
                     ` (2 more replies)
  2020-09-07  7:55 ` Daniel Vetter
  1 sibling, 3 replies; 16+ messages in thread
From: Vaibhav Gupta @ 2020-08-06  7:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, Vaibhav Gupta, linux-kernel, dri-devel,
	linux-kernel-mentees

Linux Kernel Mentee: Remove Legacy Power Management. 

The original goal of the patch series is to upgrade the power management
framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
and .resume() callbacks.

The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.

During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
redundant. This was introduced in the commit
42ddb453a0cd ("radeon: Conditionally compile PM code").

------------

Before 42ddb453a0cd:
$ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"

Based on output in terminal:

547:#ifdef CONFIG_PM
       |-- 959:#ifdef CONFIG_PPC_PMAC
       |-- 972:#endif
       |-- 1291:#ifdef CONFIG_PPC_OF
       |-- 1301:#endif /* CONFIG_PPC_OF */
       |-- 1943:#ifdef CONFIG_PPC_OF
                   |-- 2206:#if 0 /* Not ready yet */
                   |-- 2508:#endif /* 0 */
       |-- 2510:#endif /* CONFIG_PPC_OF */
       |-- 2648:#ifdef CONFIG_PPC_PMAC
       |-- 2654:#endif /* CONFIG_PPC_PMAC */
       |-- 2768:#ifdef CONFIG_PPC_PMAC
       |-- 2774:#endif /* CONFIG_PPC_PMAC */
       |-- 2791:#ifdef CONFIG_PPC_OF__disabled
       |-- 2801:#endif /* CONFIG_PPC_OF */
2803:#endif /* CONFIG_PM */

------------

After 42ddb453a0cd:
$ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"

Based on output in terminal:

547:#ifdef CONFIG_PM
       |-- 959:#ifdef CONFIG_PPC_PMAC
       |-- 972:#endif
       |-- 1291:#ifdef CONFIG_PPC_OF
       |-- 1301:#endif /* CONFIG_PPC_OF */
       |-- 1430:#if defined(CONFIG_PM)
                   |-- 1431:#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
                   |-- 1944:#endif
                   |-- 1946:#ifdef CONFIG_PPC_OF
                               |-- 1947:#ifdef CONFIG_PPC_PMAC
                               |-- 2208:#endif
                   |-- 2209:#endif
                   |-- 2211:#if 0 /* Not ready yet */
                   |-- 2513:#endif /* 0 */
       |-- 2515:#endif /* CONFIG_PPC_OF */
       |-- 2653:#ifdef CONFIG_PPC_PMAC
       |-- 2659:#endif /* CONFIG_PPC_PMAC */
       |-- 2773:#ifdef CONFIG_PPC_PMAC
       |-- 2779:#endif /* CONFIG_PPC_PMAC */
       |-- 2796:#ifdef CONFIG_PPC_OF__disabled
       |-- 2806:#endif /* CONFIG_PPC_OF */
2808:#endif /* CONFIG_PM */

------------

This also affected the CONFIG_PPC_OF container (line 1943 at commit 65122f7e80b5)

The patch-series fixes it along with PM upgrade.

All patches are compile-tested only.

Test tools:
    - Compiler: gcc (GCC) 10.1.0
    - allmodconfig build: make -j$(nproc) W=1 all

Vaibhav Gupta (2):
  video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  fbdev: radeonfb:use generic power management

 drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
 drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++-------
 drivers/video/fbdev/aty/radeonfb.h    |  3 +--
 3 files changed, 35 insertions(+), 16 deletions(-)

-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  2020-08-06  7:26 ` Vaibhav Gupta
@ 2020-08-06  7:26   ` Vaibhav Gupta
  2020-09-07  6:33     ` Vaibhav Gupta
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management Vaibhav Gupta
  2020-09-07  6:31   ` [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
  2 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Gupta @ 2020-08-06  7:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, Vaibhav Gupta, linux-kernel, dri-devel,
	linux-kernel-mentees

Fixes commit 42ddb453a0cd ("radeon: Conditionally compile PM code")

Before the above mentioned patch, codes between the line number 547 and
2803 were already inside "#ifdef CONFIG_PM" container. Thus, addition of
"#if defined(CONFIG_PM)" was not required in the patch. It also affected
the "#ifdef CONFIG_PPC_OF" container (line 1943-2510).

From the current snapshot of radeon_pm.c, remove:
    1434 | #if defined(CONFIG_PM)
and,
    2213 | #endif

This removes the redundant CONFIG_PM directive as well as fixes the
CONFIG_PPC (earlier CONFIG_PPC_OF) container.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/video/fbdev/aty/radeon_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_pm.c b/drivers/video/fbdev/aty/radeon_pm.c
index 7c4483c7f313..b9af70bd656a 100644
--- a/drivers/video/fbdev/aty/radeon_pm.c
+++ b/drivers/video/fbdev/aty/radeon_pm.c
@@ -1431,7 +1431,6 @@ static void radeon_pm_full_reset_sdram(struct radeonfb_info *rinfo)
 	mdelay( 15);
 }
 
-#if defined(CONFIG_PM)
 #if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
 static void radeon_pm_reset_pad_ctlr_strength(struct radeonfb_info *rinfo)
 {
@@ -2210,7 +2209,6 @@ static void radeon_reinitialize_M9P(struct radeonfb_info *rinfo)
 	radeon_pm_m10_enable_lvds_spread_spectrum(rinfo);
 }
 #endif
-#endif
 
 #if 0 /* Not ready yet */
 static void radeon_reinitialize_QW(struct radeonfb_info *rinfo)
-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management
  2020-08-06  7:26 ` Vaibhav Gupta
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container Vaibhav Gupta
@ 2020-08-06  7:26   ` Vaibhav Gupta
  2020-09-07  6:34     ` Vaibhav Gupta
  2020-09-07  6:31   ` [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
  2 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Gupta @ 2020-08-06  7:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, Vaibhav Gupta, linux-kernel, dri-devel,
	linux-kernel-mentees

Drivers using legacy PCI power management .suspend()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.

Switch to generic power management framework using a "struct dev_pm_ops"
variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/video/fbdev/aty/radeon_base.c | 10 +++++---
 drivers/video/fbdev/aty/radeon_pm.c   | 36 +++++++++++++++++++++------
 drivers/video/fbdev/aty/radeonfb.h    |  3 +--
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index e116a3f9ad56..232dbe154666 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2559,16 +2559,18 @@ static void radeonfb_pci_unregister(struct pci_dev *pdev)
         framebuffer_release(info);
 }
 
+#ifdef CONFIG_PM
+#define RADEONFB_PCI_PM_OPS (&radeonfb_pci_pm_ops)
+#else
+#define RADEONFB_PCI_PM_OPS NULL
+#endif
 
 static struct pci_driver radeonfb_driver = {
 	.name		= "radeonfb",
 	.id_table	= radeonfb_pci_table,
 	.probe		= radeonfb_pci_register,
 	.remove		= radeonfb_pci_unregister,
-#ifdef CONFIG_PM
-	.suspend       	= radeonfb_pci_suspend,
-	.resume		= radeonfb_pci_resume,
-#endif /* CONFIG_PM */
+	.driver.pm	= RADEONFB_PCI_PM_OPS,
 };
 
 #ifndef MODULE
diff --git a/drivers/video/fbdev/aty/radeon_pm.c b/drivers/video/fbdev/aty/radeon_pm.c
index b9af70bd656a..352d0bb4773a 100644
--- a/drivers/video/fbdev/aty/radeon_pm.c
+++ b/drivers/video/fbdev/aty/radeon_pm.c
@@ -2611,8 +2611,9 @@ static void radeon_set_suspend(struct radeonfb_info *rinfo, int suspend)
 	}
 }
 
-int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
+static int radeonfb_pci_suspend_late(struct device *dev, pm_message_t mesg)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
         struct fb_info *info = pci_get_drvdata(pdev);
         struct radeonfb_info *rinfo = info->par;
 
@@ -2660,11 +2661,6 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 	pmac_suspend_agp_for_card(pdev);
 #endif /* CONFIG_PPC_PMAC */
 
-	/* It's unclear whether or when the generic code will do that, so let's
-	 * do it ourselves. We save state before we do any power management
-	 */
-	pci_save_state(pdev);
-
 	/* If we support wakeup from poweroff, we save all regs we can including cfg
 	 * space
 	 */
@@ -2689,7 +2685,6 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 			msleep(20);
 			OUTREG(LVDS_GEN_CNTL, INREG(LVDS_GEN_CNTL) & ~(LVDS_DIGON));
 		}
-		pci_disable_device(pdev);
 	}
 	/* If we support D2, we go to it (should be fixed later with a flag forcing
 	 * D3 only for some laptops)
@@ -2705,6 +2700,21 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 	return 0;
 }
 
+static int radeonfb_pci_suspend(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_SUSPEND);
+}
+
+static int radeonfb_pci_hibernate(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_HIBERNATE);
+}
+
+static int radeonfb_pci_freeze(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_FREEZE);
+}
+
 static int radeon_check_power_loss(struct radeonfb_info *rinfo)
 {
 	return rinfo->save_regs[4] != INPLL(CLK_PIN_CNTL) ||
@@ -2712,8 +2722,9 @@ static int radeon_check_power_loss(struct radeonfb_info *rinfo)
 	       rinfo->save_regs[3] != INPLL(SCLK_CNTL);
 }
 
-int radeonfb_pci_resume(struct pci_dev *pdev)
+static int radeonfb_pci_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
         struct fb_info *info = pci_get_drvdata(pdev);
         struct radeonfb_info *rinfo = info->par;
 	int rc = 0;
@@ -2795,6 +2806,15 @@ int radeonfb_pci_resume(struct pci_dev *pdev)
 	return rc;
 }
 
+const struct dev_pm_ops radeonfb_pci_pm_ops = {
+	.suspend	= radeonfb_pci_suspend,
+	.resume		= radeonfb_pci_resume,
+	.freeze		= radeonfb_pci_freeze,
+	.thaw		= radeonfb_pci_resume,
+	.poweroff	= radeonfb_pci_hibernate,
+	.restore	= radeonfb_pci_resume,
+};
+
 #ifdef CONFIG_PPC__disabled
 static void radeonfb_early_resume(void *data)
 {
diff --git a/drivers/video/fbdev/aty/radeonfb.h b/drivers/video/fbdev/aty/radeonfb.h
index 131b34dd65af..93f403cbb415 100644
--- a/drivers/video/fbdev/aty/radeonfb.h
+++ b/drivers/video/fbdev/aty/radeonfb.h
@@ -483,8 +483,7 @@ extern void radeon_delete_i2c_busses(struct radeonfb_info *rinfo);
 extern int radeon_probe_i2c_connector(struct radeonfb_info *rinfo, int conn, u8 **out_edid);
 
 /* PM Functions */
-extern int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t state);
-extern int radeonfb_pci_resume(struct pci_dev *pdev);
+extern const struct dev_pm_ops radeonfb_pci_pm_ops;
 extern void radeonfb_pm_init(struct radeonfb_info *rinfo, int dynclk, int ignore_devlist, int force_sleep);
 extern void radeonfb_pm_exit(struct radeonfb_info *rinfo);
 
-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-08-06  7:26 ` Vaibhav Gupta
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container Vaibhav Gupta
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management Vaibhav Gupta
@ 2020-09-07  6:31   ` Vaibhav Gupta
  2020-09-07  6:47     ` Greg KH
  2 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  6:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, linux-kernel-mentees, linux-kernel, dri-devel

Please review this patch-series.

Thank you
Vaibhav Gupta
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container Vaibhav Gupta
@ 2020-09-07  6:33     ` Vaibhav Gupta
  2020-09-07  6:48       ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  6:33 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, linux-kernel-mentees, linux-kernel, dri-devel


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management
  2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management Vaibhav Gupta
@ 2020-09-07  6:34     ` Vaibhav Gupta
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  6:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, linux-kernel-mentees, linux-kernel, dri-devel


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-09-07  6:31   ` [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
@ 2020-09-07  6:47     ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2020-09-07  6:47 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On Mon, Sep 07, 2020 at 12:01:53PM +0530, Vaibhav Gupta wrote:
> Please review this patch-series.

I see no patch here :(
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  2020-09-07  6:33     ` Vaibhav Gupta
@ 2020-09-07  6:48       ` Greg KH
  2020-09-07  6:52         ` Vaibhav Gupta
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2020-09-07  6:48 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On Mon, Sep 07, 2020 at 12:03:47PM +0530, Vaibhav Gupta wrote:
> 

Why did you send empty emails out?

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
  2020-09-07  6:48       ` Greg KH
@ 2020-09-07  6:52         ` Vaibhav Gupta
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  6:52 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On Mon, Sep 07, 2020 at 08:48:10AM +0200, Greg KH wrote:
> On Mon, Sep 07, 2020 at 12:03:47PM +0530, Vaibhav Gupta wrote:
> > 
> 
> Why did you send empty emails out?
> 
> greg k-h
I was trying to re-ping the patches. Guess it went empty. I will send patches
again.

Vaibhav
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-08-06  7:22 [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
  2020-08-06  7:26 ` Vaibhav Gupta
@ 2020-09-07  7:55 ` Daniel Vetter
  2020-09-07  9:16   ` Vaibhav Gupta
                     ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-07  7:55 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On Thu, Aug 06, 2020 at 12:52:54PM +0530, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management. 
> 
> The original goal of the patch series is to upgrade the power management
> framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
> and .resume() callbacks.
> 
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
> 
> During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
> redundant. This was introduced in the commit
> 42ddb453a0cd ("radeon: Conditionally compile PM code").

I do wonder whether it wouldn't be better to just outright delete these,
we have the drm radeon driver for pretty much all the same hardware ...
-Daniel

> 
> ------------
> 
> Before 42ddb453a0cd:
> $ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> 
> Based on output in terminal:
> 
> 547:#ifdef CONFIG_PM
>        |-- 959:#ifdef CONFIG_PPC_PMAC
>        |-- 972:#endif
>        |-- 1291:#ifdef CONFIG_PPC_OF
>        |-- 1301:#endif /* CONFIG_PPC_OF */
>        |-- 1943:#ifdef CONFIG_PPC_OF
>                    |-- 2206:#if 0 /* Not ready yet */
>                    |-- 2508:#endif /* 0 */
>        |-- 2510:#endif /* CONFIG_PPC_OF */
>        |-- 2648:#ifdef CONFIG_PPC_PMAC
>        |-- 2654:#endif /* CONFIG_PPC_PMAC */
>        |-- 2768:#ifdef CONFIG_PPC_PMAC
>        |-- 2774:#endif /* CONFIG_PPC_PMAC */
>        |-- 2791:#ifdef CONFIG_PPC_OF__disabled
>        |-- 2801:#endif /* CONFIG_PPC_OF */
> 2803:#endif /* CONFIG_PM */
> 
> ------------
> 
> After 42ddb453a0cd:
> $ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> 
> Based on output in terminal:
> 
> 547:#ifdef CONFIG_PM
>        |-- 959:#ifdef CONFIG_PPC_PMAC
>        |-- 972:#endif
>        |-- 1291:#ifdef CONFIG_PPC_OF
>        |-- 1301:#endif /* CONFIG_PPC_OF */
>        |-- 1430:#if defined(CONFIG_PM)
>                    |-- 1431:#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
>                    |-- 1944:#endif
>                    |-- 1946:#ifdef CONFIG_PPC_OF
>                                |-- 1947:#ifdef CONFIG_PPC_PMAC
>                                |-- 2208:#endif
>                    |-- 2209:#endif
>                    |-- 2211:#if 0 /* Not ready yet */
>                    |-- 2513:#endif /* 0 */
>        |-- 2515:#endif /* CONFIG_PPC_OF */
>        |-- 2653:#ifdef CONFIG_PPC_PMAC
>        |-- 2659:#endif /* CONFIG_PPC_PMAC */
>        |-- 2773:#ifdef CONFIG_PPC_PMAC
>        |-- 2779:#endif /* CONFIG_PPC_PMAC */
>        |-- 2796:#ifdef CONFIG_PPC_OF__disabled
>        |-- 2806:#endif /* CONFIG_PPC_OF */
> 2808:#endif /* CONFIG_PM */
> 
> ------------
> 
> This also affected the CONFIG_PPC_OF container (line 1943 at commit 65122f7e80b5)
> 
> The patch-series fixes it along with PM upgrade.
> 
> All patches are compile-tested only.
> 
> Test tools:
>     - Compiler: gcc (GCC) 10.1.0
>     - allmodconfig build: make -j$(nproc) W=1 all
> 
> Vaibhav Gupta (2):
>   video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
>   fbdev: radeonfb:use generic power management
> 
>  drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
>  drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++-------
>  drivers/video/fbdev/aty/radeonfb.h    |  3 +--
>  3 files changed, 35 insertions(+), 16 deletions(-)
> 
> -- 
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-09-07  7:55 ` Daniel Vetter
@ 2020-09-07  9:16   ` Vaibhav Gupta
  2020-09-07 17:57     ` Daniel Vetter
  2020-09-07 10:29   ` Michel Dänzer
  2020-09-07 11:58   ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  9:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding, linux-fbdev, linux-kernel,
	dri-devel, Shuah Khan, linux-kernel-mentees

On Mon, Sep 07, 2020 at 09:55:59AM +0200, Daniel Vetter wrote:
> On Thu, Aug 06, 2020 at 12:52:54PM +0530, Vaibhav Gupta wrote:
> > Linux Kernel Mentee: Remove Legacy Power Management. 
> > 
> > The original goal of the patch series is to upgrade the power management
> > framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
> > and .resume() callbacks.
> > 
> > The upgrade makes sure that the involvement of PCI Core does not change the
> > order of operations executed in a driver. Thus, does not change its behavior.
> > 
> > During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
> > redundant. This was introduced in the commit
> > 42ddb453a0cd ("radeon: Conditionally compile PM code").
> 
> I do wonder whether it wouldn't be better to just outright delete these,
> we have the drm radeon driver for pretty much all the same hardware ...
> -Daniel
> 
Hello Daniel,
I don't have any problem in either way. My priority is to get rid of the
legacy .suspend and .resume pointers from "struct pci_driver" . Hence, modifying
every driver that is using them.

Vaibhav Gupta
> > 
> > ------------
> > 
> > Before 42ddb453a0cd:
> > $ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > 
> > Based on output in terminal:
> > 
> > 547:#ifdef CONFIG_PM
> >        |-- 959:#ifdef CONFIG_PPC_PMAC
> >        |-- 972:#endif
> >        |-- 1291:#ifdef CONFIG_PPC_OF
> >        |-- 1301:#endif /* CONFIG_PPC_OF */
> >        |-- 1943:#ifdef CONFIG_PPC_OF
> >                    |-- 2206:#if 0 /* Not ready yet */
> >                    |-- 2508:#endif /* 0 */
> >        |-- 2510:#endif /* CONFIG_PPC_OF */
> >        |-- 2648:#ifdef CONFIG_PPC_PMAC
> >        |-- 2654:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2768:#ifdef CONFIG_PPC_PMAC
> >        |-- 2774:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2791:#ifdef CONFIG_PPC_OF__disabled
> >        |-- 2801:#endif /* CONFIG_PPC_OF */
> > 2803:#endif /* CONFIG_PM */
> > 
> > ------------
> > 
> > After 42ddb453a0cd:
> > $ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > 
> > Based on output in terminal:
> > 
> > 547:#ifdef CONFIG_PM
> >        |-- 959:#ifdef CONFIG_PPC_PMAC
> >        |-- 972:#endif
> >        |-- 1291:#ifdef CONFIG_PPC_OF
> >        |-- 1301:#endif /* CONFIG_PPC_OF */
> >        |-- 1430:#if defined(CONFIG_PM)
> >                    |-- 1431:#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
> >                    |-- 1944:#endif
> >                    |-- 1946:#ifdef CONFIG_PPC_OF
> >                                |-- 1947:#ifdef CONFIG_PPC_PMAC
> >                                |-- 2208:#endif
> >                    |-- 2209:#endif
> >                    |-- 2211:#if 0 /* Not ready yet */
> >                    |-- 2513:#endif /* 0 */
> >        |-- 2515:#endif /* CONFIG_PPC_OF */
> >        |-- 2653:#ifdef CONFIG_PPC_PMAC
> >        |-- 2659:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2773:#ifdef CONFIG_PPC_PMAC
> >        |-- 2779:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2796:#ifdef CONFIG_PPC_OF__disabled
> >        |-- 2806:#endif /* CONFIG_PPC_OF */
> > 2808:#endif /* CONFIG_PM */
> > 
> > ------------
> > 
> > This also affected the CONFIG_PPC_OF container (line 1943 at commit 65122f7e80b5)
> > 
> > The patch-series fixes it along with PM upgrade.
> > 
> > All patches are compile-tested only.
> > 
> > Test tools:
> >     - Compiler: gcc (GCC) 10.1.0
> >     - allmodconfig build: make -j$(nproc) W=1 all
> > 
> > Vaibhav Gupta (2):
> >   video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
> >   fbdev: radeonfb:use generic power management
> > 
> >  drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
> >  drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++-------
> >  drivers/video/fbdev/aty/radeonfb.h    |  3 +--
> >  3 files changed, 35 insertions(+), 16 deletions(-)
> > 
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-09-07  7:55 ` Daniel Vetter
  2020-09-07  9:16   ` Vaibhav Gupta
@ 2020-09-07 10:29   ` Michel Dänzer
  2020-09-07 11:58   ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 16+ messages in thread
From: Michel Dänzer @ 2020-09-07 10:29 UTC (permalink / raw)
  To: Vaibhav Gupta, Daniel Vetter
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On 2020-09-07 9:55 a.m., Daniel Vetter wrote:
> On Thu, Aug 06, 2020 at 12:52:54PM +0530, Vaibhav Gupta wrote:
>> Linux Kernel Mentee: Remove Legacy Power Management.
>>
>> The original goal of the patch series is to upgrade the power management
>> framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
>> and .resume() callbacks.
>>
>> The upgrade makes sure that the involvement of PCI Core does not change the
>> order of operations executed in a driver. Thus, does not change its behavior.
>>
>> During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
>> redundant. This was introduced in the commit
>> 42ddb453a0cd ("radeon: Conditionally compile PM code").
> 
> I do wonder whether it wouldn't be better to just outright delete these,
> we have the drm radeon driver for pretty much all the same hardware ...

In contrast to radeonfb, the radeon driver doesn't support 
suspend-to-RAM on Apple PowerPC notebooks.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-09-07  7:55 ` Daniel Vetter
  2020-09-07  9:16   ` Vaibhav Gupta
  2020-09-07 10:29   ` Michel Dänzer
@ 2020-09-07 11:58   ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 16+ messages in thread
From: Benjamin Herrenschmidt @ 2020-09-07 11:58 UTC (permalink / raw)
  To: Daniel Vetter, Vaibhav Gupta
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Vaibhav Gupta,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Bjorn Helgaas, Thierry Reding, linux-kernel-mentees

On Mon, 2020-09-07 at 09:55 +0200, Daniel Vetter wrote:
> On Thu, Aug 06, 2020 at 12:52:54PM +0530, Vaibhav Gupta wrote:
> > Linux Kernel Mentee: Remove Legacy Power Management. 
> > 
> > The original goal of the patch series is to upgrade the power
> > management
> > framework of radeonfb fbdev driver. This has been done by upgrading
> > .suspend()
> > and .resume() callbacks.
> > 
> > The upgrade makes sure that the involvement of PCI Core does not
> > change the
> > order of operations executed in a driver. Thus, does not change its
> > behavior.
> > 
> > During this process, it was found that "#if defined(CONFIG_PM)" at
> > line 1434 is
> > redundant. This was introduced in the commit
> > 42ddb453a0cd ("radeon: Conditionally compile PM code").
> 
> I do wonder whether it wouldn't be better to just outright delete
> these,
> we have the drm radeon driver for pretty much all the same hardware
> ...

The only thing is, afaik, the DRM drivers never got the D2/D3 code that
I wrote for radeonfb to get old powerbooks to suspend/resume.

Cheers,
Ben.

> -Daniel
> 
> > 
> > ------------
> > 
> > Before 42ddb453a0cd:
> > $ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n
> > "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > 
> > Based on output in terminal:
> > 
> > 547:#ifdef CONFIG_PM
> >        |-- 959:#ifdef CONFIG_PPC_PMAC
> >        |-- 972:#endif
> >        |-- 1291:#ifdef CONFIG_PPC_OF
> >        |-- 1301:#endif /* CONFIG_PPC_OF */
> >        |-- 1943:#ifdef CONFIG_PPC_OF
> >                    |-- 2206:#if 0 /* Not ready yet */
> >                    |-- 2508:#endif /* 0 */
> >        |-- 2510:#endif /* CONFIG_PPC_OF */
> >        |-- 2648:#ifdef CONFIG_PPC_PMAC
> >        |-- 2654:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2768:#ifdef CONFIG_PPC_PMAC
> >        |-- 2774:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2791:#ifdef CONFIG_PPC_OF__disabled
> >        |-- 2801:#endif /* CONFIG_PPC_OF */
> > 2803:#endif /* CONFIG_PM */
> > 
> > ------------
> > 
> > After 42ddb453a0cd:
> > $ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n
> > "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > 
> > Based on output in terminal:
> > 
> > 547:#ifdef CONFIG_PM
> >        |-- 959:#ifdef CONFIG_PPC_PMAC
> >        |-- 972:#endif
> >        |-- 1291:#ifdef CONFIG_PPC_OF
> >        |-- 1301:#endif /* CONFIG_PPC_OF */
> >        |-- 1430:#if defined(CONFIG_PM)
> >                    |-- 1431:#if defined(CONFIG_X86) ||
> > defined(CONFIG_PPC_PMAC)
> >                    |-- 1944:#endif
> >                    |-- 1946:#ifdef CONFIG_PPC_OF
> >                                |-- 1947:#ifdef CONFIG_PPC_PMAC
> >                                |-- 2208:#endif
> >                    |-- 2209:#endif
> >                    |-- 2211:#if 0 /* Not ready yet */
> >                    |-- 2513:#endif /* 0 */
> >        |-- 2515:#endif /* CONFIG_PPC_OF */
> >        |-- 2653:#ifdef CONFIG_PPC_PMAC
> >        |-- 2659:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2773:#ifdef CONFIG_PPC_PMAC
> >        |-- 2779:#endif /* CONFIG_PPC_PMAC */
> >        |-- 2796:#ifdef CONFIG_PPC_OF__disabled
> >        |-- 2806:#endif /* CONFIG_PPC_OF */
> > 2808:#endif /* CONFIG_PM */
> > 
> > ------------
> > 
> > This also affected the CONFIG_PPC_OF container (line 1943 at commit
> > 65122f7e80b5)
> > 
> > The patch-series fixes it along with PM upgrade.
> > 
> > All patches are compile-tested only.
> > 
> > Test tools:
> >     - Compiler: gcc (GCC) 10.1.0
> >     - allmodconfig build: make -j$(nproc) W=1 all
> > 
> > Vaibhav Gupta (2):
> >   video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM
> > container
> >   fbdev: radeonfb:use generic power management
> > 
> >  drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
> >  drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++---
> > ----
> >  drivers/video/fbdev/aty/radeonfb.h    |  3 +--
> >  3 files changed, 35 insertions(+), 16 deletions(-)
> > 
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
  2020-09-07  9:16   ` Vaibhav Gupta
@ 2020-09-07 17:57     ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-07 17:57 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	linux-kernel, dri-devel, Thierry Reding, Bjorn Helgaas,
	Vaibhav Gupta, Bjorn Helgaas, Thierry Reding,
	linux-kernel-mentees

On Mon, Sep 07, 2020 at 02:46:21PM +0530, Vaibhav Gupta wrote:
> On Mon, Sep 07, 2020 at 09:55:59AM +0200, Daniel Vetter wrote:
> > On Thu, Aug 06, 2020 at 12:52:54PM +0530, Vaibhav Gupta wrote:
> > > Linux Kernel Mentee: Remove Legacy Power Management. 
> > > 
> > > The original goal of the patch series is to upgrade the power management
> > > framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
> > > and .resume() callbacks.
> > > 
> > > The upgrade makes sure that the involvement of PCI Core does not change the
> > > order of operations executed in a driver. Thus, does not change its behavior.
> > > 
> > > During this process, it was found that "#if defined(CONFIG_PM)" at line 1434 is
> > > redundant. This was introduced in the commit
> > > 42ddb453a0cd ("radeon: Conditionally compile PM code").
> > 
> > I do wonder whether it wouldn't be better to just outright delete these,
> > we have the drm radeon driver for pretty much all the same hardware ...
> > -Daniel
> > 
> Hello Daniel,
> I don't have any problem in either way. My priority is to get rid of the
> legacy .suspend and .resume pointers from "struct pci_driver" . Hence, modifying
> every driver that is using them.

Ok, also sounds like we can't just ditch it outright and merging your
patches makes sense.

Please note that Bart (he's usually picking up the fbdev patches) is on
vacations until next week, I guess he'll then go and vacuum up everything
for 5.10 as he usually does.

Cheers, Daniel

> 
> Vaibhav Gupta
> > > 
> > > ------------
> > > 
> > > Before 42ddb453a0cd:
> > > $ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > > 
> > > Based on output in terminal:
> > > 
> > > 547:#ifdef CONFIG_PM
> > >        |-- 959:#ifdef CONFIG_PPC_PMAC
> > >        |-- 972:#endif
> > >        |-- 1291:#ifdef CONFIG_PPC_OF
> > >        |-- 1301:#endif /* CONFIG_PPC_OF */
> > >        |-- 1943:#ifdef CONFIG_PPC_OF
> > >                    |-- 2206:#if 0 /* Not ready yet */
> > >                    |-- 2508:#endif /* 0 */
> > >        |-- 2510:#endif /* CONFIG_PPC_OF */
> > >        |-- 2648:#ifdef CONFIG_PPC_PMAC
> > >        |-- 2654:#endif /* CONFIG_PPC_PMAC */
> > >        |-- 2768:#ifdef CONFIG_PPC_PMAC
> > >        |-- 2774:#endif /* CONFIG_PPC_PMAC */
> > >        |-- 2791:#ifdef CONFIG_PPC_OF__disabled
> > >        |-- 2801:#endif /* CONFIG_PPC_OF */
> > > 2803:#endif /* CONFIG_PM */
> > > 
> > > ------------
> > > 
> > > After 42ddb453a0cd:
> > > $ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
> > > 
> > > Based on output in terminal:
> > > 
> > > 547:#ifdef CONFIG_PM
> > >        |-- 959:#ifdef CONFIG_PPC_PMAC
> > >        |-- 972:#endif
> > >        |-- 1291:#ifdef CONFIG_PPC_OF
> > >        |-- 1301:#endif /* CONFIG_PPC_OF */
> > >        |-- 1430:#if defined(CONFIG_PM)
> > >                    |-- 1431:#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
> > >                    |-- 1944:#endif
> > >                    |-- 1946:#ifdef CONFIG_PPC_OF
> > >                                |-- 1947:#ifdef CONFIG_PPC_PMAC
> > >                                |-- 2208:#endif
> > >                    |-- 2209:#endif
> > >                    |-- 2211:#if 0 /* Not ready yet */
> > >                    |-- 2513:#endif /* 0 */
> > >        |-- 2515:#endif /* CONFIG_PPC_OF */
> > >        |-- 2653:#ifdef CONFIG_PPC_PMAC
> > >        |-- 2659:#endif /* CONFIG_PPC_PMAC */
> > >        |-- 2773:#ifdef CONFIG_PPC_PMAC
> > >        |-- 2779:#endif /* CONFIG_PPC_PMAC */
> > >        |-- 2796:#ifdef CONFIG_PPC_OF__disabled
> > >        |-- 2806:#endif /* CONFIG_PPC_OF */
> > > 2808:#endif /* CONFIG_PM */
> > > 
> > > ------------
> > > 
> > > This also affected the CONFIG_PPC_OF container (line 1943 at commit 65122f7e80b5)
> > > 
> > > The patch-series fixes it along with PM upgrade.
> > > 
> > > All patches are compile-tested only.
> > > 
> > > Test tools:
> > >     - Compiler: gcc (GCC) 10.1.0
> > >     - allmodconfig build: make -j$(nproc) W=1 all
> > > 
> > > Vaibhav Gupta (2):
> > >   video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
> > >   fbdev: radeonfb:use generic power management
> > > 
> > >  drivers/video/fbdev/aty/radeon_base.c | 10 ++++---
> > >  drivers/video/fbdev/aty/radeon_pm.c   | 38 ++++++++++++++++++++-------
> > >  drivers/video/fbdev/aty/radeonfb.h    |  3 +--
> > >  3 files changed, 35 insertions(+), 16 deletions(-)
> > > 
> > > -- 
> > > 2.27.0
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management
  2020-09-07  7:02 Vaibhav Gupta
@ 2020-09-07  7:02 ` Vaibhav Gupta
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Gupta @ 2020-09-07  7:02 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Benjamin Herrenschmidt, Bartlomiej Zolnierkiewicz,
	Thierry Reding, Thierry Reding
  Cc: linux-fbdev, linux-kernel, dri-devel, linux-kernel-mentees

Drivers using legacy PCI power management .suspend()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.

Switch to generic power management framework using a "struct dev_pm_ops"
variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/video/fbdev/aty/radeon_base.c | 10 +++++---
 drivers/video/fbdev/aty/radeon_pm.c   | 36 +++++++++++++++++++++------
 drivers/video/fbdev/aty/radeonfb.h    |  3 +--
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index e116a3f9ad56..232dbe154666 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2559,16 +2559,18 @@ static void radeonfb_pci_unregister(struct pci_dev *pdev)
         framebuffer_release(info);
 }
 
+#ifdef CONFIG_PM
+#define RADEONFB_PCI_PM_OPS (&radeonfb_pci_pm_ops)
+#else
+#define RADEONFB_PCI_PM_OPS NULL
+#endif
 
 static struct pci_driver radeonfb_driver = {
 	.name		= "radeonfb",
 	.id_table	= radeonfb_pci_table,
 	.probe		= radeonfb_pci_register,
 	.remove		= radeonfb_pci_unregister,
-#ifdef CONFIG_PM
-	.suspend       	= radeonfb_pci_suspend,
-	.resume		= radeonfb_pci_resume,
-#endif /* CONFIG_PM */
+	.driver.pm	= RADEONFB_PCI_PM_OPS,
 };
 
 #ifndef MODULE
diff --git a/drivers/video/fbdev/aty/radeon_pm.c b/drivers/video/fbdev/aty/radeon_pm.c
index b9af70bd656a..352d0bb4773a 100644
--- a/drivers/video/fbdev/aty/radeon_pm.c
+++ b/drivers/video/fbdev/aty/radeon_pm.c
@@ -2611,8 +2611,9 @@ static void radeon_set_suspend(struct radeonfb_info *rinfo, int suspend)
 	}
 }
 
-int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
+static int radeonfb_pci_suspend_late(struct device *dev, pm_message_t mesg)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
         struct fb_info *info = pci_get_drvdata(pdev);
         struct radeonfb_info *rinfo = info->par;
 
@@ -2660,11 +2661,6 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 	pmac_suspend_agp_for_card(pdev);
 #endif /* CONFIG_PPC_PMAC */
 
-	/* It's unclear whether or when the generic code will do that, so let's
-	 * do it ourselves. We save state before we do any power management
-	 */
-	pci_save_state(pdev);
-
 	/* If we support wakeup from poweroff, we save all regs we can including cfg
 	 * space
 	 */
@@ -2689,7 +2685,6 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 			msleep(20);
 			OUTREG(LVDS_GEN_CNTL, INREG(LVDS_GEN_CNTL) & ~(LVDS_DIGON));
 		}
-		pci_disable_device(pdev);
 	}
 	/* If we support D2, we go to it (should be fixed later with a flag forcing
 	 * D3 only for some laptops)
@@ -2705,6 +2700,21 @@ int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
 	return 0;
 }
 
+static int radeonfb_pci_suspend(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_SUSPEND);
+}
+
+static int radeonfb_pci_hibernate(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_HIBERNATE);
+}
+
+static int radeonfb_pci_freeze(struct device *dev)
+{
+	return radeonfb_pci_suspend_late(dev, PMSG_FREEZE);
+}
+
 static int radeon_check_power_loss(struct radeonfb_info *rinfo)
 {
 	return rinfo->save_regs[4] != INPLL(CLK_PIN_CNTL) ||
@@ -2712,8 +2722,9 @@ static int radeon_check_power_loss(struct radeonfb_info *rinfo)
 	       rinfo->save_regs[3] != INPLL(SCLK_CNTL);
 }
 
-int radeonfb_pci_resume(struct pci_dev *pdev)
+static int radeonfb_pci_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
         struct fb_info *info = pci_get_drvdata(pdev);
         struct radeonfb_info *rinfo = info->par;
 	int rc = 0;
@@ -2795,6 +2806,15 @@ int radeonfb_pci_resume(struct pci_dev *pdev)
 	return rc;
 }
 
+const struct dev_pm_ops radeonfb_pci_pm_ops = {
+	.suspend	= radeonfb_pci_suspend,
+	.resume		= radeonfb_pci_resume,
+	.freeze		= radeonfb_pci_freeze,
+	.thaw		= radeonfb_pci_resume,
+	.poweroff	= radeonfb_pci_hibernate,
+	.restore	= radeonfb_pci_resume,
+};
+
 #ifdef CONFIG_PPC__disabled
 static void radeonfb_early_resume(void *data)
 {
diff --git a/drivers/video/fbdev/aty/radeonfb.h b/drivers/video/fbdev/aty/radeonfb.h
index 131b34dd65af..93f403cbb415 100644
--- a/drivers/video/fbdev/aty/radeonfb.h
+++ b/drivers/video/fbdev/aty/radeonfb.h
@@ -483,8 +483,7 @@ extern void radeon_delete_i2c_busses(struct radeonfb_info *rinfo);
 extern int radeon_probe_i2c_connector(struct radeonfb_info *rinfo, int conn, u8 **out_edid);
 
 /* PM Functions */
-extern int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t state);
-extern int radeonfb_pci_resume(struct pci_dev *pdev);
+extern const struct dev_pm_ops radeonfb_pci_pm_ops;
 extern void radeonfb_pm_init(struct radeonfb_info *rinfo, int dynclk, int ignore_devlist, int force_sleep);
 extern void radeonfb_pm_exit(struct radeonfb_info *rinfo);
 
-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-09-07 17:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06  7:22 [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
2020-08-06  7:26 ` Vaibhav Gupta
2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 1/2] video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container Vaibhav Gupta
2020-09-07  6:33     ` Vaibhav Gupta
2020-09-07  6:48       ` Greg KH
2020-09-07  6:52         ` Vaibhav Gupta
2020-08-06  7:26   ` [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management Vaibhav Gupta
2020-09-07  6:34     ` Vaibhav Gupta
2020-09-07  6:31   ` [Linux-kernel-mentees] [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups Vaibhav Gupta
2020-09-07  6:47     ` Greg KH
2020-09-07  7:55 ` Daniel Vetter
2020-09-07  9:16   ` Vaibhav Gupta
2020-09-07 17:57     ` Daniel Vetter
2020-09-07 10:29   ` Michel Dänzer
2020-09-07 11:58   ` Benjamin Herrenschmidt
2020-09-07  7:02 Vaibhav Gupta
2020-09-07  7:02 ` [Linux-kernel-mentees] [PATCH v1 2/2] fbdev: radeonfb:use generic power management Vaibhav Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).