We can use engine busy stats instead of the sampling timer for better accuracy. By doing this we replace the stohastic sampling with busyness metric derived directly from engine activity. This is context switch interrupt driven, so as accurate as we can get from software tracking. As a secondary benefit, we can also not run the sampling timer in cases only busyness metric is enabled. v2: Rebase. v3: * Rebase, comments. * Leave engine busyness controls out of workers. v4: Checkpatch cleanup. v5: Added comment to pmu_needs_timer change. v6: * Rebase. * Fix style of some comments. (Chris Wilson) v7: Rebase and commit message update. (Chris Wilson) v8: Add delayed stats disabling to improve accuracy in face of CPU hotplug events. v9: Rebase. v10: Rebase - i915_modparams.enable_execlists removal.
Unified · Split
| @@ -90,6 +90,11 @@ static unsigned int event_enabled_bit(struct perf_event *event) · 89 unmodified lines | |||
| 90 | return config_enabled_bit(event->attr.config); | 90 | return config_enabled_bit(event->attr.config); |
| 91 | } | 91 | } |
| 92 | 92 | ||
| 93 | static bool supports_busy_stats(struct drm_i915_private *i915) | ||
| 94 | { | ||
| 95 | return INTEL_GEN(i915) >= 8; | ||
| 96 | } | ||
| 97 | |||
| 93 | static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active) | 98 | static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active) |
| 94 | { | 99 | { |
| 95 | u64 enable; | 100 | u64 enable; |
| @@ -115,6 +120,12 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active) · 19 unmodified lines | |||
| 115 | */ | 120 | */ |
| 116 | if (!gpu_active) | 121 | if (!gpu_active) |
| 117 | enable &= ~ENGINE_SAMPLE_MASK; | 122 | enable &= ~ENGINE_SAMPLE_MASK; |
| 123 | /* | ||
| 124 | * Also there is software busyness tracking available we do not | ||
| 125 | * need the timer for I915_SAMPLE_BUSY counter. | ||
| 126 | */ | ||
| 127 | else if (supports_busy_stats(i915)) | ||
| 128 | enable &= ~BIT(I915_SAMPLE_BUSY); | ||
| 118 | 129 | ||
| 119 | /* | 130 | /* |
| 120 | * If some bits remain it means we need the sampling timer running. | 131 | * If some bits remain it means we need the sampling timer running. |
| @@ -363,6 +374,9 @@ static u64 __i915_pmu_event_read(struct perf_event *event) · 242 unmodified lines | |||
| 363 | 374 | ||
| 364 | if (WARN_ON_ONCE(!engine)) { | 375 | if (WARN_ON_ONCE(!engine)) { |
| 365 | /* Do nothing */ | 376 | /* Do nothing */ |
| 377 | } else if (sample == I915_SAMPLE_BUSY && | ||
| 378 | engine->pmu.busy_stats) { | ||
| 379 | val = ktime_to_ns(intel_engine_get_busy_time(engine)); | ||
| 366 | } else { | 380 | } else { |
| 367 | val = engine->pmu.sample[sample].cur; | 381 | val = engine->pmu.sample[sample].cur; |
| 368 | } | 382 | } |
| @@ -399,6 +413,12 @@ again: · 30 unmodified lines | |||
| 399 | local64_add(new - prev, &event->count); | 413 | local64_add(new - prev, &event->count); |
| 400 | } | 414 | } |
| 401 | 415 | ||
| 416 | static bool engine_needs_busy_stats(struct intel_engine_cs *engine) | ||
| 417 | { | ||
| 418 | return supports_busy_stats(engine->i915) && | ||
| 419 | (engine->pmu.enable & BIT(I915_SAMPLE_BUSY)); | ||
| 420 | } | ||
| 421 | |||
| 402 | static void i915_pmu_enable(struct perf_event *event) | 422 | static void i915_pmu_enable(struct perf_event *event) |
| 403 | { | 423 | { |
| 404 | struct drm_i915_private *i915 = | 424 | struct drm_i915_private *i915 = |
| @@ -438,7 +458,21 @@ static void i915_pmu_enable(struct perf_event *event) · 33 unmodified lines | |||
| 438 | 458 | ||
| 439 | GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS); | 459 | GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS); |
| 440 | GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0); | 460 | GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0); |
| 441 | engine->pmu.enable_count[sample]++; | 461 | if (engine->pmu.enable_count[sample]++ == 0) { |
| 462 | /* | ||
| 463 | * Enable engine busy stats tracking if needed or | ||
| 464 | * alternatively cancel the scheduled disable. | ||
| 465 | * | ||
| 466 | * If the delayed disable was pending, cancel it and | ||
| 467 | * in this case do not enable since it already is. | ||
| 468 | */ | ||
| 469 | if (engine_needs_busy_stats(engine) && | ||
| 470 | !engine->pmu.busy_stats) { | ||
| 471 | engine->pmu.busy_stats = true; | ||
| 472 | if (!cancel_delayed_work(&engine->pmu.disable_busy_stats)) | ||
| 473 | intel_enable_engine_stats(engine); | ||
| 474 | } | ||
| 475 | } | ||
| 442 | } | 476 | } |
| 443 | 477 | ||
| 444 | /* | 478 | /* |
| @@ -451,6 +485,14 @@ static void i915_pmu_enable(struct perf_event *event) · 6 unmodified lines | |||
| 451 | spin_unlock_irqrestore(&i915->pmu.lock, flags); | 485 | spin_unlock_irqrestore(&i915->pmu.lock, flags); |
| 452 | } | 486 | } |
| 453 | 487 | ||
| 488 | static void __disable_busy_stats(struct work_struct *work) | ||
| 489 | { | ||
| 490 | struct intel_engine_cs *engine = | ||
| 491 | container_of(work, typeof(*engine), pmu.disable_busy_stats.work); | ||
| 492 | |||
| 493 | intel_disable_engine_stats(engine); | ||
| 494 | } | ||
| 495 | |||
| 454 | static void i915_pmu_disable(struct perf_event *event) | 496 | static void i915_pmu_disable(struct perf_event *event) |
| 455 | { | 497 | { |
| 456 | struct drm_i915_private *i915 = | 498 | struct drm_i915_private *i915 = |
| @@ -474,8 +516,26 @@ static void i915_pmu_disable(struct perf_event *event) · 17 unmodified lines | |||
| 474 | * Decrement the reference count and clear the enabled | 516 | * Decrement the reference count and clear the enabled |
| 475 | * bitmask when the last listener on an event goes away. | 517 | * bitmask when the last listener on an event goes away. |
| 476 | */ | 518 | */ |
| 477 | if (--engine->pmu.enable_count[sample] == 0) | 519 | if (--engine->pmu.enable_count[sample] == 0) { |
| 478 | engine->pmu.enable &= ~BIT(sample); | 520 | engine->pmu.enable &= ~BIT(sample); |
| 521 | if (!engine_needs_busy_stats(engine) && | ||
| 522 | engine->pmu.busy_stats) { | ||
| 523 | engine->pmu.busy_stats = false; | ||
| 524 | /* | ||
| 525 | * We request a delayed disable to handle the | ||
| 526 | * rapid on/off cycles on events, which can | ||
| 527 | * happen when tools like perf stat start, in a | ||
| 528 | * nicer way. | ||
| 529 | * | ||
| 530 | * In addition, this also helps with busy stats | ||
| 531 | * accuracy with background CPU offline/online | ||
| 532 | * migration events. | ||
| 533 | */ | ||
| 534 | queue_delayed_work(system_wq, | ||
| 535 | &engine->pmu.disable_busy_stats, | ||
| 536 | round_jiffies_up_relative(HZ)); | ||
| 537 | } | ||
| 538 | } | ||
| 479 | } | 539 | } |
| 480 | 540 | ||
| 481 | GEM_BUG_ON(bit >= I915_PMU_MASK_BITS); | 541 | GEM_BUG_ON(bit >= I915_PMU_MASK_BITS); |
| @@ -702,6 +762,8 @@ static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915) · 220 unmodified lines | |||
| 702 | 762 | ||
| 703 | void i915_pmu_register(struct drm_i915_private *i915) | 763 | void i915_pmu_register(struct drm_i915_private *i915) |
| 704 | { | 764 | { |
| 765 | struct intel_engine_cs *engine; | ||
| 766 | enum intel_engine_id id; | ||
| 705 | int ret; | 767 | int ret; |
| 706 | 768 | ||
| 707 | if (INTEL_GEN(i915) <= 2) { | 769 | if (INTEL_GEN(i915) <= 2) { |
| @@ -723,6 +785,10 @@ void i915_pmu_register(struct drm_i915_private *i915) · 15 unmodified lines | |||
| 723 | hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); | 785 | hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); |
| 724 | i915->pmu.timer.function = i915_sample; | 786 | i915->pmu.timer.function = i915_sample; |
| 725 | 787 | ||
| 788 | for_each_engine(engine, i915, id) | ||
| 789 | INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats, | ||
| 790 | __disable_busy_stats); | ||
| 791 | |||
| 726 | ret = perf_pmu_register(&i915->pmu.base, "i915", -1); | 792 | ret = perf_pmu_register(&i915->pmu.base, "i915", -1); |
| 727 | if (ret) | 793 | if (ret) |
| 728 | goto err; | 794 | goto err; |
| @@ -742,6 +808,9 @@ err: · 13 unmodified lines | |||
| 742 | 808 | ||
| 743 | void i915_pmu_unregister(struct drm_i915_private *i915) | 809 | void i915_pmu_unregister(struct drm_i915_private *i915) |
| 744 | { | 810 | { |
| 811 | struct intel_engine_cs *engine; | ||
| 812 | enum intel_engine_id id; | ||
| 813 | |||
| 745 | if (!i915->pmu.base.event_init) | 814 | if (!i915->pmu.base.event_init) |
| 746 | return; | 815 | return; |
| 747 | 816 | ||
| @@ -749,6 +818,11 @@ void i915_pmu_unregister(struct drm_i915_private *i915) · 1 unmodified line | |||
| 749 | 818 | ||
| 750 | hrtimer_cancel(&i915->pmu.timer); | 819 | hrtimer_cancel(&i915->pmu.timer); |
| 751 | 820 | ||
| 821 | for_each_engine(engine, i915, id) { | ||
| 822 | GEM_BUG_ON(engine->pmu.busy_stats); | ||
| 823 | flush_delayed_work(&engine->pmu.disable_busy_stats); | ||
| 824 | } | ||
| 825 | |||
| 752 | i915_pmu_unregister_cpuhp_state(i915); | 826 | i915_pmu_unregister_cpuhp_state(i915); |
| 753 | 827 | ||
| 754 | perf_pmu_unregister(&i915->pmu.base); | 828 | perf_pmu_unregister(&i915->pmu.base); |
| @@ -359,6 +359,20 @@ struct intel_engine_cs { · 358 unmodified lines | |||
| 359 | * Our internal timer stores the current counters in this field. | 359 | * Our internal timer stores the current counters in this field. |
| 360 | */ | 360 | */ |
| 361 | struct i915_pmu_sample sample[I915_ENGINE_SAMPLE_MAX]; | 361 | struct i915_pmu_sample sample[I915_ENGINE_SAMPLE_MAX]; |
| 362 | /** | ||
| 363 | * @busy_stats: Has enablement of engine stats tracking been | ||
| 364 | * requested. | ||
| 365 | */ | ||
| 366 | bool busy_stats; | ||
| 367 | /** | ||
| 368 | * @disable_busy_stats: Work item for busy stats disabling. | ||
| 369 | * | ||
| 370 | * Same as with @enable_busy_stats action, with the difference | ||
| 371 | * that we delay it in case there are rapid enable-disable | ||
| 372 | * actions, which can happen during tool startup (like perf | ||
| 373 | * stat). | ||
| 374 | */ | ||
| 375 | struct delayed_work disable_busy_stats; | ||
| 362 | } pmu; | 376 | } pmu; |
| 363 | 377 | ||
| 364 | /* | 378 | /* |