Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers
from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt
call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event")
call. This is a problem if some AML code reads the GPIO pin before we
run the deferred acpi_gpiochip_request_interrupt, because in that case
acpi_gpio_adr_space_handler() will already have called
gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from
acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to
register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt
already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG
ID pin on a Prowise PT301 tablet not registering, keeping the port stuck
in whatever mode it was in during boot and e.g. only allowing charging
after a reboot.
This commit fixes this by only deferring the request_irq call and the
initial run of edge-triggered IRQs instead of deferring all of
acpi_gpiochip_request_interrupt.
Cc: stable(a)vger.kernel.org
Fixes: 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event ...")
Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
---
Changes in v2:
-add kernel-doc describing struct acpi_gpio_event members (requested by Mika)
-s/acpi_gpiochip_request_interrupt_desc/acpi_gpiochip_alloc_event/ (req Mika)
-minor style fixups (requested by Andy)
---
drivers/gpio/gpiolib-acpi.c | 144 +++++++++++++++++++++---------------
1 file changed, 84 insertions(+), 60 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 55b72fbe1631..7f93954c58ea 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -19,11 +19,28 @@
#include "gpiolib.h"
+/**
+ * struct acpi_gpio_event - ACPI GPIO event handler data
+ *
+ * @node: list-entry of the events list of the struct acpi_gpio_chip
+ * @handle: handle of ACPI method to execute when the IRQ triggers
+ * @handler: irq_handler to pass to request_irq when requesting the IRQ
+ * @pin: GPIO pin number on the gpio_chip
+ * @irq: Linux IRQ number for the event, for request_ / free_irq
+ * @irqflags: flags to pass to request_irq when requesting the IRQ
+ * @irq_is_wake: If the ACPI flags indicate the IRQ is a wakeup source
+ * @is_requested: True if request_irq has been done
+ * @desc: gpio_desc for the GPIO pin for this event
+ */
struct acpi_gpio_event {
struct list_head node;
acpi_handle handle;
+ irq_handler_t handler;
unsigned int pin;
unsigned int irq;
+ unsigned long irqflags;
+ bool irq_is_wake;
+ bool irq_requested;
struct gpio_desc *desc;
};
@@ -49,10 +66,10 @@ struct acpi_gpio_chip {
/*
* For gpiochips which call acpi_gpiochip_request_interrupts() before late_init
- * (so builtin drivers) we register the ACPI GpioInt event handlers from a
+ * (so builtin drivers) we register the ACPI GpioInt IRQ handlers from a
* late_initcall_sync handler, so that other builtin drivers can register their
* OpRegions before the event handlers can run. This list contains gpiochips
- * for which the acpi_gpiochip_request_interrupts() has been deferred.
+ * for which the acpi_gpiochip_request_irqs() call has been deferred.
*/
static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock);
static LIST_HEAD(acpi_gpio_deferred_req_irqs_list);
@@ -133,8 +150,42 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
}
EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource);
-static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
- void *context)
+static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
+ struct acpi_gpio_event *event)
+{
+ int ret, value;
+
+ ret = request_threaded_irq(event->irq, NULL, event->handler,
+ event->irqflags, "ACPI:Event", event);
+ if (ret) {
+ dev_err(acpi_gpio->chip->parent,
+ "Failed to setup interrupt handler for %d\n",
+ event->irq);
+ return;
+ }
+
+ if (event->irq_is_wake)
+ enable_irq_wake(event->irq);
+
+ event->irq_requested = true;
+
+ /* Make sure we trigger the initial state of edge-triggered IRQs */
+ value = gpiod_get_raw_value_cansleep(event->desc);
+ if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
+ ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
+ event->handler(event->irq, event);
+}
+
+static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
+{
+ struct acpi_gpio_event *event;
+
+ list_for_each_entry(event, &acpi_gpio->events, node)
+ acpi_gpiochip_request_irq(acpi_gpio, event);
+}
+
+static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
+ void *context)
{
struct acpi_gpio_chip *acpi_gpio = context;
struct gpio_chip *chip = acpi_gpio->chip;
@@ -143,8 +194,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
struct acpi_gpio_event *event;
irq_handler_t handler = NULL;
struct gpio_desc *desc;
- unsigned long irqflags;
- int ret, pin, irq, value;
+ int ret, pin, irq;
if (!acpi_gpio_get_irq_resource(ares, &agpio))
return AE_OK;
@@ -175,8 +225,6 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
gpiod_direction_input(desc);
- value = gpiod_get_value_cansleep(desc);
-
ret = gpiochip_lock_as_irq(chip, pin);
if (ret) {
dev_err(chip->parent, "Failed to lock GPIO as interrupt\n");
@@ -189,64 +237,42 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
goto fail_unlock_irq;
}
- irqflags = IRQF_ONESHOT;
+ event = kzalloc(sizeof(*event), GFP_KERNEL);
+ if (!event)
+ goto fail_unlock_irq;
+
+ event->irqflags = IRQF_ONESHOT;
if (agpio->triggering == ACPI_LEVEL_SENSITIVE) {
if (agpio->polarity == ACPI_ACTIVE_HIGH)
- irqflags |= IRQF_TRIGGER_HIGH;
+ event->irqflags |= IRQF_TRIGGER_HIGH;
else
- irqflags |= IRQF_TRIGGER_LOW;
+ event->irqflags |= IRQF_TRIGGER_LOW;
} else {
switch (agpio->polarity) {
case ACPI_ACTIVE_HIGH:
- irqflags |= IRQF_TRIGGER_RISING;
+ event->irqflags |= IRQF_TRIGGER_RISING;
break;
case ACPI_ACTIVE_LOW:
- irqflags |= IRQF_TRIGGER_FALLING;
+ event->irqflags |= IRQF_TRIGGER_FALLING;
break;
default:
- irqflags |= IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING;
+ event->irqflags |= IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING;
break;
}
}
- event = kzalloc(sizeof(*event), GFP_KERNEL);
- if (!event)
- goto fail_unlock_irq;
-
event->handle = evt_handle;
+ event->handler = handler;
event->irq = irq;
+ event->irq_is_wake = agpio->wake_capable == ACPI_WAKE_CAPABLE;
event->pin = pin;
event->desc = desc;
- ret = request_threaded_irq(event->irq, NULL, handler, irqflags,
- "ACPI:Event", event);
- if (ret) {
- dev_err(chip->parent,
- "Failed to setup interrupt handler for %d\n",
- event->irq);
- goto fail_free_event;
- }
-
- if (agpio->wake_capable == ACPI_WAKE_CAPABLE)
- enable_irq_wake(irq);
-
list_add_tail(&event->node, &acpi_gpio->events);
- /*
- * Make sure we trigger the initial state of the IRQ when using RISING
- * or FALLING. Note we run the handlers on late_init, the AML code
- * may refer to OperationRegions from other (builtin) drivers which
- * may be probed after us.
- */
- if (((irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
- ((irqflags & IRQF_TRIGGER_FALLING) && value == 0))
- handler(event->irq, event);
-
return AE_OK;
-fail_free_event:
- kfree(event);
fail_unlock_irq:
gpiochip_unlock_as_irq(chip, pin);
fail_free_desc:
@@ -283,6 +309,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
if (ACPI_FAILURE(status))
return;
+ acpi_walk_resources(handle, "_AEI",
+ acpi_gpiochip_alloc_event, acpi_gpio);
+
mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
defer = !acpi_gpio_deferred_req_irqs_done;
if (defer)
@@ -293,8 +322,7 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
if (defer)
return;
- acpi_walk_resources(handle, "_AEI",
- acpi_gpiochip_request_interrupt, acpi_gpio);
+ acpi_gpiochip_request_irqs(acpi_gpio);
}
EXPORT_SYMBOL_GPL(acpi_gpiochip_request_interrupts);
@@ -331,10 +359,13 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) {
struct gpio_desc *desc;
- if (irqd_is_wakeup_set(irq_get_irq_data(event->irq)))
- disable_irq_wake(event->irq);
+ if (event->irq_requested) {
+ if (event->irq_is_wake)
+ disable_irq_wake(event->irq);
+
+ free_irq(event->irq, event);
+ }
- free_irq(event->irq, event);
desc = event->desc;
if (WARN_ON(IS_ERR(desc)))
continue;
@@ -1200,23 +1231,16 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
return con_id == NULL;
}
-/* Run deferred acpi_gpiochip_request_interrupts() */
-static int acpi_gpio_handle_deferred_request_interrupts(void)
+/* Run deferred acpi_gpiochip_request_irqs() */
+static int acpi_gpio_handle_deferred_request_irqs(void)
{
struct acpi_gpio_chip *acpi_gpio, *tmp;
mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
list_for_each_entry_safe(acpi_gpio, tmp,
&acpi_gpio_deferred_req_irqs_list,
- deferred_req_irqs_list_entry) {
- acpi_handle handle;
-
- handle = ACPI_HANDLE(acpi_gpio->chip->parent);
- acpi_walk_resources(handle, "_AEI",
- acpi_gpiochip_request_interrupt, acpi_gpio);
-
- list_del_init(&acpi_gpio->deferred_req_irqs_list_entry);
- }
+ deferred_req_irqs_list_entry)
+ acpi_gpiochip_request_irqs(acpi_gpio);
acpi_gpio_deferred_req_irqs_done = true;
mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
@@ -1224,4 +1248,4 @@ static int acpi_gpio_handle_deferred_request_interrupts(void)
return 0;
}
/* We must use _sync so that this runs after the first deferred_probe run */
-late_initcall_sync(acpi_gpio_handle_deferred_request_interrupts);
+late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
--
2.19.1
From: Adrian Hunter <adrian.hunter(a)intel.com>
thread__resolve() is used in the sample_addr_correlates_sym() cases
where 'addr' is a destination of a branch which does not necessarily
have the same cpumode as the 'ip'. Use the fallback function in that
case.
This patch depends on patch "perf tools: Add fallback functions for
cases where cpumode is insufficient".
Signed-off-by: Adrian Hunter <adrian.hunter(a)intel.com>
Cc: Andi Kleen <ak(a)linux.intel.com>
Cc: David S. Miller <davem(a)davemloft.net>
Cc: Jiri Olsa <jolsa(a)redhat.com>
Cc: Leo Yan <leo.yan(a)linaro.org>
Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Cc: stable(a)vger.kernel.org
Link: http://lkml.kernel.org/r/20181106210712.12098-3-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
---
tools/perf/util/event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9431b20c1337..24493200cf80 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1706,7 +1706,7 @@ bool sample_addr_correlates_sym(struct perf_event_attr *attr)
void thread__resolve(struct thread *thread, struct addr_location *al,
struct perf_sample *sample)
{
- thread__find_map(thread, sample->cpumode, sample->addr, al);
+ thread__find_map_fb(thread, sample->cpumode, sample->addr, al);
al->cpu = sample->cpu;
al->sym = NULL;
--
2.19.2
From: Adrian Hunter <adrian.hunter(a)intel.com>
Some architectures have a single address space for kernel and user
addresses, which makes it possible to determine if an address is in
kernel space or user space. Some don't, e.g.: sparc.
Cache that info in perf_env so that, for instance, code needing to
fallback failed symbol lookups at the kernel space in single address
space arches can lookup at userspace.
Signed-off-by: Adrian Hunter <adrian.hunter(a)intel.com>
Cc: Andi Kleen <ak(a)linux.intel.com>
Cc: David S. Miller <davem(a)davemloft.net>
Cc: Jiri Olsa <jolsa(a)redhat.com>
Cc: Leo Yan <leo.yan(a)linaro.org>
Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Cc: stable(a)vger.kernel.org
Link: http://lkml.kernel.org/r/20181106210712.12098-2-adrian.hunter@intel.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
---
tools/perf/arch/common.c | 10 ++++++++++
tools/perf/arch/common.h | 1 +
tools/perf/util/machine.h | 1 +
tools/perf/util/session.c | 4 ++++
4 files changed, 16 insertions(+)
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 82657c01a3b8..5f69fd0b745a 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -200,3 +200,13 @@ int perf_env__lookup_objdump(struct perf_env *env, const char **path)
return perf_env__lookup_binutils_path(env, "objdump", path);
}
+
+/*
+ * Some architectures have a single address space for kernel and user addresses,
+ * which makes it possible to determine if an address is in kernel space or user
+ * space.
+ */
+bool perf_env__single_address_space(struct perf_env *env)
+{
+ return strcmp(perf_env__arch(env), "sparc");
+}
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
index 2167001b18c5..c298a446d1f6 100644
--- a/tools/perf/arch/common.h
+++ b/tools/perf/arch/common.h
@@ -5,5 +5,6 @@
#include "../util/env.h"
int perf_env__lookup_objdump(struct perf_env *env, const char **path);
+bool perf_env__single_address_space(struct perf_env *env);
#endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index d856b85862e2..ca897a73014c 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -42,6 +42,7 @@ struct machine {
u16 id_hdr_size;
bool comm_exec;
bool kptr_restrict_warned;
+ bool single_address_space;
char *root_dir;
char *mmap_name;
struct threads threads[THREADS__TABLE_SIZE];
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7d2c8ce6cfad..f8eab197f35c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -24,6 +24,7 @@
#include "thread.h"
#include "thread-stack.h"
#include "stat.h"
+#include "arch/common.h"
static int perf_session__deliver_event(struct perf_session *session,
union perf_event *event,
@@ -150,6 +151,9 @@ struct perf_session *perf_session__new(struct perf_data *data,
session->machines.host.env = &perf_env;
}
+ session->machines.host.single_address_space =
+ perf_env__single_address_space(session->machines.host.env);
+
if (!data || perf_data__is_write(data)) {
/*
* In O_RDONLY mode this will be performed when reading the
--
2.19.2
From: Arnaldo Carvalho de Melo <acme(a)redhat.com>
We'll set a new machine field based on env->arch, which for live mode,
like with 'perf top' means we need to use uname() to figure the name of
the arch, fix perf_env__arch() to consider both (env == NULL) and
(env->arch == NULL) as local operation.
Cc: Adrian Hunter <adrian.hunter(a)intel.com>
Cc: Andi Kleen <ak(a)linux.intel.com>
Cc: David Ahern <dsahern(a)gmail.com>
Cc: David S. Miller <davem(a)davemloft.net>
Cc: Jiri Olsa <jolsa(a)redhat.com>
Cc: Leo Yan <leo.yan(a)linaro.org>
Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Cc: Namhyung Kim <namhyung(a)kernel.org>
Cc: Wang Nan <wangnan0(a)huawei.com>
Cc: stable(a)vger.kernel.org
Link: https://lkml.kernel.org/n/tip-vcz4ufzdon7cwy8dm2ua53xk@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
---
tools/perf/util/env.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 59f38c7693f8..4c23779e271a 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -166,7 +166,7 @@ const char *perf_env__arch(struct perf_env *env)
struct utsname uts;
char *arch_name;
- if (!env) { /* Assume local operation */
+ if (!env || !env->arch) { /* Assume local operation */
if (uname(&uts) < 0)
return NULL;
arch_name = uts.machine;
--
2.19.2
Braswell is really picky about having our writes posted to memory before
we execute or else the GPU may see stale values. A wmb() is insufficient
as it only ensures the writes are visible to other cores, we need a full
mb() to ensure the writes are in memory and visible to the GPU.
The most frequent failure in flushing before execution is that we see
stale PTE values and execute the wrong pages.
References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala(a)linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen(a)linux.intel.com>
Cc: stable(a)vger.kernel.org
---
drivers/gpu/drm/i915/intel_lrc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index de1e9dc6aec0..e6a86fa4502d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -379,8 +379,13 @@ static u64 execlists_update_context(struct i915_request *rq)
* may not be visible to the HW prior to the completion of the UC
* register write and that we may begin execution from the context
* before its image is complete leading to invalid PD chasing.
+ *
+ * Furthermore, Braswell, at least, wants a full mb to be sure that
+ * the writes are coherent in memory (visible to the GPU) prior to
+ * execution, and not just visible to other CPUs (as is the result of
+ * wmb).
*/
- wmb();
+ mb();
return ce->lrc_desc;
}
--
2.20.0.rc2