From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2053.outbound.protection.outlook.com [40.107.237.53]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9E4B710E337 for ; Mon, 4 Apr 2022 18:05:18 +0000 (UTC) Message-ID: <9aff75af-8b59-e7d4-ad48-8791d9789270@amd.com> Date: Mon, 4 Apr 2022 14:05:07 -0400 Content-Language: en-US To: David Zhang , igt-dev@lists.freedesktop.org References: <20220404161955.4087858-1-dingchen.zhang@amd.com> From: Rodrigo Siqueira Jordao In-Reply-To: <20220404161955.4087858-1-dingchen.zhang@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH] lib/igt_amd: return negative if PSR state debugfs open fails List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 2022-04-04 12:19, David Zhang wrote: > [why & how] > The PSR state read helper should return an integer which is not > the same as PSR state enumeration item for the case of debugfs > interface opening fail. Currently it return false which is casted > to 0 that is the same as PSR_STATE0, this is incorrect. > > Instead of returning 0, a negative (e.g. -1) value is returned > when debugfs interface of PSR state opening fails. And adding the > check of such negative value in amd_psr test case as well. > > Cc: Rodrigo Siqueira > Cc: Harry Wentland > Cc: Leo Li > Cc: Aurabindo Pillai > Cc: Wayne Lin > > Signed-off-by: David Zhang > --- > lib/igt_amd.c | 2 +- > tests/amdgpu/amd_psr.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/igt_amd.c b/lib/igt_amd.c > index 888da44a..664602da 100644 > --- a/lib/igt_amd.c > +++ b/lib/igt_amd.c > @@ -1064,7 +1064,7 @@ int igt_amd_read_psr_state(int drm_fd, char *connector_name) > fd = igt_debugfs_connector_dir(drm_fd, connector_name, O_RDONLY); > if (fd < 0) { > igt_info("Couldn't open connector %s debugfs directory\n", connector_name); > - return false; > + return -1; > } > > ret = igt_debugfs_simple_read(fd, DEBUGFS_EDP_PSR_STATE, buf, sizeof(buf)); > diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c > index b9d8a53b..d21d41e3 100644 > --- a/tests/amdgpu/amd_psr.c > +++ b/tests/amdgpu/amd_psr.c > @@ -179,6 +179,7 @@ static void run_check_psr(data_t *data, bool test_null_crtc) { > continue; > > psr_state = igt_amd_read_psr_state(data->fd, output->name); > + igt_fail_on_f(psr_state < 0, "Open PSR state debugfs failed\n"); > igt_fail_on_f(psr_state < 1, "PSR was not enabled for connector %s\n", output->name); > igt_fail_on_f(psr_state == 0xff, "PSR is invalid for connector %s\n", output->name); > igt_fail_on_f(psr_state != 5, "PSR state is expected to be at 5 on a " > @@ -295,6 +296,7 @@ static void run_check_psr_su_mpo(data_t *data) > /* check PSR state */ > if (i > PSR_SETTLE_DELAY * frame_rate) { > psr_state = igt_amd_read_psr_state(data->fd, data->output->name); > + igt_fail_on_f(psr_state < 0, "Open PSR state debugfs failed\n"); > igt_fail_on_f(psr_state == PSR_STATE0, > "PSR was not enabled for connector %s\n", data->output->name); > igt_fail_on_f(psr_state == PSR_STATE_INVALID, Reviewed-by: Rodrigo Siqueria