[RFC/RFT,4/5] ACPI / LPSS: Support for device latency tolerance PM QoS
diff mbox series

Message ID 12327024.lrSi2qfq01@vostro.rjw.lan
State New, archived
Headers show
Series
  • PM / QoS: Introduce latency tolerance device PM QoS type
Related show

Commit Message

Rafael J. Wysocki Jan. 17, 2014, 2:46 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a new routine, acpi_lpss_set_ltr(), for setting latency tolerance
values for LPSS devices having LTR (Latency Tolerance Reporting)
registers.  Modify acpi_lpss_platform_notify() to set the LPSS
devices' power.set_latency_tolerance callback pointers to
acpi_lpss_set_ltr() during device addition.

That will cause the device latency tolerance PM QoS to work for
the devices in question as documented.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_lpss.c |   66 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Mika Westerberg Jan. 20, 2014, 11:15 a.m. UTC | #1
On Fri, Jan 17, 2014 at 03:46:40PM +0100, Rafael J. Wysocki wrote:
> @@ -415,11 +472,12 @@ static int acpi_lpss_platform_notify(str
>  		return 0;
>  	}
>  
> -	if (action == BUS_NOTIFY_ADD_DEVICE)
> +	if (action == BUS_NOTIFY_ADD_DEVICE) {
>  		ret = sysfs_create_group(&pdev->dev.kobj, &lpss_attr_group);
> -	else if (action == BUS_NOTIFY_DEL_DEVICE)
> +		pdev->dev.power.set_latency_tolerance = acpi_lpss_set_ltr;

While trying to test this I noticed that BUS_NOTIFY_ADD_DEVICE happens
after call to dpm_sysfs_add(), so LTR field is never exposed to the
userspace.

> +	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
>  		sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
> -
> +	}
>  	return ret;
>  }
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Rafael J. Wysocki Jan. 20, 2014, 1:10 p.m. UTC | #2
On Monday, January 20, 2014 01:15:19 PM Mika Westerberg wrote:
> On Fri, Jan 17, 2014 at 03:46:40PM +0100, Rafael J. Wysocki wrote:
> > @@ -415,11 +472,12 @@ static int acpi_lpss_platform_notify(str
> >  		return 0;
> >  	}
> >  
> > -	if (action == BUS_NOTIFY_ADD_DEVICE)
> > +	if (action == BUS_NOTIFY_ADD_DEVICE) {
> >  		ret = sysfs_create_group(&pdev->dev.kobj, &lpss_attr_group);
> > -	else if (action == BUS_NOTIFY_DEL_DEVICE)
> > +		pdev->dev.power.set_latency_tolerance = acpi_lpss_set_ltr;
> 
> While trying to test this I noticed that BUS_NOTIFY_ADD_DEVICE happens
> after call to dpm_sysfs_add(), so LTR field is never exposed to the
> userspace.

Ahh, I confused things, thanks for reporting this!

I'll need to hook it up to acpi_platform_notify() somehow I think.

> > +	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
> >  		sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
> > -
> > +	}
> >  	return ret;
> >  }
> >
Rafael J. Wysocki Jan. 21, 2014, 11:23 p.m. UTC | #3
On Monday, January 20, 2014 02:10:10 PM Rafael J. Wysocki wrote:
> On Monday, January 20, 2014 01:15:19 PM Mika Westerberg wrote:
> > On Fri, Jan 17, 2014 at 03:46:40PM +0100, Rafael J. Wysocki wrote:
> > > @@ -415,11 +472,12 @@ static int acpi_lpss_platform_notify(str
> > >  		return 0;
> > >  	}
> > >  
> > > -	if (action == BUS_NOTIFY_ADD_DEVICE)
> > > +	if (action == BUS_NOTIFY_ADD_DEVICE) {
> > >  		ret = sysfs_create_group(&pdev->dev.kobj, &lpss_attr_group);
> > > -	else if (action == BUS_NOTIFY_DEL_DEVICE)
> > > +		pdev->dev.power.set_latency_tolerance = acpi_lpss_set_ltr;
> > 
> > While trying to test this I noticed that BUS_NOTIFY_ADD_DEVICE happens
> > after call to dpm_sysfs_add(), so LTR field is never exposed to the
> > userspace.
> 
> Ahh, I confused things, thanks for reporting this!
> 
> I'll need to hook it up to acpi_platform_notify() somehow I think.

OK, two patches to replace this one ([4/5]) will follow.

On top of them we can move the code from acpi_lpss_platform_notify() to
acpi_lpss_(bind|unbind)() and get rid of acpi_lpss_nb, which I think would be
an improvement too. :-)

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Patch
diff mbox series

Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -33,6 +33,12 @@  ACPI_MODULE_NAME("acpi_lpss");
 #define LPSS_GENERAL_UART_RTS_OVRD	BIT(3)
 #define LPSS_SW_LTR			0x10
 #define LPSS_AUTO_LTR			0x14
