diff mbox

nouveau: add coherent BO attribute

Message ID 1426228051-20298-1-git-send-email-acourbot@nvidia.com
State Deferred
Headers show

Commit Message

Alexandre Courbot March 13, 2015, 6:27 a.m. UTC
Add a flag allowing Nouveau to specify that an object should be coherent
at allocation time. This is required for some class of objects like
fences which are randomly-accessed by both the CPU and GPU. This flag
instructs the kernel driver to make sure the object remains coherent
even on architectures for which coherency is not guaranteed by the bus.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 include/drm/nouveau_drm.h | 1 +
 nouveau/abi16.c           | 3 +++
 nouveau/nouveau.h         | 1 +
 3 files changed, 5 insertions(+)

Comments

Ilia Mirkin March 13, 2015, 6:36 a.m. UTC | #1
Doesn't this require a kernel version that has your other patch? What
happens when this runs on an older kernel? Does it get silently
ignored, or does it end up erroring out? If it errors out, that's
fine. Otherwise some sort of version check should be put in, no?

On Fri, Mar 13, 2015 at 2:27 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Add a flag allowing Nouveau to specify that an object should be coherent
> at allocation time. This is required for some class of objects like
> fences which are randomly-accessed by both the CPU and GPU. This flag
> instructs the kernel driver to make sure the object remains coherent
> even on architectures for which coherency is not guaranteed by the bus.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  include/drm/nouveau_drm.h | 1 +
>  nouveau/abi16.c           | 3 +++
>  nouveau/nouveau.h         | 1 +
>  3 files changed, 5 insertions(+)
>
> diff --git a/include/drm/nouveau_drm.h b/include/drm/nouveau_drm.h
> index b18cad02419b..87aefc5e9d2f 100644
> --- a/include/drm/nouveau_drm.h
> +++ b/include/drm/nouveau_drm.h
> @@ -96,6 +96,7 @@ struct drm_nouveau_setparam {
>  #define NOUVEAU_GEM_DOMAIN_VRAM      (1 << 1)
>  #define NOUVEAU_GEM_DOMAIN_GART      (1 << 2)
>  #define NOUVEAU_GEM_DOMAIN_MAPPABLE  (1 << 3)
> +#define NOUVEAU_GEM_DOMAIN_COHERENT  (1 << 4)
>
>  #define NOUVEAU_GEM_TILE_LAYOUT_MASK 0x0000ff00
>  #define NOUVEAU_GEM_TILE_16BPP       0x00000001
> diff --git a/nouveau/abi16.c b/nouveau/abi16.c
> index ae13821bc0cc..d2d1d0d1942d 100644
> --- a/nouveau/abi16.c
> +++ b/nouveau/abi16.c
> @@ -195,6 +195,9 @@ abi16_bo_init(struct nouveau_bo *bo, uint32_t alignment,
>         if (bo->flags & NOUVEAU_BO_MAP)
>                 info->domain |= NOUVEAU_GEM_DOMAIN_MAPPABLE;
>
> +       if (bo->flags & NOUVEAU_BO_COHERENT)
> +               info->domain |= NOUVEAU_GEM_DOMAIN_COHERENT;
> +
>         if (!(bo->flags & NOUVEAU_BO_CONTIG))
>                 info->tile_flags = NOUVEAU_GEM_TILE_NONCONTIG;
>
> diff --git a/nouveau/nouveau.h b/nouveau/nouveau.h
> index a55e2b020778..4adda0e3594c 100644
> --- a/nouveau/nouveau.h
> +++ b/nouveau/nouveau.h
> @@ -127,6 +127,7 @@ union nouveau_bo_config {
>  #define NOUVEAU_BO_MAP     0x80000000
>  #define NOUVEAU_BO_CONTIG  0x40000000
>  #define NOUVEAU_BO_NOSNOOP 0x20000000
> +#define NOUVEAU_BO_COHERENT 0x10000000
>
>  struct nouveau_bo {
>         struct nouveau_device *device;
> --
> 2.3.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot March 13, 2015, 6:39 a.m. UTC | #2
On Fri, Mar 13, 2015 at 3:36 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> Doesn't this require a kernel version that has your other patch? What
> happens when this runs on an older kernel? Does it get silently
> ignored, or does it end up erroring out? If it errors out, that's
> fine. Otherwise some sort of version check should be put in, no?

The corresponding kernel patch is already merged in Ben's tree. If
running with an older kernel, this flag will be a no-op, which is fine
since GK20A's GPU (the reason for this patch as you have guessed :))
is not enabled for these kernels.

I am fine with adding a version check if you think it is necessary.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot March 13, 2015, 9:45 a.m. UTC | #3
On Fri, Mar 13, 2015 at 3:39 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Fri, Mar 13, 2015 at 3:36 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> Doesn't this require a kernel version that has your other patch? What
>> happens when this runs on an older kernel? Does it get silently
>> ignored, or does it end up erroring out? If it errors out, that's
>> fine. Otherwise some sort of version check should be put in, no?
>
> The corresponding kernel patch is already merged in Ben's tree. If
> running with an older kernel, this flag will be a no-op, which is fine
> since GK20A's GPU (the reason for this patch as you have guessed :))
> is not enabled for these kernels.
>
> I am fine with adding a version check if you think it is necessary.

So as discussed on IRC, I will check the DRM version from Mesa. This
patch should be good to go - it will require a version increase in
libdrm though. Let me know if you want me to send a patch for this
too.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maarten Lankhorst March 13, 2015, 7:33 p.m. UTC | #4
Hey,

Op 13-03-15 om 07:27 schreef Alexandre Courbot:
> Add a flag allowing Nouveau to specify that an object should be coherent
> at allocation time. This is required for some class of objects like
> fences which are randomly-accessed by both the CPU and GPU. This flag
> instructs the kernel driver to make sure the object remains coherent
> even on architectures for which coherency is not guaranteed by the bus.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
I don't see a problem with this patch, but similar patches to intel to libdrm have been shot down when the changes weren't in an official kernel yet, so I think this should wait until the change is at least in drm-next. ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot March 15, 2015, 8:41 a.m. UTC | #5
On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
> Hey,
>
> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>> Add a flag allowing Nouveau to specify that an object should be coherent
>> at allocation time. This is required for some class of objects like
>> fences which are randomly-accessed by both the CPU and GPU. This flag
>> instructs the kernel driver to make sure the object remains coherent
>> even on architectures for which coherency is not guaranteed by the bus.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> I don't see a problem with this patch, but similar patches to intel to libdrm have been shot down when the changes weren't in an official kernel yet, so I think this should wait until the change is at least in drm-next. ;-)

Sounds good. I will ping you again once the kernel change reaches -next.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot April 22, 2015, 9:08 a.m. UTC | #6
On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>
>> Hey,
>>
>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>
>>> Add a flag allowing Nouveau to specify that an object should be coherent
>>> at allocation time. This is required for some class of objects like
>>> fences which are randomly-accessed by both the CPU and GPU. This flag
>>> instructs the kernel driver to make sure the object remains coherent
>>> even on architectures for which coherency is not guaranteed by the bus.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>
>> I don't see a problem with this patch, but similar patches to intel to
>> libdrm have been shot down when the changes weren't in an official kernel
>> yet, so I think this should wait until the change is at least in drm-next.
>> ;-)
>
>
> Sounds good. I will ping you again once the kernel change reaches -next.

Hi Marteen,

The kernel change required for this patch is now in -next. Do you
think we can merge it now?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot May 15, 2015, 7:11 a.m. UTC | #7
Re-pinging Marteen on an email address that still exists :P

On Wed, Apr 22, 2015 at 6:08 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>>
>>> Hey,
>>>
>>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>>
>>>> Add a flag allowing Nouveau to specify that an object should be coherent
>>>> at allocation time. This is required for some class of objects like
>>>> fences which are randomly-accessed by both the CPU and GPU. This flag
>>>> instructs the kernel driver to make sure the object remains coherent
>>>> even on architectures for which coherency is not guaranteed by the bus.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>
>>> I don't see a problem with this patch, but similar patches to intel to
>>> libdrm have been shot down when the changes weren't in an official kernel
>>> yet, so I think this should wait until the change is at least in drm-next.
>>> ;-)
>>
>>
>> Sounds good. I will ping you again once the kernel change reaches -next.
>
> Hi Marteen,
>
> The kernel change required for this patch is now in -next. Do you
> think we can merge it now?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maarten Lankhorst May 15, 2015, 11:39 a.m. UTC | #8
Op 15-05-15 om 09:11 schreef Alexandre Courbot:
> Re-pinging Marteen on an email address that still exists :P
>
> On Wed, Apr 22, 2015 at 6:08 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>>> Hey,
>>>>
>>>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>>> Add a flag allowing Nouveau to specify that an object should be coherent
>>>>> at allocation time. This is required for some class of objects like
>>>>> fences which are randomly-accessed by both the CPU and GPU. This flag
>>>>> instructs the kernel driver to make sure the object remains coherent
>>>>> even on architectures for which coherency is not guaranteed by the bus.
>>>>>
>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> I don't see a problem with this patch, but similar patches to intel to
>>>> libdrm have been shot down when the changes weren't in an official kernel
>>>> yet, so I think this should wait until the change is at least in drm-next.
>>>> ;-)
>>>
>>> Sounds good. I will ping you again once the kernel change reaches -next.
>> Hi Marteen,
>>
>> The kernel change required for this patch is now in -next. Do you
>> think we can merge it now?
I think it would be ok to merge now.

