[PATCH mesa 0/4] nv30: Various fixes

[PATCH mesa 0/4] nv30: Various fixes

[Hans de Goede]

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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[PATCH mesa 1/4] nv30: Fix creation of scanout buffers

[Hans de Goede]

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@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH mesa 2/4] nv30: Implement color resolve for msaa

[Hans de Goede]

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@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Hans de Goede]

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@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH mesa 4/4] nv30: Disable msaa on nv4x because it causes gpu lockups

[Hans de Goede]

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@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 0/4] nv30: Various fixes

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH mesa 1/4] nv30: Fix creation of scanout buffers

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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@alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH mesa 2/4] nv30: Implement color resolve for msaa

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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@alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable@lists.freedesktop.org>

>
> And on nv3x we end up using the cpu which is really slow.
>
> Signed-off-by: Hans de Goede <hdegoede@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 3/4] nv30: Do not export msaa capabable visuals on nv3x

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 4/4] nv30: Disable msaa on nv4x because it causes gpu lockups

[Ilia Mirkin]

On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH mesa 0/4] nv30: Various fixes

[Hans de Goede]

Hi,

On 03-09-15 19:14, Ilia Mirkin wrote:
> On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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@alum.mit.edu>
Cc: "10.6 11.0" <mesa-stable@lists.freedesktop.org>

Added, you can find this here:

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

Regards,

Hans
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH mesa 3/4] nv30: Do not export msaa capabable visuals on nv3x

[Hans de Goede]

Hi,

On 03-09-15 19:32, Ilia Mirkin wrote:
> On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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.

Ok.

> 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.

Hmm, my first hunch when reading this was: "that is not going to work,
apps pick a visual and then alloc surfaces for that visual and only
that visual, they will not fall back to another visual if the alloc
fails."

Still I've given this a try, resulting in this:

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
index 76bb8b8..172ffc1 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
@@ -325,6 +325,7 @@ nv30_miptree_create(struct pipe_screen *pscreen,
                      const struct pipe_resource *tmpl)
  {
     struct nouveau_device *dev = nouveau_screen(pscreen)->device;
+   struct nv30_screen *screen = nv30_screen(pscreen);
     struct nv30_miptree *mt = CALLOC_STRUCT(nv30_miptree);
     struct pipe_resource *pt = &mt->base.base;
     unsigned blocksz, size;
@@ -356,6 +357,15 @@ nv30_miptree_create(struct pipe_screen *pscreen,

     w = pt->width0 << mt->ms_x;
     h = pt->height0 << mt->ms_y;
+
+   /* On nv3x ms requires sifm, make sure we meet the sifm constraints */
+   if (screen->eng3d->oclass < NV40_3D_CLASS && tmpl->nr_samples > 0 &&
+       ((w | h) > 1024 || w < 2 || h < 2)) {
+      debug_printf("refusing to create ms buffer with a size of %dx%d\n", w, h);
+      FREE(mt);
+      return NULL;
+   }
+
     d = (pt->target == PIPE_TEXTURE_3D) ? pt->depth0 : 1;
     blocksz = util_format_get_blocksize(pt->format);


But as I guessed already this does not make impress happy, it still tries
to use a ms visuals, and then hobbles along showing an uninitialized
frontbuffer, as it was doing before the patch to implement color
resolve.

I've checked and the debug printf I added works, so either I'm doing this
at the wrong place, or this is just not a good idea.

BTW there are more arguments to disable msaa on nv3x:

1) nv3x is slow enough without it
2) nv3x cards typically only have 64M of memory and msaa
takes a significant amount of extra memory.

Regards,

Hans



>
>> Signed-off-by: Hans de Goede <hdegoede@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 4/4] nv30: Disable msaa on nv4x because it causes gpu lockups

[Hans de Goede]

Hi,

On 03-09-15 19:33, Ilia Mirkin wrote:
> On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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.

