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 DE700C0755A for ; Mon, 27 Nov 2023 16:55:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AA9CB10E3B0; Mon, 27 Nov 2023 16:55:14 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id D049510E3B0 for ; Mon, 27 Nov 2023 16:55:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701104113; x=1732640113; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=D8Dhy6YT4T9o047LwwIwcENmD6s7zwUPpP1Rk+1S8Io=; b=fRPASDdy/zILnfbflg5YMMB/cwHkCZXHkei7nWGi+CJzw1tMEPLPCZ4z m+Y0Odyy2VmJ1tchDKvXIYpCZhpIVcCWpwLr2rNDqAMWcfp96jGxk96ga fI5rFmBxP4IeDpgXnh+qV9GMyqmDXdtglFXDoOZBaCQPLPdyqvPCvJ00f 3xZOx4Qs5QzWDm/YvMnqe5f/UT0S8MtMO+KFXFNDtq3yrbArFXDkC7iAp UTD3CjRRjOYGnWh1G5HFwXMhGpVp4mdlkTnOAECfvv9oW3IeI3j1su6NH QzMlkg5P34zjxLQf69YkikbxG2XFd7byjAB5OxPIQATWdWlOND8LngPHI A==; X-IronPort-AV: E=McAfee;i="6600,9927,10907"; a="5995190" X-IronPort-AV: E=Sophos;i="6.04,231,1695711600"; d="scan'208";a="5995190" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2023 08:55:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.04,231,1695711600"; d="scan'208";a="9672585" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.104.148]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2023 08:55:12 -0800 Date: Mon, 27 Nov 2023 08:36:57 -0800 Message-ID: <87edgbywye.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Riana Tauro In-Reply-To: References: <20231124122755.91279-1-sujaritha.sundaresan@intel.com> <20231124122755.91279-3-sujaritha.sundaresan@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 2/2] drm/xe: Add vram frequency sysfs attributes 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: Sujaritha Sundaresan , intel-xe@lists.freedesktop.org, rodrigo.vivi@intel.com Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Mon, 27 Nov 2023 02:00:10 -0800, Riana Tauro wrote: > > Hi Suja > > There is an error on module unload. > > [ 1042.525614] ------------[ cut here ]------------ > [ 1042.525622] kernfs: can not remove 'physical_vram_size_bytes', no > directory > [ 1042.525641] WARNING: CPU: 3 PID: 2234 at fs/kernfs/dir.c:1662 > kernfs_remove_by_name_ns+0xb3/0xc0 > [ 1042.525965] Call Trace: > [ 1042.525970] > [ 1042.525976] ? __warn+0xa5/0x200 > [ 1042.525986] ? kernfs_remove_by_name_ns+0xb3/0xc0 > [ 1042.525996] ? report_bug+0x216/0x220 > [ 1042.526011] ? handle_bug+0x3c/0x70 > [ 1042.526018] ? exc_invalid_op+0x18/0x50 > [ 1042.526025] ? asm_exc_invalid_op+0x1a/0x20 > [ 1042.526033] ? __pfx_tile_sysfs_fini+0x10/0x10 [xe] > [ 1042.526275] ? irq_work_claim+0x1e/0x40 > [ 1042.526288] ? kernfs_remove_by_name_ns+0xb3/0xc0 > [ 1042.526298] ? kernfs_remove_by_name_ns+0xb3/0xc0 > [ 1042.526310] tile_sysfs_fini+0x1e/0x40 [xe] > [ 1042.526515] drm_managed_release+0x117/0x250 [drm] > [ 1042.526626] drm_dev_release+0x49/0x60 [drm] > [ 1042.526723] release_nodes+0x59/0x190 > [ 1042.526731] ? lockdep_hardirqs_on_prepare+0x136/0x210 > [ 1042.526739] ? _raw_spin_unlock_irqrestore+0x51/0x70 > [ 1042.526752] devres_release_all+0xf8/0x140 > [ 1042.526761] ? __pfx_devres_release_all+0x10/0x10 > [ 1042.526779] device_unbind_cleanup+0x16/0xc0 > [ 1042.526788] device_release_driver_internal+0x10d/0x160 > [ 1042.526799] unbind_store+0x98/0xa0 > [ 1042.526809] ? __pfx_sysfs_kf_write+0x10/0x10 > [ 1042.526815] kernfs_fop_write_iter+0x1bc/0x260 > [ 1042.526828] vfs_write+0x553/0x770 > [ 1042.526839] ? __pfx_vfs_write+0x10/0x10 > [ 1042.526850] ? do_sys_openat2+0x266/0x350 > [ 1042.526867] ? __fget_light+0x9e/0x100 > [ 1042.526882] ksys_write+0xc7/0x170 > [ 1042.526889] ? __pfx_ksys_write+0x10/0x10 > [ 1042.526895] ? mark_held_locks+0x24/0x90 > [ 1042.526906] ? lockdep_hardirqs_on_prepare+0x136/0x210 > [ 1042.526920] do_syscall_64+0x3c/0x90 > [ 1042.526929] entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Thanks Riana. Hi Suja, Let's follow the steps below to test these sysfs patches before posting them: * Make sure sysfs entries appear in the correct directory * Make sure read/write works as expected (using cat/echo) * After doing these things please unload the driver and make sure the driver can be unloaded without crashing. Then reload the driver and test again. Check dmesg after each of these steps (leave 'dmesg -w' running in a separate console window) to see if there's any sign of crash or kernel oops. Thanks, Ashutosh > > > On 11/24/2023 5:57 PM, Sujaritha Sundaresan wrote: > > Add vram frequency sysfs attributes under the below hierarchy; > > > > /device/tile/memory/freq > > |-vram_rp0_freq > > |-vram_rpn_freq > > > > Signed-off-by: Sujaritha Sundaresan > > --- > > drivers/gpu/drm/xe/xe_pcode_api.h | 8 +++ > > drivers/gpu/drm/xe/xe_tile_sysfs.c | 78 +++++++++++++++++++++++++++++- > > 2 files changed, 84 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h > > index 5935cfe30204..edde5335bdb1 100644 > > --- a/drivers/gpu/drm/xe/xe_pcode_api.h > > +++ b/drivers/gpu/drm/xe/xe_pcode_api.h > > @@ -42,6 +42,14 @@ > > #define POWER_SETUP_I1_SHIFT 6 /* 10.6 fixed point format */ > > #define POWER_SETUP_I1_DATA_MASK REG_GENMASK(15, 0) > > +#define XEHP_PCODE_FREQUENCY_CONFIG 0x6e /* > > xehp, pvc */ > > +/* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */ > > +#define PCODE_MBOX_FC_SC_READ_FUSED_P0 0x0 > > +#define PCODE_MBOX_FC_SC_READ_FUSED_PN 0x1 > > +/* PCODE_MBOX_DOMAIN_* - mailbox domain IDs */ > > +/* XEHP_PCODE_FREQUENCY_CONFIG param2 */ > > +#define PCODE_MBOX_DOMAIN_HBM 0x2 > > + > > struct pcode_err_decode { > > int errno; > > const char *str; > > diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c b/drivers/gpu/drm/xe/xe_tile_sysfs.c > > index f354c8b2bfc6..ddf5072c40eb 100644 > > --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c > > +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c > > @@ -7,9 +7,14 @@ > > #include > > #include > > +#include "xe_gt_types.h" > > +#include "xe_pcode.h" > > +#include "xe_pcode_api.h" > > #include "xe_tile.h" > > #include "xe_tile_sysfs.h" > > +#define GT_FREQUENCY_MULTIPLIER 50 > > + > > static void xe_tile_sysfs_kobj_release(struct kobject *kobj) > > { > > kfree(kobj); > > @@ -35,11 +40,72 @@ static DEVICE_ATTR_RO(physical_vram_size_bytes); > > static const struct attribute *physical_memsize_attr = > > &dev_attr_physical_vram_size_bytes.attr; > > +static ssize_t vram_rp0_freq_show(struct device *kdev, struct > > device_attribute *attr, > > + char *buf) > > +{ > > + struct kobject *kobj = &kdev->kobj; > > + struct xe_tile *tile = kobj_to_tile(kobj->parent); > > + struct xe_gt *gt = tile->primary_gt; > > + u32 val, mbox; > > + int err; > > + > > + mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, XEHP_PCODE_FREQUENCY_CONFIG) > > + | REG_FIELD_PREP(PCODE_MB_PARAM1, PCODE_MBOX_FC_SC_READ_FUSED_P0) > > + | REG_FIELD_PREP(PCODE_MB_PARAM2, PCODE_MBOX_DOMAIN_HBM); > > + > > + err = xe_pcode_read(gt, mbox, &val, NULL); > > + if (err) > > + return err; > > + > > + /* data_out - Fused P0 for domain ID in units of 50 MHz */ > > + val *= GT_FREQUENCY_MULTIPLIER; > > + > > + return sysfs_emit(buf, "%u\n", val); > > +} > > +static DEVICE_ATTR_RO(vram_rp0_freq); > > + > > +static ssize_t vram_rpn_freq_show(struct device *kdev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct kobject *kobj = &kdev->kobj; > > + struct xe_tile *tile = kobj_to_tile(kobj->parent); > > + struct xe_gt *gt = tile->primary_gt; > > + u32 val, mbox; > > + int err; > > + > > + mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, XEHP_PCODE_FREQUENCY_CONFIG) > > + | REG_FIELD_PREP(PCODE_MB_PARAM1, PCODE_MBOX_FC_SC_READ_FUSED_PN) > > + | REG_FIELD_PREP(PCODE_MB_PARAM2, PCODE_MBOX_DOMAIN_HBM); > > + > > + err = xe_pcode_read(gt, mbox, &val, NULL); > > + if (err) > > + return err; > > + > > + /* data_out - Fused Pn for domain ID in units of 50 MHz */ > > + val *= GT_FREQUENCY_MULTIPLIER; > > + > > + return sysfs_emit(buf, "%u\n", val); > > +} > > +static DEVICE_ATTR_RO(vram_rpn_freq); > > + > > +static struct attribute *vram_freq_attrs[] = { > > + &dev_attr_vram_rp0_freq.attr, > > + &dev_attr_vram_rpn_freq.attr, > > + NULL > > +}; > > + > > +static const struct attribute_group freq_group_attrs = { > > + .name = "freq", > > + .attrs = vram_freq_attrs, > > +}; > > + > > static void tile_sysfs_fini(struct drm_device *drm, void *arg) > > { > > - struct xe_tile *tile = arg; > > + struct kobject *kobj = arg; > > - kobject_put(tile->sysfs); > > + sysfs_remove_file(kobj, physical_memsize_attr); > > + sysfs_remove_group(kobj, &freq_group_attrs); > > + kobject_put(kobj); > > } > > void xe_tile_sysfs_init(struct xe_tile *tile) > > @@ -77,6 +143,14 @@ void xe_tile_sysfs_init(struct xe_tile *tile) > > drm_warn(&xe->drm, > > "Sysfs creation to read addr_range per tile failed\n"); > > + if (xe->info.platform == XE_PVC) { > > + err = sysfs_create_group(kobj, &freq_group_attrs); > indentation > Also we are using two different methods of creating subdir in a single > file. Should be uniform > > Thanks > Riana > > + if (err) { > > + drm_warn(&xe->drm, "failed to register vram freq sysfs, err: %d\n", err); > > + return; > > + } > > + } > > + > > err = drmm_add_action_or_reset(&xe->drm, tile_sysfs_fini, tile); > > if (err) { > > drm_warn(&xe->drm, "%s: drmm_add_action_or_reset failed, err: %d\n",