<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <p>Hi Marius,</p> <p>Thanks for the comments and recommendations. I will be submitting the next version of the patch shortly with the changes.</p> <p>PFB the actions taken/explanations inline.<br> </p> <br> <div class="moz-cite-prefix">On 9/7/2016 8:23 PM, Marius Vlad wrote:<br> </div> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap="">On Wed, Aug 31, 2016 at 01:46:03PM +0530, Nautiyal Ankit wrote: </pre> <blockquote type="cite"> <pre wrap="">From: aknautiy <a class="moz-txt-link-rfc2396E" href="mailto:ankit.k.nautiyal@intel.com"><ankit.k.nautiyal@intel.com></a> 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. Instance Expected State 1. Immediately after rendering the still image DRRS_HIGH_RR 2. After a delay of 1.2Sec DRRS_LOW_RR 3. After changing the frame buffer DRRS_HIGH_RR 4. After a delay of 1.2Sec DRRS_LOW_RR 5. After changing the frame buffer DRRS_HIGH_RR 6. After a delay of 1.2Sec DRRS_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 <a class="moz-txt-link-freetext" href="https://patchwork.freedesktop.org/patch/45472/">https://patchwork.freedesktop.org/patch/45472/</a> towards igt for idleness DRRS. The code is tested on Broxton BXT_T platform. Signed-off-by: aknautiy <a class="moz-txt-link-rfc2396E" href="mailto:ankit.k.nautiyal@intel.com"><ankit.k.nautiyal@intel.com></a> --- tests/Makefile.sources | 1 + tests/kms_drrs.c | 614 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 615 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 0000000..fbe78c5 --- /dev/null +++ b/tests/kms_drrs.c @@ -0,0 +1,614 @@ +/* + * 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 <time.h> +#include <pthread.h> +#include <stdlib.h> +#include <sys/time.h> +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_CNT 1000 +#define SET 1 +#define RESET 0 + +/* + * 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; + igt_output_t *output; + enum pipe pipe; + igt_plane_t *primary; + struct igt_fb fb[2]; + uint32_t fb_id[2]; +} data_t; + +/* + * Structure to count vblank and note the starting time of the counter + */ +typedef struct { + unsigned int vbl_count; + struct timeval start; +} vbl_info; + +/* + * Condition variables,mutex, and shared variables for thread synchronization + */ +pthread_mutex_t rr_mutex; +pthread_cond_t cv_rr_calc_requested; +pthread_cond_t cv_rr_calc_completed; +int rr_calc_requested = RESET; +int rr_calc_completed = RESET; +int thread_flag = SET; +int refresh_rate_shared = -1; + +/* + * Structure for refresh rate type + */ +typedef struct{ + int rate; + const char *name; +} refresh_rate_t; + +/* + * vblank_handler : + * User defined vblank event handler, to count vblanks. + * The control reaches this function after a vblank event is registered + */ +static void vblank_handler(int fd, unsigned int frame, + unsigned int sec, unsigned int usec, void *data) +{ + /* The control reaches this function after a vblank event is + * registered + */ + vbl_info *info = data; </pre> </blockquote> <pre wrap="">Doesn't this yell a warning?</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre>Ankit- The type conversion is taken care of in the next patchset<span style="color:black">.<o:p></o:p></span> </pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ + if (info == NULL) { + igt_info("ERROR: Invalid Data passed to the vblank handler\n"); + return; + } + info->vbl_count++; +} + + +/* + * read_drrs_status : + * Func to read the DRRS status from debugfs + */ +static bool read_drrs_status(data_t *data, char *str) +{ + FILE *status; + int cnt; + + status = igt_debugfs_fopen("i915_drrs_status", "r"); + igt_assert(status); + + cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status); + if (!cnt) { + if (!feof(status)) { + igt_info("ERROR: %d at fread\n", ferror(status)); + return false; + } + clearerr(status); + } + fclose(status); + return true; +} </pre> </blockquote> <pre wrap="">Can't find where data is used here.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit – Removed the unnecessary parameter :<span style="mso-spacerun:yes"> </span>data_t from the function.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ +/* + * is_drrs_enabled : + * Func to check for DRRS support + */ +static bool is_drrs_enabled(data_t *data) +{ + char str[DRRS_STATUS_BYTES_CNT] = {}; + + if (!read_drrs_status(data, str)) { + igt_info("ERROR: Debugfs read FAILED\n"); + return false; + } + return strstr(str, "DRRS Supported: Yes") != NULL; </pre> </blockquote> <pre wrap="">No need for data here.</pre> </blockquote> <br> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit – Removed the unnecessary parameter :<span style="mso-spacerun:yes"> </span>data_t from the function.</font></span></pre> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+} + +/* + * prepare_crtc : + * Func to prepare crtc for the given display + */ +static bool prepare_crtc(data_t *data) +{ + if (data == NULL) + return false; + igt_display_t *display = &data->display; + igt_output_t *output = data->output; + + /* select the pipe we want to use */ + igt_output_set_pipe(output, data->pipe); + igt_display_commit(display); + + if (!output->valid) { + igt_output_set_pipe(output, PIPE_ANY); + igt_display_commit(display); + return false; + } + + return true; +} </pre> </blockquote> <pre wrap="">The return value isn't used anywhere. </pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- the return value is checked for successful prepare_crtc()</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <blockquote type="cite"> <pre wrap="">+ +/* + * calculate_refresh_rate : + * Func to calculate the refresh rate by counting + * vblanks in 1 sec. It returns the calculated refresh rate on success. + * Returns -1 in case of error. + */ +int calculate_refresh_rate(data_t *data) +{ + drmEventContext evctx; + drmVBlank vbl; + vbl_info handler_info; + struct timeval start_time, curr_time; + double time_elapsed; + int refresh_rate = 0, vbl_count = 0; + /*set up event context for handling vblank events*/ + memset(&evctx, 0, sizeof(evctx)); + evctx.version = DRM_EVENT_CONTEXT_VERSION; + evctx.vblank_handler = vblank_handler; + evctx.page_flip_handler = NULL; + + /*initialize the vbl count to zero and set the starting time*/ + handler_info.vbl_count = 0; + gettimeofday(&start_time, NULL); + handler_info.start = start_time; + curr_time = start_time; + + time_elapsed = 0.0; + /* To count vblanks in 1 sec. Loop till + * (current-time - starting time) <= 1.0 sec + */ + while (time_elapsed <= 1.0) { + int ret; + + vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT; + vbl.request.sequence = 1; + vbl.request.signal = (unsigned long)&handler_info; + ret = drmWaitVBlank(data->drm_fd, &vbl); + if (ret != 0) { + igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret); + return -1; + } + /*call drmHandleEvent() to handle vblank event */ + ret = drmHandleEvent(data->drm_fd, &evctx); + if (ret != 0) { + igt_info("ERROR : drmHandleEvent FAILED %d\n", ret); + return -1; + } + + gettimeofday(&curr_time, NULL); + time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 - + (start_time.tv_sec + start_time.tv_usec * 1e-6); + } + if (!time_elapsed) { + igt_info("ERROR: Incorrect measurement of vblank duration\n"); + return -1; + } + vbl_count = handler_info.vbl_count; + /* calculate the refresh rate; rr=vblank_count/time_taken */ + refresh_rate = vbl_count / time_elapsed; + return refresh_rate; +} + +/* + * worker_thread_func : + * Func which is run by a worker thread to calculate the refresh_rate, + * when signalled by the master thread. + */ +void *worker_thread_func(void *ptr) +{ + data_t *data = ptr; </pre> </blockquote> <pre wrap="">Isn't a typecast needed here?</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Typecasted to data_t *</font> <o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ + while (1) { + int refresh_rate = 0; + /*wait for signal from master*/ + pthread_mutex_lock(&rr_mutex); + while (!rr_calc_requested) + pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex); + + /* checkpoint for thread termination */ + if (thread_flag == RESET) { + pthread_mutex_unlock(&rr_mutex); + pthread_exit(NULL); + } + rr_calc_requested = RESET; + pthread_mutex_unlock(&rr_mutex); + /*calculate refresh_rate*/ + refresh_rate = calculate_refresh_rate(data); + + /* signal the master */ + pthread_mutex_lock(&rr_mutex); + refresh_rate_shared = refresh_rate; + rr_calc_completed = SET; + pthread_mutex_unlock(&rr_mutex); + pthread_cond_signal(&cv_rr_calc_completed); + } + return NULL; +} + +/* + * check_refresh_rate : + * Func to check the refresh rate calculated, against the expected refresh + * rate. It signals the worker thread to calculate the refresh- + * rate, and checks if the correct refresh rate is switched. + * Returns 1 in case of pass , 0 in case of failure, and -1 for error. + */ + +static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr) +{ +/* + * The calculated refresh-rate is an approximation to estimated refresh-rate. + * The tolerance threshold for the difference is set to 2. Difference more than + * 2 is considered as failure for the check_refresh_rate. + */ +#define TOLERANCE_THRESHOLD 2 + int ret, refresh_rate = -1; + char drrs_status[DRRS_STATUS_BYTES_CNT] = {}; + + /* Signal worker thread to calculate refresh rate */ + pthread_mutex_lock(&rr_mutex); + rr_calc_requested = SET; + pthread_mutex_unlock(&rr_mutex); + pthread_cond_signal(&cv_rr_calc_requested); + + /* Get the DRRS Status from the debugfs entry */ + if (!read_drrs_status(data, drrs_status)) { + igt_info("ERROR : DRRS debugfs read FAILED\n"); + return -1; + } + /* Wait for the refresh rate calculator thread */ + pthread_mutex_lock(&rr_mutex); + while (!rr_calc_completed) + pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex); + rr_calc_completed = RESET; + refresh_rate = refresh_rate_shared; + pthread_mutex_unlock(&rr_mutex); + if (refresh_rate < 0) { + igt_info("ERROR : Refresh rate calculation FAILED\n"); + return -1; + } + igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n" + , refresh_rate); + + /* Compare with Expected Refresh Rate*/ + igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate); + if (strstr(drrs_status, expected_rr->name) != NULL + && abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) { + igt_info("INFO: Expected %s : PASSED\n", expected_rr->name); + return 1; + } + + igt_info("INFO : Expected %s : FAILED\n", expected_rr->name); + return 0; +} + +/* + * clean_up : + * Func to stop the worker thread and destroy thread syncronization + * structures. + */ +void clean_up(pthread_t rr_thread) +{ + /* Stop the worker thread */ + pthread_mutex_lock(&rr_mutex); + thread_flag = RESET; + rr_calc_requested = SET; + pthread_mutex_unlock(&rr_mutex); + pthread_cond_signal(&cv_rr_calc_requested); + pthread_join(rr_thread, NULL); + + /* destroy the condition variables and mutex */ + pthread_cond_destroy(&cv_rr_calc_requested); + pthread_cond_destroy(&cv_rr_calc_completed); + pthread_mutex_destroy(&rr_mutex); +} + +/* + * execute_test: + * This function + * 1. Initializes the thread sync structures. + * 2. Creates a worker thread which can be signaled to estimate + * the refresh rates based on counting vblanks per sec. + * 3. Creates two different Framebuffer fb0, fb1 + * and apply them in the interval of 1.2Sec. So between FB change + * DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR. + * And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR. + * + * These expected transistions are verified using the kernel debugfs + * and the refresh-rate, calculated by counting vblanks per sec by the + * worker thread. + */ +static bool execute_test(data_t *data) +{ + igt_display_t *display = &data->display; + igt_output_t *output = data->output; + drmModeModeInfo *mode, *supported_modes; + bool test_failed = false; + int ret, i, count_modes; + refresh_rate_t high_rr, low_rr; + pthread_t rr_thread; + + /*initialize the condition variables and mutex*/ + pthread_cond_init(&cv_rr_calc_requested, NULL); + pthread_cond_init(&cv_rr_calc_completed, NULL); + pthread_mutex_init(&rr_mutex, NULL); + thread_flag = SET; + rr_calc_requested = RESET; + rr_calc_completed = RESET; + /* Create a thread for calculating the refresh rate */ + ret = pthread_create(&rr_thread, NULL, worker_thread_func, + (void *) data); + if (ret == -1) { + igt_info("ERROR : Refresh rate thread creation FAILED\n"); + clean_up(rr_thread); + return false; + } + /*close the write end of the pipe as it will be used for reading only*/ </pre> </blockquote> <pre wrap="">Where is the write-end of the pipe?</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Removed this comment.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ + high_rr.name = "DRRS_HIGH_RR"; + high_rr.rate = 0; + low_rr.name = "DRRS_LOW_RR"; + low_rr.rate = 0; + /* get the max and min supported refresh rates for the display */ + supported_modes = output->config.connector->modes; + count_modes = output->config.connector->count_modes; + /* minimum 2 modes are required for DRRS */ + if (count_modes < 2) { + igt_info("ERROR: Minimum 2 modes required for DRRS\n"); + return false; + } + for (i = 0; i < count_modes; i++) { + int rr = supported_modes[i].vrefresh; + + if (i == 0) { + high_rr.rate = low_rr.rate = rr; + continue; + } + if (high_rr.rate < rr) + high_rr.rate = rr; + if (low_rr.rate > rr) + low_rr.rate = rr; + } + data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY); + mode = igt_output_get_mode(data->output); + data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay, + mode->vdisplay, + DRM_FORMAT_XRGB8888, + LOCAL_DRM_FORMAT_MOD_NONE, + 0.0, 100.1, 0.0, &data->fb[0]); + igt_assert(data->fb_id[0]); + data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay, + mode->vdisplay, + DRM_FORMAT_XRGB8888, + LOCAL_DRM_FORMAT_MOD_NONE, + 100.1, 0.0, 0.0, + &data->fb[1]); + igt_assert(data->fb_id[1]); + + data->handle[0] = data->fb[0].gem_handle; + data->handle[1] = data->fb[1].gem_handle; + + /* scanout = fb[1] */ + igt_plane_set_fb(data->primary, &data->fb[1]); + igt_display_commit(display); + </pre> </blockquote> <pre wrap=""> I assume that the behaviour is that once you have committed your frame-buffer, DRRS should be enabled by the kernel, hence the check here again. You do this (albeit manually) in igt_fixture block.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit– Yes you are right. There are two things to be checked. 1st is whether DRRS is supported from VBT/Panel side, and this is checked in igt_simple_main. The 2nd thing is whether DRRS is enabled by the driver, checked once the framebuffer is committed.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ if (!is_drrs_enabled(data)) { + igt_info("INFO : DRRS not enabled\n"); + igt_plane_set_fb(data->primary, NULL); + igt_output_set_pipe(output, PIPE_ANY); + igt_display_commit(display); + + igt_remove_fb(data->drm_fd, &data->fb[0]); + igt_remove_fb(data->drm_fd, &data->fb[1]); + clean_up(rr_thread); + return false; + } + + /* expecting High RR */ + ret = check_refresh_rate(data, &high_rr); + if (ret != 1) + test_failed = true; + /* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */ + usleep(1200000); + ret = check_refresh_rate(data, &low_rr); + if (ret != 1) + test_failed = true; + + /* scanout = fb[0] and check for High Refresh Rate*/ + igt_plane_set_fb(data->primary, &data->fb[0]); + igt_display_commit(display); + ret = check_refresh_rate(data, &high_rr); + if (ret != 1) + test_failed = true; + + /* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */ + usleep(1200000); + ret = check_refresh_rate(data, &low_rr); + if (ret != 1) + test_failed = true; + + /* scanout = fb[1] and check for High Refresh Rate*/ + igt_plane_set_fb(data->primary, &data->fb[1]); + igt_display_commit(display); + ret = check_refresh_rate(data, &high_rr); + if (ret != 1) + test_failed = true; + + /* 1.2Sec to Enter the Idleness */ + usleep(1200000); + ret = check_refresh_rate(data, &low_rr); + if (ret != 1) + test_failed = true; + + igt_assert(!test_failed); </pre> </blockquote> <pre wrap="">Do you really want to exit with an assert if DRRS test failed? This function returns false when DRRS is not present and bails out earlier if test has failed. You might want to have returned an int with -1 for errors, 0 for test succeeded, 1 when test not succeeded (you seem to be using this notation in one of the functions above).</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Returning int here, -1 for errors, 0 for test success and 1 for failure.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ clean_up(rr_thread); + return true; +} + +/* + * finish_crtc : + * Func to free the framebuffers after the test completion. + */ +static void finish_crtc(data_t *data) +{ + igt_plane_set_fb(data->primary, NULL); + igt_output_set_pipe(data->output, PIPE_ANY); + igt_display_commit(&data->display); + + igt_remove_fb(data->drm_fd, &data->fb[0]); + igt_remove_fb(data->drm_fd, &data->fb[1]); +} + +/* + * reset_display : + * Func to reset the display structures after the test completion. + */ +static void reset_display(data_t *data) +{ + igt_display_t *display = &data->display; + + for_each_connected_output(display, data->output) { + if (data->output->valid) { + data->primary = igt_output_get_plane(data->output, + IGT_PLANE_PRIMARY); + igt_plane_set_fb(data->primary, NULL); + } + igt_output_set_pipe(data->output, PIPE_ANY); + } +} +/* + * run_test : + * Func to run the test for the eDP display. + */ +static void run_test(data_t *data) +{ + igt_display_t *display = &data->display; + int valid_tests = 0; + + reset_display(data); + /* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */ + for_each_connected_output(display, data->output) { + drmModeConnectorPtr c = data->output->config.connector; + + if (c->connector_type != DRM_MODE_CONNECTOR_eDP || + c->connection != DRM_MODE_CONNECTED) + continue; + data->pipe = PIPE_A; + prepare_crtc(data); </pre> </blockquote> <pre wrap="">prepare_crtc() returns a bool.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Return value is handled properly.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ igt_info("INFO : Beginning %s on pipe %s,connector %s\n", + igt_subtest_name(), + kmstest_pipe_name(data->pipe), + igt_output_name(data->output)); + + if (!execute_test(data)) { + igt_info("INFO :%s on pipe %s, connector %s:SKIPPED\n", + igt_subtest_name(), + kmstest_pipe_name(data->pipe), + igt_output_name(data->output)); + continue; + } + + valid_tests++; + + igt_info("INFO :%s on pipe %s, connector %s: PASSED\n", + igt_subtest_name(), + kmstest_pipe_name(data->pipe), + igt_output_name(data->output)); + + finish_crtc(data); + } +} + +igt_main </pre> </blockquote> <pre wrap="">Might want to use igt_simple_main for only one test.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit – igt_simple_main is used.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+{ + data_t data = {}; + + igt_skip_on_simulation(); + + igt_fixture { + char buf[64]; + int cnt; + FILE *status; + + data.drm_fd = drm_open_driver_master(DRIVER_INTEL); + if (data.drm_fd == -1) { + igt_info("ERROR : invalid fd\n"); + return; + } + + data.devid = intel_get_drm_devid(data.drm_fd); + if (data.devid < 0) { + igt_info("ERROR : invalid dev id\n"); + return; + } + + status = igt_debugfs_fopen("i915_drrs_status", "r"); + if (status == NULL) { + igt_info("ERROR : debugfs open FAILED\n"); + return; + } + igt_require_f(status, "No i915_drrs_status found\n"); + cnt = fread(buf, sizeof(buf), 1, status); + if (!cnt) { + igt_info("ERROR : fread FAILED\n"); + return; + } + fclose(status); + buf[sizeof(buf) - 1] = '\0'; + igt_require_f(!strstr(buf, "disabled"), + "DRRS not supported:check VBT/panel caps\n"); </pre> </blockquote> <pre wrap="">You have a couple functions to test DRRS presence bou do it (again) manually. Also you check for each connected output in you do it (again) manually.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Reusing the existing function is_drrs_enabled() for checking the DRRS support. However the function name was a misnomer, so changed it to is_drrs_supported.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ /* Check if the DRRS is supported. + * If yes call the Idleness DRRS test + */ </pre> </blockquote> <pre wrap="">This comments seems to belong in a different place.</pre> </blockquote> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <pre class="MsoPlainText"><span style="color:#2E75B6;mso-themecolor:accent1; mso-themeshade:191;mso-style-textfill-fill-color:#2E75B6;mso-style-textfill-fill-themecolor: accent1;mso-style-textfill-fill-alpha:100.0%;mso-style-textfill-fill-colortransforms: lumm=75000"><font color="#000000">Ankit- Added the comments in proper place.</font><o:p></o:p></span></pre> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5CAKNAUTIY%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> <blockquote cite="mid:20160907145258.jlburgob6ecqjigf@mcvlad-wk.rb.intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+ + igt_display_init(&data.display, data.drm_fd); + } + igt_subtest_f("Idleness_DRRS") + run_test(&data); + + igt_fixture { + igt_display_fini(&data.display); + } +} -- 2.7.4 _______________________________________________ Intel-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a></pre> </blockquote> </blockquote> Thanks for the comments , suggestions pointing out the misses. I will address these comments and send new version of the patch.<br> <br> Regards,<br> Ankit<br> </body> </html>