My plan was for me to commit to at least taking a serious attempt
at fixing this, see my mail in the "nv3x libreoffice impress opengl
animations not working" thread.

In the mean time I think it would be good to have a patch such as this
one upstream, so that things will just work for nv30 family users and
we can also Cc such a patch to
"10.6 11.0" <mesa-stable@lists.freedesktop.org>

Where as the fix may not be suitable for stable.

Regards,

Hans



>
>>
>> Signed-off-by: Hans de Goede <hdegoede@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH mesa 3/4] nv30: Do not export msaa capabable visuals on nv3x

[Ilia Mirkin]

On Fri, Sep 4, 2015 at 9:10 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 03-09-15 19:32, Ilia Mirkin wrote:
>>
>> On Thu, Sep 3, 2015 at 7:25 AM, Hans de Goede <hdegoede@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.
>
>
> Ok.
>
>> 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.
>
>
> Hmm, my first hunch when reading this was: "that is not going to work,
> apps pick a visual and then alloc surfaces for that visual and only
> that visual, they will not fall back to another visual if the alloc
> fails."
>
> Still I've given this a try, resulting in this:
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> index 76bb8b8..172ffc1 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_miptree.c
> @@ -325,6 +325,7 @@ nv30_miptree_create(struct pipe_screen *pscreen,
>                      const struct pipe_resource *tmpl)
>  {
>     struct nouveau_device *dev = nouveau_screen(pscreen)->device;
> +   struct nv30_screen *screen = nv30_screen(pscreen);
>     struct nv30_miptree *mt = CALLOC_STRUCT(nv30_miptree);
>     struct pipe_resource *pt = &mt->base.base;
>     unsigned blocksz, size;
> @@ -356,6 +357,15 @@ nv30_miptree_create(struct pipe_screen *pscreen,
>
>     w = pt->width0 << mt->ms_x;
>     h = pt->height0 << mt->ms_y;
> +
> +   /* On nv3x ms requires sifm, make sure we meet the sifm constraints */
> +   if (screen->eng3d->oclass < NV40_3D_CLASS && tmpl->nr_samples > 0 &&
> +       ((w | h) > 1024 || w < 2 || h < 2)) {
> +      debug_printf("refusing to create ms buffer with a size of %dx%d\n",
> w, h);
> +      FREE(mt);
> +      return NULL;
> +   }
> +
>     d = (pt->target == PIPE_TEXTURE_3D) ? pt->depth0 : 1;
>     blocksz = util_format_get_blocksize(pt->format);
>
>
> But as I guessed already this does not make impress happy, it still tries
> to use a ms visuals, and then hobbles along showing an uninitialized
> frontbuffer, as it was doing before the patch to implement color
> resolve.
>
> I've checked and the debug printf I added works, so either I'm doing this
> at the wrong place, or this is just not a good idea.
>
> BTW there are more arguments to disable msaa on nv3x:
>
> 1) nv3x is slow enough without it
> 2) nv3x cards typically only have 64M of memory and msaa
> takes a significant amount of extra memory.

Well, I don't think anyone's expecting it to do 4K resolution at 32x
msaa... The fact that it'll die even with 640x480 is a bit unfortunate
though. MSAA doesn't take a "significant" amount of extra memory, just
2x or 4x for the RB. Textures/everything else are unaffected. Really
we should be using sifm for the whole thing irrespective of the size
and just tiling. That'll introduce seams if we're not careful about
it, but it should be fine for a resolve, since it's very direct pixel
mappings (i.e. 2:1 or 4:1, no 2.5:1 type of thing). Unless it uses
surrounding samples, then there might be very minor seams... not sure.

Should be easy to rig up sifm to ignore the w/h restrictions for MS
textures, and just do it in 1024x1024 blocks. What do you think? You
should also double-check whether the 1024 restriction is real, and
what it's actually restricting? [Note that MS miptrees are allocated
linear, not swizzled. Not sure why... probably not a bad thing since
sifm only deals with linear sources.]

  -ilia
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau