From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id A2B946E03E for ; Mon, 15 Nov 2021 15:13:56 +0000 (UTC) Message-ID: <7c3df7c4-a239-acca-6765-3cda08f634c2@intel.com> Date: Mon, 15 Nov 2021 20:43:50 +0530 MIME-Version: 1.0 Content-Language: en-US References: <20211112122752.29241-1-venkata.sai.patnana@intel.com> From: "Sharma, Swati2" In-Reply-To: <20211112122752.29241-1-venkata.sai.patnana@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/kms_dsc: To test modeset on highest mode List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: venkata.sai.patnana@intel.com, igt-dev@lists.freedesktop.org List-ID: Hi Sai, At many place indentation is missing. Please fix that. On 12-Nov-21 5:57 PM, venkata.sai.patnana@intel.com wrote: > From: Patnana Venkata Sai > > force dsc enable supports 5k abvoe resoultions on DP. nitpick: above/ resolutions > Bigjoiner does not support force bpp. > Update to pick the highest mode rather the default mode. > > Cc: Vandita Kulkarni > Cc: Karthik B S > Cc: Bhanuprakash Modem > Cc: Swati Sharma > > Signed-off-by: Patnana Venkata Sai > --- > tests/i915/kms_dsc.c | 165 ++++++++++++++++++++++++++++--------------- > 1 file changed, 109 insertions(+), 56 deletions(-) > > diff --git a/tests/i915/kms_dsc.c b/tests/i915/kms_dsc.c > index dcfe8b9669..a257ffa930 100644 > --- a/tests/i915/kms_dsc.c > +++ b/tests/i915/kms_dsc.c > @@ -63,8 +63,8 @@ typedef struct { > drmModeEncoder *encoder; > int crtc; > int compression_bpp; > + int n_pipes; > enum pipe pipe; > - char conn_name[128]; > } data_t; > > bool force_dsc_en_orig; > @@ -79,7 +79,7 @@ static void force_dsc_enable(data_t *data) > { > int ret; > > - igt_debug ("Forcing DSC enable on %s\n", data->conn_name); > + igt_debug ("Forcing DSC enable on %s\n", data->output->name); > ret = igt_force_dsc_enable(data->drm_fd, > data->output->config.connector); > igt_assert_f(ret > 0, "debugfs_write failed"); > @@ -90,7 +90,7 @@ static void force_dsc_enable_bpp(data_t *data) > int ret; > > igt_debug("Forcing DSC BPP to %d on %s\n", > - data->compression_bpp, data->conn_name); > + data->compression_bpp, data->output->name); > ret = igt_force_dsc_enable_bpp(data->drm_fd, > data->output->config.connector, > data->compression_bpp); > @@ -151,41 +151,55 @@ static bool is_external_panel(drmModeConnector *connector) > } > } > > -static bool check_dsc_on_connector(data_t *data, uint32_t drmConnector) > +static int sort_drm_modes(const void *a, const void *b) > +{ > + const drmModeModeInfo *mode1 = a, *mode2 = b; > + > + return (mode1->clock < mode2->clock) - (mode2->clock < mode1->clock); > +} > + > +static igt_output_t *check_dsc_on_connector(data_t *data, uint32_t drmConnector) > { > drmModeConnector *connector; > igt_output_t *output; > - > connector = drmModeGetConnectorCurrent(data->drm_fd, > drmConnector); > if (connector->connection != DRM_MODE_CONNECTED) > - return false; > + return NULL; > > output = igt_output_from_connector(&data->display, connector); > - sprintf(data->conn_name, "%s-%d", > - kmstest_connector_type_str(connector->connector_type), > - connector->connector_type_id); > > + /* As dsc supports >= 5k modes, we need to supress lower > + * resolutions. > + */ indentation missing > + qsort(output->config.connector->modes, > + output->config.connector->count_modes, > + sizeof(drmModeModeInfo), > + sort_drm_modes); indentation missing > + if (output->config.connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort && > + output->config.connector->modes[0].hdisplay < 5120) { > + return NULL; > + } {} not required > if (!igt_is_dsc_supported(data->drm_fd, connector)) { > igt_debug("DSC not supported on connector %s\n", > - data->conn_name); > - return false; > + output->name); > + return NULL; > } > if (is_external_panel(connector) && > !igt_is_fec_supported(data->drm_fd, connector)) { > igt_debug("DSC cannot be enabled without FEC on %s\n", > - data->conn_name); > - return false; > + output->name); > + return NULL; > } > - data->output = output; > - return true; > + return output; > } > > /* > * Re-probe connectors and do a modeset with DSC > * > */ > -static void update_display(data_t *data, enum dsc_test_type test_type) > +static void update_display(data_t *data, enum dsc_test_type test_type, > + unsigned int drm_format) indentation missing > { > bool enabled; > igt_plane_t *primary; > @@ -194,7 +208,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > igt_output_set_pipe(data->output, PIPE_NONE); > igt_display_commit(&data->display); > > - igt_debug("DSC is supported on %s\n", data->conn_name); > + igt_debug("DSC is supported on %s\n", data->output->name); > save_force_dsc_en(data); > force_dsc_enable(data); > if (test_type == test_dsc_compression_bpp) { > @@ -203,8 +217,15 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > } > > igt_output_set_pipe(data->output, data->pipe); > + qsort(data->output->config.connector->modes, > + data->output->config.connector->count_modes, > + sizeof(drmModeModeInfo), > + sort_drm_modes); indentation missing > + igt_output_override_mode(data->output, &data->output->config.connector->modes[0]); > primary = igt_output_get_plane_type(data->output, > DRM_PLANE_TYPE_PRIMARY); > + igt_skip_on_f(!igt_plane_has_format_mod(primary, drm_format, DRM_FORMAT_MOD_LINEAR), > + "plane %s format does not have support \n", igt_format_str(drm_format)); > > /* Now set the output to the desired mode */ > igt_plane_set_fb(primary, &data->fb_test_pattern); > @@ -219,52 +240,59 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > enabled = igt_is_dsc_enabled(data->drm_fd, > data->output->config.connector); > restore_force_dsc_en(); > - if (test_type == test_dsc_compression_bpp) { > - igt_debug("Rest compression BPP \n"); > - data->compression_bpp = 0; > - force_dsc_enable_bpp(data); > - } > + > + igt_debug("Reset compression BPP \n"); > + data->compression_bpp = 0; > + force_dsc_enable_bpp(data); > > igt_assert_f(enabled, > "Default DSC enable failed on Connector: %s Pipe: %s\n", > - data->conn_name, > + data->output->name, > kmstest_pipe_name(data->pipe)); > } > > -static void run_test(data_t *data, enum dsc_test_type test_type) > +static void run_test(data_t *data, enum dsc_test_type test_type, > + unsigned int drm_format) > { > enum pipe pipe; > + int i=0; > char test_name[10]; > - drmModeModeInfo *mode = igt_output_get_mode(data->output); > > - igt_create_pattern_fb(data->drm_fd, mode->hdisplay, > - mode->vdisplay, > - DRM_FORMAT_XRGB8888, > + igt_skip_on_f (test_type == test_dsc_compression_bpp && > + data->output->config.connector->modes[0].hdisplay >= 5120, > + "bigjoiner does not support force bpp\n"); > + same here > + igt_create_pattern_fb(data->drm_fd, > + data->output->config.connector->modes[0].hdisplay, > + data->output->config.connector->modes[0].vdisplay, > + drm_format, > DRM_FORMAT_MOD_LINEAR, > &data->fb_test_pattern); > > for_each_pipe(&data->display, pipe) { > - uint32_t devid = intel_get_drm_devid(data->drm_fd); > + if (i++ < (data->n_pipes - 1)) { > + uint32_t devid = intel_get_drm_devid(data->drm_fd); > > - if (data->output->config.connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort && > - pipe == PIPE_A && IS_GEN11(devid)) { > - igt_debug("DSC not supported on Pipe A on external DP in Gen11 platforms\n"); > - continue; > - } > + if (data->output->config.connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort && > + pipe == PIPE_A && IS_GEN11(devid)) { > + igt_debug("DSC not supported on Pipe A on external DP in Gen11 platforms\n"); > + continue; > + } > > - snprintf(test_name, sizeof(test_name), "-%dbpp", data->compression_bpp); > - if (igt_pipe_connector_valid(pipe, data->output)) { > - data->pipe = pipe; > + snprintf(test_name, sizeof(test_name), "-%dbpp", data->compression_bpp); > + if (igt_pipe_connector_valid(pipe, data->output)) { > + data->pipe = pipe; > > - igt_dynamic_f("%s-pipe-%s%s", data->output->name, > - kmstest_pipe_name(pipe), > - (test_type == test_dsc_compression_bpp) ? > - test_name : "") > - update_display(data, test_type); > - } > + igt_dynamic_f("%s-pipe-%s%s", data->output->name, > + kmstest_pipe_name(pipe), > + (test_type == test_dsc_compression_bpp) ? > + test_name : "") > + update_display(data, test_type, drm_format); > + } > > - if (test_type == test_dsc_compression_bpp) > - break; > + if (test_type == test_dsc_compression_bpp) > + break; > + } > } > > igt_remove_fb(data->drm_fd, &data->fb_test_pattern); > @@ -275,7 +303,17 @@ igt_main > data_t data = {}; > drmModeRes *res; > drmModeConnector *connector = NULL; > - int i, j; > + igt_output_t *outputs[IGT_MAX_PIPES] = {}; > + igt_output_t *output; > + int i, j, supported_connectors = 0; > + struct { > + unsigned int bpc; > + int format; > + char format_str[20]; > + } test_list[] = { > + {8, DRM_FORMAT_XRGB8888, "XRGB8888"}, > + {10, DRM_FORMAT_XRGB2101010, "XRGB2101010"}, > + }; > igt_fixture { > data.drm_fd = drm_open_driver_master(DRIVER_INTEL); > data.devid = intel_get_drm_devid(data.drm_fd); > @@ -283,14 +321,30 @@ igt_main > igt_install_exit_handler(kms_dsc_exit_handler); > igt_display_require(&data.display, data.drm_fd); > igt_require(res = drmModeGetResources(data.drm_fd)); > - } > - igt_subtest_with_dynamic("basic-dsc-enable") { > for (j = 0; j < res->count_connectors; j++) { > - if (!check_dsc_on_connector(&data, res->connectors[j])) > - continue; > - run_test(&data, test_basic_dsc_enable); > + output = check_dsc_on_connector(&data, res->connectors[j]); > + > + if (output) > + outputs[supported_connectors++] = output; > } > + igt_require_f(supported_connectors, "No DSC supported panel connected.\n"); > + data.n_pipes = 0; > + for_each_pipe(&data.display, i) > + data.n_pipes++; > + > } > + > + for (i=0; i < ARRAY_SIZE(test_list); i++) { > + igt_subtest_with_dynamic_f("basic-dsc-enable-%s", > + test_list[i].format_str) { > + for (j = 0; j < supported_connectors; j++) { > + data.output = outputs[j]; > + run_test(&data, test_basic_dsc_enable, > + test_list[i].format); > + } > + } > + } > + > /* currently we are validating compression bpp on XRGB8888 format only */ > igt_subtest_with_dynamic("XRGB8888-dsc-compression") { > uint32_t bpp_list[] = { > @@ -302,13 +356,12 @@ igt_main > > igt_require(intel_display_ver(data.devid) >= 13); > > - for (j = 0; j < res->count_connectors; j++) { > - if (!check_dsc_on_connector(&data, res->connectors[j])) > - continue; > - > + for (j = 0; j < supported_connectors; j++) { > + data.output = outputs[j]; > for (i = 0; i < ARRAY_SIZE(bpp_list); i++) { > data.compression_bpp = bpp_list[i]; > - run_test(&data, test_dsc_compression_bpp); > + run_test(&data, test_dsc_compression_bpp, > + DRM_FORMAT_XRGB8888); > } > } > } > -- ~Swati Sharma