From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id CE58B10E314 for ; Mon, 11 Sep 2023 15:12:58 +0000 (UTC) Message-ID: <4368f552-353c-bb7b-32a5-768e9904d9b9@intel.com> Date: Mon, 11 Sep 2023 20:42:51 +0530 MIME-Version: 1.0 Content-Language: en-US To: "Modem, Bhanuprakash" , igt-dev@lists.freedesktop.org References: <20230908175039.1291593-1-swati2.sharma@intel.com> <20230908175039.1291593-5-swati2.sharma@intel.com> From: "Sharma, Swati2" In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [igt-dev] [PATCH i-g-t 4/5] tests/intel/kms_pm_dc: Use gem_has_lmem() List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 11-Sep-23 8:21 PM, Modem, Bhanuprakash wrote: > Hi Swati, > > On Fri-08-09-2023 11:20 pm, Swati Sharma wrote: >> Instead of using i915_capabilities debugfs to know if platform >> is discrete or not, use gem_has_lmem for the same. >> >> Signed-off-by: Swati Sharma >> --- >>   tests/intel/kms_pm_dc.c | 13 +++---------- >>   1 file changed, 3 insertions(+), 10 deletions(-) >> >> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c >> index b5779eaa5..cbb419e12 100644 >> --- a/tests/intel/kms_pm_dc.c >> +++ b/tests/intel/kms_pm_dc.c >> @@ -140,15 +140,6 @@ typedef struct { >>   static bool dc_state_wait_entry(int drm_fd, int dc_flag, int >> prev_dc_count); >>   static void check_dc_counter(data_t *data, int dc_flag, uint32_t >> prev_dc_count); >> -static bool is_dgfx(data_t *data) >> -{ >> -    char buf[4096]; >> - >> -    igt_debugfs_simple_read(data->debugfs_fd, "i915_capabilities", >> -                buf, sizeof(buf)); >> -    return strstr(buf, "is_dgfx: yes"); >> -} >> - >>   static void setup_output(data_t *data) >>   { >>       igt_display_t *display = &data->display; >> @@ -661,6 +652,7 @@ static void kms_poll_state_restore(int sig) >>   igt_main >>   { >>       data_t data = {}; >> +    bool is_dgfx; >>       igt_fixture { >>           data.drm_fd = drm_open_driver_master(DRIVER_INTEL); >> @@ -742,7 +734,8 @@ igt_main >>       igt_describe("This test validates display engine entry to DC9 >> state"); >>       igt_subtest("dc9-dpms") { >> -        if (!(is_dgfx(&data))) >> +        is_dgfx = gem_has_lmem(data.drm_fd); >> +        if (!is_dgfx) >> >> igt_require_f(igt_pm_pc8_plus_residencies_enabled(data.msr_fd), > > Patch LGTM, but looks we don't even need of the variable "is_dgfx", > instead you can adjust the igt_require_f(). > > If it's make sense, please fix the above comment. IMO, this looks okay for readability. > > Reviewed-by: Bhanuprakash Modem > > - Bhanu > >>                         "PC8+ residencies not supported\n"); >>           test_dc9_dpms(&data);