From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id D541710E5BA for ; Tue, 10 Jan 2023 13:06:01 +0000 (UTC) Message-ID: <9e10dbb2-eaf3-6bbe-7461-b34fe818fbd9@intel.com> Date: Tue, 10 Jan 2023 18:35:40 +0530 To: Anshuman Gupta , , , , References: <20230109120205.2259410-1-anshuman.gupta@intel.com> <20230109120205.2259410-3-anshuman.gupta@intel.com> Content-Language: en-US From: "Tauro, Riana" In-Reply-To: <20230109120205.2259410-3-anshuman.gupta@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t 2/2] tests/device_reset: Add warm_reset test List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: Hi Anshuman, On 1/9/2023 5:32 PM, Anshuman Gupta wrote: > Adding gfx card root port warm reset support > by triggering secondary bus reset on root port. > > Signed-off-by: Anshuman Gupta > --- > lib/igt_pci.c | 28 ++++++++++++++++++++++++++++ > lib/igt_pci.h | 5 +++++ > tests/device_reset.c | 18 +++++++++++++++--- > 3 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/lib/igt_pci.c b/lib/igt_pci.c > index 61aaf939d1..3940a0ee1e 100644 > --- a/lib/igt_pci.c > +++ b/lib/igt_pci.c > @@ -51,3 +51,31 @@ int find_pci_cap_offset(struct pci_device *dev, enum pci_cap_id cap_id) > { > return find_pci_cap_offset_at(dev, cap_id, PCI_CAPS_START); > } > + > +/** > + * igt_pci_bridge_warm_reset: > + * @dev: pci bridge device > + * > + * return: > + * true on success otherwise false. > + */ > +bool igt_pci_bridge_warm_reset(struct pci_device *dev) > +{ > + uint8_t header_type; > + uint16_t bridge_ctl; > + > + if (pci_device_cfg_read_u8(dev, &header_type, PCI_HEADER_TYPE)) > + return false; > + > + igt_assert_f((header_type & 0x7f) == 1, "PCI device is not a Bridge\n"); > + > + if (pci_device_cfg_read_u16(dev, &bridge_ctl, PCI_TYPE_1_HEADER_BRIDGE_CTL)) > + return false; > + > + bridge_ctl |= PCI_TYPE_1_SEC_BUS_RESET; > + > + if (pci_device_cfg_write_u16(dev, bridge_ctl, PCI_TYPE_1_HEADER_BRIDGE_CTL)) > + return false; > + The pci code adds a minimum reset duration (Trst) delay after setting the bit. Do we need to add the same here? https://elixir.bootlin.com/linux/latest/source/drivers/pci/pci.c#L5045 Is restoring sequence not necessary? Thanks Riana > + return true; > +} > diff --git a/lib/igt_pci.h b/lib/igt_pci.h > index 92b9cc3929..f755e4baab 100644 > --- a/lib/igt_pci.h > +++ b/lib/igt_pci.h > @@ -12,6 +12,10 @@ > /* forward declaration */ > struct pci_device; > > +#define PCI_HEADER_TYPE 0xe > +#define PCI_TYPE_1_HEADER_BRIDGE_CTL 0x3e > +#define PCI_TYPE_1_SEC_BUS_RESET (1 << 6) > + > #define PCI_TYPE0_1_HEADER_SIZE 0x40 > #define PCI_CAPS_START 0x34 > #define PCI_CFG_SPACE_SIZE 0x100 > @@ -24,5 +28,6 @@ enum pci_cap_id { > #define PCI_SLOT_PWR_CTRL_PRESENT (1 << 1) > > int find_pci_cap_offset(struct pci_device *dev, enum pci_cap_id cap_id); > +bool igt_pci_bridge_warm_reset(struct pci_device *dev); > > #endif > diff --git a/tests/device_reset.c b/tests/device_reset.c > index fe26030783..cbb5bef443 100644 > --- a/tests/device_reset.c > +++ b/tests/device_reset.c > @@ -22,8 +22,9 @@ IGT_TEST_DESCRIPTION("Examine behavior of a driver on device sysfs reset"); > #define DEV_BUS_ADDR_LEN 13 /* addr has form 0000:00:00.0 */ > > enum reset { > - COLD_RESET, > - FLR_RESET > + FLR_RESET, > + WARM_RESET, > + COLD_RESET > }; > > /** > @@ -39,6 +40,7 @@ struct device_data { > } fds; > char dev_bus_addr[DEV_BUS_ADDR_LEN]; > bool snd_unload; > + struct pci_device *root; > }; > > static int __open_sysfs_dir(int fd, const char* path) > @@ -336,6 +338,8 @@ static void initiate_device_reset(struct device_data *dev, enum reset type) > > if (type == FLR_RESET) { > igt_assert(igt_sysfs_set(dev->fds.dev_dir, "reset", "1")); > + } else if (type == WARM_RESET) { > + igt_assert(igt_pci_bridge_warm_reset(dev->root)); > } else if (type == COLD_RESET) { > igt_assert(igt_sysfs_set(dev->fds.slot_dir, "power", "0")); > igt_assert(!igt_sysfs_get_boolean(dev->fds.slot_dir, "power")); > @@ -404,7 +408,7 @@ static void unbind_reset_rebind(struct device_data *dev, enum reset type) > > igt_main > { > - struct device_data dev = { .fds = {-1, -1, -1}, .dev_bus_addr = {0}, }; > + struct device_data dev = { .fds = {-1, -1, -1}, .dev_bus_addr = {0}, .root = NULL}; > > igt_fixture { > char dev_path[PATH_MAX]; > @@ -417,6 +421,7 @@ igt_main > set_device_filter(dev_path); > > igt_skip_on(!is_sysfs_reset_supported(dev.fds.dev)); > + dev.root = igt_device_get_pci_root_port(dev.fds.dev); > } > > igt_describe("Unbinds driver from device, initiates reset" > @@ -432,6 +437,13 @@ igt_main > healthcheck(&dev); > } > > + igt_describe("Unbinds driver from device, initiates warm reset" > + " then rebinds driver to device"); > + igt_subtest("unbind-warm-reset-rebind") { > + unbind_reset_rebind(&dev, WARM_RESET); > + healthcheck(&dev); > + } > + > igt_subtest_group { > igt_fixture { > igt_skip_on_f(dev.fds.slot_dir < 0, "Gfx Card does not support any "