[Bug 98897] Macbook pro 11,5 screen flicker when AC adapter plugged in
https://bugs.freedesktop.org/show_bug.cgi?id=98897 --- Comment #23 from Tyler Hampton --- Running this patch on Linux kernel version 4.9.1 fixes the problem for me. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/6dfb6e26/attachment-0001.html>
[Bug 191281] [drm:amdgpu_ib_ring_tests] *ERROR* amdgpu: failed testing IB on ring 12 (-110)
https://bugzilla.kernel.org/show_bug.cgi?id=191281 --- Comment #3 from Johannes Hirte --- With amdgpu.dpm=0 this doesn't occur. Also tested with amdgpu.powerplay=0, but it didn't help. I don't know about the meaning of the values applied in vce_v3_0_set_vce_sw_clock_gating(), but just inverting the "if (gated)" looks wrong to me. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 Christian König changed: What|Removed |Added Status|RESOLVED|CLOSED -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/b28ac721/attachment.html>
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 Christian König changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #11 from Christian König --- Yeah, agree. The number look perfectly fine. Additional to that please don't reopen a three year old bug for this. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/809780a7/attachment.html>
[PATCH 1/7] x86/platform/intel/iosf_mbi: Add a mutex for punit access
On Sun, 2017-01-08 at 16:30 +0100, Hans de Goede wrote: > Hi, > > On 08-01-17 16:16, Andy Shevchenko wrote: > > On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote: > > > One some systems the punit accesses the pmic to change various > > > voltages > > > through the same bus as other kernel drivers use for e.g. battery > > > monitoring. > > > > > > If a driver sends requests to the punit which require the punit to > > > access > > > the pmic bus while another driver is also accessing the pmic bus > > > various > > > bad things happen. > > > > > > This commit adds a mutex to protect the punit against simultaneous > > > accesses > > > and 2 functions to lock / unlock this mutex. > > > > > > Note on these systems the i2c-bus driver will request a sempahore > > > from > > > the > > > punit for exclusive access to the pmic bus when i2c drivers are > > > accessing > > > it, but this does not appear to be sufficient, we still need to > > > avoid > > > making certain punit requests during the access window to avoid > > > problems. > > > > I'm fine with the patch, but please spell > > P-Unit > > PMIC > > In the commit msg and comments, not in code you mean I assume ? Correct. -- Andy Shevchenko Intel Finland Oy
[PATCH 7/7] drm/i915: Take punit lock when modifying punit settings
On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote:
> Make sure the punit i2c bus is not in use when we send a request to
> the punit by calling iosf_mbi_punit_lock() / iosf_mbi_punit_unlock()
> around punit write accesses.
>
But should not i915 drm eventually share the same iosf_mbi driver?
Currently what you are doing you create notifier and all that not-best-
ever stuff due to having two accessors to IOSF MB. If we have only one
driver which i915 will not ignore you don't need to create such things.
That's what I was trying to imply when commenting one of the patch in
previous series.
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> Signed-off-by: Hans de Goede
> Tested-by: tagorereddy
> ---
>  drivers/gpu/drm/i915/intel_display.c    | 6 ++
>  drivers/gpu/drm/i915/intel_pm.c         | 9 +
> Â drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
> Â 3 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fec8eb3..b8be6ea 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -47,6 +47,7 @@
> Â #include
> Â #include
> Â #include
> +#include
> Â
> Â static bool is_mmio_work(struct intel_flip_work *work)
> Â {
> @@ -6423,6 +6424,8 @@ static void valleyview_set_cdclk(struct
> drm_device *dev, int cdclk)
> Â cmd = 0;
> Â
> Â mutex_lock(_priv->rps.hw_lock);
> + iosf_mbi_punit_lock();
> +
> Â val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> Â val &= ~DSPFREQGUAR_MASK;
> Â val |= (cmd << DSPFREQGUAR_SHIFT);
> @@ -6432,6 +6435,7 @@ static void valleyview_set_cdclk(struct
> drm_device *dev, int cdclk)
> Â Â Â Â Â Â 50)) {
> Â DRM_ERROR("timed out waiting for CDclk change\n");
> Â }
> + iosf_mbi_punit_unlock();
> Â mutex_unlock(_priv->rps.hw_lock);
> Â
> Â mutex_lock(_priv->sb_lock);
> @@ -6499,6 +6503,7 @@ static void cherryview_set_cdclk(struct
> drm_device *dev, int cdclk)
> Â cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
> Â
> Â mutex_lock(_priv->rps.hw_lock);
> + iosf_mbi_punit_lock();
> Â val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> Â val &= ~DSPFREQGUAR_MASK_CHV;
> Â val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
> @@ -6508,6 +6513,7 @@ static void cherryview_set_cdclk(struct
> drm_device *dev, int cdclk)
> Â Â Â Â Â Â 50)) {
> Â DRM_ERROR("timed out waiting for CDclk change\n");
> Â }
> + iosf_mbi_punit_unlock();
> Â mutex_unlock(_priv->rps.hw_lock);
> Â
> Â intel_update_cdclk(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 4b12637..0d55b61 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -32,6 +32,7 @@
> Â #include "../../../platform/x86/intel_ips.h"
> Â #include
> Â #include
> +#include
> Â
> Â /**
> Â * DOC: RC6
> @@ -276,6 +277,7 @@ static void chv_set_memory_dvfs(struct
> drm_i915_private *dev_priv, bool enable)
> Â u32 val;
> Â
> Â mutex_lock(_priv->rps.hw_lock);
> + iosf_mbi_punit_lock();
> Â
> Â val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
> Â if (enable)
> @@ -290,6 +292,7 @@ static void chv_set_memory_dvfs(struct
> drm_i915_private *dev_priv, bool enable)
> Â Â Â Â Â Â Â FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
> Â DRM_ERROR("timed out waiting for Punit DDR DVFS
> request\n");
> Â
> + iosf_mbi_punit_unlock();
> Â mutex_unlock(_priv->rps.hw_lock);
> Â }
> Â
> @@ -298,6 +301,7 @@ static void chv_set_memory_pm5(struct
> drm_i915_private *dev_priv, bool enable)
> Â u32 val;
> Â
> Â mutex_lock(_priv->rps.hw_lock);
> + iosf_mbi_punit_lock();
> Â
> Â val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> Â if (enable)
> @@ -306,6 +310,7 @@ static void chv_set_memory_pm5(struct
> drm_i915_private *dev_priv, bool enable)
> Â val &= ~DSP_MAXFIFO_PM5_ENABLE;
> Â vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> Â
> + iosf_mbi_punit_unlock();
> Â mutex_unlock(_priv->rps.hw_lock);
> Â }
> Â
> @@ -4546,6 +4551,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
> Â
> Â if (IS_CHERRYVIEW(dev_priv)) {
> Â mutex_lock(_priv->rps.hw_lock);
> + iosf_mbi_punit_lock();
> Â
> Â val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> Â if (val & DSP_MAXFIFO_PM5_ENABLE)
> @@ -4575,6 +4581,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
> Â wm->level = VLV_WM_LEVEL_DDR_DVFS;
> Â }
> Â
> + iosf_mbi_punit_unlock();
> Â mutex_unlock(_priv->rps.hw_lock);
> Â }
> Â
> @@ -4981,7 +4988,9 @@ static void valleyview_set_rps(struct
> drm_i915_private *dev_priv, u8 val)
> Â I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
> Â
> Â if (val !=
[PATCH 3/7] i2c: designware-baytrail: Take punit lock on bus acquire
On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote: > Take the punit lock to stop others from accessing the punit while the > pmic i2c bus is in use. This is necessary because accessing the punit > from the kernel may result in the punit trying to access the pmic i2c > bus, which results in a hang when it happens while we own the pmic i2c > bus semaphore. > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 > Signed-off-by: Hans de Goede > Tested-by: tagorereddy > --- > Â drivers/i2c/busses/i2c-designware-baytrail.c | 4 > Â 1 file changed, 4 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c > b/drivers/i2c/busses/i2c-designware-baytrail.c > index 3effc9a..cf7a2a0 100644 > --- a/drivers/i2c/busses/i2c-designware-baytrail.c > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c > > @@ -62,6 +62,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev) > + iosf_mbi_punit_unlock(); > @@ -79,6 +81,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev > + iosf_mbi_punit_lock(); For me it looks a bit asymmetrical. Can we use acquire()/release()? -- Andy Shevchenko Intel Finland Oy
[PATCH 4/7] i2c: designware-baytrail: Call pmic_bus_access_notifier_chain
On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote: > Call the iosf_mbi pmic_bus_access_notifier_chain on bus acquire / > release. > FWIW: Reviewed-by: Andy Shevchenko > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 > Signed-off-by: Hans de Goede > Tested-by: tagorereddy > --- > Â drivers/i2c/busses/i2c-designware-baytrail.c | 4 > Â 1 file changed, 4 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c > b/drivers/i2c/busses/i2c-designware-baytrail.c > index cf7a2a0..e8cf10a 100644 > --- a/drivers/i2c/busses/i2c-designware-baytrail.c > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c > @@ -63,6 +63,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev) > Â > Â pm_qos_update_request(>pm_qos, PM_QOS_DEFAULT_VALUE); > Â > + iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACC > ESS_END, > + Â Â Â Â Â NULL); > Â iosf_mbi_punit_unlock(); > Â } > Â > @@ -82,6 +84,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev > *dev) > Â return 0; > Â > Â iosf_mbi_punit_lock(); > + iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACC > ESS_BEGIN, > + Â Â Â Â Â NULL); > Â > Â /* > Â Â * Disallow the CPU to enter C6 or C7 state, entering these > states -- Andy Shevchenko Intel Finland Oy
4.10-rc2 i915 LVDS display noise on X61s thinkpad
Hi All, Booted 4.10-rc2 today, and the display shows random spots of flickering short horizontal bars 1px tall, maybe 8 or 16px wide. Machine is the venerable X61s thinkpad, 1.8ghz, XGA. Problem was not present on 4.9.0, same kernel config. I'll see if I can get around to bisecting, consider this a heads-up. Attached config for the curious, it's clearly visible in my drm eye candy toy https://github.com/vcaputo/rototiller. Not on list so please CC me in replies, thanks Cheers, Vito Caputo -- next part -- # # Automatically generated file; DO NOT EDIT. # Linux/x86 4.10.0-rc2 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_HAVE_INTEL_TXT=y CONFIG_X86_64_SMP=y CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_DEBUG_RODATA=y CONFIG_PGTABLE_LEVELS=4 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y # CONFIG_KERNEL_GZIP is not set # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set CONFIG_KERNEL_XZ=y # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_FHANDLE=y # CONFIG_USELIB is not set CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y CONFIG_AUDIT_WATCH=y CONFIG_AUDIT_TREE=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y # CONFIG_IRQ_DOMAIN_DEBUG is not set CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y # CONFIG_NO_HZ_FULL is not set # CONFIG_NO_HZ is not set CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_BSD_PROCESS_ACCT=y # CONFIG_BSD_PROCESS_ACCT_V3 is not set CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y # # RCU Subsystem # CONFIG_PREEMPT_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y # CONFIG_TASKS_RCU is not set CONFIG_RCU_STALL_COMMON=y # CONFIG_TREE_RCU_TRACE is not set # CONFIG_RCU_EXPEDITE_BOOT is not set CONFIG_BUILD_BIN2C=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=18 CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 CONFIG_NMI_LOG_BUF_SHIFT=13 CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y CONFIG_ARCH_SUPPORTS_INT128=y CONFIG_CGROUPS=y # CONFIG_MEMCG is not set CONFIG_BLK_CGROUP=y # CONFIG_DEBUG_BLK_CGROUP is not set CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y # CONFIG_CFS_BANDWIDTH is not set # CONFIG_RT_GROUP_SCHED is not set # CONFIG_CGROUP_PIDS is not set CONFIG_CGROUP_FREEZER=y # CONFIG_CGROUP_HUGETLB is not set CONFIG_CPUSETS=y CONFIG_PROC_PID_CPUSET=y # CONFIG_CGROUP_DEVICE is not set CONFIG_CGROUP_CPUACCT=y # CONFIG_CGROUP_PERF is not set # CONFIG_CGROUP_DEBUG is not set CONFIG_CHECKPOINT_RESTORE=y CONFIG_NAMESPACES=y CONFIG_UTS_NS=y CONFIG_IPC_NS=y # CONFIG_USER_NS is not set CONFIG_PID_NS=y
[PATCH 2/7] x86/platform/intel/iosf_mbi: Add a pmic bus access notifier
On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote:
> Some drivers may need to acquire punit managed resources from
> interrupt
> context, where they cannot call iosf_mbi_punit_lock().
>
> This commit adds a notifier chain which allows a driver to get
> notified
> (in a process context) before other drivers start accessing the pmic
> bus,
> so that the driver can acquire any resources, which it may need during
> the window the other driver is accessing the pmic, before hand.
>
Same comments as per patch 1.
I'm okay with the patch, but I would hear from other stakeholders that's
_the_ way we are going.
So, FWIW:
Reviewed-by: Andy Shevchenko
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> Signed-off-by: Hans de Goede
> Tested-by: tagorereddy
> ---
>  arch/x86/include/asm/iosf_mbi.h    | 54
> ++
> Â arch/x86/platform/intel/iosf_mbi.c | 36 +
> Â 2 files changed, 90 insertions(+)
>
> diff --git a/arch/x86/include/asm/iosf_mbi.h
> b/arch/x86/include/asm/iosf_mbi.h
> index 91f5d16..b8733bb 100644
> --- a/arch/x86/include/asm/iosf_mbi.h
> +++ b/arch/x86/include/asm/iosf_mbi.h
> @@ -47,6 +47,10 @@
> Â #define QRK_MBI_UNIT_MM 0x05
> Â #define QRK_MBI_UNIT_SOC0x31
> Â
> +/* Action values for the pmic_bus_access_notifier functions */
> +#define MBI_PMIC_BUS_ACCESS_BEGIN1
> +#define MBI_PMIC_BUS_ACCESS_END 2
> +
> Â #if IS_ENABLED(CONFIG_IOSF_MBI)
> Â
> Â bool iosf_mbi_available(void);
> @@ -115,6 +119,38 @@ void iosf_mbi_punit_lock(void);
> Â */
> Â void iosf_mbi_punit_unlock(void);
> Â
> +/**
> + * iosf_mbi_register_pmic_bus_access_notifier - Register pmic bus
> notifier
> + *
> + * This function can be used by drivers which may need to acquire
> punit
> + * managed resources from interrupt context, where
> iosf_mbi_punit_lock()
> + * can not be used.
> + *
> + * This function allows a driver to register a notifier to get
> notified (in a
> + * process context) before other drivers start accessing the pmic
> bus.
> + *
> + * This allows the driver to acquire any resources, which it may need
> during
> + * the window the other driver is accessing the pmic, before hand.
> + *
> + * @nb: notifier_block to register
> + */
> +int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block
> *nb);
> +
> +/**
> + * iosf_mbi_register_pmic_bus_access_notifier - Unregister pmic bus
> notifier
> + *
> + * @nb: notifier_block to unregister
> + */
> +int iosf_mbi_unregister_pmic_bus_access_notifier(struct
> notifier_block *nb);
> +
> +/**
> + * iosf_mbi_call_pmic_bus_access_notifier_chain - Call pmic bus
> notifier chain
> + *
> + * @val: action to pass into listener's notifier_call function
> + * @v: data pointer to pass into listener's notifier_call function
> + */
> +int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val,
> void *v);
> +
> Â #else /* CONFIG_IOSF_MBI is not enabled */
> Â static inline
> Â bool iosf_mbi_available(void)
> @@ -146,6 +182,24 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32
> offset, u32 mdr, u32 mask)
> Â static inline void iosf_mbi_punit_lock(void) {}
> Â static inline void iosf_mbi_punit_unlock(void) {}
> Â
> +static inline
> +int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block
> *nb)
> +{
> + return 0;
> +}
> +
> +static inline
> +int iosf_mbi_unregister_pmic_bus_access_notifier(struct
> notifier_block *nb)
> +{
> + return 0;
> +}
> +
> +static inline
> +int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val,
> void *v)
> +{
> + return 0;
> +}
> +
> Â #endif /* CONFIG_IOSF_MBI */
> Â
> Â #endif /* IOSF_MBI_SYMS_H */
> diff --git a/arch/x86/platform/intel/iosf_mbi.c
> b/arch/x86/platform/intel/iosf_mbi.c
> index 75d8135..a995789 100644
> --- a/arch/x86/platform/intel/iosf_mbi.c
> +++ b/arch/x86/platform/intel/iosf_mbi.c
> @@ -35,6 +35,7 @@
> Â static struct pci_dev *mbi_pdev;
> Â static DEFINE_SPINLOCK(iosf_mbi_lock);
> Â static DEFINE_MUTEX(iosf_mbi_punit_mutex);
> +static BLOCKING_NOTIFIER_HEAD(iosf_mbi_pmic_bus_access_notifier);
> Â
> Â static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
> Â {
> @@ -203,6 +204,41 @@ void iosf_mbi_punit_unlock(void)
> Â }
> Â EXPORT_SYMBOL(iosf_mbi_punit_unlock);
> Â
> +int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block
> *nb)
> +{
> + int ret;
> +
> + /* Wait for the bus to go inactive before registering */
> + mutex_lock(_mbi_punit_mutex);
> + ret = blocking_notifier_chain_register(
> + _mbi_pmic_bus_access_notifier,
> nb);
> + mutex_unlock(_mbi_punit_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(iosf_mbi_register_pmic_bus_access_notifier);
> +
> +int iosf_mbi_unregister_pmic_bus_access_notifier(struct
> notifier_block *nb)
> +{
> + int ret;
> +
> + /* Wait for the bus to go inactive before unregistering */
> + mutex_lock(_mbi_punit_mutex);
> + ret =
[PATCH 1/7] x86/platform/intel/iosf_mbi: Add a mutex for punit access
On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote:
> One some systems the punit accesses the pmic to change various
> voltages
> through the same bus as other kernel drivers use for e.g. battery
> monitoring.
>
> If a driver sends requests to the punit which require the punit to
> access
> the pmic bus while another driver is also accessing the pmic bus
> various
> bad things happen.
>
> This commit adds a mutex to protect the punit against simultaneous
> accesses
> and 2 functions to lock / unlock this mutex.
>
> Note on these systems the i2c-bus driver will request a sempahore from
> the
> punit for exclusive access to the pmic bus when i2c drivers are
> accessing
> it, but this does not appear to be sufficient, we still need to avoid
> making certain punit requests during the access window to avoid
> problems.
I'm fine with the patch, but please spell
P-Unit
PMIC
Reviewed-by: Andy Shevchenko
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> Signed-off-by: Hans de Goede
> Tested-by: tagorereddy
> ---
>  arch/x86/include/asm/iosf_mbi.h    | 31
> +++
> Â arch/x86/platform/intel/iosf_mbi.c | 13 +
> Â 2 files changed, 44 insertions(+)
>
> diff --git a/arch/x86/include/asm/iosf_mbi.h
> b/arch/x86/include/asm/iosf_mbi.h
> index b41ee16..91f5d16 100644
> --- a/arch/x86/include/asm/iosf_mbi.h
> +++ b/arch/x86/include/asm/iosf_mbi.h
> @@ -88,6 +88,33 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset,
> u32 mdr);
> Â */
> Â int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32
> mask);
> Â
> +/**
> + * iosf_mbi_punit_lock() - Lock the punit mutex
> + *
> + * One some systems the punit accesses the pmic to change various
> voltages
> + * through the same bus as other kernel drivers use for e.g. battery
> monitoring.
> + *
> + * If a driver sends requests to the punit which require the punit to
> access the
> + * pmic bus while another driver is also accessing the pmic bus
> various bad
> + * things happen.
> + *
> + * To avoid these problems this function must be called before
> accessing the
> + * punit or the pmic, be it through iosf_mbi* functions or through
> other means.
> + *
> + * Note on these systems the i2c-bus driver will request a sempahore
> from the
> + * punit for exclusive access to the pmic bus when i2c drivers are
> accessing it,
> + * but this does not appear to be sufficient, we still need to avoid
> making
> + * certain punit requests during the access window to avoid problems.
> + *
> + * This function locks a mutex, as such it may sleep.
> + */
> +void iosf_mbi_punit_lock(void);
> +
> +/**
> + * iosf_mbi_punit_unlock() - Unlock the punit mutex
> + */
> +void iosf_mbi_punit_unlock(void);
> +
> Â #else /* CONFIG_IOSF_MBI is not enabled */
> Â static inline
> Â bool iosf_mbi_available(void)
> @@ -115,6 +142,10 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32
> offset, u32 mdr, u32 mask)
> Â WARN(1, "IOSF_MBI driver not available");
> Â return -EPERM;
> Â }
> +
> +static inline void iosf_mbi_punit_lock(void) {}
> +static inline void iosf_mbi_punit_unlock(void) {}
> +
> Â #endif /* CONFIG_IOSF_MBI */
> Â
> Â #endif /* IOSF_MBI_SYMS_H */
> diff --git a/arch/x86/platform/intel/iosf_mbi.c
> b/arch/x86/platform/intel/iosf_mbi.c
> index edf2c54..75d8135 100644
> --- a/arch/x86/platform/intel/iosf_mbi.c
> +++ b/arch/x86/platform/intel/iosf_mbi.c
> @@ -34,6 +34,7 @@
> Â
> Â static struct pci_dev *mbi_pdev;
> Â static DEFINE_SPINLOCK(iosf_mbi_lock);
> +static DEFINE_MUTEX(iosf_mbi_punit_mutex);
> Â
> Â static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
> Â {
> @@ -190,6 +191,18 @@ bool iosf_mbi_available(void)
> Â }
> Â EXPORT_SYMBOL(iosf_mbi_available);
> Â
> +void iosf_mbi_punit_lock(void)
> +{
> + mutex_lock(_mbi_punit_mutex);
> +}
> +EXPORT_SYMBOL(iosf_mbi_punit_lock);
> +
> +void iosf_mbi_punit_unlock(void)
> +{
> + mutex_unlock(_mbi_punit_mutex);
> +}
> +EXPORT_SYMBOL(iosf_mbi_punit_unlock);
> +
> Â #ifdef CONFIG_IOSF_MBI_DEBUG
> Â static u32 dbg_mdr;
> Â static u32 dbg_mcr;
--
Andy Shevchenko
Intel Finland Oy
[PATCH 7/7] drm/i915: Take punit lock when modifying punit settings
Hi,
On 08-01-17 16:34, Andy Shevchenko wrote:
> On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote:
>> Make sure the punit i2c bus is not in use when we send a request to
>> the punit by calling iosf_mbi_punit_lock() / iosf_mbi_punit_unlock()
>> around punit write accesses.
>>
>
> But should not i915 drm eventually share the same iosf_mbi driver?
> Currently what you are doing you create notifier and all that not-best-
> ever stuff due to having two accessors to IOSF MB. If we have only one
> driver which i915 will not ignore you don't need to create such things.
>
> That's what I was trying to imply when commenting one of the patch in
> previous series.
Ah, yes that as an interesting point. So what do the i915 devs think
of changing the i915 code to use the iosf_mbi functions for at least
punit accesses, instead of doing those through the i915 pci config space?
Note that we will still need to have some higher level locking, or
a new iosf_mbi function which does this under a lock, for code paths
where the i915 code does a write followed by multiple reads to wait
for the punit to have changed the value to the requested value.
Regards,
Hans
>
>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
>> Signed-off-by: Hans de Goede
>> Tested-by: tagorereddy
>> ---
>> drivers/gpu/drm/i915/intel_display.c| 6 ++
>> drivers/gpu/drm/i915/intel_pm.c | 9 +
>> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
>> 3 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fec8eb3..b8be6ea 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -47,6 +47,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> static bool is_mmio_work(struct intel_flip_work *work)
>> {
>> @@ -6423,6 +6424,8 @@ static void valleyview_set_cdclk(struct
>> drm_device *dev, int cdclk)
>> cmd = 0;
>>
>> mutex_lock(_priv->rps.hw_lock);
>> +iosf_mbi_punit_lock();
>> +
>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
>> val &= ~DSPFREQGUAR_MASK;
>> val |= (cmd << DSPFREQGUAR_SHIFT);
>> @@ -6432,6 +6435,7 @@ static void valleyview_set_cdclk(struct
>> drm_device *dev, int cdclk)
>> 50)) {
>> DRM_ERROR("timed out waiting for CDclk change\n");
>> }
>> +iosf_mbi_punit_unlock();
>> mutex_unlock(_priv->rps.hw_lock);
>>
>> mutex_lock(_priv->sb_lock);
>> @@ -6499,6 +6503,7 @@ static void cherryview_set_cdclk(struct
>> drm_device *dev, int cdclk)
>> cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
>>
>> mutex_lock(_priv->rps.hw_lock);
>> +iosf_mbi_punit_lock();
>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
>> val &= ~DSPFREQGUAR_MASK_CHV;
>> val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
>> @@ -6508,6 +6513,7 @@ static void cherryview_set_cdclk(struct
>> drm_device *dev, int cdclk)
>> 50)) {
>> DRM_ERROR("timed out waiting for CDclk change\n");
>> }
>> +iosf_mbi_punit_unlock();
>> mutex_unlock(_priv->rps.hw_lock);
>>
>> intel_update_cdclk(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 4b12637..0d55b61 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -32,6 +32,7 @@
>> #include "../../../platform/x86/intel_ips.h"
>> #include
>> #include
>> +#include
>>
>> /**
>> * DOC: RC6
>> @@ -276,6 +277,7 @@ static void chv_set_memory_dvfs(struct
>> drm_i915_private *dev_priv, bool enable)
>> u32 val;
>>
>> mutex_lock(_priv->rps.hw_lock);
>> +iosf_mbi_punit_lock();
>>
>> val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
>> if (enable)
>> @@ -290,6 +292,7 @@ static void chv_set_memory_dvfs(struct
>> drm_i915_private *dev_priv, bool enable)
>>FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
>> DRM_ERROR("timed out waiting for Punit DDR DVFS
>> request\n");
>>
>> +iosf_mbi_punit_unlock();
>> mutex_unlock(_priv->rps.hw_lock);
>> }
>>
>> @@ -298,6 +301,7 @@ static void chv_set_memory_pm5(struct
>> drm_i915_private *dev_priv, bool enable)
>> u32 val;
>>
>> mutex_lock(_priv->rps.hw_lock);
>> +iosf_mbi_punit_lock();
>>
>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
>> if (enable)
>> @@ -306,6 +310,7 @@ static void chv_set_memory_pm5(struct
>> drm_i915_private *dev_priv, bool enable)
>> val &= ~DSP_MAXFIFO_PM5_ENABLE;
>> vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
>>
>> +iosf_mbi_punit_unlock();
>> mutex_unlock(_priv->rps.hw_lock);
>> }
>>
>> @@ -4546,6 +4551,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
>>
>> if (IS_CHERRYVIEW(dev_priv)) {
>> mutex_lock(_priv->rps.hw_lock);
>> +iosf_mbi_punit_lock();
>>
>> val =
[PATCH 3/7] i2c: designware-baytrail: Take punit lock on bus acquire
Hi, On 08-01-17 16:27, Andy Shevchenko wrote: > On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote: >> Take the punit lock to stop others from accessing the punit while the >> pmic i2c bus is in use. This is necessary because accessing the punit >> from the kernel may result in the punit trying to access the pmic i2c >> bus, which results in a hang when it happens while we own the pmic i2c >> bus semaphore. >> >> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 >> Signed-off-by: Hans de Goede >> Tested-by: tagorereddy >> --- >> drivers/i2c/busses/i2c-designware-baytrail.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c >> b/drivers/i2c/busses/i2c-designware-baytrail.c >> index 3effc9a..cf7a2a0 100644 >> --- a/drivers/i2c/busses/i2c-designware-baytrail.c >> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c >> > >> @@ -62,6 +62,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev) > >> +iosf_mbi_punit_unlock(); > >> @@ -79,6 +81,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev >> +iosf_mbi_punit_lock(); > > For me it looks a bit asymmetrical. > > Can we use acquire()/release()? Sure I can change things to use acquire()/release() instead. I will change this for v2. Regards, Hans
[PATCH 1/7] x86/platform/intel/iosf_mbi: Add a mutex for punit access
Hi,
On 08-01-17 16:16, Andy Shevchenko wrote:
> On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote:
>> One some systems the punit accesses the pmic to change various
>> voltages
>> through the same bus as other kernel drivers use for e.g. battery
>> monitoring.
>>
>> If a driver sends requests to the punit which require the punit to
>> access
>> the pmic bus while another driver is also accessing the pmic bus
>> various
>> bad things happen.
>>
>> This commit adds a mutex to protect the punit against simultaneous
>> accesses
>> and 2 functions to lock / unlock this mutex.
>>
>> Note on these systems the i2c-bus driver will request a sempahore from
>> the
>> punit for exclusive access to the pmic bus when i2c drivers are
>> accessing
>> it, but this does not appear to be sufficient, we still need to avoid
>> making certain punit requests during the access window to avoid
>> problems.
>
> I'm fine with the patch, but please spell
> P-Unit
> PMIC
In the commit msg and comments, not in code you mean I assume ?
Regards,
Hans
>
> Reviewed-by: Andy Shevchenko
>
>>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
>> Signed-off-by: Hans de Goede
>> Tested-by: tagorereddy
>> ---
>> arch/x86/include/asm/iosf_mbi.h| 31
>> +++
>> arch/x86/platform/intel/iosf_mbi.c | 13 +
>> 2 files changed, 44 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/iosf_mbi.h
>> b/arch/x86/include/asm/iosf_mbi.h
>> index b41ee16..91f5d16 100644
>> --- a/arch/x86/include/asm/iosf_mbi.h
>> +++ b/arch/x86/include/asm/iosf_mbi.h
>> @@ -88,6 +88,33 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset,
>> u32 mdr);
>> */
>> int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32
>> mask);
>>
>> +/**
>> + * iosf_mbi_punit_lock() - Lock the punit mutex
>> + *
>> + * One some systems the punit accesses the pmic to change various
>> voltages
>> + * through the same bus as other kernel drivers use for e.g. battery
>> monitoring.
>> + *
>> + * If a driver sends requests to the punit which require the punit to
>> access the
>> + * pmic bus while another driver is also accessing the pmic bus
>> various bad
>> + * things happen.
>> + *
>> + * To avoid these problems this function must be called before
>> accessing the
>> + * punit or the pmic, be it through iosf_mbi* functions or through
>> other means.
>> + *
>> + * Note on these systems the i2c-bus driver will request a sempahore
>> from the
>> + * punit for exclusive access to the pmic bus when i2c drivers are
>> accessing it,
>> + * but this does not appear to be sufficient, we still need to avoid
>> making
>> + * certain punit requests during the access window to avoid problems.
>> + *
>> + * This function locks a mutex, as such it may sleep.
>> + */
>> +void iosf_mbi_punit_lock(void);
>> +
>> +/**
>> + * iosf_mbi_punit_unlock() - Unlock the punit mutex
>> + */
>> +void iosf_mbi_punit_unlock(void);
>> +
>> #else /* CONFIG_IOSF_MBI is not enabled */
>> static inline
>> bool iosf_mbi_available(void)
>> @@ -115,6 +142,10 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32
>> offset, u32 mdr, u32 mask)
>> WARN(1, "IOSF_MBI driver not available");
>> return -EPERM;
>> }
>> +
>> +static inline void iosf_mbi_punit_lock(void) {}
>> +static inline void iosf_mbi_punit_unlock(void) {}
>> +
>> #endif /* CONFIG_IOSF_MBI */
>>
>> #endif /* IOSF_MBI_SYMS_H */
>> diff --git a/arch/x86/platform/intel/iosf_mbi.c
>> b/arch/x86/platform/intel/iosf_mbi.c
>> index edf2c54..75d8135 100644
>> --- a/arch/x86/platform/intel/iosf_mbi.c
>> +++ b/arch/x86/platform/intel/iosf_mbi.c
>> @@ -34,6 +34,7 @@
>>
>> static struct pci_dev *mbi_pdev;
>> static DEFINE_SPINLOCK(iosf_mbi_lock);
>> +static DEFINE_MUTEX(iosf_mbi_punit_mutex);
>>
>> static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
>> {
>> @@ -190,6 +191,18 @@ bool iosf_mbi_available(void)
>> }
>> EXPORT_SYMBOL(iosf_mbi_available);
>>
>> +void iosf_mbi_punit_lock(void)
>> +{
>> +mutex_lock(_mbi_punit_mutex);
>> +}
>> +EXPORT_SYMBOL(iosf_mbi_punit_lock);
>> +
>> +void iosf_mbi_punit_unlock(void)
>> +{
>> +mutex_unlock(_mbi_punit_mutex);
>> +}
>> +EXPORT_SYMBOL(iosf_mbi_punit_unlock);
>> +
>> #ifdef CONFIG_IOSF_MBI_DEBUG
>> static u32 dbg_mdr;
>> static u32 dbg_mcr;
>
[PATCH 7/7] drm/i915: Take punit lock when modifying punit settings
Make sure the punit i2c bus is not in use when we send a request to
the punit by calling iosf_mbi_punit_lock() / iosf_mbi_punit_unlock()
around punit write accesses.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
drivers/gpu/drm/i915/intel_display.c| 6 ++
drivers/gpu/drm/i915/intel_pm.c | 9 +
drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fec8eb3..b8be6ea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,7 @@
#include
#include
#include
+#include
static bool is_mmio_work(struct intel_flip_work *work)
{
@@ -6423,6 +6424,8 @@ static void valleyview_set_cdclk(struct drm_device *dev,
int cdclk)
cmd = 0;
mutex_lock(_priv->rps.hw_lock);
+ iosf_mbi_punit_lock();
+
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
@@ -6432,6 +6435,7 @@ static void valleyview_set_cdclk(struct drm_device *dev,
int cdclk)
50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
+ iosf_mbi_punit_unlock();
mutex_unlock(_priv->rps.hw_lock);
mutex_lock(_priv->sb_lock);
@@ -6499,6 +6503,7 @@ static void cherryview_set_cdclk(struct drm_device *dev,
int cdclk)
cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
mutex_lock(_priv->rps.hw_lock);
+ iosf_mbi_punit_lock();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK_CHV;
val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
@@ -6508,6 +6513,7 @@ static void cherryview_set_cdclk(struct drm_device *dev,
int cdclk)
50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
+ iosf_mbi_punit_unlock();
mutex_unlock(_priv->rps.hw_lock);
intel_update_cdclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4b12637..0d55b61 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -32,6 +32,7 @@
#include "../../../platform/x86/intel_ips.h"
#include
#include
+#include
/**
* DOC: RC6
@@ -276,6 +277,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private
*dev_priv, bool enable)
u32 val;
mutex_lock(_priv->rps.hw_lock);
+ iosf_mbi_punit_lock();
val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
if (enable)
@@ -290,6 +292,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private
*dev_priv, bool enable)
FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
DRM_ERROR("timed out waiting for Punit DDR DVFS request\n");
+ iosf_mbi_punit_unlock();
mutex_unlock(_priv->rps.hw_lock);
}
@@ -298,6 +301,7 @@ static void chv_set_memory_pm5(struct drm_i915_private
*dev_priv, bool enable)
u32 val;
mutex_lock(_priv->rps.hw_lock);
+ iosf_mbi_punit_lock();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
if (enable)
@@ -306,6 +310,7 @@ static void chv_set_memory_pm5(struct drm_i915_private
*dev_priv, bool enable)
val &= ~DSP_MAXFIFO_PM5_ENABLE;
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+ iosf_mbi_punit_unlock();
mutex_unlock(_priv->rps.hw_lock);
}
@@ -4546,6 +4551,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
if (IS_CHERRYVIEW(dev_priv)) {
mutex_lock(_priv->rps.hw_lock);
+ iosf_mbi_punit_lock();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
if (val & DSP_MAXFIFO_PM5_ENABLE)
@@ -4575,6 +4581,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
wm->level = VLV_WM_LEVEL_DDR_DVFS;
}
+ iosf_mbi_punit_unlock();
mutex_unlock(_priv->rps.hw_lock);
}
@@ -4981,7 +4988,9 @@ static void valleyview_set_rps(struct drm_i915_private
*dev_priv, u8 val)
I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
if (val != dev_priv->rps.cur_freq) {
+ iosf_mbi_punit_lock();
vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+ iosf_mbi_punit_unlock();
if (!IS_CHERRYVIEW(dev_priv))
gen6_set_rps_thresholds(dev_priv, val);
}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index c0b7e95..17922ae 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -28,6 +28,7 @@
#include
#include
+#include
#include "i915_drv.h"
#include "intel_drv.h"
@@ -1027,6 +1028,8 @@ static void
[PATCH 6/7] drm/i915: Listen for pmic bus access notifications
Listen for pmic bus access notifications and get FORCEWAKE_ALL while
the bus is accessed to avoid needing to do any forcewakes, which need
pmic bus access, while the pmic bus is busy:
This fixes errors like these showing up in dmesg, usually followed
by a gfx or system freeze:
[drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack
request.
[drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack
request.
i2c_designware 808622C1:06: punit semaphore timed out, resetting
i2c_designware 808622C1:06: PUNIT SEM: 2
i2c_designware 808622C1:06: couldn't acquire bus ownership
Downside of this approach is that it causes wakeups whenever the pmic
bus is accessed. Unfortunately we cannot simply wait for the pmic bus
to go idle when we hit a race, as forcewakes may be done from interrupt
handlers where we cannot sleep to wait for the i2c pmic bus access to
finish.
Note that the notifications and thus the wakeups will only happen on
baytrail / cherrytrail devices using pmic-s with a shared i2c bus for
punit and host pmic access (i2c busses with a _SEM method in their
APCI node), e.g. an axp288 pmic.
I plan to write some patches for drivers accessing the pmic bus to
limit their bus accesses to a bare minimum (e.g. cache registers, do not
update battery level more often then 4 times a minute), to limit the
amount of wakeups.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 35 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7cd0363..60297de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -731,6 +731,7 @@ struct intel_uncore {
const struct intel_forcewake_range *fw_domains_table;
unsigned int fw_domains_table_entries;
+ struct notifier_block pmic_bus_access_nb;
struct intel_uncore_funcs funcs;
unsigned fifo_count;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index c7fa222..1668de4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -25,6 +25,7 @@
#include "intel_drv.h"
#include "i915_vgpu.h"
+#include
#include
#define FORCEWAKE_ACK_TIMEOUT_MS 50
@@ -429,12 +430,16 @@ static void __intel_uncore_early_sanitize(struct
drm_i915_private *dev_priv,
void intel_uncore_suspend(struct drm_i915_private *dev_priv)
{
+ iosf_mbi_unregister_pmic_bus_access_notifier(
+ _priv->uncore.pmic_bus_access_nb);
__intel_uncore_forcewake_reset(dev_priv, false);
}
void intel_uncore_resume(struct drm_i915_private *dev_priv)
{
__intel_uncore_early_sanitize(dev_priv, true);
+ iosf_mbi_register_pmic_bus_access_notifier(
+ _priv->uncore.pmic_bus_access_nb);
i915_check_and_clear_faults(dev_priv);
}
@@ -1390,6 +1395,28 @@ static void intel_uncore_fw_domains_init(struct
drm_i915_private *dev_priv)
dev_priv->uncore.fw_domains_table_entries = ARRAY_SIZE((d)); \
}
+static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
+unsigned long action, void *data)
+{
+ struct drm_i915_private *dev_priv = container_of(nb,
+ struct drm_i915_private, uncore.pmic_bus_access_nb);
+
+ switch (action) {
+ case MBI_PMIC_BUS_ACCESS_BEGIN:
+ /*
+* forcewake all to make sure that we don't need to forcewake
+* any power-planes while the pmic bus is busy.
+*/
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ break;
+ case MBI_PMIC_BUS_ACCESS_END:
+ intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
void intel_uncore_init(struct drm_i915_private *dev_priv)
{
i915_check_vgpu(dev_priv);
@@ -1399,6 +1426,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
__intel_uncore_early_sanitize(dev_priv, false);
dev_priv->uncore.unclaimed_mmio_check = 1;
+ dev_priv->uncore.pmic_bus_access_nb.notifier_call =
+ i915_pmic_bus_access_notifier;
switch (INTEL_INFO(dev_priv)->gen) {
default:
@@ -1458,6 +1487,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
ASSIGN_READ_MMIO_VFUNCS(vgpu);
}
+ iosf_mbi_register_pmic_bus_access_notifier(
+ _priv->uncore.pmic_bus_access_nb);
+
i915_check_and_clear_faults(dev_priv);
}
#undef ASSIGN_WRITE_MMIO_VFUNCS
@@ -1465,6 +1497,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
void intel_uncore_fini(struct drm_i915_private *dev_priv)
{
+
[PATCH 5/7] drm/i915: Add intel_uncore_suspend / resume functions
Rename intel_uncore_early_sanitize to intel_uncore_resume, dropping the
(always true) restore_forcewake argument and add a new intel_uncore_resume
function to replace the intel_uncore_forcewake_reset(dev_priv, false)
calls done from the suspend / runtime_suspend functions and make
intel_uncore_forcewake_reset private.
This is a preparation patch for adding pmic bus access notifier support.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
drivers/gpu/drm/i915/i915_drv.c | 6 +++---
drivers/gpu/drm/i915/i915_drv.h | 6 ++
drivers/gpu/drm/i915/intel_uncore.c | 18 +++---
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2c020ea..ec61e6e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1445,7 +1445,7 @@ static int i915_drm_suspend(struct drm_device *dev)
opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
intel_opregion_notify_adapter(dev_priv, opregion_target_state);
- intel_uncore_forcewake_reset(dev_priv, false);
+ intel_uncore_suspend(dev_priv);
intel_opregion_unregister(dev_priv);
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
@@ -1690,7 +1690,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
DRM_ERROR("Resume prepare failed: %d, continuing anyway\n",
ret);
- intel_uncore_early_sanitize(dev_priv, true);
+ intel_uncore_resume(dev_priv);
if (IS_BROXTON(dev_priv)) {
if (!dev_priv->suspended_to_idle)
@@ -2343,7 +2343,7 @@ static int intel_runtime_suspend(struct device *kdev)
return ret;
}
- intel_uncore_forcewake_reset(dev_priv, false);
+ intel_uncore_suspend(dev_priv);
enable_rpm_wakeref_asserts(dev_priv);
WARN_ON_ONCE(atomic_read(_priv->pm.wakeref_count));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5194686..7cd0363 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3052,14 +3052,12 @@ int intel_irq_install(struct drm_i915_private
*dev_priv);
void intel_irq_uninstall(struct drm_i915_private *dev_priv);
extern void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
-extern void intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
- bool restore_forcewake);
extern void intel_uncore_init(struct drm_i915_private *dev_priv);
extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
extern bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private
*dev_priv);
extern void intel_uncore_fini(struct drm_i915_private *dev_priv);
-extern void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
-bool restore);
+extern void intel_uncore_suspend(struct drm_i915_private *dev_priv);
+extern void intel_uncore_resume(struct drm_i915_private *dev_priv);
const char *intel_uncore_forcewake_domain_to_str(const enum
forcewake_domain_id id);
void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
enum forcewake_domains domains);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 8fc5f29..c7fa222 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -250,7 +250,7 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
return HRTIMER_NORESTART;
}
-void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
+static void __intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
bool restore)
{
unsigned long irqflags;
@@ -424,13 +424,17 @@ static void __intel_uncore_early_sanitize(struct
drm_i915_private *dev_priv,
if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
info->has_decoupled_mmio = false;
- intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
+ __intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
}
-void intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
-bool restore_forcewake)
+void intel_uncore_suspend(struct drm_i915_private *dev_priv)
{
- __intel_uncore_early_sanitize(dev_priv, restore_forcewake);
+ __intel_uncore_forcewake_reset(dev_priv, false);
+}
+
+void intel_uncore_resume(struct drm_i915_private *dev_priv)
+{
+ __intel_uncore_early_sanitize(dev_priv, true);
i915_check_and_clear_faults(dev_priv);
}
@@ -1463,7 +1467,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv)
{
/* Paranoia: make sure we have disabled everything before we exit. */
intel_uncore_sanitize(dev_priv);
-
[PATCH 4/7] i2c: designware-baytrail: Call pmic_bus_access_notifier_chain
Call the iosf_mbi pmic_bus_access_notifier_chain on bus acquire / release. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 Signed-off-by: Hans de Goede Tested-by: tagorereddy --- drivers/i2c/busses/i2c-designware-baytrail.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index cf7a2a0..e8cf10a 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -63,6 +63,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev) pm_qos_update_request(>pm_qos, PM_QOS_DEFAULT_VALUE); + iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACCESS_END, +NULL); iosf_mbi_punit_unlock(); } @@ -82,6 +84,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev) return 0; iosf_mbi_punit_lock(); + iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACCESS_BEGIN, +NULL); /* * Disallow the CPU to enter C6 or C7 state, entering these states -- 2.9.3
[PATCH 3/7] i2c: designware-baytrail: Take punit lock on bus acquire
Take the punit lock to stop others from accessing the punit while the pmic i2c bus is in use. This is necessary because accessing the punit from the kernel may result in the punit trying to access the pmic i2c bus, which results in a hang when it happens while we own the pmic i2c bus semaphore. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 Signed-off-by: Hans de Goede Tested-by: tagorereddy --- drivers/i2c/busses/i2c-designware-baytrail.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index 3effc9a..cf7a2a0 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -62,6 +62,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev) dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n"); pm_qos_update_request(>pm_qos, PM_QOS_DEFAULT_VALUE); + + iosf_mbi_punit_unlock(); } static int baytrail_i2c_acquire(struct dw_i2c_dev *dev) @@ -79,6 +81,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev) if (!dev->release_lock) return 0; + iosf_mbi_punit_lock(); + /* * Disallow the CPU to enter C6 or C7 state, entering these states * requires the punit to talk to the pmic and if this happens while -- 2.9.3
[PATCH 2/7] x86/platform/intel/iosf_mbi: Add a pmic bus access notifier
Some drivers may need to acquire punit managed resources from interrupt
context, where they cannot call iosf_mbi_punit_lock().
This commit adds a notifier chain which allows a driver to get notified
(in a process context) before other drivers start accessing the pmic bus,
so that the driver can acquire any resources, which it may need during
the window the other driver is accessing the pmic, before hand.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
arch/x86/include/asm/iosf_mbi.h| 54 ++
arch/x86/platform/intel/iosf_mbi.c | 36 +
2 files changed, 90 insertions(+)
diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
index 91f5d16..b8733bb 100644
--- a/arch/x86/include/asm/iosf_mbi.h
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -47,6 +47,10 @@
#define QRK_MBI_UNIT_MM0x05
#define QRK_MBI_UNIT_SOC 0x31
+/* Action values for the pmic_bus_access_notifier functions */
+#define MBI_PMIC_BUS_ACCESS_BEGIN 1
+#define MBI_PMIC_BUS_ACCESS_END2
+
#if IS_ENABLED(CONFIG_IOSF_MBI)
bool iosf_mbi_available(void);
@@ -115,6 +119,38 @@ void iosf_mbi_punit_lock(void);
*/
void iosf_mbi_punit_unlock(void);
+/**
+ * iosf_mbi_register_pmic_bus_access_notifier - Register pmic bus notifier
+ *
+ * This function can be used by drivers which may need to acquire punit
+ * managed resources from interrupt context, where iosf_mbi_punit_lock()
+ * can not be used.
+ *
+ * This function allows a driver to register a notifier to get notified (in a
+ * process context) before other drivers start accessing the pmic bus.
+ *
+ * This allows the driver to acquire any resources, which it may need during
+ * the window the other driver is accessing the pmic, before hand.
+ *
+ * @nb: notifier_block to register
+ */
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb);
+
+/**
+ * iosf_mbi_register_pmic_bus_access_notifier - Unregister pmic bus notifier
+ *
+ * @nb: notifier_block to unregister
+ */
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb);
+
+/**
+ * iosf_mbi_call_pmic_bus_access_notifier_chain - Call pmic bus notifier chain
+ *
+ * @val: action to pass into listener's notifier_call function
+ * @v: data pointer to pass into listener's notifier_call function
+ */
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v);
+
#else /* CONFIG_IOSF_MBI is not enabled */
static inline
bool iosf_mbi_available(void)
@@ -146,6 +182,24 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32
mdr, u32 mask)
static inline void iosf_mbi_punit_lock(void) {}
static inline void iosf_mbi_punit_unlock(void) {}
+static inline
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v)
+{
+ return 0;
+}
+
#endif /* CONFIG_IOSF_MBI */
#endif /* IOSF_MBI_SYMS_H */
diff --git a/arch/x86/platform/intel/iosf_mbi.c
b/arch/x86/platform/intel/iosf_mbi.c
index 75d8135..a995789 100644
--- a/arch/x86/platform/intel/iosf_mbi.c
+++ b/arch/x86/platform/intel/iosf_mbi.c
@@ -35,6 +35,7 @@
static struct pci_dev *mbi_pdev;
static DEFINE_SPINLOCK(iosf_mbi_lock);
static DEFINE_MUTEX(iosf_mbi_punit_mutex);
+static BLOCKING_NOTIFIER_HEAD(iosf_mbi_pmic_bus_access_notifier);
static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
{
@@ -203,6 +204,41 @@ void iosf_mbi_punit_unlock(void)
}
EXPORT_SYMBOL(iosf_mbi_punit_unlock);
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+ int ret;
+
+ /* Wait for the bus to go inactive before registering */
+ mutex_lock(_mbi_punit_mutex);
+ ret = blocking_notifier_chain_register(
+ _mbi_pmic_bus_access_notifier, nb);
+ mutex_unlock(_mbi_punit_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_register_pmic_bus_access_notifier);
+
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+ int ret;
+
+ /* Wait for the bus to go inactive before unregistering */
+ mutex_lock(_mbi_punit_mutex);
+ ret = blocking_notifier_chain_unregister(
+ _mbi_pmic_bus_access_notifier, nb);
+ mutex_unlock(_mbi_punit_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_unregister_pmic_bus_access_notifier);
+
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v)
+{
+ return blocking_notifier_call_chain(
+ _mbi_pmic_bus_access_notifier, val, v);
+}
+EXPORT_SYMBOL(iosf_mbi_call_pmic_bus_access_notifier_chain);
+
#ifdef CONFIG_IOSF_MBI_DEBUG
static u32 dbg_mdr;
[PATCH 1/7] x86/platform/intel/iosf_mbi: Add a mutex for punit access
One some systems the punit accesses the pmic to change various voltages
through the same bus as other kernel drivers use for e.g. battery
monitoring.
If a driver sends requests to the punit which require the punit to access
the pmic bus while another driver is also accessing the pmic bus various
bad things happen.
This commit adds a mutex to protect the punit against simultaneous accesses
and 2 functions to lock / unlock this mutex.
Note on these systems the i2c-bus driver will request a sempahore from the
punit for exclusive access to the pmic bus when i2c drivers are accessing
it, but this does not appear to be sufficient, we still need to avoid
making certain punit requests during the access window to avoid problems.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
arch/x86/include/asm/iosf_mbi.h| 31 +++
arch/x86/platform/intel/iosf_mbi.c | 13 +
2 files changed, 44 insertions(+)
diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
index b41ee16..91f5d16 100644
--- a/arch/x86/include/asm/iosf_mbi.h
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -88,6 +88,33 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
*/
int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+/**
+ * iosf_mbi_punit_lock() - Lock the punit mutex
+ *
+ * One some systems the punit accesses the pmic to change various voltages
+ * through the same bus as other kernel drivers use for e.g. battery
monitoring.
+ *
+ * If a driver sends requests to the punit which require the punit to access
the
+ * pmic bus while another driver is also accessing the pmic bus various bad
+ * things happen.
+ *
+ * To avoid these problems this function must be called before accessing the
+ * punit or the pmic, be it through iosf_mbi* functions or through other means.
+ *
+ * Note on these systems the i2c-bus driver will request a sempahore from the
+ * punit for exclusive access to the pmic bus when i2c drivers are accessing
it,
+ * but this does not appear to be sufficient, we still need to avoid making
+ * certain punit requests during the access window to avoid problems.
+ *
+ * This function locks a mutex, as such it may sleep.
+ */
+void iosf_mbi_punit_lock(void);
+
+/**
+ * iosf_mbi_punit_unlock() - Unlock the punit mutex
+ */
+void iosf_mbi_punit_unlock(void);
+
#else /* CONFIG_IOSF_MBI is not enabled */
static inline
bool iosf_mbi_available(void)
@@ -115,6 +142,10 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32
mdr, u32 mask)
WARN(1, "IOSF_MBI driver not available");
return -EPERM;
}
+
+static inline void iosf_mbi_punit_lock(void) {}
+static inline void iosf_mbi_punit_unlock(void) {}
+
#endif /* CONFIG_IOSF_MBI */
#endif /* IOSF_MBI_SYMS_H */
diff --git a/arch/x86/platform/intel/iosf_mbi.c
b/arch/x86/platform/intel/iosf_mbi.c
index edf2c54..75d8135 100644
--- a/arch/x86/platform/intel/iosf_mbi.c
+++ b/arch/x86/platform/intel/iosf_mbi.c
@@ -34,6 +34,7 @@
static struct pci_dev *mbi_pdev;
static DEFINE_SPINLOCK(iosf_mbi_lock);
+static DEFINE_MUTEX(iosf_mbi_punit_mutex);
static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
{
@@ -190,6 +191,18 @@ bool iosf_mbi_available(void)
}
EXPORT_SYMBOL(iosf_mbi_available);
+void iosf_mbi_punit_lock(void)
+{
+ mutex_lock(_mbi_punit_mutex);
+}
+EXPORT_SYMBOL(iosf_mbi_punit_lock);
+
+void iosf_mbi_punit_unlock(void)
+{
+ mutex_unlock(_mbi_punit_mutex);
+}
+EXPORT_SYMBOL(iosf_mbi_punit_unlock);
+
#ifdef CONFIG_IOSF_MBI_DEBUG
static u32 dbg_mdr;
static u32 dbg_mcr;
--
2.9.3
[PATCH 0/7] coordinate cht i2c-pmic and i915-punit accesses
Hi All, Here is a non RFC version of my previous RFC series for coordinating cht i2c-pmic and i915-punit accesses. New in this non RFC version is that forcewake accesses are coodinated now too. The problem: This series fixes an issue where the following messages are shown in dmesg: [drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request. [drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request. i2c_designware 808622C1:06: punit semaphore timed out, resetting i2c_designware 808622C1:06: PUNIT SEM: 2 i2c_designware 808622C1:06: couldn't acquire bus ownership Usually followed by a gfx or system freeze, this is happening both on my cherrytrail tablet as well as being seen by an user commenting on: https://bugzilla.kernel.org/show_bug.cgi?id=155241 This problem is triggered by my patch series to fix the punit i2c bus semaphore code in drivers/i2c/busses/designware-baytrail.c : https://patchwork.ozlabs.org/patch/710067/ https://patchwork.ozlabs.org/patch/710068/ https://patchwork.ozlabs.org/patch/710069/ https://patchwork.ozlabs.org/patch/710070/ https://patchwork.ozlabs.org/patch/710071/ https://patchwork.ozlabs.org/patch/710072/ Which actually allows i2c access to pmic-s wich need bus-access arbritration for the first time on cherrytrail, combined with patches from me to actually make the axp288_fuel_gauge driver load, which leads to regular pmic i2c accesses (when userspace checks the battery level). The fix / workaround Testing has shown that just taking the punit i2c bus semaphore on i2c accesses to a shared pmic i2c bus is not enough to avoid problem, e.g. besides this series to coordinate i915 access, we also need: https://patchwork.ozlabs.org/patch/710070/ This series introduces 2 mechanisms to coordinate direct pmic accesses vs punit requests which lead to pmic accesses. The first mechanism is a simple mutex, which works well for the direct punit accesses the i915 code does. However testing has shown this is not enough, we also need to avoid doing a forcewake when a driver is directly accessing the pmic i2c bus, otherwise we get the timeout errors quoted above, the fact that after the forcewake errors the designware-baytrail.c code also fails to acquire the lock, suggests that doing forcewakes during the access window causes the i2c bus to get stuck (or the punit to get confused). The deal with the forcewake issue a notifier is introduced which allows drivers to get notified before any pmic i2c transfer begins (and after it ends). This is used in the i915 driver to do a forcewake_get(FORCEWAKE_ALL) before the access begins avoiding needing to it while the access is in progress. Testing has shown that this avoids the problem for both me and the user, where as before it could be reproduced at will. The implementation of the fix = I've decided to put the mutex and the notifier in the iosf_mbi code, as that is somewhat of a common middle ground between the i915 driver and the designware-baytrail.c code, with the latter already depending on the iosf_mbi code. I'm open for moving them if someone has a suggestion for a better place to put them. If iosf_mbi support is not enabled then all the functions used will become static inline NOPs and nothing changes on the i915 side. If the code is enabled and runs on a system which does not need pmic i2c bus arbritration, then the mutex lock / unlock calls added to the i915 code will still be made, but there won't be any contention, so other then the overhead of the lock / unlock there will not be any delays. These calls are not made in any hot paths. The biggest downside IMHO is that taking FORCEWAKE_ALL before each bus access will cause extra GPU wakeups, and thus powerdrain. I plan to make the impact of this minimal by modifying any drivers (axp288 code, with which I'm quite familiar by now) to limit their i2c accesses to a bare minimum. Specifically I plan to e.g. cache the battery level and wait at least 15 seconds (max 4 times a minute) before getting a fresh reading even if userspace asks for it more often, combined with some mechanism to only do 1 access-begin and 1 access-end notification for i2c accesses in quick succession. Alternatives It would be tempting to say that this is not worth the trouble and we should just disallow direct i2c accesses to the pmic bus. Unfortunately that will cause some major issues on affected devices: -No battery monitoring -No "AC" plugged in monitoring -If booted with a normal USB-A -> micro-USB cable, or no cable, plugged in and then the user replaces the cable with an otg USB-host cable / adapter, the id-pin shorting will enable a 5v boost convertor, but we need to disable the pmic's USB-Vbus path otherwise it will start drawing current from the boost convertor, leading to aprox 300mA of extra battery drain, this is done by the axp288_charger driver, which needs
[Bug 107381] radeon VCE init error (-110) -- AMD/Intel Mars Hybrid Graphics
https://bugzilla.kernel.org/show_bug.cgi?id=107381 --- Comment #17 from Jean-Pierre van Riel --- Until VCE gets fixed for Mars/, there might be a way to disable VCE (I need to look into it more) https://github.com/torvalds/linux/commit/fabb5935871db1f31fcd2684fd154e24de04d917#diff-9bc1b4aaf15dd521a1991717e4e2a2e0 -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 97861] [amdgpu SI] purple line is visible on left side of the screen connected by HDMI
https://bugs.freedesktop.org/show_bug.cgi?id=97861 --- Comment #3 from Gregor Münch --- I have the same problem. Purple Line on left side and overall blurry screen. When Audio off, the issues go away. AMDGPU 1.20 Kernel 4.10 RC2 Radeon HD 7970 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/ca4ab04c/attachment.html>
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 --- Comment #10 from Andy Furniss --- That looks normal, I have that sample and it's 1444 frames / 60Hz = 24 seconds. You can't really turn off vsync for vdpau and mplayer doesn't do gl interop. Cpu times can mislead if cpufreq on_demand is in use as you don't know what freq the cores are in. UVD/powerplay is tuned for playback (at least on recent GPUs) if using with eg. ffmpeg you can get more perf by forcing GPU clocks to high. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/894ea0a4/attachment.html>
[Bug 99321] Videoplaying applications hang on latest r600
https://bugs.freedesktop.org/show_bug.cgi?id=99321 Vlad changed: What|Removed |Added Summary|Videoplaying applications |Videoplaying applications |hangs on latest r600|hang on latest r600 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/605a08f7/attachment.html>
[Bug 99321] Videoplaying applications hangs on latest r600
https://bugs.freedesktop.org/show_bug.cgi?id=99321 Bug ID: 99321 Summary: Videoplaying applications hangs on latest r600 Product: Mesa Version: 13.0 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r600 Assignee: dri-devel at lists.freedesktop.org Reporter: krnlbg at gmail.com QA Contact: dri-devel at lists.freedesktop.org Hello. After updating to Fedora 25 applications that play videos started to hang periodically. Firefox, Chrome, Steam and mpv are affected when play video. Also I found that this can happen in 2d native games like Crimsonland. 3D games works just fine. I can't find any logs related to this problem and the only manifestation of this is that the process goes into "futex_wait_queue_me" status and can only be killed. One thing I noticed: starting mpv with --vo=x11 solves the problem. (default is --vo=opengl). It is 100% reproducible for me. I think the problem is driver related because I tried putting my ssd into another pc with Intel graphics and everything was working just fine. More info in redhat bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1401479 I don't know how to get more information but i'm willing to try if somebody suggests me. My Hardware: AMD Phenom(tm) II X4 975 Processor AMD Radeon 6850 GPU with r600g driver -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/eea39e71/attachment.html>
[Bug 98743] Incorrect colormapping in Verdun game
https://bugs.freedesktop.org/show_bug.cgi?id=98743 --- Comment #6 from piejacker875 at gmail.com --- I'm running mesa 13.0.3-1 and llvm 3.9.1-2 on Arch. R9 390. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/dd799d59/attachment-0001.html>
[Bug 96600] [RV630]: a lot of artifacts appears on a screen playing some videos through VDPAU with hardware acceleration
https://bugs.freedesktop.org/show_bug.cgi?id=96600 --- Comment #12 from Vladimir Usikov --- No more artifacts in this video on mesa-git 1d529cba02 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/2f278458/attachment.html>
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 --- Comment #9 from Vladimir Usikov --- mplayer -benchmark -nosound -lavdopts threads=16 Planet_Earth_From_Pole_to_Pole_1080p_sample.mkv -vo gl BENCHMARKs: VC: 0.827s VO: 22.917s A: 0.000s Sys: 0.494s = 24.238s BENCHMARK%: VC: 3.4108% VO: 94.5502% A: 0.% Sys: 2.0390% = 100.% $ mplayer -benchmark -nosound -lavdopts threads=16 Planet_Earth_From_Pole_to_Pole_1080p_sample.mkv -vo xv BENCHMARKs: VC: 3.292s VO: 2.707s A: 0.000s Sys: 0.667s =6.666s BENCHMARK%: VC: 49.3886% VO: 40.6098% A: 0.% Sys: 10.0016% = 100.% mplayer -benchmark -nosound -lavdopts threads=16 Planet_Earth_From_Pole_to_Pole_1080p_sample.mkv -vo vdpau BENCHMARKs: VC: 0.752s VO: 22.925s A: 0.000s Sys: 0.482s = 24.158s BENCHMARK%: VC: 3.1113% VO: 94.8942% A: 0.% Sys: 1.9945% = 100.% mplayer -benchmark -nosound -lavdopts threads=16 Planet_Earth_From_Pole_to_Pole_1080p_sample.mkv -vo null BENCHMARKs: VC: 4.836s VO: 0.004s A: 0.000s Sys: 0.391s =5.231s BENCHMARK%: VC: 92.4320% VO: 0.0849% A: 0.% Sys: 7.4831% = 100.% mplayer -benchmark -nosound -lavdopts threads=16 Planet_Earth_From_Pole_to_Pole_1080p_sample.mkv -vo vdpau -vc ffh264vdpau BENCHMARKs: VC: 1.624s VO: 22.466s A: 0.000s Sys: 0.501s = 24.590s BENCHMARK%: VC: 6.6026% VO: 91.3590% A: 0.% Sys: 2.0384% = 100.% -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/f2374a5a/attachment.html>
[Bug 99316] Radeon crash when laptop on AC power
https://bugs.freedesktop.org/show_bug.cgi?id=99316 --- Comment #6 from Samuel Anderson --- Created attachment 128812 --> https://bugs.freedesktop.org/attachment.cgi?id=128812=edit kernel panic I get these sometimes when I reboot after testing the card. Usually after testing it with something like Portal or Unigine Heaven. It doesn't always happen. Sorry it's a picture, I haven't been able to get kdump to work, though if necessary I'll give it another try. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/da27faac/attachment.html>
[Bug 99316] Radeon crash when laptop on AC power
https://bugs.freedesktop.org/show_bug.cgi?id=99316 --- Comment #5 from Samuel Anderson --- Created attachment 128811 --> https://bugs.freedesktop.org/attachment.cgi?id=128811=edit hwinfo -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/8a21a6b0/attachment.html>
[Bug 99316] Radeon crash when laptop on AC power
https://bugs.freedesktop.org/show_bug.cgi?id=99316 --- Comment #4 from Samuel Anderson --- It has no effect. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20170108/fb789490/attachment.html>