~Maarten
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot May 20, 2015, 5:11 a.m. UTC | #9
On Fri, May 15, 2015 at 8:39 PM, Maarten Lankhorst
<maarten@mblankhorst.nl> wrote:
> Op 15-05-15 om 09:11 schreef Alexandre Courbot:
>> Re-pinging Marteen on an email address that still exists :P
>>
>> On Wed, Apr 22, 2015 at 6:08 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>> On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>>>> Hey,
>>>>>
>>>>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>>>> Add a flag allowing Nouveau to specify that an object should be coherent
>>>>>> at allocation time. This is required for some class of objects like
>>>>>> fences which are randomly-accessed by both the CPU and GPU. This flag
>>>>>> instructs the kernel driver to make sure the object remains coherent
>>>>>> even on architectures for which coherency is not guaranteed by the bus.
>>>>>>
>>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>> I don't see a problem with this patch, but similar patches to intel to
>>>>> libdrm have been shot down when the changes weren't in an official kernel
>>>>> yet, so I think this should wait until the change is at least in drm-next.
>>>>> ;-)
>>>>
>>>> Sounds good. I will ping you again once the kernel change reaches -next.
>>> Hi Marteen,
>>>
>>> The kernel change required for this patch is now in -next. Do you
>>> think we can merge it now?
> I think it would be ok to merge now.

