From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: References: <20200220174155.2162242-1-chris@chris-wilson.co.uk> <023b88f4-7b98-a376-aee1-db09cec21a98@linux.intel.com> <158227331733.3099.1298656919493160116@skylake-alporthouse-com> From: Martin Peres Message-ID: <37e6fded-72f3-6480-6ed8-6591c2d2733b@linux.intel.com> Date: Fri, 21 Feb 2020 10:28:16 +0200 MIME-Version: 1.0 In-Reply-To: <158227331733.3099.1298656919493160116@skylake-alporthouse-com> 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-21 10:21, Chris Wilson wrote: > Quoting Martin Peres (2020-02-21 07:33:59) >> 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? > > We always do a check before after each file. We start in a known state, > and expect to be able to return to that suspended state, and the _real_ > guts of the test is that any device access is accounted for. > > assert(suspended) would be a better check for non-interference. I would feel better with assert(suspended) added, but would it really speed anything up since I assume wait_for_suspended() should be instantaneous if we are already suspended, right? > >> 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. > > Exactly. Good to hear :) > -Chris > _______________________________________________ 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=-5.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, 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 B2602C35643 for ; Fri, 21 Feb 2020 08:28:24 +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 89F7724682 for ; Fri, 21 Feb 2020 08:28:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 89F7724682 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 C31566EEB6; Fri, 21 Feb 2020 08:28:22 +0000 (UTC) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id AD4886EEB5; Fri, 21 Feb 2020 08:28:20 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Feb 2020 00:28:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,467,1574150400"; d="scan'208";a="229777811" Received: from linux.intel.com ([10.54.29.200]) by orsmga008.jf.intel.com with ESMTP; 21 Feb 2020 00:28:20 -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 20B8C58056A; Fri, 21 Feb 2020 00:28:18 -0800 (PST) To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <20200220174155.2162242-1-chris@chris-wilson.co.uk> <023b88f4-7b98-a376-aee1-db09cec21a98@linux.intel.com> <158227331733.3099.1298656919493160116@skylake-alporthouse-com> From: Martin Peres Message-ID: <37e6fded-72f3-6480-6ed8-6591c2d2733b@linux.intel.com> Date: Fri, 21 Feb 2020 10:28:16 +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: <158227331733.3099.1298656919493160116@skylake-alporthouse-com> 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-21 10:21, Chris Wilson wrote: > Quoting Martin Peres (2020-02-21 07:33:59) >> 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? > > We always do a check before after each file. We start in a known state, > and expect to be able to return to that suspended state, and the _real_ > guts of the test is that any device access is accounted for. > > assert(suspended) would be a better check for non-interference. I would feel better with assert(suspended) added, but would it really speed anything up since I assume wait_for_suspended() should be instantaneous if we are already suspended, right? > >> 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. > > Exactly. Good to hear :) > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx