Ilia Mirkin
2016-Sep-19 03:43 UTC
[Nouveau] [PATCH 1/2] Revert "bus: remove cpu_coherent flag"
On Sun, Sep 18, 2016 at 11:39 PM, Alexandre Courbot <gnurou at gmail.com> wrote:> On Sun, Sep 18, 2016 at 7:21 PM, Karol Herbst <karolherbst at gmail.com> wrote: >> This reverts commit 01bbcb69f80e1058395b737ae399c6f4ef48691b.I think you meant aff51175cdbf345740ec9203eff88e772af88059 - that's the commit id that matters, not the one in Ben's non-linux repo.>> >> The commit caused fence timeouts within nvc0_screen_destroy and most likely >> other places as well. >> >> The most obvious effect is, that userspace processes take minutes to actually >> quit. > > Acked-by: Alexandre Courbot <acourbot at nvidia.com>If this doesn't make it into v4.8 (which is due to be released in a week), it should get tagged for stable as well. By the way, Alexandre, I believe Martin was reporting issues like this on his TK1 before this change as well. Probably worth investigating - sounds like there's a wider issue with TTM_PL_FLAG_UNCACHED and/or force_coherent.> >> >> Signed-off-by: Karol Herbst <karolherbst at gmail.com> >> --- >> drm/nouveau/include/nvkm/core/device.h | 1 + >> drm/nouveau/nouveau_bo.c | 3 ++- >> drm/nouveau/nvkm/engine/device/pci.c | 1 + >> drm/nouveau/nvkm/engine/device/tegra.c | 1 + >> lib/include/nvif/os.h | 6 ++++++ >> 5 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drm/nouveau/include/nvkm/core/device.h b/drm/nouveau/include/nvkm/core/device.h >> index 7ea8aa7..6bc712f 100644 >> --- a/drm/nouveau/include/nvkm/core/device.h >> +++ b/drm/nouveau/include/nvkm/core/device.h >> @@ -175,6 +175,7 @@ struct nvkm_device_func { >> void (*fini)(struct nvkm_device *, bool suspend); >> resource_size_t (*resource_addr)(struct nvkm_device *, unsigned bar); >> resource_size_t (*resource_size)(struct nvkm_device *, unsigned bar); >> + bool cpu_coherent; >> }; >> >> struct nvkm_device_quirk { >> diff --git a/drm/nouveau/nouveau_bo.c b/drm/nouveau/nouveau_bo.c >> index 528bdef..6887d0a 100644 >> --- a/drm/nouveau/nouveau_bo.c >> +++ b/drm/nouveau/nouveau_bo.c >> @@ -209,7 +209,8 @@ nouveau_bo_new(struct drm_device *dev, int size, int align, >> nvbo->tile_flags = tile_flags; >> nvbo->bo.bdev = &drm->ttm.bdev; >> >> - nvbo->force_coherent = flags & TTM_PL_FLAG_UNCACHED; >> + if (!nvxx_device(&drm->device)->func->cpu_coherent) >> + nvbo->force_coherent = flags & TTM_PL_FLAG_UNCACHED; >> >> nvbo->page_shift = 12; >> if (drm->client.vm) { >> diff --git a/drm/nouveau/nvkm/engine/device/pci.c b/drm/nouveau/nvkm/engine/device/pci.c >> index b1b6932..62ad030 100644 >> --- a/drm/nouveau/nvkm/engine/device/pci.c >> +++ b/drm/nouveau/nvkm/engine/device/pci.c >> @@ -1614,6 +1614,7 @@ nvkm_device_pci_func = { >> .fini = nvkm_device_pci_fini, >> .resource_addr = nvkm_device_pci_resource_addr, >> .resource_size = nvkm_device_pci_resource_size, >> + .cpu_coherent = !IS_ENABLED(CONFIG_ARM), >> }; >> >> int >> diff --git a/drm/nouveau/nvkm/engine/device/tegra.c b/drm/nouveau/nvkm/engine/device/tegra.c >> index 939682f..9b638bd 100644 >> --- a/drm/nouveau/nvkm/engine/device/tegra.c >> +++ b/drm/nouveau/nvkm/engine/device/tegra.c >> @@ -245,6 +245,7 @@ nvkm_device_tegra_func = { >> .fini = nvkm_device_tegra_fini, >> .resource_addr = nvkm_device_tegra_resource_addr, >> .resource_size = nvkm_device_tegra_resource_size, >> + .cpu_coherent = false, >> }; >> >> int >> diff --git a/lib/include/nvif/os.h b/lib/include/nvif/os.h >> index b45a186..2f34c5a 100644 >> --- a/lib/include/nvif/os.h >> +++ b/lib/include/nvif/os.h >> @@ -124,6 +124,12 @@ typedef dma_addr_t resource_size_t; >> #define __printf(a,b) >> #define __user >> >> +#if defined(CONFIG_ARM) >> +#define IS_ENABLED_CONFIG_ARM 1 >> +#else >> +#define IS_ENABLED_CONFIG_ARM 0 >> +#endif >> + >> #if defined(CONFIG_IOMMU_API) >> #define IS_ENABLED_CONFIG_IOMMU_API 1 >> #else >> -- >> 2.10.0 >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Alexandre Courbot
2016-Sep-19 03:50 UTC
[Nouveau] [PATCH 1/2] Revert "bus: remove cpu_coherent flag"
On Mon, Sep 19, 2016 at 12:43 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:> On Sun, Sep 18, 2016 at 11:39 PM, Alexandre Courbot <gnurou at gmail.com> wrote: >> On Sun, Sep 18, 2016 at 7:21 PM, Karol Herbst <karolherbst at gmail.com> wrote: >>> This reverts commit 01bbcb69f80e1058395b737ae399c6f4ef48691b. > > I think you meant aff51175cdbf345740ec9203eff88e772af88059 - that's > the commit id that matters, not the one in Ben's non-linux repo. > >>> >>> The commit caused fence timeouts within nvc0_screen_destroy and most likely >>> other places as well. >>> >>> The most obvious effect is, that userspace processes take minutes to actually >>> quit. >> >> Acked-by: Alexandre Courbot <acourbot at nvidia.com> > > If this doesn't make it into v4.8 (which is due to be released in a > week), it should get tagged for stable as well. > > By the way, Alexandre, I believe Martin was reporting issues like this > on his TK1 before this change as well. Probably worth investigating - > sounds like there's a wider issue with TTM_PL_FLAG_UNCACHED and/or > force_coherent.Yes, it may be indeed related. I need to sync with Martin to understand his exact repro case.
Martin Peres
2016-Sep-19 06:57 UTC
[Nouveau] [PATCH 1/2] Revert "bus: remove cpu_coherent flag"
On 19/09/16 06:50, Alexandre Courbot wrote:> On Mon, Sep 19, 2016 at 12:43 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote: >> On Sun, Sep 18, 2016 at 11:39 PM, Alexandre Courbot <gnurou at gmail.com> wrote: >>> On Sun, Sep 18, 2016 at 7:21 PM, Karol Herbst <karolherbst at gmail.com> wrote: >>>> This reverts commit 01bbcb69f80e1058395b737ae399c6f4ef48691b. >> I think you meant aff51175cdbf345740ec9203eff88e772af88059 - that's >> the commit id that matters, not the one in Ben's non-linux repo. >> >>>> The commit caused fence timeouts within nvc0_screen_destroy and most likely >>>> other places as well. >>>> >>>> The most obvious effect is, that userspace processes take minutes to actually >>>> quit. >>> Acked-by: Alexandre Courbot <acourbot at nvidia.com> >> If this doesn't make it into v4.8 (which is due to be released in a >> week), it should get tagged for stable as well. >> >> By the way, Alexandre, I believe Martin was reporting issues like this >> on his TK1 before this change as well. Probably worth investigating - >> sounds like there's a wider issue with TTM_PL_FLAG_UNCACHED and/or >> force_coherent. > Yes, it may be indeed related. I need to sync with Martin to > understand his exact repro case.Running piglit on the tk1 is enough for me, but I will try to update the kernel again to see if it helps. The issue is that mesa is waiting on a fence that never gets synced on in the gbm destructor. The probably of having the timeout is about 50/50, so it is easy-enough to reproduce in my case :s Martin