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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 479B6C001DE for ; Fri, 11 Aug 2023 03:56:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D27FA10E63A; Fri, 11 Aug 2023 03:56:47 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0859110E63A for ; Fri, 11 Aug 2023 03:56:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1691726206; x=1723262206; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=uk5s8AafIFSuQ9XoVlRbNWos7kMqJXDR/sR+QeIoyRI=; b=XbWZhU0+NaapBWo1laKl1OFO+Wn0+wkquR4qEIsO24apVzndS4z9rSNr DXudPnWHIEfkaM2LgY2gUvPhCozn7Db1AZ2HvZGymsfn+dkTHbNarI8C2 6xUU/mayGf+ldxXlL9cL9hYMAn3yCleb/2sOzKM0tpi5txpv8eelospwQ qfyNY8w5N0vb1xZeiSkj/cXfqf9dSYNvJkhq1wo8uFoxkPqrFzpxGPh1q 54RoutxTxQwzDAMxOeTqbG7C9A0PMf/rkbCmUKgwe1rnjo5kxxUi8me2t l+aKvklu0678j6Xfwxpb6RQXHZnbpCuVD04SCAMQA9IcCVJDga8GBclpv Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10798"; a="435480980" X-IronPort-AV: E=Sophos;i="6.01,164,1684825200"; d="scan'208";a="435480980" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Aug 2023 20:56:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10798"; a="906298021" X-IronPort-AV: E=Sophos;i="6.01,164,1684825200"; d="scan'208";a="906298021" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.101.232]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Aug 2023 20:56:45 -0700 Date: Thu, 10 Aug 2023 20:38:45 -0700 Message-ID: <875y5m1c56.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Rodrigo Vivi In-Reply-To: References: <20230808115436.400611-1-aravind.iddamsetty@intel.com> <20230808115436.400611-3-aravind.iddamsetty@intel.com> <877cq4y8s3.wl-ashutosh.dixit@intel.com> <95750d7f-df24-a9c5-ab39-86db69767bac@intel.com> <87msyz39iv.wl-ashutosh.dixit@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-xe] [PATCH v3 2/2] drm/xe/pmu: Enable PMU interface X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Bommu Krishnaiah , intel-xe@lists.freedesktop.org, Tvrtko Ursulin Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 10 Aug 2023 14:55:41 -0700, Rodrigo Vivi wrote: Hi Rodrigo/Aravind, > On Thu, Aug 10, 2023 at 01:40:16PM +0530, Iddamsetty, Aravind wrote: > > On 10-08-2023 08:10, Dixit, Ashutosh wrote: > > > > > On Wed, 09 Aug 2023 06:11:48 -0700, Iddamsetty, Aravind wrote: > > > > > >> On 09-08-2023 17:27, Iddamsetty, Aravind wrote: > > >>> On 09-08-2023 15:25, Iddamsetty, Aravind wrote: > > >>>> On 09-08-2023 12:58, Dixit, Ashutosh wrote: > > >>>>> On Tue, 08 Aug 2023 04:54:36 -0700, Aravind Iddamsetty wrote: > > >>>>> > > >>>>> Spotted a few remaining things. See if it's possible to fix these up and > > >>>>> send another version. > > >>>>> > > >>>>>> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c > > >>>>>> new file mode 100644 > > >>>>>> index 000000000000..9637f8283641 > > >>>>>> --- /dev/null > > >>>>>> +++ b/drivers/gpu/drm/xe/xe_pmu.c > > >>>>>> @@ -0,0 +1,673 @@ > > >>>> > > >>>> > > >>>>>> +static u64 __engine_group_busyness_read(struct xe_gt *gt, int sample_type) > > >>>>>> +{ > > >>>>>> + u64 val = 0; > > >>>>>> + > > >>>>> > > >>>>> What is the forcewake domain for these registers? Don't we need to get > > >>>>> forcewake before reading these. Something like: > > >>>>> > > >>>>> XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)); > > >>>> > > >>>> based on BSPEC:67609 these belong to GT power domain, so acquiring that > > >>>> should be sufficient. > > >>> > > >>> But if i understand correctly taking forcewake is not allowed here as it > > >>> is an atomic context and forcewake can sleep and that is what I'm seeing > > >>> as well, might also be the reason why i915 didn't do that as well. > > >>> > > >>> [ 899.114316] BUG: sleeping function called from invalid context at > > >>> kernel/locking/mutex.c:580 > > >>> [ 899.115768] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: > > >>> 290, name: kworker/27:1 > > >> > > >> that is the reason why in i915 we were doing similar thing of storing > > >> the counter as we enter rc6, not sure how do we do that in xe. > > > > > > Just to check, which code path(s) is/are aotmic context: > > > > > > a. xe_pm_suspend > > > b. xe_pm_runtime_suspend > > > c. xe_pmu_event_read > > > > pmu_event_read and runtime_suspend are atomic contexts. > > what about doing this at xe_pci_runtime_idle() ? > > This will run after the autosuspend time elapses, > but before calling any suspend. Also, there's no requirement of > that function to be in atomic context. So you could forcewake_get/put > and stash your registers before we go runtime_suspend. Thanks for the suggestion. Though Anshuman was saying that rpm_suspend callback itself is not called in atomic context, Aravind seems to have made a mistake. Aravind could you please confirm? In any case there seems to be a way out here, we should work to save off the registers while suspending in either the idle or suspend callbacks. > > > Now I am wondering if GuC should provide these counters too along with > > > other busyness values it provides, since GuC is what control RC6 > > > entry/exit. But let's try to understand the issue some more first. > > > > do you mean GuC reading these registers and presenting us in a way, will > > need to think over how does it fit in the PMU. I think better to leave GuC out since it's a long process to modify the GuC API. So let's cancel that. About xe_pmu_event_read being atomic context, since the registers might be getting updated while xe_pmu_event_read calls are happening, the only way out I am seeing is to run a kthread (specifically a delayed work item) while PMU is active. The work item will run every 10 ms or so and save off the registers (since we cannot take forcewake in xe_pmu_event_read and read the registers). So this way we should be able to report register values which are at most 10 ms old. Any other ideas here? Thanks. -- Ashutosh > > >>>>> > > >>>>>> + switch (sample_type) { > > >>>>>> + case __XE_SAMPLE_RENDER_GROUP_BUSY: > > >>>>>> + val = xe_mmio_read32(gt, XE_OAG_RENDER_BUSY_FREE); > > >>>>>> + break; > > >>>>>> + case __XE_SAMPLE_COPY_GROUP_BUSY: > > >>>>>> + val = xe_mmio_read32(gt, XE_OAG_BLT_BUSY_FREE); > > >>>>>> + break; > > >>>>>> + case __XE_SAMPLE_MEDIA_GROUP_BUSY: > > >>>>>> + val = xe_mmio_read32(gt, XE_OAG_ANY_MEDIA_FF_BUSY_FREE); > > >>>>>> + break; > > >>>>>> + case __XE_SAMPLE_ANY_ENGINE_GROUP_BUSY: > > >>>>>> + val = xe_mmio_read32(gt, XE_OAG_RC0_ANY_ENGINE_BUSY_FREE); > > >>>>>> + break; > > >>>>>> + default: > > >>>>>> + drm_warn(>->tile->xe->drm, "unknown pmu event\n"); > > >>>>>> + } > > >>>>> > > >>>>> And similarly here: > > >>>>> > > >>>>> XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));