Nouveau - Sep 2016 - [PATCH 2/2] nouveau: Fix compile error due to bad include

This reverts commit 01bbcb69f80e1058395b737ae399c6f4ef48691b.

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.

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

Cought while working on travis-ci integration

Signed-off-by: Karol Herbst <karolherbst at gmail.com>
---
 drm/nouveau/uapi/drm/nouveau_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h
b/drm/nouveau/uapi/drm/nouveau_drm.h
index 259588a..0bfaae8 100644
--- a/drm/nouveau/uapi/drm/nouveau_drm.h
+++ b/drm/nouveau/uapi/drm/nouveau_drm.h
@@ -27,7 +27,7 @@
 
 #define DRM_NOUVEAU_EVENT_NVIF                                       0x80000000
 
-#include "drm.h"
+#include "drm/drm.h"
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.10.0

Every other *_drm.h does #include "drm.h". Something else is wrong.

On Sun, Sep 18, 2016 at 6:21 AM, Karol Herbst <karolherbst at gmail.com>
wrote:
> Cought while working on travis-ci integration
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> ---
>  drm/nouveau/uapi/drm/nouveau_drm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h
b/drm/nouveau/uapi/drm/nouveau_drm.h
> index 259588a..0bfaae8 100644
> --- a/drm/nouveau/uapi/drm/nouveau_drm.h
> +++ b/drm/nouveau/uapi/drm/nouveau_drm.h
> @@ -27,7 +27,7 @@
>
>  #define DRM_NOUVEAU_EVENT_NVIF                                      
0x80000000
>
> -#include "drm.h"
> +#include "drm/drm.h"
>
>  #if defined(__cplusplus)
>  extern "C" {
> --
> 2.10.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

On Sun, Sep 18, 2016 at 7:21 PM, Karol Herbst <karolherbst at gmail.com>
wrote:
> This reverts commit 01bbcb69f80e1058395b737ae399c6f4ef48691b.
>
> 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>
>
> 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

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

On 18 September 2016 at 11:21, Karol Herbst <karolherbst at gmail.com>
wrote:
> Cought while working on travis-ci integration
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> ---
>  drm/nouveau/uapi/drm/nouveau_drm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h
b/drm/nouveau/uapi/drm/nouveau_drm.h
> index 259588a..0bfaae8 100644
> --- a/drm/nouveau/uapi/drm/nouveau_drm.h
> +++ b/drm/nouveau/uapi/drm/nouveau_drm.h
> @@ -27,7 +27,7 @@
>
>  #define DRM_NOUVEAU_EVENT_NVIF                                      
0x80000000
>
> -#include "drm.h"
> +#include "drm/drm.h"
>
Afaict this is a side effect of an "incomplete sync" with the kernel
code. In there we have the respective header alongside the
nouveau_drm.h one.

One workaround is to track drm.h in the nouveau repo, with another to
adjust KBUILD_OPTS to honour the upstream one.

KBUILD_OPTS += KCPPFLAGS="-I$(LINUXDIR)/include/uapi/drm"

The above (untested) sounds like the better solution imho.

Regards,
Emil