[Intel-gfx] [RFC] Idleness DRRS test
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Fri Sep 23 12:07:44 UTC 2016
Hi Marius,
Thanks for the comments and recommendations. I will be submitting the
next version of the patch shortly with the changes.
PFB the actions taken/explanations inline.
On 9/7/2016 8:23 PM, Marius Vlad wrote:
> On Wed, Aug 31, 2016 at 01:46:03PM +0530, Nautiyal Ankit wrote:
>> From: aknautiy <ankit.k.nautiyal at intel.com>
>>
>> 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
>> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
>>
>> DRRS. The code is tested on Broxton BXT_T platform.
>>
>> Signed-off-by: aknautiy <ankit.k.nautiyal at intel.com>
>> ---
>> 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;
> Doesn't this yell a warning?
Ankit- The type conversion is taken care of in the next patchset.
>> +
>> + 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;
>> +}
> Can't find where data is used here.
Ankit – Removed the unnecessary parameter :data_t from the function.
>> +
>> +/*
>> + * 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;
> No need for data here.
Ankit – Removed the unnecessary parameter :data_t from the function.
>> +}
>> +
>> +/*
>> + * 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;
>> +}
> The return value isn't used anywhere.
Ankit- the return value is checked for successful prepare_crtc()
>> +
>> +/*
>> + * 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;
> Isn't a typecast needed here?
Ankit- Typecasted to data_t *
>> +
>> + 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*/
> Where is the write-end of the pipe?
Ankit- Removed this comment.
>> +
>> + 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);
>> +
> 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.
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.
>
>> + 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);
> 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).
Ankit- Returning int here, -1 for errors, 0 for test success and 1 for
failure.
>
>> + 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);
> prepare_crtc() returns a bool.
Ankit- Return value is handled properly.
>> + 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
> Might want to use igt_simple_main for only one test.
Ankit – igt_simple_main is used.
>> +{
>> + 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");
> 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.
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.
>> + /* Check if the DRRS is supported.
>> + * If yes call the Idleness DRRS test
>> + */
> This comments seems to belong in a different place.
Ankit- Added the comments in proper place.
>> +
>> + 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
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks for the comments , suggestions pointing out the misses. I will
address these comments and send new version of the patch.
Regards,
Ankit
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20160923/34883717/attachment-0001.html>
More information about the Intel-gfx
mailing list