From 6e59de2048eb375a9bfcd39461ef841cd2a78962 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Thu, 8 Feb 2018 17:46:01 +0800 Subject: [PATCH 01/18] drm/amdgpu: add new device to use atpx quirk The affected system (0x0813) is pretty similar to another one (0x0812), it also needs to use ATPX power control. Signed-off-by: Kai-Heng Feng Signed-off-by: Alex Deucher Cc: stable@vger.kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c index e2c3c5ec42d1..c53095b3b0fb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c @@ -568,6 +568,7 @@ static const struct amdgpu_px_quirk amdgpu_px_quirk_list[] = { /* HG _PR3 doesn't seem to work on this A+A weston board */ { 0x1002, 0x6900, 0x1002, 0x0124, AMDGPU_PX_QUIRK_FORCE_ATPX }, { 0x1002, 0x6900, 0x1028, 0x0812, AMDGPU_PX_QUIRK_FORCE_ATPX }, + { 0x1002, 0x6900, 0x1028, 0x0813, AMDGPU_PX_QUIRK_FORCE_ATPX }, { 0, 0, 0, 0, 0 }, }; From 75b0e73023ef7994348d619e9adadab0e96bb195 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 8 Feb 2018 10:24:02 +0000 Subject: [PATCH 02/18] drm/i915/perf: Fix compiler warning for string truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drivers/gpu/drm/i915/i915_oa_cflgt3.c: In function ‘i915_perf_load_test_config_cflgt3’: drivers/gpu/drm/i915/i915_oa_cflgt3.c:87:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation] v2: strlcpy Fixes: 4407eaa9b0cc ("drm/i915/perf: add support for Coffeelake GT3") Signed-off-by: Chris Wilson Cc: Lionel Landwerlin Cc: Matthew Auld Reviewed-by: Lionel Landwerlin Link: https://patchwork.freedesktop.org/patch/msgid/20180208102403.5587-1-chris@chris-wilson.co.uk (cherry picked from commit 43df81d324cdd7056ad0ce3df709aff8dce856b7) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_oa_cflgt3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_oa_cflgt3.c b/drivers/gpu/drm/i915/i915_oa_cflgt3.c index 42ff06fe54a3..792facdb6702 100644 --- a/drivers/gpu/drm/i915/i915_oa_cflgt3.c +++ b/drivers/gpu/drm/i915/i915_oa_cflgt3.c @@ -84,9 +84,9 @@ show_test_oa_id(struct device *kdev, struct device_attribute *attr, char *buf) void i915_perf_load_test_config_cflgt3(struct drm_i915_private *dev_priv) { - strncpy(dev_priv->perf.oa.test_config.uuid, + strlcpy(dev_priv->perf.oa.test_config.uuid, "577e8e2c-3fa0-4875-8743-3538d585e3b0", - UUID_STRING_LEN); + sizeof(dev_priv->perf.oa.test_config.uuid)); dev_priv->perf.oa.test_config.id = 1; dev_priv->perf.oa.test_config.mux_regs = mux_config_test_oa; From 73b0fcd24ef1b8e20b7f6e6babcde540d96d0cb2 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 8 Feb 2018 10:24:03 +0000 Subject: [PATCH 03/18] drm/i915/perf: Fix compiler warning for string truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drivers/gpu/drm/i915/i915_oa_cnl.c: In function ‘i915_perf_load_test_config_cnl’: drivers/gpu/drm/i915/i915_oa_cnl.c:99:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation] v2: strlcpy Fixes: 95690a02fb5d ("drm/i915/perf: enable perf support on CNL") Signed-off-by: Chris Wilson Cc: Lionel Landwerlin Cc: Matthew Auld Reviewed-by: Lionel Landwerlin Link: https://patchwork.freedesktop.org/patch/msgid/20180208102403.5587-2-chris@chris-wilson.co.uk (cherry picked from commit 020580ff8edd50e64ae1bf47e560c61e5e2f29fc) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_oa_cnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_oa_cnl.c b/drivers/gpu/drm/i915/i915_oa_cnl.c index ff0ac3627cc4..ba9140c87cc0 100644 --- a/drivers/gpu/drm/i915/i915_oa_cnl.c +++ b/drivers/gpu/drm/i915/i915_oa_cnl.c @@ -96,9 +96,9 @@ show_test_oa_id(struct device *kdev, struct device_attribute *attr, char *buf) void i915_perf_load_test_config_cnl(struct drm_i915_private *dev_priv) { - strncpy(dev_priv->perf.oa.test_config.uuid, + strlcpy(dev_priv->perf.oa.test_config.uuid, "db41edd4-d8e7-4730-ad11-b9a2d6833503", - UUID_STRING_LEN); + sizeof(dev_priv->perf.oa.test_config.uuid)); dev_priv->perf.oa.test_config.id = 1; dev_priv->perf.oa.test_config.mux_regs = mux_config_test_oa; From 33afe065b66f226ee5f90ab24ff55799c896e381 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 8 Feb 2018 08:51:51 +0000 Subject: [PATCH 04/18] drm/i915: Avoid truncation before clamping userspace's priority value Userspace provides a 64b value for the priority, we need to be careful to preserve the full range before validation to prevent truncation (and letting an illegal value pass). Reported-by: Antonio Argenziano Fixes: ac14fbd460d0 ("drm/i915/scheduler: Support user-defined priorities") Signed-off-by: Chris Wilson Cc: Antonio Argenziano Cc: Michal Winiarski Cc: Mika Kuoppala Cc: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20180208085151.11480-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen (cherry picked from commit 11a18f631959fd1ca10856c836a827683536770c) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 648e7536ff51..0c963fcf31ff 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -803,7 +803,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data, case I915_CONTEXT_PARAM_PRIORITY: { - int priority = args->value; + s64 priority = args->value; if (args->size) ret = -EINVAL; From 7292b9e6586534fb43e4316ad8b508bf3d1212f7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 12 Feb 2018 09:39:28 +0000 Subject: [PATCH 05/18] drm/i915: Don't wake the device up to check if the engine is asleep If the entire device is powered off, we can safely assume that the engine is also asleep (and idle). Reported-by: Tvrtko Ursulin Fixes: a091d4ee931b ("drm/i915: Hold a wakeref for probing the ring registers") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20180212093928.6005-1-chris@chris-wilson.co.uk (cherry picked from commit 74d00d28a15c8452f65de0a9477b52d95639cc63) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index d790bdc227ff..acc661aa9c0c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1458,7 +1458,9 @@ static bool ring_is_idle(struct intel_engine_cs *engine) struct drm_i915_private *dev_priv = engine->i915; bool idle = true; - intel_runtime_pm_get(dev_priv); + /* If the whole device is asleep, the engine must be idle */ + if (!intel_runtime_pm_get_if_in_use(dev_priv)) + return true; /* First check that no commands are left in the ring */ if ((I915_READ_HEAD(engine) & HEAD_ADDR) != From 117172c8f9d40ba1de8cb35c6e614422faa03330 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 13 Feb 2018 09:01:54 +0000 Subject: [PATCH 06/18] drm/i915/breadcrumbs: Ignore unsubmitted signalers When a request is preempted, it is unsubmitted from the HW queue and removed from the active list of breadcrumbs. In the process, this however triggers the signaler and it may see the clear rbtree with the old, and still valid, seqno, or it may match the cleared seqno with the now zero rq->global_seqno. This confuses the signaler into action and signaling the fence. Fixes: d6a2289d9d6b ("drm/i915: Remove the preempted request from the execution queue") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Cc: # v4.12+ Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180206094633.30181-1-chris@chris-wilson.co.uk (cherry picked from commit fd10e2ce9905030d922e179a8047a4d50daffd8e) Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20180213090154.17373-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 29 ++++++++---------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index bd40fea16b4f..f54ddda9fdad 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -594,29 +594,16 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, spin_unlock_irq(&b->rb_lock); } -static bool signal_valid(const struct drm_i915_gem_request *request) -{ - return intel_wait_check_request(&request->signaling.wait, request); -} - static bool signal_complete(const struct drm_i915_gem_request *request) { if (!request) return false; - /* If another process served as the bottom-half it may have already - * signalled that this wait is already completed. - */ - if (intel_wait_complete(&request->signaling.wait)) - return signal_valid(request); - - /* Carefully check if the request is complete, giving time for the + /* + * Carefully check if the request is complete, giving time for the * seqno to be visible or if the GPU hung. */ - if (__i915_request_irq_complete(request)) - return true; - - return false; + return __i915_request_irq_complete(request); } static struct drm_i915_gem_request *to_signaler(struct rb_node *rb) @@ -659,9 +646,13 @@ static int intel_breadcrumbs_signaler(void *arg) request = i915_gem_request_get_rcu(request); rcu_read_unlock(); if (signal_complete(request)) { - local_bh_disable(); - dma_fence_signal(&request->fence); - local_bh_enable(); /* kick start the tasklets */ + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, + &request->fence.flags)) { + local_bh_disable(); + dma_fence_signal(&request->fence); + GEM_BUG_ON(!i915_gem_request_completed(request)); + local_bh_enable(); /* kick start the tasklets */ + } spin_lock_irq(&b->rb_lock); From edb76b01ac1629bfe17158bea56fcc16bfb57854 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 13 Feb 2018 09:57:44 +0000 Subject: [PATCH 07/18] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats In order to prevent a race condition where we may end up overaccounting the active state and leaving the busy-stats believing the GPU is 100% busy, lock out the tasklet while we reconstruct the busy state. There is no direct spinlock guard for the execlists->port[], so we need to utilise tasklet_disable() as a synchronous barrier to prevent it, the only writer to execlists->port[], from running at the same time as the enable. Fixes: 4900727d35bb ("drm/i915/pmu: Reconstruct active state on starting busy-stats") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180115092041.13509-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin (cherry picked from commit 99e48bf98dd036090b480a12c39e8b971731247e) Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20180213095747.2424-1-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/i915/intel_engine_cs.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index acc661aa9c0c..fa960cfd2764 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1945,16 +1945,22 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance) */ int intel_enable_engine_stats(struct intel_engine_cs *engine) { + struct intel_engine_execlists *execlists = &engine->execlists; unsigned long flags; + int err = 0; if (!intel_engine_supports_stats(engine)) return -ENODEV; + tasklet_disable(&execlists->tasklet); spin_lock_irqsave(&engine->stats.lock, flags); - if (engine->stats.enabled == ~0) - goto busy; + + if (unlikely(engine->stats.enabled == ~0)) { + err = -EBUSY; + goto unlock; + } + if (engine->stats.enabled++ == 0) { - struct intel_engine_execlists *execlists = &engine->execlists; const struct execlist_port *port = execlists->port; unsigned int num_ports = execlists_num_ports(execlists); @@ -1969,14 +1975,12 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) if (engine->stats.active) engine->stats.start = engine->stats.enabled_at; } + +unlock: spin_unlock_irqrestore(&engine->stats.lock, flags); + tasklet_enable(&execlists->tasklet); - return 0; - -busy: - spin_unlock_irqrestore(&engine->stats.lock, flags); - - return -EBUSY; + return err; } static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine) From d3f84c8b097001e3f31f584b793493cb0033a7ae Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Tue, 13 Feb 2018 09:57:45 +0000 Subject: [PATCH 08/18] drm/i915/pmu: Fix PMU enable vs execlists tasklet race Commit 99e48bf98dd0 ("drm/i915: Lock out execlist tasklet while peeking inside for busy-stats") added a tasklet_disable call in busy stats enabling, but we failed to understand that the PMU enable callback runs as an hard IRQ (IPI). Consequence of this is that the PMU enable callback can interrupt the execlists tasklet, and will then deadlock when it calls intel_engine_stats_enable->tasklet_disable. To fix this, I realized it is possible to move the engine stats enablement and disablement to PMU event init and destroy hooks. This allows for much simpler implementation since those hooks run in normal context (can sleep). v2: Extract engine_event_destroy. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Fixes: 99e48bf98dd0 ("drm/i915: Lock out execlist tasklet while peeking inside for busy-stats") Testcase: igt/perf_pmu/enable-race-* Cc: Chris Wilson Cc: Tvrtko Ursulin Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org Reviewed-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20180205093448.13877-1-tvrtko.ursulin@linux.intel.com (cherry picked from commit b2f78cda260bc6a1a2d382b1d85a29e69b5b3724) Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20180213095747.2424-2-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/i915/i915_pmu.c | 127 ++++++++++-------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 14 --- 2 files changed, 53 insertions(+), 88 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 55a8a1e29424..337eaa6ede52 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -285,26 +285,41 @@ static u64 count_interrupts(struct drm_i915_private *i915) return sum; } -static void i915_pmu_event_destroy(struct perf_event *event) -{ - WARN_ON(event->parent); -} - -static int engine_event_init(struct perf_event *event) +static void engine_event_destroy(struct perf_event *event) { struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); + struct intel_engine_cs *engine; - if (!intel_engine_lookup_user(i915, engine_event_class(event), - engine_event_instance(event))) - return -ENODEV; + engine = intel_engine_lookup_user(i915, + engine_event_class(event), + engine_event_instance(event)); + if (WARN_ON_ONCE(!engine)) + return; - switch (engine_event_sample(event)) { + if (engine_event_sample(event) == I915_SAMPLE_BUSY && + intel_engine_supports_stats(engine)) + intel_disable_engine_stats(engine); +} + +static void i915_pmu_event_destroy(struct perf_event *event) +{ + WARN_ON(event->parent); + + if (is_engine_event(event)) + engine_event_destroy(event); +} + +static int +engine_event_status(struct intel_engine_cs *engine, + enum drm_i915_pmu_engine_sample sample) +{ + switch (sample) { case I915_SAMPLE_BUSY: case I915_SAMPLE_WAIT: break; case I915_SAMPLE_SEMA: - if (INTEL_GEN(i915) < 6) + if (INTEL_GEN(engine->i915) < 6) return -ENODEV; break; default: @@ -314,6 +329,30 @@ static int engine_event_init(struct perf_event *event) return 0; } +static int engine_event_init(struct perf_event *event) +{ + struct drm_i915_private *i915 = + container_of(event->pmu, typeof(*i915), pmu.base); + struct intel_engine_cs *engine; + u8 sample; + int ret; + + engine = intel_engine_lookup_user(i915, engine_event_class(event), + engine_event_instance(event)); + if (!engine) + return -ENODEV; + + sample = engine_event_sample(event); + ret = engine_event_status(engine, sample); + if (ret) + return ret; + + if (sample == I915_SAMPLE_BUSY && intel_engine_supports_stats(engine)) + ret = intel_enable_engine_stats(engine); + + return ret; +} + static int i915_pmu_event_init(struct perf_event *event) { struct drm_i915_private *i915 = @@ -387,7 +426,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event) if (WARN_ON_ONCE(!engine)) { /* Do nothing */ } else if (sample == I915_SAMPLE_BUSY && - engine->pmu.busy_stats) { + intel_engine_supports_stats(engine)) { val = ktime_to_ns(intel_engine_get_busy_time(engine)); } else { val = engine->pmu.sample[sample].cur; @@ -442,12 +481,6 @@ static void i915_pmu_event_read(struct perf_event *event) local64_add(new - prev, &event->count); } -static bool engine_needs_busy_stats(struct intel_engine_cs *engine) -{ - return intel_engine_supports_stats(engine) && - (engine->pmu.enable & BIT(I915_SAMPLE_BUSY)); -} - static void i915_pmu_enable(struct perf_event *event) { struct drm_i915_private *i915 = @@ -487,21 +520,7 @@ static void i915_pmu_enable(struct perf_event *event) GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS); GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0); - if (engine->pmu.enable_count[sample]++ == 0) { - /* - * Enable engine busy stats tracking if needed or - * alternatively cancel the scheduled disable. - * - * If the delayed disable was pending, cancel it and - * in this case do not enable since it already is. - */ - if (engine_needs_busy_stats(engine) && - !engine->pmu.busy_stats) { - engine->pmu.busy_stats = true; - if (!cancel_delayed_work(&engine->pmu.disable_busy_stats)) - intel_enable_engine_stats(engine); - } - } + engine->pmu.enable_count[sample]++; } /* @@ -514,14 +533,6 @@ static void i915_pmu_enable(struct perf_event *event) spin_unlock_irqrestore(&i915->pmu.lock, flags); } -static void __disable_busy_stats(struct work_struct *work) -{ - struct intel_engine_cs *engine = - container_of(work, typeof(*engine), pmu.disable_busy_stats.work); - - intel_disable_engine_stats(engine); -} - static void i915_pmu_disable(struct perf_event *event) { struct drm_i915_private *i915 = @@ -545,26 +556,8 @@ static void i915_pmu_disable(struct perf_event *event) * Decrement the reference count and clear the enabled * bitmask when the last listener on an event goes away. */ - if (--engine->pmu.enable_count[sample] == 0) { + if (--engine->pmu.enable_count[sample] == 0) engine->pmu.enable &= ~BIT(sample); - if (!engine_needs_busy_stats(engine) && - engine->pmu.busy_stats) { - engine->pmu.busy_stats = false; - /* - * We request a delayed disable to handle the - * rapid on/off cycles on events, which can - * happen when tools like perf stat start, in a - * nicer way. - * - * In addition, this also helps with busy stats - * accuracy with background CPU offline/online - * migration events. - */ - queue_delayed_work(system_wq, - &engine->pmu.disable_busy_stats, - round_jiffies_up_relative(HZ)); - } - } } GEM_BUG_ON(bit >= I915_PMU_MASK_BITS); @@ -797,8 +790,6 @@ static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915) void i915_pmu_register(struct drm_i915_private *i915) { - struct intel_engine_cs *engine; - enum intel_engine_id id; int ret; if (INTEL_GEN(i915) <= 2) { @@ -820,10 +811,6 @@ void i915_pmu_register(struct drm_i915_private *i915) hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); i915->pmu.timer.function = i915_sample; - for_each_engine(engine, i915, id) - INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats, - __disable_busy_stats); - ret = perf_pmu_register(&i915->pmu.base, "i915", -1); if (ret) goto err; @@ -843,9 +830,6 @@ void i915_pmu_register(struct drm_i915_private *i915) void i915_pmu_unregister(struct drm_i915_private *i915) { - struct intel_engine_cs *engine; - enum intel_engine_id id; - if (!i915->pmu.base.event_init) return; @@ -853,11 +837,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915) hrtimer_cancel(&i915->pmu.timer); - for_each_engine(engine, i915, id) { - GEM_BUG_ON(engine->pmu.busy_stats); - flush_delayed_work(&engine->pmu.disable_busy_stats); - } - i915_pmu_unregister_cpuhp_state(i915); perf_pmu_unregister(&i915->pmu.base); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index c5ff203e42d6..a0e7a6c2a57c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -366,20 +366,6 @@ struct intel_engine_cs { */ #define I915_ENGINE_SAMPLE_MAX (I915_SAMPLE_SEMA + 1) struct i915_pmu_sample sample[I915_ENGINE_SAMPLE_MAX]; - /** - * @busy_stats: Has enablement of engine stats tracking been - * requested. - */ - bool busy_stats; - /** - * @disable_busy_stats: Work item for busy stats disabling. - * - * Same as with @enable_busy_stats action, with the difference - * that we delay it in case there are rapid enable-disable - * actions, which can happen during tool startup (like perf - * stat). - */ - struct delayed_work disable_busy_stats; } pmu; /* From 4c83f0a788ccf58864f781585d8ae7c7e6a7e07d Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Tue, 13 Feb 2018 09:57:46 +0000 Subject: [PATCH 09/18] drm/i915/pmu: Fix sleep under atomic in RC6 readout We are not allowed to call intel_runtime_pm_get from the PMU counter read callback since the former can sleep, and the latter is running under IRQ context. To workaround this, we record the last known RC6 and while runtime suspended estimate its increase by querying the runtime PM core timestamps. Downside of this approach is that we can temporarily lose a chunk of RC6 time, from the last PMU read-out to runtime suspend entry, but that will eventually catch up, once device comes back online and in the presence of PMU queries. Also, we have to be careful not to overshoot the RC6 estimate, so once resumed after a period of approximation, we only update the counter once it catches up. With the observation that RC6 is increasing while the device is suspended, this should not pose a problem and can only cause slight inaccuracies due clock base differences. v2: Simplify by estimating on top of PM core counters. (Imre) Signed-off-by: Tvrtko Ursulin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104943 Fixes: 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics") Testcase: igt/perf_pmu/rc6-runtime-pm Cc: Tvrtko Ursulin Cc: Chris Wilson Cc: Imre Deak Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: David Airlie Cc: intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Reviewed-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20180206183311.17924-1-tvrtko.ursulin@linux.intel.com (cherry picked from commit 1fe699e30113ed6f6e853ff44710d256072ea627) Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20180213095747.2424-3-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/i915/i915_pmu.c | 93 +++++++++++++++++++++++++++------ drivers/gpu/drm/i915/i915_pmu.h | 6 +++ 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 337eaa6ede52..e13859aaa2a3 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -409,7 +409,81 @@ static int i915_pmu_event_init(struct perf_event *event) return 0; } -static u64 __i915_pmu_event_read(struct perf_event *event) +static u64 get_rc6(struct drm_i915_private *i915, bool locked) +{ + unsigned long flags; + u64 val; + + if (intel_runtime_pm_get_if_in_use(i915)) { + val = intel_rc6_residency_ns(i915, IS_VALLEYVIEW(i915) ? + VLV_GT_RENDER_RC6 : + GEN6_GT_GFX_RC6); + + if (HAS_RC6p(i915)) + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6p); + + if (HAS_RC6pp(i915)) + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6pp); + + intel_runtime_pm_put(i915); + + /* + * If we are coming back from being runtime suspended we must + * be careful not to report a larger value than returned + * previously. + */ + + if (!locked) + spin_lock_irqsave(&i915->pmu.lock, flags); + + if (val >= i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) { + i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = 0; + i915->pmu.sample[__I915_SAMPLE_RC6].cur = val; + } else { + val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur; + } + + if (!locked) + spin_unlock_irqrestore(&i915->pmu.lock, flags); + } else { + struct pci_dev *pdev = i915->drm.pdev; + struct device *kdev = &pdev->dev; + unsigned long flags2; + + /* + * We are runtime suspended. + * + * Report the delta from when the device was suspended to now, + * on top of the last known real value, as the approximated RC6 + * counter value. + */ + if (!locked) + spin_lock_irqsave(&i915->pmu.lock, flags); + + spin_lock_irqsave(&kdev->power.lock, flags2); + + if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) + i915->pmu.suspended_jiffies_last = + kdev->power.suspended_jiffies; + + val = kdev->power.suspended_jiffies - + i915->pmu.suspended_jiffies_last; + val += jiffies - kdev->power.accounting_timestamp; + + spin_unlock_irqrestore(&kdev->power.lock, flags2); + + val = jiffies_to_nsecs(val); + val += i915->pmu.sample[__I915_SAMPLE_RC6].cur; + i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val; + + if (!locked) + spin_unlock_irqrestore(&i915->pmu.lock, flags); + } + + return val; +} + +static u64 __i915_pmu_event_read(struct perf_event *event, bool locked) { struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); @@ -447,18 +521,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event) val = count_interrupts(i915); break; case I915_PMU_RC6_RESIDENCY: - intel_runtime_pm_get(i915); - val = intel_rc6_residency_ns(i915, - IS_VALLEYVIEW(i915) ? - VLV_GT_RENDER_RC6 : - GEN6_GT_GFX_RC6); - if (HAS_RC6p(i915)) - val += intel_rc6_residency_ns(i915, - GEN6_GT_GFX_RC6p); - if (HAS_RC6pp(i915)) - val += intel_rc6_residency_ns(i915, - GEN6_GT_GFX_RC6pp); - intel_runtime_pm_put(i915); + val = get_rc6(i915, locked); break; } } @@ -473,7 +536,7 @@ static void i915_pmu_event_read(struct perf_event *event) again: prev = local64_read(&hwc->prev_count); - new = __i915_pmu_event_read(event); + new = __i915_pmu_event_read(event, false); if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev) goto again; @@ -528,7 +591,7 @@ static void i915_pmu_enable(struct perf_event *event) * for all listeners. Even when the event was already enabled and has * an existing non-zero value. */ - local64_set(&event->hw.prev_count, __i915_pmu_event_read(event)); + local64_set(&event->hw.prev_count, __i915_pmu_event_read(event, true)); spin_unlock_irqrestore(&i915->pmu.lock, flags); } diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h index 40c154d13565..bb62df15afa4 100644 --- a/drivers/gpu/drm/i915/i915_pmu.h +++ b/drivers/gpu/drm/i915/i915_pmu.h @@ -27,6 +27,8 @@ enum { __I915_SAMPLE_FREQ_ACT = 0, __I915_SAMPLE_FREQ_REQ, + __I915_SAMPLE_RC6, + __I915_SAMPLE_RC6_ESTIMATED, __I915_NUM_PMU_SAMPLERS }; @@ -94,6 +96,10 @@ struct i915_pmu { * struct intel_engine_cs. */ struct i915_pmu_sample sample[__I915_NUM_PMU_SAMPLERS]; + /** + * @suspended_jiffies_last: Cached suspend time from PM core. + */ + unsigned long suspended_jiffies_last; }; #ifdef CONFIG_PERF_EVENTS From 4b8b41d15d9db54703958fbd2928a2fd319563f6 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 13 Feb 2018 09:57:47 +0000 Subject: [PATCH 10/18] drm/i915/pmu: Fix building without CONFIG_PM As we peek inside struct device to query members guarded by CONFIG_PM, so must be the code. Reported-by: kbuild test robot Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180207160428.17015-1-chris@chris-wilson.co.uk (cherry picked from commit 05273c950a3c93c5f96be8807eaf24f2cc9f1c1e) Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20180213095747.2424-4-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/i915/i915_pmu.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index e13859aaa2a3..0e9b98c32b62 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -409,22 +409,32 @@ static int i915_pmu_event_init(struct perf_event *event) return 0; } +static u64 __get_rc6(struct drm_i915_private *i915) +{ + u64 val; + + val = intel_rc6_residency_ns(i915, + IS_VALLEYVIEW(i915) ? + VLV_GT_RENDER_RC6 : + GEN6_GT_GFX_RC6); + + if (HAS_RC6p(i915)) + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6p); + + if (HAS_RC6pp(i915)) + val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6pp); + + return val; +} + static u64 get_rc6(struct drm_i915_private *i915, bool locked) { +#if IS_ENABLED(CONFIG_PM) unsigned long flags; u64 val; if (intel_runtime_pm_get_if_in_use(i915)) { - val = intel_rc6_residency_ns(i915, IS_VALLEYVIEW(i915) ? - VLV_GT_RENDER_RC6 : - GEN6_GT_GFX_RC6); - - if (HAS_RC6p(i915)) - val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6p); - - if (HAS_RC6pp(i915)) - val += intel_rc6_residency_ns(i915, GEN6_GT_GFX_RC6pp); - + val = __get_rc6(i915); intel_runtime_pm_put(i915); /* @@ -481,6 +491,9 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) } return val; +#else + return __get_rc6(i915); +#endif } static u64 __i915_pmu_event_read(struct perf_event *event, bool locked) From 37ad4e68783088ed61493f54194cfccd3c87ab35 Mon Sep 17 00:00:00 2001 From: Weinan Li Date: Fri, 9 Feb 2018 16:01:34 +0800 Subject: [PATCH 11/18] drm/i915/gvt: add 0xe4f0 into gen9 render list Guest may set this register on KBL platform, it can impact hardware behavior, so add it into the gen9 render list. Otherwise gpu hang issue may happen during different vgpu switch. v2: separate it from patch set. Cc: Zhi Wang Cc: Zhenyu Wang Signed-off-by: Weinan Li Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/mmio_context.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c index 73ad6e90e49d..256f1bb522b7 100644 --- a/drivers/gpu/drm/i915/gvt/mmio_context.c +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c @@ -118,6 +118,7 @@ static struct engine_mmio gen9_engine_mmio_list[] __cacheline_aligned = { {RCS, HALF_SLICE_CHICKEN3, 0xffff, true}, /* 0xe184 */ {RCS, GEN9_HALF_SLICE_CHICKEN5, 0xffff, true}, /* 0xe188 */ {RCS, GEN9_HALF_SLICE_CHICKEN7, 0xffff, true}, /* 0xe194 */ + {RCS, GEN8_ROW_CHICKEN, 0xffff, true}, /* 0xe4f0 */ {RCS, TRVATTL3PTRDW(0), 0, false}, /* 0x4de0 */ {RCS, TRVATTL3PTRDW(1), 0, false}, /* 0x4de4 */ {RCS, TRNULLDETCT, 0, false}, /* 0x4de8 */ From a26ca6ad4c4aa4afcbfe4c46c33ad98859736245 Mon Sep 17 00:00:00 2001 From: Tina Zhang Date: Sun, 11 Feb 2018 14:59:19 +0800 Subject: [PATCH 12/18] drm/i915/gvt: Support BAR0 8-byte reads/writes GGTT is in BAR0 with 8 bytes aligned. With a qemu patch (commit: 38d49e8c1523d97d2191190d3f7b4ce7a0ab5aa3), VFIO can use 8-byte reads/ writes to access it. This patch is to support the 8-byte GGTT reads/writes. Ideally, we would like to support 8-byte reads/writes for the total BAR0. But it needs more work for handling 8-byte MMIO reads/writes. This patch can fix the issue caused by partial updating GGTT entry, during guest booting up. v3: - Use intel_vgpu_get_bar_gpa() stead. (Zhenyu) - Include all the GGTT checking logic in gtt_entry(). (Zhenyu) v2: - Limit to GGTT entry. (Zhenyu) Signed-off-by: Tina Zhang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/kvmgt.c | 51 ++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 909499b73d03..021f722e2481 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -733,6 +733,25 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf, return ret == 0 ? count : ret; } +static bool gtt_entry(struct mdev_device *mdev, loff_t *ppos) +{ + struct intel_vgpu *vgpu = mdev_get_drvdata(mdev); + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + struct intel_gvt *gvt = vgpu->gvt; + int offset; + + /* Only allow MMIO GGTT entry access */ + if (index != PCI_BASE_ADDRESS_0) + return false; + + offset = (u64)(*ppos & VFIO_PCI_OFFSET_MASK) - + intel_vgpu_get_bar_gpa(vgpu, PCI_BASE_ADDRESS_0); + + return (offset >= gvt->device_info.gtt_start_offset && + offset < gvt->device_info.gtt_start_offset + gvt_ggtt_sz(gvt)) ? + true : false; +} + static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, size_t count, loff_t *ppos) { @@ -742,7 +761,21 @@ static ssize_t intel_vgpu_read(struct mdev_device *mdev, char __user *buf, while (count) { size_t filled; - if (count >= 4 && !(*ppos % 4)) { + /* Only support GGTT entry 8 bytes read */ + if (count >= 8 && !(*ppos % 8) && + gtt_entry(mdev, ppos)) { + u64 val; + + ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ppos, false); + if (ret <= 0) + goto read_err; + + if (copy_to_user(buf, &val, sizeof(val))) + goto read_err; + + filled = 8; + } else if (count >= 4 && !(*ppos % 4)) { u32 val; ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), @@ -802,7 +835,21 @@ static ssize_t intel_vgpu_write(struct mdev_device *mdev, while (count) { size_t filled; - if (count >= 4 && !(*ppos % 4)) { + /* Only support GGTT entry 8 bytes write */ + if (count >= 8 && !(*ppos % 8) && + gtt_entry(mdev, ppos)) { + u64 val; + + if (copy_from_user(&val, buf, sizeof(val))) + goto write_err; + + ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val), + ppos, true); + if (ret <= 0) + goto write_err; + + filled = 8; + } else if (count >= 4 && !(*ppos % 4)) { u32 val; if (copy_from_user(&val, buf, sizeof(val))) From 3cc7644e4af179e79153b1fd60f9dd937ee32684 Mon Sep 17 00:00:00 2001 From: Weinan Li Date: Mon, 12 Feb 2018 15:28:42 +0800 Subject: [PATCH 13/18] drm/i915/gvt: fix one typo of render_mmio trace Fix one typo of render_mmio trace, exchange the mmio value of old and new. Signed-off-by: Weinan Li Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/trace.h b/drivers/gpu/drm/i915/gvt/trace.h index 7a2511538f34..736bd2bc5127 100644 --- a/drivers/gpu/drm/i915/gvt/trace.h +++ b/drivers/gpu/drm/i915/gvt/trace.h @@ -333,7 +333,7 @@ TRACE_EVENT(render_mmio, TP_PROTO(int old_id, int new_id, char *action, unsigned int reg, unsigned int old_val, unsigned int new_val), - TP_ARGS(old_id, new_id, action, reg, new_val, old_val), + TP_ARGS(old_id, new_id, action, reg, old_val, new_val), TP_STRUCT__entry( __field(int, old_id) From 405cacc947f7b58969b2a8ab1568c2d98b245308 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 20 Dec 2017 11:50:17 +0100 Subject: [PATCH 14/18] drm/i915/vlv: Add cdclk workaround for DSI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image shifted aprox. 20% to the left (with wraparound) and sometimes also wrong colors, showing that the panel controller is starting with sampling the datastream somewhere mid-line. This happens after the first blanking and re-init of the panel. After looking at drm.debug output I noticed that initially we inherit the cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667 KHz, which turns out to be the cause of this problem, a quick hack to hard code the cdclk to 333333 KHz makes the problem go away. I've tested this on various Bay Trail devices, to make sure this not does cause regressions on other devices and the higher cdclk does not cause any problems on the following devices: -GP-electronic T701 1024x600 333333 KHz cdclk after this patch -PEAQ C1010 1920x1200 333333 KHz cdclk after this patch -PoV mobii-wintab-800w 800x1280 333333 KHz cdclk after this patch -Asus Transformer-T100TA 1368x768 320000 KHz cdclk after this patch Also interesting wrt this is the comment in vlv_calc_cdclk about the existing workaround to avoid 200 Mhz as clock because that causes issues in some cases. This commit extends the "do not use 200 Mhz" workaround with an extra check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI panel is active. Changes in v2: -Change the commit message and the code comment to not treat the GOP as a reference, the GOP should not be treated as a reference Acked-by: Ville Syrjälä Signed-off-by: Hans de Goede Link: https://patchwork.freedesktop.org/patch/msgid/20171220105017.11259-1-hdegoede@redhat.com (cherry picked from commit c8dae55a8ced625038d52d26e48273707fab2688) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index 5dc118f26b51..1704c8897afd 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1952,6 +1952,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk); + /* + * On Valleyview some DSI panels lose (v|h)sync when the clock is lower + * than 320000KHz. + */ + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) && + IS_VALLEYVIEW(dev_priv)) + min_cdclk = max(320000, min_cdclk); + if (min_cdclk > dev_priv->max_cdclk_freq) { DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n", min_cdclk, dev_priv->max_cdclk_freq); From 7928e9bb09dc7f108a1a2b589ef1c7b86843569c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Feb 2018 09:21:49 +0100 Subject: [PATCH 15/18] drm/i915: Add intel_bios_cleanup() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an intel_bios_cleanup() function to act as counterpart of intel_bios_init() and move the cleanup of vbt related resources there, putting it in the same file as the allocation. Changed in v2: -While touching the code anyways, remove the unnecessary: if (dev_priv->vbt.child_dev) done before kfree(dev_priv->vbt.child_dev) Reviewed-by: Ville Syrjälä Signed-off-by: Hans de Goede Link: https://patchwork.freedesktop.org/patch/msgid/20180214082151.25015-1-hdegoede@redhat.com (cherry picked from commit 785f076b3ba781804f2b22b347b4431e3efb0ab3) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_drv.c | 14 +------------- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 173d0095e3b2..2f5209de0391 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1433,19 +1433,7 @@ void i915_driver_unload(struct drm_device *dev) intel_modeset_cleanup(dev); - /* - * free the memory space allocated for the child device - * config parsed from VBT - */ - if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) { - kfree(dev_priv->vbt.child_dev); - dev_priv->vbt.child_dev = NULL; - dev_priv->vbt.child_dev_num = 0; - } - kfree(dev_priv->vbt.sdvo_lvds_vbt_mode); - dev_priv->vbt.sdvo_lvds_vbt_mode = NULL; - kfree(dev_priv->vbt.lfp_lvds_vbt_mode); - dev_priv->vbt.lfp_lvds_vbt_mode = NULL; + intel_bios_cleanup(dev_priv); vga_switcheroo_unregister_client(pdev); vga_client_register(pdev, NULL, NULL, NULL); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a42deebedb0f..d2fc519bc592 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3657,6 +3657,7 @@ extern void intel_i2c_reset(struct drm_i915_private *dev_priv); /* intel_bios.c */ void intel_bios_init(struct drm_i915_private *dev_priv); +void intel_bios_cleanup(struct drm_i915_private *dev_priv); bool intel_bios_is_valid_vbt(const void *buf, size_t size); bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv); bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin); diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index f7f771749e48..57db816f962b 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1588,6 +1588,21 @@ void intel_bios_init(struct drm_i915_private *dev_priv) pci_unmap_rom(pdev, bios); } +/** + * intel_bios_cleanup - Free any resources allocated by intel_bios_init() + * @dev_priv: i915 device instance + */ +void intel_bios_cleanup(struct drm_i915_private *dev_priv) +{ + kfree(dev_priv->vbt.child_dev); + dev_priv->vbt.child_dev = NULL; + dev_priv->vbt.child_dev_num = 0; + kfree(dev_priv->vbt.sdvo_lvds_vbt_mode); + dev_priv->vbt.sdvo_lvds_vbt_mode = NULL; + kfree(dev_priv->vbt.lfp_lvds_vbt_mode); + dev_priv->vbt.lfp_lvds_vbt_mode = NULL; +} + /** * intel_bios_is_tv_present - is integrated TV present in VBT * @dev_priv: i915 device instance From ed0545a7fbb5241a27f45a084dd71522cdaea5b9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Feb 2018 09:21:50 +0100 Subject: [PATCH 16/18] drm/i915: Free memdup-ed DSI VBT data structures on driver_unload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make intel_bios_cleanup function free the DSI VBT data structures which are memdup-ed by parse_mipi_config() and parse_mipi_sequence(). Reviewed-by: Ville Syrjälä Signed-off-by: Hans de Goede Link: https://patchwork.freedesktop.org/patch/msgid/20180214082151.25015-2-hdegoede@redhat.com (cherry picked from commit e1b86c85f6c2029c31dba99823b6f3d9e15eaacd) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_bios.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 57db816f962b..9a9b62c93889 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1601,6 +1601,12 @@ void intel_bios_cleanup(struct drm_i915_private *dev_priv) dev_priv->vbt.sdvo_lvds_vbt_mode = NULL; kfree(dev_priv->vbt.lfp_lvds_vbt_mode); dev_priv->vbt.lfp_lvds_vbt_mode = NULL; + kfree(dev_priv->vbt.dsi.data); + dev_priv->vbt.dsi.data = NULL; + kfree(dev_priv->vbt.dsi.pps); + dev_priv->vbt.dsi.pps = NULL; + kfree(dev_priv->vbt.dsi.config); + dev_priv->vbt.dsi.config = NULL; } /** From ee622fe757f6de612dad0f01805eea815a5b3025 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Feb 2018 09:21:51 +0100 Subject: [PATCH 17/18] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far models of the Dell Venue 8 Pro, with a panel with MIPI panel index = 3, one of which has been kindly provided to me by Jan Brummer, where not working with the i915 driver, giving a black screen on the first modeset. The problem with at least these Dells is that their VBT defines a MIPI ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the reset in their INIT_OTP sequence, but the deassert must be done before calling intel_dsi_device_ready(), so that is too late. Simply doing the INIT_OTP sequence earlier is not enough to fix this, because the INIT_OTP sequence also sends various MIPI packets to the panel, which can only happen after calling intel_dsi_device_ready(). This commit fixes this by splitting the INIT_OTP sequence into everything before the first DSI packet and everything else, including the first DSI packet. The first part (everything before the first DSI packet) is then used as deassert sequence. Changed in v2: -Split the init OTP sequence into a deassert reset and the actual init OTP sequence, instead of calling it earlier and then having the first mipi_exec_send_packet() call call intel_dsi_device_ready(). Changes in v3: -Move the whole shebang to intel_bios.c Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82880 References: https://bugs.freedesktop.org/show_bug.cgi?id=101205 Cc: Jan-Michael Brummer Reported-by: Jan-Michael Brummer Tested-by: Hans de Goede Reviewed-by: Ville Syrjälä Acked-by: Jani Nikula Signed-off-by: Hans de Goede Link: https://patchwork.freedesktop.org/patch/msgid/20180214082151.25015-3-hdegoede@redhat.com (cherry picked from commit fb38e7ade9af4f3e96f5916c3f6f19bfc7d5f961) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_bios.c | 84 +++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d2fc519bc592..d307429a5ae0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1349,6 +1349,7 @@ struct intel_vbt_data { u32 size; u8 *data; const u8 *sequence[MIPI_SEQ_MAX]; + u8 *deassert_seq; /* Used by fixup_mipi_sequences() */ } dsi; int crt_ddc_pin; diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 9a9b62c93889..b49a2df44430 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int index, int total) return 0; } +/* + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence, + * skip all delay + gpio operands and stop at the first DSI packet op. + */ +static int get_init_otp_deassert_fragment_len(struct drm_i915_private *dev_priv) +{ + const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP]; + int index, len; + + if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1)) + return 0; + + /* index = 1 to skip sequence byte */ + for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) { + switch (data[index]) { + case MIPI_SEQ_ELEM_SEND_PKT: + return index == 1 ? 0 : index; + case MIPI_SEQ_ELEM_DELAY: + len = 5; /* 1 byte for operand + uint32 */ + break; + case MIPI_SEQ_ELEM_GPIO: + len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */ + break; + default: + return 0; + } + } + + return 0; +} + +/* + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence. + * The deassert must be done before calling intel_dsi_device_ready, so for + * these devices we split the init OTP sequence into a deassert sequence and + * the actual init OTP part. + */ +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv) +{ + u8 *init_otp; + int len; + + /* Limit this to VLV for now. */ + if (!IS_VALLEYVIEW(dev_priv)) + return; + + /* Limit this to v1 vid-mode sequences */ + if (dev_priv->vbt.dsi.config->is_cmd_mode || + dev_priv->vbt.dsi.seq_version != 1) + return; + + /* Only do this if there are otp and assert seqs and no deassert seq */ + if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] || + !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] || + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET]) + return; + + /* The deassert-sequence ends at the first DSI packet */ + len = get_init_otp_deassert_fragment_len(dev_priv); + if (!len) + return; + + DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n"); + + /* Copy the fragment, update seq byte and terminate it */ + init_otp = (u8 *)dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP]; + dev_priv->vbt.dsi.deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL); + if (!dev_priv->vbt.dsi.deassert_seq) + return; + dev_priv->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET; + dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END; + /* Use the copy for deassert */ + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] = + dev_priv->vbt.dsi.deassert_seq; + /* Replace the last byte of the fragment with init OTP seq byte */ + init_otp[len - 1] = MIPI_SEQ_INIT_OTP; + /* And make MIPI_MIPI_SEQ_INIT_OTP point to it */ + dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1; +} + static void parse_mipi_sequence(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv, dev_priv->vbt.dsi.size = seq_size; dev_priv->vbt.dsi.seq_version = sequence->version; + fixup_mipi_sequences(dev_priv); + DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n"); return; @@ -1607,6 +1689,8 @@ void intel_bios_cleanup(struct drm_i915_private *dev_priv) dev_priv->vbt.dsi.pps = NULL; kfree(dev_priv->vbt.dsi.config); dev_priv->vbt.dsi.config = NULL; + kfree(dev_priv->vbt.dsi.deassert_seq); + dev_priv->vbt.dsi.deassert_seq = NULL; } /** From 92256269893e96e5f9e8ac6dd882a0bef63fcea7 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Wed, 7 Feb 2018 18:40:27 +0100 Subject: [PATCH 18/18] drm/nouveau: Make clock gate support conditional The recently introduced clock gate support breaks on Tegra chips because no thermal support is enabled for those devices. Conditionalize the code on the existence of thermal support to fix this. Fixes: b138eca661cc ("drm/nouveau: Add support for basic clockgating on Kepler1") Cc: Martin Peres Cc: Lyude Paul Signed-off-by: Thierry Reding Reviewed-by: Lyude Paul Signed-off-by: Ben Skeggs --- drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c index bf62303571b3..3695cde669f8 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c @@ -301,7 +301,7 @@ nvkm_therm_attr_set(struct nvkm_therm *therm, void nvkm_therm_clkgate_enable(struct nvkm_therm *therm) { - if (!therm->func->clkgate_enable || !therm->clkgating_enabled) + if (!therm || !therm->func->clkgate_enable || !therm->clkgating_enabled) return; nvkm_debug(&therm->subdev, @@ -312,7 +312,7 @@ nvkm_therm_clkgate_enable(struct nvkm_therm *therm) void nvkm_therm_clkgate_fini(struct nvkm_therm *therm, bool suspend) { - if (!therm->func->clkgate_fini || !therm->clkgating_enabled) + if (!therm || !therm->func->clkgate_fini || !therm->clkgating_enabled) return; nvkm_debug(&therm->subdev, @@ -395,7 +395,7 @@ void nvkm_therm_clkgate_init(struct nvkm_therm *therm, const struct nvkm_therm_clkgate_pack *p) { - if (!therm->func->clkgate_init || !therm->clkgating_enabled) + if (!therm || !therm->func->clkgate_init || !therm->clkgating_enabled) return; therm->func->clkgate_init(therm, p);