Great - who could do this? :P
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin Peres May 20, 2015, 6:53 a.m. UTC | #10
On 20/05/15 08:11, Alexandre Courbot wrote:
> On Fri, May 15, 2015 at 8:39 PM, Maarten Lankhorst
> <maarten@mblankhorst.nl> wrote:
>> Op 15-05-15 om 09:11 schreef Alexandre Courbot:
>>> Re-pinging Marteen on an email address that still exists :P
>>>
>>> On Wed, Apr 22, 2015 at 6:08 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>> On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>>> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>>>>> Hey,
>>>>>>
>>>>>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>>>>> Add a flag allowing Nouveau to specify that an object should be coherent
>>>>>>> at allocation time. This is required for some class of objects like
>>>>>>> fences which are randomly-accessed by both the CPU and GPU. This flag
>>>>>>> instructs the kernel driver to make sure the object remains coherent
>>>>>>> even on architectures for which coherency is not guaranteed by the bus.
>>>>>>>
>>>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>>> I don't see a problem with this patch, but similar patches to intel to
>>>>>> libdrm have been shot down when the changes weren't in an official kernel
>>>>>> yet, so I think this should wait until the change is at least in drm-next.
>>>>>> ;-)
>>>>> Sounds good. I will ping you again once the kernel change reaches -next.
>>>> Hi Marteen,
>>>>
>>>> The kernel change required for this patch is now in -next. Do you
>>>> think we can merge it now?
>> I think it would be ok to merge now.
> Great - who could do this? :P

