From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: References: <20200220174155.2162242-1-chris@chris-wilson.co.uk> From: Martin Peres Message-ID: <023b88f4-7b98-a376-aee1-db09cec21a98@linux.intel.com> Date: Fri, 21 Feb 2020 09:33:59 +0200 MIME-Version: 1.0 In-Reply-To: <20200220174155.2162242-1-chris@chris-wilson.co.uk> Content-Language: en-US Subject: Re: [igt-dev] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: igt-dev@lists.freedesktop.org List-ID: On 2020-02-20 19:41, Chris Wilson wrote: > Since we check before and then after each debugfs entry, we do not need > to check before each time as well. We will error out as soon as it does > fail, at all other times we know the system to be idle. > > No impact on runtime for glk (which apparently is one of the better > behaving systems). > > Signed-off-by: Chris Wilson > Cc: Martin Peres I don't like this patch because the first read might not have the gpu suspended, and there shouldn't be much overhead in checking twice rather than once. What's your rationale here? To me, the issue is that some platforms suspend in milliseconds while some take seconds, and that might be indicative a real bug in the driver. Martin > --- > tests/i915/i915_pm_rpm.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c > index 0c2821122..bf412b5cc 100644 > --- a/tests/i915/i915_pm_rpm.c > +++ b/tests/i915/i915_pm_rpm.c > @@ -932,9 +932,6 @@ static int read_entry(const char *filepath, > int fd; > int rc; > > - igt_assert_f(wait_for_suspended(), "Before opening: %s (%s)\n", > - filepath + pathinfo->base, filepath); > - > fd = open(filepath, O_RDONLY | O_NONBLOCK); > if (fd < 0) { > igt_debug("Failed to open '%s': %m\n", filepath); > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 76D27C35640 for ; Fri, 21 Feb 2020 07:34:05 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 52D6C20801 for ; Fri, 21 Feb 2020 07:34:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52D6C20801 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 919156EEAD; Fri, 21 Feb 2020 07:34:04 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 386366EEAC; Fri, 21 Feb 2020 07:34:03 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Feb 2020 23:34:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,467,1574150400"; d="scan'208";a="315982253" Received: from linux.intel.com ([10.54.29.200]) by orsmga001.jf.intel.com with ESMTP; 20 Feb 2020 23:34:02 -0800 Received: from [10.237.72.173] (mperes-desk.fi.intel.com [10.237.72.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by linux.intel.com (Postfix) with ESMTPS id 6D8005802C8; Thu, 20 Feb 2020 23:34:01 -0800 (PST) To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <20200220174155.2162242-1-chris@chris-wilson.co.uk> From: Martin Peres Message-ID: <023b88f4-7b98-a376-aee1-db09cec21a98@linux.intel.com> Date: Fri, 21 Feb 2020 09:33:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200220174155.2162242-1-chris@chris-wilson.co.uk> Content-Language: en-US Subject: Re: [Intel-gfx] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 2020-02-20 19:41, Chris Wilson wrote: > Since we check before and then after each debugfs entry, we do not need > to check before each time as well. We will error out as soon as it does > fail, at all other times we know the system to be idle. > > No impact on runtime for glk (which apparently is one of the better > behaving systems). > > Signed-off-by: Chris Wilson > Cc: Martin Peres I don't like this patch because the first read might not have the gpu suspended, and there shouldn't be much overhead in checking twice rather than once. What's your rationale here? To me, the issue is that some platforms suspend in milliseconds while some take seconds, and that might be indicative a real bug in the driver. Martin > --- > tests/i915/i915_pm_rpm.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c > index 0c2821122..bf412b5cc 100644 > --- a/tests/i915/i915_pm_rpm.c > +++ b/tests/i915/i915_pm_rpm.c > @@ -932,9 +932,6 @@ static int read_entry(const char *filepath, > int fd; > int rc; > > - igt_assert_f(wait_for_suspended(), "Before opening: %s (%s)\n", > - filepath + pathinfo->base, filepath); > - > fd = open(filepath, O_RDONLY | O_NONBLOCK); > if (fd < 0) { > igt_debug("Failed to open '%s': %m\n", filepath); > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx