Re: [Intel-gfx] [PATCH v3] Idleness DRRS test

[Daniel Vetter]

On Tue, Nov 29, 2016 at 03:06:17PM +0530, Nautiyal, Ankit K wrote:
> As per discussion with Chris, on IRC following were the suggestions :
> 
> - Chris has suggested an event based approach to avoid using pthreads.
> 
> - Avoid using kernel-specific info like transitional delays and instead use
> either polling or wait-for-event approach. Need to focus on user-observable
> behavior rather than the kernel standpoint.
> 
> - Check for transition latency is unnecessary in this test, as this is more
> of a performance/power benchmark.
> 
> 
> I will try the event based mechanism to do away with pthreads, and
> incorporate these suggestions in the next patch-set.

Again, why is kms_frontbuffer_tracking not considered? Reinventing wheels
is not good, and kms_frontbuffer_tracking implements all of the above
stuff afaik.
-Daniel

> 
> -Ankit
> 
> 
> On 11/15/2016 2:28 PM, Daniel Vetter wrote:
> > On Mon, Nov 14, 2016 at 01:06:09PM +, Chris Wilson wrote:
> > > On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> > > > Chris, happy with this revision?
> > > Me? No. It still uses a thread instead of events, so I don't think it
> > > qualifies as a good example for anyone else wanting to do the same thing.
> > > Lots of hardcoded expectations (specific sleep patterns, rather than
> > > waiting for the kernel to change with a timeout for failure). It doesn't
> > > check all the possible ways that the output maybe changed (and if they
> > > are irrelevent, that too also needs to be tested to establish expected
> > > behaviour and catch future regressions). Important question, how is
> > > userspace expected to know that the vrefresh changed? How do get around
> > > that userspace is expecting a fixed vblank frequency?
> > for display power testing we already have the kms_frontbuffer_tracking
> > testcase, adding a DRRS variants to that one should resolve all this.
> > 
> > And then kms_drrs would be empty, except when we (if ever) do explicit
> > drrs through modeset requests. And that would just be checking that the
> > observed timings match the requested timings.
> > -Daniel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] Idleness DRRS test

[Nautiyal, Ankit K]

As per discussion with Chris, on IRC following were the suggestions :

- Chris has suggested an event based approach to avoid using pthreads.

- Avoid using kernel-specific info like transitional delays and instead 
use either polling or wait-for-event approach. Need to focus on 
user-observable behavior rather than the kernel standpoint.


- Check for transition latency is unnecessary in this test, as this is 
more of a performance/power benchmark.



I will try the event based mechanism to do away with pthreads, and 
incorporate these suggestions in the next patch-set.


-Ankit


On 11/15/2016 2:28 PM, Daniel Vetter wrote:

On Mon, Nov 14, 2016 at 01:06:09PM +, Chris Wilson wrote:

On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:

Chris, happy with this revision?

Me? No. It still uses a thread instead of events, so I don't think it
qualifies as a good example for anyone else wanting to do the same thing.
Lots of hardcoded expectations (specific sleep patterns, rather than
waiting for the kernel to change with a timeout for failure). It doesn't
check all the possible ways that the output maybe changed (and if they
are irrelevent, that too also needs to be tested to establish expected
behaviour and catch future regressions). Important question, how is
userspace expected to know that the vrefresh changed? How do get around
that userspace is expecting a fixed vblank frequency?

for display power testing we already have the kms_frontbuffer_tracking
testcase, adding a DRRS variants to that one should resolve all this.

And then kms_drrs would be empty, except when we (if ever) do explicit
drrs through modeset requests. And that would just be checking that the
observed timings match the requested timings.
-Daniel


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

Re: [Intel-gfx] [PATCH v3] Idleness DRRS test

[Daniel Vetter]

On Mon, Nov 14, 2016 at 01:06:09PM +, Chris Wilson wrote:
> On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> > Chris, happy with this revision?
> 
> Me? No. It still uses a thread instead of events, so I don't think it
> qualifies as a good example for anyone else wanting to do the same thing.
> Lots of hardcoded expectations (specific sleep patterns, rather than
> waiting for the kernel to change with a timeout for failure). It doesn't
> check all the possible ways that the output maybe changed (and if they
> are irrelevent, that too also needs to be tested to establish expected
> behaviour and catch future regressions). Important question, how is
> userspace expected to know that the vrefresh changed? How do get around
> that userspace is expecting a fixed vblank frequency?