+#define LPSS_LTR_SNOOP_REQ		BIT(15)
+#define LPSS_LTR_SNOOP_MASK		0x0000FFFF
+#define LPSS_LTR_SNOOP_LAT_1US		0x800
+#define LPSS_LTR_SNOOP_LAT_32US		0xC00
+#define LPSS_LTR_SNOOP_LAT_SHIFT	5
+#define LPSS_LTR_MAX_VAL		0x3FF
 #define LPSS_TX_INT			0x20
 #define LPSS_TX_INT_MASK		BIT(1)
 
@@ -316,6 +322,17 @@  static int acpi_lpss_create_device(struc
 	return ret;
 }
 
+static u32 __lpss_reg_read(struct lpss_private_data *pdata, unsigned int reg)
+{
+	return readl(pdata->mmio_base + pdata->dev_desc->prv_offset + reg);
+}
+
+static void __lpss_reg_write(u32 val, struct lpss_private_data *pdata,
+			     unsigned int reg)
+{
+	writel(val, pdata->mmio_base + pdata->dev_desc->prv_offset + reg);
+}
+
 static int lpss_reg_read(struct device *dev, unsigned int reg, u32 *val)
 {
 	struct acpi_device *adev;
@@ -337,7 +354,7 @@  static int lpss_reg_read(struct device *
 		ret = -ENODEV;
 		goto out;
 	}
-	*val = readl(pdata->mmio_base + pdata->dev_desc->prv_offset + reg);
+	*val = __lpss_reg_read(pdata, reg);
 
  out:
 	spin_unlock_irqrestore(&dev->power.lock, flags);
@@ -390,6 +407,46 @@  static struct attribute_group lpss_attr_
 	.name = "lpss_ltr",
 };
 
+static void acpi_lpss_set_ltr(struct device *dev, s32 val)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct lpss_private_data *pdata;
+	u32 ltr_mode, ltr_val;
+
+	if (WARN_ON(!adev))
+		return;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->mmio_base))
+		return;
+
+	ltr_mode = __lpss_reg_read(pdata, LPSS_GENERAL);
+	if (val < 0) {
+		if (ltr_mode & LPSS_GENERAL_LTR_MODE_SW) {
+			ltr_mode &= ~LPSS_GENERAL_LTR_MODE_SW;
+			__lpss_reg_write(ltr_mode, pdata, LPSS_GENERAL);
+		}
+		return;
+	}
+	ltr_val = __lpss_reg_read(pdata, LPSS_SW_LTR) & ~LPSS_LTR_SNOOP_MASK;
+	if (val > LPSS_LTR_MAX_VAL) {
+		ltr_val |= LPSS_LTR_SNOOP_LAT_32US | LPSS_LTR_SNOOP_REQ;
+		val >>= LPSS_LTR_SNOOP_LAT_SHIFT;
+		if (val > LPSS_LTR_MAX_VAL)
+			val = LPSS_LTR_MAX_VAL;
+	} else {
+		ltr_val |= LPSS_LTR_SNOOP_LAT_1US;
+		if (val > 0)
+			ltr_val |= LPSS_LTR_SNOOP_REQ;
+	}
+	ltr_val |= val;
+	__lpss_reg_write(ltr_val, pdata, LPSS_SW_LTR);
+	if (!(ltr_mode & LPSS_GENERAL_LTR_MODE_SW)) {
+		ltr_mode |= LPSS_GENERAL_LTR_MODE_SW;
+		__lpss_reg_write(ltr_mode, pdata, LPSS_GENERAL);
+	}
+}
+
 static int acpi_lpss_platform_notify(struct notifier_block *nb,
 				     unsigned long action, void *data)
 {
@@ -415,11 +472,12 @@  static int acpi_lpss_platform_notify(str
 		return 0;
 	}
 
-	if (action == BUS_NOTIFY_ADD_DEVICE)
+	if (action == BUS_NOTIFY_ADD_DEVICE) {
 		ret = sysfs_create_group(&pdev->dev.kobj, &lpss_attr_group);
-	else if (action == BUS_NOTIFY_DEL_DEVICE)
+		pdev->dev.power.set_latency_tolerance = acpi_lpss_set_ltr;
+	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
 		sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
-
+	}
 	return ret;
 }