Hi Vinay, On Mon, Apr 17, 2023 at 11:04:31PM -0700, Belgaumkar, Vinay wrote: > > On 4/17/2023 6:39 PM, Andi Shyti wrote: > > Hi Vinay, > > > > Looks good, just few minor comments below, > > > > [...] > > > > > @@ -267,13 +267,11 @@ static int run_test(struct intel_gt *gt, int test_type) > > > } > > > /* > > > - * Set min frequency to RPn so that we can test the whole > > > - * range of RPn-RP0. This also turns off efficient freq > > > - * usage and makes results more predictable. > > > + * Turn off efficient freq so RPn/RP0 ranges are obeyed > > > */ > > > - err = slpc_set_min_freq(slpc, slpc->min_freq); > > > + err = intel_guc_slpc_set_ignore_eff_freq(slpc, true); > > > if (err) { > > > - pr_err("Unable to update min freq!"); > > > + pr_err("Unable to turn off efficient freq!"); > > drm_err()? or gt_err()? As we are here we can use a proper > > printing. > > > > How is this change related to the scope of this patch? > The selftest was relying on setting min freq < RP1 to disable efficient > freq, now that we have an interface, the test should use that (former method > will not work). Should this be a separate patch? I would have placed it in a separate patch. But I'm not strong with it, up to you. Thanks, Andi