for display power testing we already have the kms_frontbuffer_tracking
testcase, adding a DRRS variants to that one should resolve all this.

And then kms_drrs would be empty, except when we (if ever) do explicit
drrs through modeset requests. And that would just be checking that the
observed timings match the requested timings.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] Idleness DRRS test

[Chris Wilson]

On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> Chris, happy with this revision?

Me? No. It still uses a thread instead of events, so I don't think it
qualifies as a good example for anyone else wanting to do the same thing.
Lots of hardcoded expectations (specific sleep patterns, rather than
waiting for the kernel to change with a timeout for failure). It doesn't
check all the possible ways that the output maybe changed (and if they
are irrelevent, that too also needs to be tested to establish expected
behaviour and catch future regressions). Important question, how is
userspace expected to know that the vrefresh changed? How do get around
that userspace is expecting a fixed vblank frequency?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] Idleness DRRS test

[Petri Latvala]

Chris, happy with this revision?


On Fri, Oct 21, 2016 at 09:22:13AM +0530, Nautiyal Ankit wrote:
> From: Ramalingam C 
> 
> Idleness DRRS:
>   By default the DRRS state will be at DRRS_HIGH_RR. When a Display
>   content is Idle for more than 1Sec Idleness will be declared and
>   DRRS_LOW_RR will be invoked, changing the refresh rate to the
>   lower most refresh rate supported by the panel. As soon as there
>   is a display content change there will be a DRRS state transition
>   as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
>   highest refresh rate supported by the panel.
> 
> To test this, Idleness DRRS IGT will probe the DRRS state at below
> instances and compare with the expected state.
> 
>   InstanceExpected State
> 1. Immediately after rendering the still imageDRRS_HIGH_RR
> 2. After a delay of 1.2SecDRRS_LOW_RR
> 3. After changing the frame bufferDRRS_HIGH_RR
> 4. After a delay of 1.2SecDRRS_LOW_RR
> 5. After changing the frame bufferDRRS_HIGH_RR
> 6. After a delay of 1.2SecDRRS_LOW_RR
> 
> The test checks the driver DRRS state from the debugfs entry. To check the
> actual refresh-rate, a separate thread counts the number of vblanks
> received per sec. The refresh-rate calculated is checked against the
> expected refresh-rate with a tolerance value of 2.
> 
> This patch is a continuation of the earlier work
> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
> 
> DRRS. The code is tested on Broxton BXT_T platform.
> 
> v2: Addressed the comments and suggestions from Vlad, Marius.
> The signoff details from the earlier work are also included.
> 
> v3: Modified vblank rate calculation by using reply-sequence, provided by
> drmWaitVBlank, as suggested by Chris Wilson.
> 
> Signed-off-by: Ramalingam C 
> Signed-off-by: Vandana Kannan 
> Signed-off-by: aknautiy 
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_drrs.c   | 599 
> +
>  2 files changed, 600 insertions(+)
>  create mode 100644 tests/kms_drrs.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a837977..5f31521 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>   kms_cursor_crc \
>   kms_cursor_legacy \
>   kms_draw_crc \
> + kms_drrs \
>   kms_fbc_crc \
>   kms_fbcon_fbt \
>   kms_flip \
> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
> new file mode 100644
> index 000..bd5a135
> --- /dev/null
> +++ b/tests/kms_drrs.c
> @@ -0,0 +1,599 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +#include "intel_batchbuffer.h"
> +#include "ioctl_wrappers.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +IGT_TEST_DESCRIPTION(
> +"Performs write operations and then waits for DRRS to invoke the"
> +"Low Refresh Rate and then disturbs the contents of the screen once"
> +"again hence DRRS revert back to High Refresh Rate(Default).");
> +
> +#define DRRS_STATUS_BYTES_CNT1000
> +#define SET  1
> +#define RESET0
> +
> +/*
> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a 
> given
> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] 
> and
> + * vice versa.
> + */
> +typedef struct {
> + int drm_fd;
> + uint32_t devid;
> + uint32_t handle[2];
> + igt_display_t display;
> +