[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHp75Vf2ioDkNgA42J14p+_t7Hjdszmbpwh9nMSwQtxANuzLtg@mail.gmail.com>
Date: Mon, 29 Oct 2018 14:00:03 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Darren Hart <dvhart@...radead.org>,
Andy Shevchenko <andy@...radead.org>,
Platform Driver <platform-driver-x86@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] platform/x86: Add Intel AtomISP2 dummy /
power-management driver
On Sun, Oct 14, 2018 at 8:54 PM Hans de Goede <hdegoede@...hat.com> wrote:
>
> The Image Signal Processor found on Cherry Trail devices is brought up in
> D0 state on devices which have camera sensors attached to it. The ISP will
> not enter D3 state again without some massaging of its registers beforehand
> and the ISP not being in D3 state blocks the SoC from entering S0ix modes.
>
> There was a driver for the ISP in drivers/staging but that got removed
> again because it never worked. It does not seem likely that a real
> driver for the ISP will be added to the mainline kernel anytime soon.
>
> This commit adds a dummy driver which contains the necessary magic from
> the staging driver to powerdown the ISP, so that Cherry Trail devices where
> the ISP is used will properly use S0ix modes when suspended.
>
> Together with other recent S0ix related fixes this allows S0ix modes to
> be entered on e.g. a Chuwi Hi8 Pro and a HP x2 210.
>
There are might be slight improvements, but in general it's okay.
Applied, thanks!
P.S. It's possible we need to add Merrifield ID there, I will do it
myself along with cosmetic changes as a separate patches later on.
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=196915
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> ---
> Changes in v2:
> -Rename the driver and Kconfig option from intel-isp-dummy to intel-atomisp2-pm
> ---
> MAINTAINERS | 6 ++
> drivers/platform/x86/Kconfig | 12 +++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/intel_atomisp2_pm.c | 119 +++++++++++++++++++++++
> 4 files changed, 138 insertions(+)
> create mode 100644 drivers/platform/x86/intel_atomisp2_pm.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2c5d5c6859b8..ba9e5da792ea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7306,6 +7306,12 @@ L: alsa-devel@...a-project.org (moderated for non-subscribers)
> S: Supported
> F: sound/soc/intel/
>
> +INTEL ATOMISP2 DUMMY / POWER-MANAGEMENT DRIVER
> +M: Hans de Goede <hdegoede@...hat.com>
> +L: platform-driver-x86@...r.kernel.org
> +S: Maintained
> +F: drivers/platform/x86/intel_atomisp2_pm.c
> +
> INTEL C600 SERIES SAS CONTROLLER DRIVER
> M: Intel SCU Linux support <intel-linux-scu@...el.com>
> M: Artur Paszkiewicz <artur.paszkiewicz@...el.com>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 48bec711e5e3..5eedcec0fefe 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1232,6 +1232,18 @@ config I2C_MULTI_INSTANTIATE
> To compile this driver as a module, choose M here: the module
> will be called i2c-multi-instantiate.
>
> +config INTEL_ATOMISP2_PM
> + tristate "Intel AtomISP2 dummy / power-management driver"
> + depends on PCI && IOSF_MBI && PM
> + help
> + Power-management driver for Intel's Image Signal Processor found on
> + Bay and Cherry Trail devices. This dummy driver's sole purpose is to
> + turn the ISP off (put it in D3) to save power and to allow entering
> + of S0ix modes.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called intel_atomisp2_pm.
> +
> endif # X86_PLATFORM_DEVICES
>
> config PMC_ATOM
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index e6d1becf81ce..dc29af4d8e2f 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -92,3 +92,4 @@ obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o
> obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
> obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN) += intel_chtdc_ti_pwrbtn.o
> obj-$(CONFIG_I2C_MULTI_INSTANTIATE) += i2c-multi-instantiate.o
> +obj-$(CONFIG_INTEL_ATOMISP2_PM) += intel_atomisp2_pm.o
> diff --git a/drivers/platform/x86/intel_atomisp2_pm.c b/drivers/platform/x86/intel_atomisp2_pm.c
> new file mode 100644
> index 000000000000..9371603a0ac9
> --- /dev/null
> +++ b/drivers/platform/x86/intel_atomisp2_pm.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Dummy driver for Intel's Image Signal Processor found on Bay and Cherry
> + * Trail devices. The sole purpose of this driver is to allow the ISP to
> + * be put in D3.
> + *
> + * Copyright (C) 2018 Hans de Goede <hdegoede@...hat.com>
> + *
> + * Based on various non upstream patches for ISP support:
> + * Copyright (C) 2010-2017 Intel Corporation. All rights reserved.
> + * Copyright (c) 2010 Silicon Hive www.siliconhive.com.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <asm/iosf_mbi.h>
> +
> +/* PCI configuration regs */
> +#define PCI_INTERRUPT_CTRL 0x9c
> +
> +#define PCI_CSI_CONTROL 0xe8
> +#define PCI_CSI_CONTROL_PORTS_OFF_MASK 0x7
> +
> +/* IOSF BT_MBI_UNIT_PMC regs */
> +#define ISPSSPM0 0x39
> +#define ISPSSPM0_ISPSSC_OFFSET 0
> +#define ISPSSPM0_ISPSSC_MASK 0x00000003
> +#define ISPSSPM0_ISPSSS_OFFSET 24
> +#define ISPSSPM0_ISPSSS_MASK 0x03000000
> +#define ISPSSPM0_IUNIT_POWER_ON 0x0
> +#define ISPSSPM0_IUNIT_POWER_OFF 0x3
> +
> +static int isp_probe(struct pci_dev *dev, const struct pci_device_id *id)
> +{
> + unsigned long timeout;
> + u32 val;
> +
> + pci_write_config_dword(dev, PCI_INTERRUPT_CTRL, 0);
> +
> + /*
> + * MRFLD IUNIT DPHY is located in an always-power-on island
> + * MRFLD HW design need all CSI ports are disabled before
> + * powering down the IUNIT.
> + */
> + pci_read_config_dword(dev, PCI_CSI_CONTROL, &val);
> + val |= PCI_CSI_CONTROL_PORTS_OFF_MASK;
> + pci_write_config_dword(dev, PCI_CSI_CONTROL, val);
> +
> + /* Write 0x3 to ISPSSPM0 bit[1:0] to power off the IUNIT */
> + iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, ISPSSPM0,
> + ISPSSPM0_IUNIT_POWER_OFF, ISPSSPM0_ISPSSC_MASK);
> +
> + /*
> + * There should be no IUNIT access while power-down is
> + * in progress HW sighting: 4567865
> + * Wait up to 50 ms for the IUNIT to shut down.
> + */
> + timeout = jiffies + msecs_to_jiffies(50);
> + while (1) {
> + /* Wait until ISPSSPM0 bit[25:24] shows 0x3 */
> + iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, ISPSSPM0, &val);
> + val = (val & ISPSSPM0_ISPSSS_MASK) >> ISPSSPM0_ISPSSS_OFFSET;
> + if (val == ISPSSPM0_IUNIT_POWER_OFF)
> + break;
> +
> + if (time_after(jiffies, timeout)) {
> + dev_err(&dev->dev, "IUNIT power-off timeout.\n");
> + return -EBUSY;
> + }
> + usleep_range(1000, 2000);
> + }
> +
> + pm_runtime_allow(&dev->dev);
> + pm_runtime_put_sync_suspend(&dev->dev);
> +
> + return 0;
> +}
> +
> +static void isp_remove(struct pci_dev *dev)
> +{
> + pm_runtime_get_sync(&dev->dev);
> + pm_runtime_forbid(&dev->dev);
> +}
> +
> +static int isp_pci_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int isp_pci_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static UNIVERSAL_DEV_PM_OPS(isp_pm_ops, isp_pci_suspend,
> + isp_pci_resume, NULL);
> +
> +static const struct pci_device_id isp_id_table[] = {
> + { PCI_VDEVICE(INTEL, 0x22b8), },
> + { 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, isp_id_table);
> +
> +static struct pci_driver isp_pci_driver = {
> + .name = "intel_atomisp2_pm",
> + .id_table = isp_id_table,
> + .probe = isp_probe,
> + .remove = isp_remove,
> + .driver.pm = &isp_pm_ops,
> +};
> +
> +module_pci_driver(isp_pci_driver);
> +
> +MODULE_DESCRIPTION("Intel AtomISP2 dummy / power-management drv (for suspend)");
> +MODULE_AUTHOR("Hans de Goede <hdegoede@...hat.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.19.0
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists