Re: [Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 09:34:26AM +0200, Mika Westerberg wrote: > On Fri, Feb 11, 2022 at 03:45:46PM -0600, Bjorn Helgaas wrote: > > My expectation is that "USB" (like "PCI" and "PCIe") tells me > > something about how a device is electrically connected and how > > software can operate it. It doesn't really tell me anything about > > whether those electrical connections are permanent, made through an > > internal slot, or made through an external connector and cable. > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > DisplayPort). Tunnels are created by software (in Linux it is the > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > USB Type-C cable which also is something user can plug/unplug freely. > > I would say it is reasonable expectation that anything behind these > ports can be assumed as "removable". USB gadgets may be soldered to the mainboard. Those cannot be unplugged freely. It is common practice to solder USB Ethernet or USB FTDI serial ports and nothing's preventing a vendor to solder USB4/Thunderbolt gadgets.
Re: [Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 09:52:02AM +0100, Lukas Wunner wrote: > On Mon, Feb 14, 2022 at 09:34:26AM +0200, Mika Westerberg wrote: > > On Fri, Feb 11, 2022 at 03:45:46PM -0600, Bjorn Helgaas wrote: > > > My expectation is that "USB" (like "PCI" and "PCIe") tells me > > > something about how a device is electrically connected and how > > > software can operate it. It doesn't really tell me anything about > > > whether those electrical connections are permanent, made through an > > > internal slot, or made through an external connector and cable. > > > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > > DisplayPort). Tunnels are created by software (in Linux it is the > > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > > USB Type-C cable which also is something user can plug/unplug freely. > > > > I would say it is reasonable expectation that anything behind these > > ports can be assumed as "removable". > > USB gadgets may be soldered to the mainboard. Those cannot be > unplugged freely. It is common practice to solder USB Ethernet > or USB FTDI serial ports and nothing's preventing a vendor to solder > USB4/Thunderbolt gadgets. Right, that's why I say it is "reasonable expectation" that anything behind these ports can be assumed "removable" :) Of course they don't have to be but if we assume that in the driver where this actually matters we should be on the safe side, no?
Re: [Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`
On Mon, Feb 14, 2022 at 01:11:05PM +0200, Mika Westerberg wrote: > > > It is used to identify "tunneled" ports (whether PCIe, USB 3.x or > > > DisplayPort). Tunnels are created by software (in Linux it is the > > > Thunderbolt driver) and are dynamic in nature. The USB4 links go over > > > USB Type-C cable which also is something user can plug/unplug freely. > > > > > > I would say it is reasonable expectation that anything behind these > > > ports can be assumed as "removable". > > > > USB gadgets may be soldered to the mainboard. Those cannot be > > unplugged freely. It is common practice to solder USB Ethernet > > or USB FTDI serial ports and nothing's preventing a vendor to solder > > USB4/Thunderbolt gadgets. > > Right, that's why I say it is "reasonable expectation" that anything > behind these ports can be assumed "removable" :) Of course they don't > have to be but if we assume that in the driver where this actually > matters we should be on the safe side, no? Also the tunnels are not permanent anyway.
Re: [Nouveau] [PATCH] drm/nouveau/bios: Use HWSQ entry 1 for PowerBook6, 1
I'm not saying this is wrong, but could you file a bug at gitlab.freedesktop.org/drm/nouveau/-/issues and include the VBIOS (/sys/kernel/debug/dri/0/vbios.rom)? That would make it easier to review the full situation. On Mon, Feb 14, 2022 at 11:03 AM Icenowy Zheng wrote: > > On PowerBook6,1 (PowerBook G4 867 12") HWSQ entry 0 (which is currently > always used by nouveau) fails, but the BIOS declares 2 HWSQ entries and > entry 1 works. > > Add a quirk to use HWSQ entry 1. > > Signed-off-by: Icenowy Zheng > --- > drivers/gpu/drm/nouveau/nouveau_bios.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c > b/drivers/gpu/drm/nouveau/nouveau_bios.c > index e8c445eb11004..2691d0e0cf9f1 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bios.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c > @@ -1977,6 +1977,13 @@ static int load_nv17_hw_sequencer_ucode(struct > drm_device *dev, > if (!hwsq_offset) > return 0; > > +#ifdef __powerpc__ > + /* HWSQ entry 0 fails on PowerBook G4 867 12" (Al) */ > + if (of_machine_is_compatible("PowerBook6,1")) > + return load_nv17_hwsq_ucode_entry(dev, bios, > + hwsq_offset + sz, 1); > +#endif > + > /* always use entry 0? */ > return load_nv17_hwsq_ucode_entry(dev, bios, hwsq_offset + sz, 0); > } > -- > 2.30.2 >
Re: [Nouveau] [PATCH 09/27] mm: generalize the pgmap based page_free infrastructure
On 2022-02-10 12:28 a.m., Christoph Hellwig wrote: > Key off on the existence of ->page_free to prepare for adding support for > more pgmap types that are device managed and thus need the free callback. > > Signed-off-by: Christoph Hellwig Great! This makes my patch simpler. Reviewed-by: Logan Gunthorpe > --- > mm/memremap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/memremap.c b/mm/memremap.c > index fef5734d5e4933..e00ffcdba7b632 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -452,7 +452,7 @@ EXPORT_SYMBOL_GPL(get_dev_pagemap); > > void free_zone_device_page(struct page *page) > { > - if (WARN_ON_ONCE(!is_device_private_page(page))) > + if (WARN_ON_ONCE(!page->pgmap->ops || !page->pgmap->ops->page_free)) > return; > > __ClearPageWaiters(page); > @@ -460,7 +460,7 @@ void free_zone_device_page(struct page *page) > mem_cgroup_uncharge(page_folio(page)); > > /* > - * When a device_private page is freed, the page->mapping field > + * When a device managed page is freed, the page->mapping field >* may still contain a (stale) mapping value. For example, the >* lower bits of page->mapping may still identify the page as an >* anonymous page. Ultimately, this entire field is just stale >
Re: [Nouveau] [PATCH v4 00/10] Overhaul `is_thunderbolt`
On Mon, Feb 14, 2022 at 06:01:50PM -0600, Mario Limonciello wrote: > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_vga.c | 4 +- > drivers/gpu/drm/radeon/radeon_device.c | 4 +- > drivers/gpu/drm/radeon/radeon_kms.c | 2 +- > drivers/pci/hotplug/pciehp_hpc.c| 6 +- > drivers/pci/pci-acpi.c | 15 - > drivers/pci/pci.c | 17 +++-- > drivers/pci/probe.c | 52 ++- > drivers/pci/quirks.c| 84 + > drivers/platform/x86/apple-gmux.c | 2 +- > drivers/thunderbolt/nhi.h | 2 - > include/linux/pci.h | 25 +--- > include/linux/pci_ids.h | 3 + > 14 files changed, 173 insertions(+), 47 deletions(-) That's an awful lot of additional LoC for what is primarily a refactoring job with the intent to simplify things. Honestly this looks like an attempt to fix something that isn't broken. Specifically, the is_thunderbolt bit apparently can't be removed without adding new bits to struct pci_dev. Not sure if that can be called progress. Thanks, Lukas