[PATCH 2/3] drm/i915: WARN if RPS adjustment is more than range
Kamble, Sagar A
sagar.a.kamble at intel.com
Fri Jan 13 17:43:47 UTC 2017
Hi Chris,
Details of the issue with GuC and trace that shows the adjustment overflow to negative values is given below.
Even when Host RPS has requested RP0, Up Threshold Interrupts are coming due to kernel bug.
With many such interrupts driver adjustment goes 1,2,4,8...,0x40000000,0x80000000(-2147483648) and then request goes down to Rpe Then ramp starts again which is impacting workload performance.
Patch at https://patchwork.freedesktop.org/patch/133084/ will mask interrupts correctly with GuC.
Configuration:
Rpe = 300MHz
Rp0 = 900MHz
Kernel = 4.10.0-rc3+
System = SKL GT3 RVP7
Scores:
Submission/RPS/glb_egypt_fixedtime score
Host/Host/165
GuC/Host/153
GuC/Fix at Rp0/165
GuC/SLPC/165
Ftrace Data: Analysis shows after reaching RP0, RP UP interrupts keep coming as they are unmasked due to kernel bug.
Due to multiple such interrupts (32) driver adjustment of frequency goes bad and sets frequency to min and from thereonwards ramp starts back again. Fix is to avoid the unnecessary interrupts that are leading to miscalculation of adjustments.
Ftrace snippet for reference:
GLBenchmark-1887 [003] .... 45.829860: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.835706: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.835812: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.835826: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.835829: gen6_pm_rps_work: 0 20 900 (printing client_boost, pm_iir, cur_freq)
kworker/3:1-148 [003] .... 45.835831: gen6_pm_rps_work: 4194304 54 (printing adj, cur_freq)
kworker/3:1-148 [003] .... 45.835832: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.835832: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.835832: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.835833: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.835833: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.835835: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.839068: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.842144: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.845141: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] dNh. 45.845968: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.845987: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.845991: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.845993: gen6_pm_rps_work: 8388608 54
kworker/3:1-148 [003] .... 45.845993: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.845993: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.845993: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.845997: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.845997: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.845999: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.847333: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.853224: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 45.856096: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.856117: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.856120: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.856121: gen6_pm_rps_work: 16777216 54
kworker/3:1-148 [003] .... 45.856121: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.856121: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.856121: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.856125: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.856126: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.856127: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.856377: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.859554: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.862424: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.864454: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.866296: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.866350: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.866357: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.866359: gen6_pm_rps_work: 33554432 54
kworker/3:1-148 [003] .... 45.866360: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.866360: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.866360: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.866366: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.866366: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.866369: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.870367: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.873350: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.876323: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.876406: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.876422: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.876426: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.876427: gen6_pm_rps_work: 67108864 54
kworker/3:1-148 [003] .... 45.876427: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.876427: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.876427: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.876428: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.876429: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.876431: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.878445: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.884232: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 45.886632: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.886679: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.886687: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.886689: gen6_pm_rps_work: 134217728 54
kworker/3:1-148 [003] .... 45.886690: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.886691: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.886691: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.886700: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.886700: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.886704: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.896136: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.896640: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.896674: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.896676: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.896677: gen6_pm_rps_work: 268435456 54
kworker/3:1-148 [003] .... 45.896677: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.896678: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.896678: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.896680: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.896680: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.896681: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] d.h. 45.906758: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.906791: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.906795: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.906796: gen6_pm_rps_work: 536870912 54
kworker/3:1-148 [003] .... 45.906797: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.906797: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.906797: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.906805: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.906805: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.906808: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.910951: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 45.916810: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.916823: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.916826: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.916827: gen6_pm_rps_work: 1073741824 54
kworker/3:1-148 [003] .... 45.916827: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.916828: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.916828: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.916829: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.916829: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.916830: intel_gpu_freq_change: new_freq=900
GLBenchmark-1887 [003] .... 45.926438: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 45.926809: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.926823: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.926829: gen6_pm_rps_work: 0 20 900
kworker/3:1-148 [003] .... 45.926830: gen6_pm_rps_work: -2147483648 18 <---------------------- This is where rollover to negative happens
kworker/3:1-148 [003] .... 45.926830: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.926830: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.926830: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.926831: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 45.926836: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.926837: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.926838: intel_gpu_freq_change: new_freq=300
GLBenchmark-1887 [003] .... 45.927746: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.938185: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.943129: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.943551: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.943566: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.943568: gen6_pm_rps_work: 0 20 300
kworker/3:1-148 [003] .... 45.943569: gen6_pm_rps_work: 1 19
kworker/3:1-148 [003] .... 45.943570: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.943570: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.943570: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.943571: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 45.943572: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.943572: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.943574: intel_gpu_freq_change: new_freq=317
GLBenchmark-1887 [003] .... 45.947825: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.952229: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 45.960215: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:0-29 [003] .... 45.960228: gen6_pm_rps_work <-process_one_work
kworker/3:0-29 [003] .... 45.960231: gen6_pm_rps_work: 0 20 317
kworker/3:0-29 [003] .... 45.960232: gen6_pm_rps_work: 2 21
kworker/3:0-29 [003] .... 45.960232: intel_set_rps <-gen6_pm_rps_work
kworker/3:0-29 [003] .... 45.960232: intel_set_rps.part.35 <-intel_set_rps
kworker/3:0-29 [003] .... 45.960232: gen6_set_rps <-intel_set_rps.part.35
kworker/3:0-29 [003] .... 45.960232: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:0-29 [003] .... 45.960235: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:0-29 [003] .... 45.960236: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:0-29 [003] .... 45.960237: intel_gpu_freq_change: new_freq=350
GLBenchmark-1887 [003] .... 45.960706: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.965016: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.969320: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.973807: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.973845: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.973849: gen6_pm_rps_work: 0 20 350
kworker/3:1-148 [003] .... 45.973850: gen6_pm_rps_work: 1 22
kworker/3:1-148 [003] .... 45.973850: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.973850: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.973851: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.973851: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 45.973854: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.973855: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.973857: intel_gpu_freq_change: new_freq=367
GLBenchmark-1887 [003] .... 45.977368: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.981260: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 45.985129: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 45.987310: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 45.987342: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 45.987344: gen6_pm_rps_work: 0 20 367
kworker/3:1-148 [003] .... 45.987345: gen6_pm_rps_work: 2 24
kworker/3:1-148 [003] .... 45.987345: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 45.987345: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 45.987346: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 45.987346: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 45.987348: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 45.987349: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 45.987350: intel_gpu_freq_change: new_freq=400
GLBenchmark-1887 [003] .... 45.992560: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] d.h. 46.000630: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 46.000654: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 46.000657: gen6_pm_rps_work: 0 20 400
kworker/3:1-148 [003] .... 46.000658: gen6_pm_rps_work: 4 28
kworker/3:1-148 [003] .... 46.000658: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 46.000658: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 46.000658: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 46.000658: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 46.000662: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 46.000662: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 46.000663: intel_gpu_freq_change: new_freq=467
GLBenchmark-1887 [003] .... 46.000955: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 46.007412: gen6_rps_boost <-i915_gem_object_wait_fence
GLBenchmark-1887 [003] .... 46.011889: gen6_rps_boost <-i915_gem_object_wait_fence
<idle>-0 [003] d.h. 46.014216: gen6_rps_irq_handler <-gen8_gt_irq_handler
kworker/3:1-148 [003] .... 46.014229: gen6_pm_rps_work <-process_one_work
kworker/3:1-148 [003] .... 46.014232: gen6_pm_rps_work: 0 20 467
kworker/3:1-148 [003] .... 46.014232: gen6_pm_rps_work: 8 36
kworker/3:1-148 [003] .... 46.014233: intel_set_rps <-gen6_pm_rps_work
kworker/3:1-148 [003] .... 46.014233: intel_set_rps.part.35 <-intel_set_rps
kworker/3:1-148 [003] .... 46.014233: gen6_set_rps <-intel_set_rps.part.35
kworker/3:1-148 [003] .... 46.014233: gen6_set_rps_thresholds <-gen6_set_rps
kworker/3:1-148 [003] .... 46.014234: gen6_rps_pm_mask <-gen6_set_rps
kworker/3:1-148 [003] .... 46.014235: gen6_sanitize_rps_pm_mask <-gen6_rps_pm_mask
kworker/3:1-148 [003] .... 46.014236: intel_gpu_freq_change: new_freq=600
Thanks
Sagar
-----Original Message-----
From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
Sent: Friday, January 13, 2017 8:33 PM
To: Kamble, Sagar A <sagar.a.kamble at intel.com>
Cc: Intel-gfx-trybot at lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: WARN if RPS adjustment is more than range
On Fri, Jan 13, 2017 at 08:21:45PM +0530, Kamble, Sagar A wrote:
>
>
> On 1/13/2017 4:26 PM, Chris Wilson wrote:
> >On Fri, Jan 13, 2017 at 04:12:18PM +0530, Kamble, Sagar A wrote:
> >>
> >>On 1/13/2017 3:55 PM, Chris Wilson wrote:
> >>>On Fri, Jan 13, 2017 at 03:46:11PM +0530, Sagar Arun Kamble wrote:
> >>>>Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >>>>Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> >>>>---
> >>>> drivers/gpu/drm/i915/i915_irq.c | 2 ++
> >>>> 1 file changed, 2 insertions(+)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >>>>b/drivers/gpu/drm/i915/i915_irq.c index ce5663d..b85bef9 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_irq.c
> >>>>+++ b/drivers/gpu/drm/i915/i915_irq.c
> >>>>@@ -1195,6 +1195,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
> >>>> adj = 0;
> >>>> }
> >>>>+ WARN_ON(abs(adj) > (max-min));
> >>>Do we actually care? The adjustment is just an exponential function
> >>>that will be clamped (and then when it is finally clamped will become zero).
> >>>Before it is clamped, it is quite permissible that the last adj
> >>>will be larger than the entire range.
> >>>-Chris
> >>Oh. right. I coded this incorrectly. Will it be okay if we WARN on
> >>-ve adjustment for Up interrupts and +ve for Down interrupts.
> >The code has to assume that the pm_iir may contain multiple
> >interrupts (i.e. both up, down, timeout) because we coalesce the hw
> >interrupt into the worker. I fear you will just end up repeating the
> >if-else chain, at which point you could just review the code above.
> >-Chris
> Ok. I am thinking of following options. Could you suggest which one to
> go ahead.
> 1. Adjustment rollover to negative value due to overflow of adj on
> consecutive UP interrupts can be avoided if Up interrupts are masked
> properly. Masking with GUC is fixed in earlier patch.
How can we overflow? Hmm, we are generating interrupts that we don't care for. That's the actual bug, right?
> Should we just have that patch and ignore the WARN_ON or bounds
> check altogether.
> 2. Add WARN_ON in individual condition blocks for Up/Down interrupts
> on invalid adjustment.
> 3. Without WARN_ON on invalid adjustment just reset adjustment to zero
Something like:
if (WARN_ON_ONCE(slpc && (pm_iir & INVALID_SLPC_MASK)))
pm_iir &= ~INVALID_SLPC_MASK
near the start?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx-trybot
mailing list