Re: [PATCH v3] drm/i915/pmu: Clear the previous sample value when parking

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

On 11/23/2017 3:07 PM, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-11-23 09:27:40)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> When turning off the engines, and the pmu sampling, clear the previous
>> value as the current measurement should be 0.
>>
>> v2: Use a for-loop
>> v3:
>>   * Move clearing to timer self-dis-arm to avoid race with parking.
>>   * Clear frequency samples as well.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> That makes sense (wrt to the sampling),
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> (However, for freq.sample we should be using idle-freq not 0, and init
> sample.prev to idle-freq?)
>
> Hmm, actual-freq depends on gt.awake, but we are not including that in
> pmu_needs_timer.
frequency_sample is reading cur_freq as SAMPLE_FREQ_ACT if GT is not 
awake, else it reads CAGF.
Are we sure CAGF will not fall below Rpn (I seem to have seen on SKL it 
go to 300MHz with Rpn=350Mhz),
this made me think that shouldn't we always be reading CAGF?
>
> @@ -119,7 +119,8 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
>           * running so clear those bits out.
>           */
>          if (!gpu_active)
> -               enable &= ~ENGINE_SAMPLE_MASK;
> +               enable &= ~(config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY) |
> +                           ENGINE_SAMPLE_MASK);
>
> I think.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx