[igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

This completes the change started by:

commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri May 19 09:41:40 2017 +0100

    igt/pm_rps: Allow CUR to be greater than MAX (overclocking)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 tests/pm_rps.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index f0455e78..1a322459 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -145,7 +145,6 @@ static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
 static void checkit(const int *freqs)
 {
 	igt_assert_lte(freqs[MIN], freqs[MAX]);
-	igt_assert_lte(freqs[CUR], freqs[MAX]);
 	igt_assert_lte(freqs[RPn], freqs[CUR]);
 	igt_assert_lte(freqs[RPn], freqs[MIN]);
 	igt_assert_lte(freqs[MAX], freqs[RP0]);
-- 
2.11.0

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

Re: [igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[Szwichtenberg, Radoslaw]

On Wed, 2017-06-14 at 12:44 -0700, jeff.mcgee@intel.com wrote:
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> This completes the change started by:
> 
> commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri May 19 09:41:40 2017 +0100
> 
>     igt/pm_rps: Allow CUR to be greater than MAX (overclocking)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[Arkadiusz Hiler]

On Tue, Jun 20, 2017 at 01:54:54PM +0000, Szwichtenberg, Radoslaw wrote:
> On Wed, 2017-06-14 at 12:44 -0700, jeff.mcgee@intel.com wrote:
> > From: Jeff McGee <jeff.mcgee@intel.com>
> > 
> > This completes the change started by:
> > 
> > commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Fri May 19 09:41:40 2017 +0100
> > 
> >     igt/pm_rps: Allow CUR to be greater than MAX (overclocking)
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>

Pushed. Thanks for the patch and the review.

Excerpt from CONTRIBUTING:
-----------------------------------------------------------------------------
Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily
identified in the massive amount mails on intel-gfx. To ensure this is always
done, autogen.sh will run:

  git config format.subjectprefix "PATCH i-g-t"

on its first invocation.
-----------------------------------------------------------------------------

Lack of proper prefix breaks filtering / patchwork and makes changes
harder to track, and effectilvely it takes longer to get the patch in.

The autogen.sh thing is a recent addition.

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

Re: [igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[Arkadiusz Hiler]

On Tue, Jun 20, 2017 at 06:15:52PM +0300, Arkadiusz Hiler wrote:
> On Tue, Jun 20, 2017 at 01:54:54PM +0000, Szwichtenberg, Radoslaw wrote:
> > On Wed, 2017-06-14 at 12:44 -0700, jeff.mcgee@intel.com wrote:
> > > From: Jeff McGee <jeff.mcgee@intel.com>
> > > 
> > > This completes the change started by:
> > > 
> > > commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date:   Fri May 19 09:41:40 2017 +0100
> > > 
> > >     igt/pm_rps: Allow CUR to be greater than MAX (overclocking)
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> 
> Pushed. Thanks for the patch and the review.
> 
> Excerpt from CONTRIBUTING:
> -----------------------------------------------------------------------------
> Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily
> identified in the massive amount mails on intel-gfx. To ensure this is always
> done, autogen.sh will run:
> 
>   git config format.subjectprefix "PATCH i-g-t"
> 
> on its first invocation.
> -----------------------------------------------------------------------------
> 
> Lack of proper prefix breaks filtering / patchwork and makes changes
> harder to track, and effectilvely it takes longer to get the patch in.
> 
> The autogen.sh thing is a recent addition.

Hey Jeff,

The change has been reverted.

At first the it looked like a sensible folloup to previous changes and
it even got r-bed, but as pointed out by Michal, this constraint still
should hold, as it's checked in a non-boost scenario.

It hides bug introduced into the kernel by the commit:
"drm/i915: Define a separate variable and control for RPS waitboost frequency"

for GuC submission path, as the check below always holds true as we have
waiters all the time:
     if (client_boost || any_waiters) max = hw_max;

What was the point of this change?
Were you aware of the fail in the guc_submission scenario?

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

Re: [igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[Jeff McGee]

On Wed, Jun 21, 2017 at 12:55:42PM +0300, Arkadiusz Hiler wrote:
> On Tue, Jun 20, 2017 at 06:15:52PM +0300, Arkadiusz Hiler wrote:
> > On Tue, Jun 20, 2017 at 01:54:54PM +0000, Szwichtenberg, Radoslaw wrote:
> > > On Wed, 2017-06-14 at 12:44 -0700, jeff.mcgee@intel.com wrote:
> > > > From: Jeff McGee <jeff.mcgee@intel.com>
> > > > 
> > > > This completes the change started by:
> > > > 
> > > > commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
> > > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Date:   Fri May 19 09:41:40 2017 +0100
> > > > 
> > > >     igt/pm_rps: Allow CUR to be greater than MAX (overclocking)
> > > > 
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > > Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > 
> > Pushed. Thanks for the patch and the review.
> > 
> > Excerpt from CONTRIBUTING:
> > -----------------------------------------------------------------------------
> > Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily
> > identified in the massive amount mails on intel-gfx. To ensure this is always
> > done, autogen.sh will run:
> > 
> >   git config format.subjectprefix "PATCH i-g-t"
> > 
> > on its first invocation.
> > -----------------------------------------------------------------------------
> > 
> > Lack of proper prefix breaks filtering / patchwork and makes changes
> > harder to track, and effectilvely it takes longer to get the patch in.
> > 
> > The autogen.sh thing is a recent addition.
> 
> Hey Jeff,
> 
> The change has been reverted.
> 
> At first the it looked like a sensible folloup to previous changes and
> it even got r-bed, but as pointed out by Michal, this constraint still
> should hold, as it's checked in a non-boost scenario.
> 
> It hides bug introduced into the kernel by the commit:
> "drm/i915: Define a separate variable and control for RPS waitboost frequency"
> 
> for GuC submission path, as the check below always holds true as we have
> waiters all the time:
>      if (client_boost || any_waiters) max = hw_max;
> 
> What was the point of this change?
> Were you aware of the fail in the guc_submission scenario?
> 
> -- 
> Cheers,
> Arek

Hi Arek. Yes, this was prompted by a regression of this subtest in Yocto
which uses GuC submission. I root caused it to this same i915 commit
from Chris but incorrectly thought that the new "policy" was that sysfs
max freq could be ignored in pretty much any situation. I understand now
that it should be ignored in only boost and real user wait situations
(not those triggered by GuC). So I do agree with this revert.

Is there an i915 fix coming soon? Chris?

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

Re: [igt PATCH] igt/pm_rps: Remove remaining assert on CUR <= MAX

[Arkadiusz Hiler]

On Wed, Jun 21, 2017 at 08:57:21AM -0700, Jeff McGee wrote:
> On Wed, Jun 21, 2017 at 12:55:42PM +0300, Arkadiusz Hiler wrote:
> > On Tue, Jun 20, 2017 at 06:15:52PM +0300, Arkadiusz Hiler wrote:
> > > On Tue, Jun 20, 2017 at 01:54:54PM +0000, Szwichtenberg, Radoslaw wrote:
> > > > On Wed, 2017-06-14 at 12:44 -0700, jeff.mcgee@intel.com wrote:
> > > > > From: Jeff McGee <jeff.mcgee@intel.com>
> > > > > 
> > > > > This completes the change started by:
> > > > > 
> > > > > commit 39cccab83b7c515a2b57abe679a8cb304c8933ef
> > > > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Date:   Fri May 19 09:41:40 2017 +0100
> > > > > 
> > > > >     igt/pm_rps: Allow CUR to be greater than MAX (overclocking)
> > > > > 
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > > > Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> > > 
> > > Pushed. Thanks for the patch and the review.
> > > 
> > > Excerpt from CONTRIBUTING:
> > > -----------------------------------------------------------------------------
> > > Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily
> > > identified in the massive amount mails on intel-gfx. To ensure this is always
> > > done, autogen.sh will run:
> > > 
> > >   git config format.subjectprefix "PATCH i-g-t"
> > > 
> > > on its first invocation.
> > > -----------------------------------------------------------------------------
> > > 
> > > Lack of proper prefix breaks filtering / patchwork and makes changes
> > > harder to track, and effectilvely it takes longer to get the patch in.
> > > 
> > > The autogen.sh thing is a recent addition.
> > 
> > Hey Jeff,
> > 
> > The change has been reverted.
> > 
> > At first the it looked like a sensible folloup to previous changes and
> > it even got r-bed, but as pointed out by Michal, this constraint still
> > should hold, as it's checked in a non-boost scenario.
> > 
> > It hides bug introduced into the kernel by the commit:
> > "drm/i915: Define a separate variable and control for RPS waitboost frequency"
> > 
> > for GuC submission path, as the check below always holds true as we have
> > waiters all the time:
> >      if (client_boost || any_waiters) max = hw_max;
> > 
> > What was the point of this change?
> > Were you aware of the fail in the guc_submission scenario?
> > 
> > -- 
> > Cheers,
> > Arek
> 
> Hi Arek. Yes, this was prompted by a regression of this subtest in Yocto
> which uses GuC submission. I root caused it to this same i915 commit
> from Chris but incorrectly thought that the new "policy" was that sysfs
> max freq could be ignored in pretty much any situation. I understand now
> that it should be ignored in only boost and real user wait situations
> (not those triggered by GuC). So I do agree with this revert.

Hey,

Thank you for the explanation. I see where to confusion issued from -
the change seemed to make sense on it's own and to be just a simple
follow up to the previous patch.

> Is there an i915 fix coming soon? Chris?

I think it's under the review on the ML already:
https://patchwork.freedesktop.org/patch/163146/

-- 
Cheers,
Arek

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