I could do it. Please provide me with the patch with the necessary R-b 
and I can push it to our libdrm (and/or mesa).
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot May 21, 2015, 6:09 a.m. UTC | #11
On Wed, May 20, 2015 at 3:53 PM, Martin Peres <martin.peres@free.fr> wrote:
> On 20/05/15 08:11, Alexandre Courbot wrote:
>>
>> On Fri, May 15, 2015 at 8:39 PM, Maarten Lankhorst
>> <maarten@mblankhorst.nl> wrote:
>>>
>>> Op 15-05-15 om 09:11 schreef Alexandre Courbot:
>>>>
>>>> Re-pinging Marteen on an email address that still exists :P
>>>>
>>>> On Wed, Apr 22, 2015 at 6:08 PM, Alexandre Courbot <gnurou@gmail.com>
>>>> wrote:
>>>>>
>>>>> On Sun, Mar 15, 2015 at 5:41 PM, Alexandre Courbot
>>>>> <acourbot@nvidia.com> wrote:
>>>>>>
>>>>>> On 03/14/2015 04:33 AM, Maarten Lankhorst wrote:
>>>>>>>
>>>>>>> Hey,
>>>>>>>
>>>>>>> Op 13-03-15 om 07:27 schreef Alexandre Courbot:
>>>>>>>>
>>>>>>>> Add a flag allowing Nouveau to specify that an object should be
>>>>>>>> coherent
>>>>>>>> at allocation time. This is required for some class of objects like
>>>>>>>> fences which are randomly-accessed by both the CPU and GPU. This
>>>>>>>> flag
>>>>>>>> instructs the kernel driver to make sure the object remains coherent
>>>>>>>> even on architectures for which coherency is not guaranteed by the
>>>>>>>> bus.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>>>>
>>>>>>> I don't see a problem with this patch, but similar patches to intel
>>>>>>> to
>>>>>>> libdrm have been shot down when the changes weren't in an official
>>>>>>> kernel
>>>>>>> yet, so I think this should wait until the change is at least in
>>>>>>> drm-next.
>>>>>>> ;-)
>>>>>>
>>>>>> Sounds good. I will ping you again once the kernel change reaches
>>>>>> -next.
>>>>>
>>>>> Hi Marteen,
>>>>>
>>>>> The kernel change required for this patch is now in -next. Do you
>>>>> think we can merge it now?
>>>
>>> I think it would be ok to merge now.
>>
>> Great - who could do this? :P
>
>
> I could do it. Please provide me with the patch with the necessary R-b and I
> can push it to our libdrm (and/or mesa).

Thanks, I just sent a v2 with you included. I have yet to receive
formal R-b and A-b for it though.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/drm/nouveau_drm.h b/include/drm/nouveau_drm.h
index b18cad02419b..87aefc5e9d2f 100644
--- a/include/drm/nouveau_drm.h
+++ b/include/drm/nouveau_drm.h
@@ -96,6 +96,7 @@  struct drm_nouveau_setparam {
 #define NOUVEAU_GEM_DOMAIN_VRAM      (1 << 1)
 #define NOUVEAU_GEM_DOMAIN_GART      (1 << 2)
 #define NOUVEAU_GEM_DOMAIN_MAPPABLE  (1 << 3)
+#define NOUVEAU_GEM_DOMAIN_COHERENT  (1 << 4)
 
 #define NOUVEAU_GEM_TILE_LAYOUT_MASK 0x0000ff00
 #define NOUVEAU_GEM_TILE_16BPP       0x00000001
diff --git a/nouveau/abi16.c b/nouveau/abi16.c
index ae13821bc0cc..d2d1d0d1942d 100644
--- a/nouveau/abi16.c
+++ b/nouveau/abi16.c
@@ -195,6 +195,9 @@  abi16_bo_init(struct nouveau_bo *bo, uint32_t alignment,
 	if (bo->flags & NOUVEAU_BO_MAP)
 		info->domain |= NOUVEAU_GEM_DOMAIN_MAPPABLE;
 
+	if (bo->flags & NOUVEAU_BO_COHERENT)
+		info->domain |= NOUVEAU_GEM_DOMAIN_COHERENT;
+
 	if (!(bo->flags & NOUVEAU_BO_CONTIG))
 		info->tile_flags = NOUVEAU_GEM_TILE_NONCONTIG;
 
diff --git a/nouveau/nouveau.h b/nouveau/nouveau.h
index a55e2b020778..4adda0e3594c 100644
--- a/nouveau/nouveau.h
+++ b/nouveau/nouveau.h
@@ -127,6 +127,7 @@  union nouveau_bo_config {
 #define NOUVEAU_BO_MAP     0x80000000
 #define NOUVEAU_BO_CONTIG  0x40000000
 #define NOUVEAU_BO_NOSNOOP 0x20000000
+#define NOUVEAU_BO_COHERENT 0x10000000
 
 struct nouveau_bo {
 	struct nouveau_device *device;