Nouveau - Sep 2015 - [PATCH mesa 3/4] nv30: Do not export msaa capabable visuals on nv3x

Hi All,

Here is a bunch of fixes for nv30 cards, the first patch is a resend of
a patch I send a while back. AFAICT that one is ready for merging, but
it is not entirely clear to me what the process is for getting (nouveau)
mesa patches merged.

Should I request commit rights, and push my own patches once they have
been reviewed ?

Regards,

Hans

Scanout buffers on nv30 must always be non-swizzled and have special
width alignment constraints.

These constrains have been taken from the xf86-video-nouveau
src/nv_accel_common.c: nouveau_allocate_surface() function.

nouveau_allocate_surface() applies these width constraints only when a
tiled attribute is set, which it sets for all surfaces allocated via
dri, and this "tiling" is not the same as swizzling, scanout surfaces
must be linear / have a uniform_pitch or only complete garbage is shown.

This commit fixes dri3 on nv30 showing a garbled display, with dri3 the
scanout buffers are allocated by mesa, rather then by the ddx, and the
wrong stride of these buffers was causing the garbled display.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 src/gallium/drivers/nouveau/nv30/nv30_miptree.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
index c75b4b9..2276347 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
@@ -28,6 +28,7 @@
 #include "util/u_surface.h"
 
 #include "nv_m2mf.xml.h"
+#include "nv_object.xml.h"
 #include "nv30/nv30_screen.h"
 #include "nv30/nv30_context.h"
 #include "nv30/nv30_resource.h"
@@ -362,6 +363,7 @@ nv30_miptree_create(struct pipe_screen *pscreen,
    blocksz = util_format_get_blocksize(pt->format);
 
    if ((pt->target == PIPE_TEXTURE_RECT) ||
+       (pt->bind & PIPE_BIND_SCANOUT) ||
        !util_is_power_of_two(pt->width0) ||
        !util_is_power_of_two(pt->height0) ||
        !util_is_power_of_two(pt->depth0) ||
@@ -369,6 +371,14 @@ nv30_miptree_create(struct pipe_screen *pscreen,
        util_format_is_float(pt->format) || mt->ms_mode) {
       mt->uniform_pitch = util_format_get_nblocksx(pt->format, w) *
blocksz;
       mt->uniform_pitch = align(mt->uniform_pitch, 64);
+      if (pt->bind & PIPE_BIND_SCANOUT) {
+         struct nv30_screen *screen = nv30_screen(pscreen);
+         int pitch_align = MAX2(
+               screen->eng3d->oclass >= NV40_3D_CLASS ? 1024 : 256,
+               /* round_down_pow2(mt->uniform_pitch / 4) */
+               1 << (util_last_bit(mt->uniform_pitch / 4) - 1));
+         mt->uniform_pitch = align(mt->uniform_pitch, pitch_align);
+      }
    }
 
    if (!mt->uniform_pitch)
-- 
2.4.3

Note this is not ideal. Since the sifm can only do source sizes upto
1024x1024 we end up using the blitter on nv4x, which is not that fast.

And on nv3x we end up using the cpu which is really slow.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 src/gallium/drivers/nouveau/nv30/nv30_miptree.c  | 19 ++++++++-----------
 src/gallium/drivers/nouveau/nv30/nv30_resource.h |  3 ---
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
index 2276347..76bb8b8 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
@@ -145,21 +145,18 @@ nv30_resource_copy_region(struct pipe_context *pipe,
    nv30_transfer_rect(nv30, NEAREST, &src, &dst);
 }
 
-void
-nv30_resource_resolve(struct pipe_context *pipe,
-                      const struct pipe_resolve_info *info)
+static void
+nv30_resource_resolve(struct nv30_context *nv30,
+                      const struct pipe_blit_info *info)
 {
-#if 0
-   struct nv30_context *nv30 = nv30_context(pipe);
    struct nv30_rect src, dst;
 
-   define_rect(info->src.res, 0, 0, info->src.x0, info->src.y0,
-               info->src.x1 - info->src.x0, info->src.y1 -
info->src.y0, &src);
-   define_rect(info->dst.res, info->dst.level, 0, info->dst.x0,
info->dst.y0,
-               info->dst.x1 - info->dst.x0, info->dst.y1 -
info->dst.y0, &dst);
+   define_rect(info->src.resource, 0, info->src.box.z,
info->src.box.x,
+      info->src.box.y, info->src.box.width, info->src.box.height,
&src);
+   define_rect(info->dst.resource, 0, info->dst.box.z,
info->dst.box.x,
+      info->dst.box.y, info->dst.box.width, info->dst.box.height,
&dst);
 
    nv30_transfer_rect(nv30, BILINEAR, &src, &dst);
-#endif
 }
 
 void
@@ -173,7 +170,7 @@ nv30_blit(struct pipe_context *pipe,
        info.dst.resource->nr_samples <= 1 &&
        !util_format_is_depth_or_stencil(info.src.resource->format)
&&
        !util_format_is_pure_integer(info.src.resource->format)) {
-      debug_printf("nv30: color resolve unimplemented\n");
+      nv30_resource_resolve(nv30, blit_info);
       return;
    }
 
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_resource.h
b/src/gallium/drivers/nouveau/nv30/nv30_resource.h
index 8dac779..20d86b6 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_resource.h
+++ b/src/gallium/drivers/nouveau/nv30/nv30_resource.h
@@ -66,9 +66,6 @@ nv30_resource_copy_region(struct pipe_context *pipe,
                           const struct pipe_box *src_box);
 
 void
-nv30_resource_resolve(struct pipe_context *, const struct pipe_resolve_info *);
-
-void
 nv30_blit(struct pipe_context *pipe,
           const struct pipe_blit_info *blit_info);
 
-- 
2.4.3

On nv3x we will likely end up using the cpu to do color resolving for msaa
blits. Disable msaa on these cards so that we do not end up using the cpu.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 src/gallium/drivers/nouveau/nv30/nv30_screen.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 7aad26b..69acc38 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -319,8 +319,16 @@ nv30_screen_is_format_supported(struct pipe_screen
*pscreen,
                                 unsigned sample_count,
                                 unsigned bindings)
 {
-   if (sample_count > 4)
-      return false;
+   struct nv30_screen *screen = nv30_screen(pscreen);
+
+   if (screen->eng3d->oclass >= NV40_3D_CLASS) {
+      if (sample_count > 4)
+         return false;
+   } else {
+      if (sample_count > 0)
+         return false;
+   }
+
    if (!(0x00000017 & (1 << sample_count)))
       return false;
 
-- 
2.4.3

On nv4x with a msaa visual after a while the gpu locks up, attach gdb to
impress shows it is hanging waiting for a fence which never comes.

Killing ooimpress at this point works exactly once, trying to do any
3d operations after killing impress will lockup the entire system.

This needs further investigation, but for now disable msaa because no
msaa is better then crashing the system.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 src/gallium/drivers/nouveau/nv30/nv30_screen.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 69acc38..57262b8 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -321,7 +321,8 @@ nv30_screen_is_format_supported(struct pipe_screen *pscreen,
 {
    struct nv30_screen *screen = nv30_screen(pscreen);
 
-   if (screen->eng3d->oclass >= NV40_3D_CLASS) {
+   /* MSAA visuals work somwhat on nv4x, but cause gpu lockups */
+   if (0 && screen->eng3d->oclass >= NV40_3D_CLASS) {
       if (sample_count > 4)
          return false;
    } else {
-- 
2.4.3

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> Hi All,
>
> Here is a bunch of fixes for nv30 cards, the first patch is a resend of
> a patch I send a while back. AFAICT that one is ready for merging, but
> it is not entirely clear to me what the process is for getting (nouveau)
> mesa patches merged.
>
> Should I request commit rights, and push my own patches once they have
> been reviewed ?
The usual process is for you to have a few patches accepted before
requesting commit access. I suppose I'm the de-facto nouveau mesa
maintainer nowadays, not a highly-contended position though.

I'll take a look at these soon, and if I should forget, feel free to
ping me early and often.

  -ilia

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> Scanout buffers on nv30 must always be non-swizzled and have special
> width alignment constraints.
>
> These constrains have been taken from the xf86-video-nouveau
> src/nv_accel_common.c: nouveau_allocate_surface() function.
>
> nouveau_allocate_surface() applies these width constraints only when a
> tiled attribute is set, which it sets for all surfaces allocated via
> dri, and this "tiling" is not the same as swizzling, scanout
surfaces
> must be linear / have a uniform_pitch or only complete garbage is shown.
I think the deal is that it's actually not tiling at all. There's no
"other" tiling (esp not on pre-tesla). Hence the confusion. The
nouveau driver does indeed do

PUSH_DATA (push, NV30_3D_RT_FORMAT_TYPE_LINEAR |
NV30_3D_RT_FORMAT_ZETA_Z24S8 | fmt->card_fmt);

without OR'ing in the log2(w/h). So this looks good. I wondered how it
worked at all with swizzling. I guess it didn't :)

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable at lists.freedesktop.org>

[by the way, having a git branch where I can pull these from would
definitely *not* slow down the speed with which I push these out to
master...]
>
> This commit fixes dri3 on nv30 showing a garbled display, with dri3 the
> scanout buffers are allocated by mesa, rather then by the ddx, and the
> wrong stride of these buffers was causing the garbled display.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/gallium/drivers/nouveau/nv30/nv30_miptree.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> index c75b4b9..2276347 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> @@ -28,6 +28,7 @@
>  #include "util/u_surface.h"
>
>  #include "nv_m2mf.xml.h"
> +#include "nv_object.xml.h"
>  #include "nv30/nv30_screen.h"
>  #include "nv30/nv30_context.h"
>  #include "nv30/nv30_resource.h"
> @@ -362,6 +363,7 @@ nv30_miptree_create(struct pipe_screen *pscreen,
>     blocksz = util_format_get_blocksize(pt->format);
>
>     if ((pt->target == PIPE_TEXTURE_RECT) ||
> +       (pt->bind & PIPE_BIND_SCANOUT) ||
>         !util_is_power_of_two(pt->width0) ||
>         !util_is_power_of_two(pt->height0) ||
>         !util_is_power_of_two(pt->depth0) ||
> @@ -369,6 +371,14 @@ nv30_miptree_create(struct pipe_screen *pscreen,
>         util_format_is_float(pt->format) || mt->ms_mode) {
>        mt->uniform_pitch = util_format_get_nblocksx(pt->format, w) *
blocksz;
>        mt->uniform_pitch = align(mt->uniform_pitch, 64);
> +      if (pt->bind & PIPE_BIND_SCANOUT) {
> +         struct nv30_screen *screen = nv30_screen(pscreen);
> +         int pitch_align = MAX2(
> +               screen->eng3d->oclass >= NV40_3D_CLASS ? 1024 :
256,
> +               /* round_down_pow2(mt->uniform_pitch / 4) */
> +               1 << (util_last_bit(mt->uniform_pitch / 4) - 1));
> +         mt->uniform_pitch = align(mt->uniform_pitch, pitch_align);
> +      }
>     }
>
>     if (!mt->uniform_pitch)
> --
> 2.4.3
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> Note this is not ideal. Since the sifm can only do source sizes upto
> 1024x1024 we end up using the blitter on nv4x, which is not that fast.
Moral of the story: don't do use nv3x/nv4x :)

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable at lists.freedesktop.org>
>
> And on nv3x we end up using the cpu which is really slow.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/gallium/drivers/nouveau/nv30/nv30_miptree.c  | 19 ++++++++-----------
>  src/gallium/drivers/nouveau/nv30/nv30_resource.h |  3 ---
>  2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> index 2276347..76bb8b8 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> @@ -145,21 +145,18 @@ nv30_resource_copy_region(struct pipe_context *pipe,
>     nv30_transfer_rect(nv30, NEAREST, &src, &dst);
>  }
>
> -void
> -nv30_resource_resolve(struct pipe_context *pipe,
> -                      const struct pipe_resolve_info *info)
> +static void
> +nv30_resource_resolve(struct nv30_context *nv30,
> +                      const struct pipe_blit_info *info)
>  {
> -#if 0
> -   struct nv30_context *nv30 = nv30_context(pipe);
>     struct nv30_rect src, dst;
>
> -   define_rect(info->src.res, 0, 0, info->src.x0, info->src.y0,
> -               info->src.x1 - info->src.x0, info->src.y1 -
info->src.y0, &src);
> -   define_rect(info->dst.res, info->dst.level, 0, info->dst.x0,
info->dst.y0,
> -               info->dst.x1 - info->dst.x0, info->dst.y1 -
info->dst.y0, &dst);
> +   define_rect(info->src.resource, 0, info->src.box.z,
info->src.box.x,
> +      info->src.box.y, info->src.box.width, info->src.box.height,
&src);
> +   define_rect(info->dst.resource, 0, info->dst.box.z,
info->dst.box.x,
> +      info->dst.box.y, info->dst.box.width, info->dst.box.height,
&dst);
>
>     nv30_transfer_rect(nv30, BILINEAR, &src, &dst);
> -#endif
>  }
>
>  void
> @@ -173,7 +170,7 @@ nv30_blit(struct pipe_context *pipe,
>         info.dst.resource->nr_samples <= 1 &&
>         !util_format_is_depth_or_stencil(info.src.resource->format)
&&
>         !util_format_is_pure_integer(info.src.resource->format)) {
> -      debug_printf("nv30: color resolve unimplemented\n");
> +      nv30_resource_resolve(nv30, blit_info);
>        return;
>     }
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_resource.h
b/src/gallium/drivers/nouveau/nv30/nv30_resource.h
> index 8dac779..20d86b6 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_resource.h
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_resource.h
> @@ -66,9 +66,6 @@ nv30_resource_copy_region(struct pipe_context *pipe,
>                            const struct pipe_box *src_box);
>
>  void
> -nv30_resource_resolve(struct pipe_context *, const struct
pipe_resolve_info *);
> -
> -void
>  nv30_blit(struct pipe_context *pipe,
>            const struct pipe_blit_info *blit_info);
>
> --
> 2.4.3
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> On nv3x we will likely end up using the cpu to do color resolving for msaa
> blits. Disable msaa on these cards so that we do not end up using the cpu.
Actually the CPU fallback won't do scaled, so it's stuck with SIFM or
assert(false). Which isn't great, but... it's what the HW does. I
don't see a reason to shut that off. I'd rather disallow allocating MS
surfaces that SIFM won't later be able to resolve on nv3x.
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index 7aad26b..69acc38 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -319,8 +319,16 @@ nv30_screen_is_format_supported(struct pipe_screen
*pscreen,
>                                  unsigned sample_count,
>                                  unsigned bindings)
>  {
> -   if (sample_count > 4)
> -      return false;
> +   struct nv30_screen *screen = nv30_screen(pscreen);
> +
> +   if (screen->eng3d->oclass >= NV40_3D_CLASS) {
> +      if (sample_count > 4)
> +         return false;
> +   } else {
> +      if (sample_count > 0)
> +         return false;
> +   }
> +
>     if (!(0x00000017 & (1 << sample_count)))
>        return false;
>
> --
> 2.4.3
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> On nv4x with a msaa visual after a while the gpu locks up, attach gdb to
> impress shows it is hanging waiting for a fence which never comes.
>
> Killing ooimpress at this point works exactly once, trying to do any
> 3d operations after killing impress will lockup the entire system.
>
> This needs further investigation, but for now disable msaa because no
> msaa is better then crashing the system.
That's an argument I'm willing to live with -- can you generate such a
patch without your previous patch?

However if we do this, this is basically saying that MSAA will never
work on nv3x/nv4x, since it's unlikely that anyone will look further
into it. It'd be nice to put a little effort in first to make sure
there isn't something simple we can fix.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index 69acc38..57262b8 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -321,7 +321,8 @@ nv30_screen_is_format_supported(struct pipe_screen
*pscreen,
>  {
>     struct nv30_screen *screen = nv30_screen(pscreen);
>
> -   if (screen->eng3d->oclass >= NV40_3D_CLASS) {
> +   /* MSAA visuals work somwhat on nv4x, but cause gpu lockups */
> +   if (0 && screen->eng3d->oclass >= NV40_3D_CLASS) {
>        if (sample_count > 4)
>           return false;
>     } else {
> --
> 2.4.3
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Hi,

On 03-09-15 19:14, Ilia Mirkin wrote:
> On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede at
redhat.com> wrote:
>> Hi All,
>>
>> Here is a bunch of fixes for nv30 cards, the first patch is a resend of
>> a patch I send a while back. AFAICT that one is ready for merging, but
>> it is not entirely clear to me what the process is for getting
(nouveau)
>> mesa patches merged.
>>
>> Should I request commit rights, and push my own patches once they have
>> been reviewed ?
>
> The usual process is for you to have a few patches accepted before
> requesting commit access.
Fair enough.
> I suppose I'm the de-facto nouveau mesa
> maintainer nowadays, not a highly-contended position though.
Ok.
> I'll take a look at these soon, and if I should forget, feel free to
> ping me early and often.
You requested a git branch to pull from, I've put one up here
with the first 2 patches from this set with your:

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable at lists.freedesktop.org>

Added, you can find this here:

http://cgit.freedesktop.org/~jwrdegoede/mesa/log/?h=for-ilia

Regards,

Hans