[Mesa-dev] [PATCH kmscube] Search for a suitable config
Instead of assuming the first will be suitable. kmscube fails to start
for me without this change.
---
common.c | 52 +---
common.h | 1 +
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/common.c b/common.c
index f9bd280..45c074d 100644
--- a/common.c
+++ b/common.c
@@ -24,11 +24,47 @@
#include
#include
#include
+#include
#include
#include
#include "common.h"
+static bool egl_get_config(EGLDisplay disp, EGLint *attribs,
+ EGLConfig *out, EGLint visual_id) {
+ EGLint count = 0, matched = 0, ret;
+
+ ret = eglGetConfigs(disp, NULL, 0, &count);
+ if (ret == EGL_FALSE || count == 0) {
+ printf("eglGetConfigs returned no configs\n");
+ return false;
+ }
+
+ EGLConfig configs[count];
+
+ ret = eglChooseConfig(disp, attribs, configs, count, &matched);
+ if (ret == EGL_FALSE) {
+ printf("eglChooseConfig failed\n");
+ return false;
+ }
+
+ for (int i = 0; i < matched; ++i) {
+ EGLint visual;
+ if (!eglGetConfigAttrib(disp, configs[i],
+ EGL_NATIVE_VISUAL_ID, &visual)) {
+ continue;
+ }
+
+ if (!visual_id || visual == visual_id) {
+ *out = configs[i];
+ return true;
+ }
+ }
+
+ printf("no valid egl config found\n");
+ return false;
+}
+
struct gbm * init_gbm(int drm_fd, int w, int h)
{
struct gbm *gbm = calloc(1, sizeof (struct gbm));
@@ -59,7 +95,7 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
EGL_NONE
};
- static const EGLint config_attribs[] = {
+ static EGLint config_attribs[] = {
EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
EGL_RED_SIZE, 1,
EGL_GREEN_SIZE, 1,
@@ -81,9 +117,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
get_proc(eglDestroySyncKHR);
get_proc(eglWaitSyncKHR);
get_proc(eglDupNativeFenceFDANDROID);
+ get_proc(eglCreatePlatformWindowSurfaceEXT);
if (egl->eglGetPlatformDisplayEXT) {
- egl->display =
egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_KHR,
+ egl->display =
egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
gbm->dev, NULL);
} else {
egl->display = eglGetDisplay((void *)gbm->dev);
@@ -106,8 +143,9 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
return -1;
}
- if (!eglChooseConfig(egl->display, config_attribs, &egl->config, 1, &n)
|| n != 1) {
- printf("failed to choose config: %d\n", n);
+ if (!egl_get_config(egl->display, config_attribs,
+ &egl->config, GBM_FORMAT_XRGB)) {
+ printf("Failed to get EGL config\n");
return -1;
}
@@ -118,10 +156,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
return -1;
}
- egl->surface = eglCreateWindowSurface(egl->display, egl->config,
- (EGLNativeWindowType)gbm->surface, NULL);
+ egl->surface = egl->eglCreatePlatformWindowSurfaceEXT(
+ egl->display, egl->config, gbm->surface, NULL);
if (egl->surface == EGL_NO_SURFACE) {
- printf("failed to create egl surface\n");
+ printf("failed to create egl surface: %d\n", eglGetError());
return -1;
}
diff --git a/common.h b/common.h
index 1ddf04b..1675f98 100644
--- a/common.h
+++ b/common.h
@@ -69,6 +69,7 @@ struct egl {
EGLSurface surface;
PFNEGLGETPLATFORMDISPLAYEXTPROC eglGetPlatformDisplayEXT;
+ PFNEGLCREATEPLATFORMWINDOWSURFACEEXTPROC
eglCreatePlatformWindowSurfaceEXT;
PFNEGLCREATEIMAGEKHRPROC eglCreateImageKHR;
PFNEGLDESTROYIMAGEKHRPROC eglDestroyImageKHR;
PFNGLEGLIMAGETARGETTEXTURE2DOESPROC glEGLImageTargetTexture2DOES;
--
2.22.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH kmscube] Search for a suitable config
Hi Drew, On Wed, 3 Jul 2019 at 08:16, Drew DeVault wrote: > Instead of assuming the first will be suitable. kmscube fails to start > for me without this change. There are a couple of unrelated changes combined in here, but I think the core one is good. eglChooseConfig has some really useful properties, where it strictly specifies a sort order, does not include EGL_NATIVE_VISUAL_ID in that sort order, and silently accepts attributes which are not relevant for the sort. Your change which takes all possible configs and then queries them to make sure that the native visual ID is actually correct is the right thing to do. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] radeonsi: glmark2 - regression (GL_INVALID_OPERATION in glFramebufferTexture2D) - bisected
On 2019-07-02 7:29 p.m., Marek Olšák wrote: > If you don't wanna see the messages, don't use debugoptimized. Makes sense, but means it needs to be guarded by DEBUG, not NDEBUG. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] panfrost: Rewrite u-interleaving code
On Tue, 2019-06-25 at 12:36 -0700, Vasily Khoruzhick wrote:
> On Tue, Jun 25, 2019 at 11:54 AM Alyssa Rosenzweig
> wrote:
>
> Hi Alyssa,
>
> > Rather than using a magic lookup table with no explanations, let's
> > add
> > liberal comments to the code to explain what this tiling scheme is
> > and
> > how to encode/decode it efficiently.
> >
> > It's not so mysterious after all -- just reordering bits with some
> > XORs
> > thrown in.
> >
> > Signed-off-by: Alyssa Rosenzweig
> > Cc: Vasily Khoruzhick
> > ---
> > src/panfrost/shared/pan_tiling.c | 330 +
> > --
> > 1 file changed, 229 insertions(+), 101 deletions(-)
> >
> > diff --git a/src/panfrost/shared/pan_tiling.c
> > b/src/panfrost/shared/pan_tiling.c
> > index 413cd89420b..de0d6812831 100644
> > --- a/src/panfrost/shared/pan_tiling.c
> > +++ b/src/panfrost/shared/pan_tiling.c
> > @@ -2,6 +2,7 @@
> > * Copyright (c) 2011-2013 Luc Verhaegen
> > * Copyright (c) 2018 Alyssa Rosenzweig
> > * Copyright (c) 2018 Vasily Khoruzhick
> > + * Copyright (c) 2019 Collabora
> > *
> > * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > * copy of this software and associated documentation files (the
> > "Software"),
> > @@ -26,127 +27,230 @@
> >
> > #include "pan_tiling.h"
> >
> > -uint32_t space_filler[16][16] = {
> > - {
> > 0, 1, 4, 5, 16, 17, 20, 21, 64, 65, 68, 69, 80, 81
> > , 84, 85, },
> > - {
> > 3, 2, 7, 6, 19, 18, 23, 22, 67, 66, 71, 70, 83, 82
> > , 87, 86, },
> > - {
> > 12, 13, 8, 9, 28, 29, 24, 25, 76, 77, 72, 73, 92, 93
> > , 88, 89, },
> > - {
> > 15, 14, 11, 10, 31, 30, 27, 26, 79, 78, 75, 74, 95, 94
> > , 91, 90, },
> > - { 48, 49, 52, 53, 32, 33, 36, 37, 112, 113, 116, 117,
> > 96, 97, 100, 101, },
> > - { 51, 50, 55, 54, 35, 34, 39, 38, 115, 114, 119, 118,
> > 99, 98, 103, 102, },
> > - { 60, 61, 56, 57, 44, 45, 40, 41, 124, 125, 120, 121,
> > 108, 109, 104, 105, },
> > - { 63, 62, 59, 58, 47, 46, 43, 42, 127, 126, 123, 122,
> > 111, 110, 107, 106, },
> > - { 192, 193, 196, 197, 208, 209, 212, 213, 128, 129, 132, 133,
> > 144, 145, 148, 149, },
> > - { 195, 194, 199, 198, 211, 210, 215, 214, 131, 130, 135, 134,
> > 147, 146, 151, 150, },
> > - { 204, 205, 200, 201, 220, 221, 216, 217, 140, 141, 136, 137,
> > 156, 157, 152, 153, },
> > - { 207, 206, 203, 202, 223, 222, 219, 218, 143, 142, 139, 138,
> > 159, 158, 155, 154, },
> > - { 240, 241, 244, 245, 224, 225, 228, 229, 176, 177, 180, 181,
> > 160, 161, 164, 165, },
> > - { 243, 242, 247, 246, 227, 226, 231, 230, 179, 178, 183, 182,
> > 163, 162, 167, 166, },
> > - { 252, 253, 248, 249, 236, 237, 232, 233, 188, 189, 184, 185,
> > 172, 173, 168, 169, },
> > - { 255, 254, 251, 250, 239, 238, 235, 234, 191, 190, 187, 186,
> > 175, 174, 171, 170, },
> > +/* This file implements software encode/decode of the tiling
> > format used for
> > + * textures and framebuffers primarily on Utgard GPUs. Names for
> > this format
> > + * include "Utgard-style tiling", "(Mali) swizzled textures", and
> > + * "U-interleaved" (the former two names being used in the
> > community
> > + * Lima/Panfrost drivers; the latter name used internally at Arm).
> > + * Conceptually, like any tiling scheme, the pixel reordering
> > attempts to 2D
> > + * spatial locality, to improve cache locality in both horizontal
> > and vertical
> > + * directions.
> > + *
> > + * This format is tiled: first, the image dimensions must be
> > aligned to 16
> > + * pixels in each axis. Once aligned, the image is divided into
> > 16x16 tiles.
> > + * This size harmonizes with other properties of the GPU; on
> > Midgard,
> > + * framebuffer tiles are logically 16x16 (this is the tile size
> > used in
> > + * Transaction Elimination and the minimum tile size used in
> > Hierarchical
> > + * Tiling). Conversely, for a standard 4 bytes-per-pixel format
> > (like
> > + * RGBA), 16 pixels * 4 bytes/pixel = 64 bytes, equal to the
> > cache line
> > + * size.
> > + *
> > + * Within each 16x16 block, the bits are reordered according to
> > this pattern:
> > + *
> > + * | y3 | (x3 ^ y3) | y2 | (y2 ^ x2) | y1 | (y1 ^ x1) | y0 | (y0 ^
> > x0) |
> > + *
> > + * Basically, interleaving the X and Y bits, with XORs thrown in
> > for every
> > + * adjacent bit pair.
> > + *
> > + * This is cheap to implement both encode/decode in both hardware
> > and software.
> > + * In hardware, lines are simply rerouted to reorder and some XOR
> > gates are
> > + * thrown in. Software has to be a bit more clever.
> > + *
> > + * In software, the trick is to divide the pattern into two lines:
> > + *
> > + *| y3 | y3 | y2 | y2 | y1 | y1 | y0 | y0 |
> > + * ^ | 0 | x3 | 0 | x2 | 0 | x1 | 0 | x0 |
> > + *
> > + * That is, duplicate the bits of the Y and space out the bits of
> > the X. The
> > + * top line is a function only of Y, so it can be c
Re: [Mesa-dev] [PATCH v4 0/5] EGL_KHR_partial_update support
On Tue, 2019-06-25 at 18:37 +0200, Boris Brezillon wrote: > This is an attempt at resurrecting Daniel's MR [1] which was already > resurrecting Harish's EGL_KHR_partial_update series [2]. This version > implements Marek's suggestion to pass the set_damage_region() > directly > to the gallium driver and let it decide how to handle the request. > Some > drivers might just calculate the damage extent (as done in Daniel's > initial proposal and in the panfrost implementation), others might do > extra optimizations like trying to reduce the area we're supposed to > reload (only valid for tile-based rendering) even further. > > This patch series has been tested with weston (see Daniel's MR[3]) on > panfrost. Note that the panfrost implementation is rather simple > (just > limits the rendering area to the damage extent and picks the biggest > damage rect as the only damage region) but we can improve it if we > feel > the need. > > Any feedback and suggestions on how to do it better are welcome. > I haven't looked at the panfrost end of it, but the overall approach seems solid to me, and it solves the issue I had with the initial approach. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 4/5] st/dri2: Implement DRI2bufferDamageExtension
On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> Ah-ha, now we're into parts of the stack I can claim to understand!
> >:)
>
> Reviewed-by: Alyssa Rosenzweig
>
> On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon wrote:
> > From: Daniel Stone
> >
> > Add a pipe_screen->set_damage_region() hook to propagate
> > set-damage-region requests to the driver, it's then up to the
> > driver to
> > decide what to do with this piece of information.
> >
> > If the hook is left unassigned, the buffer-damage extension is
> > considered unsupported.
> >
> > Signed-off-by: Daniel Stone
> > Signed-off-by: Boris Brezillon
> > ---
> > src/gallium/include/pipe/p_screen.h | 7 +++
> > src/gallium/state_trackers/dri/dri2.c | 22 ++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/src/gallium/include/pipe/p_screen.h
> > b/src/gallium/include/pipe/p_screen.h
> > index 3f9bad470950..8df12ee4f865 100644
> > --- a/src/gallium/include/pipe/p_screen.h
> > +++ b/src/gallium/include/pipe/p_screen.h
> > @@ -464,6 +464,13 @@ struct pipe_screen {
> > bool (*is_parallel_shader_compilation_finished)(struct
> > pipe_screen *screen,
> > void *shader,
> > unsigned
> > shader_type);
> > +
> > + /**
> > +* Set damage region.
> > +*/
> > + void (*set_damage_region)(struct pipe_screen *screen,
> > + struct pipe_resource *resource,
> > + unsigned int nrects, int *rects);
I would kinda have expected rects to be an array of pipe_box instead of
just an array of integers, as that'd be a bit easier to know the
semantics of...
> > };
> >
> >
> > diff --git a/src/gallium/state_trackers/dri/dri2.c
> > b/src/gallium/state_trackers/dri/dri2.c
> > index 5caaa9deac41..df22e7c41642 100644
> > --- a/src/gallium/state_trackers/dri/dri2.c
> > +++ b/src/gallium/state_trackers/dri/dri2.c
> > @@ -1806,6 +1806,23 @@ static const __DRI2interopExtension
> > dri2InteropExtension = {
> > .export_object = dri2_interop_export_object
> > };
> >
> > +/**
> > + * \brief the DRI2bufferDamageExtension set_damage_region method
> > + */
> > +static void
> > +dri2_set_damage_region(__DRIdrawable *dPriv, unsigned int nrects,
> > int *rects)
> > +{
> > + struct dri_drawable *drawable = dri_drawable(dPriv);
> > + struct pipe_resource *resource = drawable-
> > >textures[ST_ATTACHMENT_BACK_LEFT];
> > + struct pipe_screen *screen = resource->screen;
> > +
> > + screen->set_damage_region(screen, resource, nrects, rects);
> > +}
> > +
> > +static __DRI2bufferDamageExtension dri2BufferDamageExtension = {
> > + .base = { __DRI2_BUFFER_DAMAGE, 1 },
> > +};
> > +
> > /**
> > * \brief the DRI2ConfigQueryExtension configQueryb method
> > */
> > @@ -1907,6 +1924,7 @@ static const __DRIextension
> > *dri_screen_extensions[] = {
> > &dri2GalliumConfigQueryExtension.base,
> > &dri2ThrottleExtension.base,
> > &dri2FenceExtension.base,
> > + &dri2BufferDamageExtension.base,
> > &dri2InteropExtension.base,
> > &dri2NoErrorExtension.base,
> > &driBlobExtension.base,
> > @@ -1922,6 +1940,7 @@ static const __DRIextension
> > *dri_robust_screen_extensions[] = {
> > &dri2ThrottleExtension.base,
> > &dri2FenceExtension.base,
> > &dri2InteropExtension.base,
> > + &dri2BufferDamageExtension.base,
> > &dri2Robustness.base,
> > &dri2NoErrorExtension.base,
> > &driBlobExtension.base,
> > @@ -1982,6 +2001,9 @@ dri2_init_screen(__DRIscreen * sPriv)
> >}
> > }
> >
> > + if (pscreen->set_damage_region)
> > + dri2BufferDamageExtension.set_damage_region =
> > dri2_set_damage_region;
> > +
> > if (pscreen->get_param(pscreen,
> > PIPE_CAP_DEVICE_RESET_STATUS_QUERY)) {
> >sPriv->extensions = dri_robust_screen_extensions;
> >screen->has_reset_status_query = true;
> > --
> > 2.20.1
> >
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] boolean usage in gallium
On Sat, 2019-06-29 at 00:08 -0400, Ilia Mirkin wrote: > Ken pointed out on IRC today that there was still a lot of "boolean" > (vs bool/_Bool) usage in gallium. In fact, many interfaces are > specified with boolean. > > I had it in my mind that I had at some point removed most boolean > usage, but that is just not the case - first of all, the interfaces > remain with it, and I could find no evidence of such a commit. I must > have imagined it. > > Is there any reason to keep boolean around? I know conversions must > be > done carefully (since incorrect-but-working usage would not currently > be caught by the compiler), but are there any practical reasons to > avoid C99 _Bool in gallium code? > > If not, I may begin converting things over. > I would love to see patches for this! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 4/5] st/dri2: Implement DRI2bufferDamageExtension
On Wed, 03 Jul 2019 11:54:29 +0200
Erik Faye-Lund wrote:
> On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> > Ah-ha, now we're into parts of the stack I can claim to understand!
> > >:)
> >
> > Reviewed-by: Alyssa Rosenzweig
> >
> > On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon wrote:
> > > From: Daniel Stone
> > >
> > > Add a pipe_screen->set_damage_region() hook to propagate
> > > set-damage-region requests to the driver, it's then up to the
> > > driver to
> > > decide what to do with this piece of information.
> > >
> > > If the hook is left unassigned, the buffer-damage extension is
> > > considered unsupported.
> > >
> > > Signed-off-by: Daniel Stone
> > > Signed-off-by: Boris Brezillon
> > > ---
> > > src/gallium/include/pipe/p_screen.h | 7 +++
> > > src/gallium/state_trackers/dri/dri2.c | 22 ++
> > > 2 files changed, 29 insertions(+)
> > >
> > > diff --git a/src/gallium/include/pipe/p_screen.h
> > > b/src/gallium/include/pipe/p_screen.h
> > > index 3f9bad470950..8df12ee4f865 100644
> > > --- a/src/gallium/include/pipe/p_screen.h
> > > +++ b/src/gallium/include/pipe/p_screen.h
> > > @@ -464,6 +464,13 @@ struct pipe_screen {
> > > bool (*is_parallel_shader_compilation_finished)(struct
> > > pipe_screen *screen,
> > > void *shader,
> > > unsigned
> > > shader_type);
> > > +
> > > + /**
> > > +* Set damage region.
> > > +*/
> > > + void (*set_damage_region)(struct pipe_screen *screen,
> > > + struct pipe_resource *resource,
> > > + unsigned int nrects, int *rects);
>
> I would kinda have expected rects to be an array of pipe_box instead of
> just an array of integers, as that'd be a bit easier to know the
> semantics of...
Sure, I can do that. Should I do the Y-flip as part of the ints -> box
conversion or should I keep the "origin is bottom-left" semantic?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 4/5] st/dri2: Implement DRI2bufferDamageExtension
On Wed, 2019-07-03 at 12:33 +0200, Boris Brezillon wrote:
> On Wed, 03 Jul 2019 11:54:29 +0200
> Erik Faye-Lund wrote:
>
> > On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> > > Ah-ha, now we're into parts of the stack I can claim to
> > > understand!
> > > > :)
> > >
> > > Reviewed-by: Alyssa Rosenzweig
> > >
> > > On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon
> > > wrote:
> > > > From: Daniel Stone
> > > >
> > > > Add a pipe_screen->set_damage_region() hook to propagate
> > > > set-damage-region requests to the driver, it's then up to the
> > > > driver to
> > > > decide what to do with this piece of information.
> > > >
> > > > If the hook is left unassigned, the buffer-damage extension is
> > > > considered unsupported.
> > > >
> > > > Signed-off-by: Daniel Stone
> > > > Signed-off-by: Boris Brezillon
> > > > ---
> > > > src/gallium/include/pipe/p_screen.h | 7 +++
> > > > src/gallium/state_trackers/dri/dri2.c | 22
> > > > ++
> > > > 2 files changed, 29 insertions(+)
> > > >
> > > > diff --git a/src/gallium/include/pipe/p_screen.h
> > > > b/src/gallium/include/pipe/p_screen.h
> > > > index 3f9bad470950..8df12ee4f865 100644
> > > > --- a/src/gallium/include/pipe/p_screen.h
> > > > +++ b/src/gallium/include/pipe/p_screen.h
> > > > @@ -464,6 +464,13 @@ struct pipe_screen {
> > > > bool (*is_parallel_shader_compilation_finished)(struct
> > > > pipe_screen *screen,
> > > > void
> > > > *shader,
> > > > unsigned
> > > > shader_type);
> > > > +
> > > > + /**
> > > > +* Set damage region.
> > > > +*/
> > > > + void (*set_damage_region)(struct pipe_screen *screen,
> > > > + struct pipe_resource *resource,
> > > > + unsigned int nrects, int
> > > > *rects);
> >
> > I would kinda have expected rects to be an array of pipe_box
> > instead of
> > just an array of integers, as that'd be a bit easier to know the
> > semantics of...
>
> Sure, I can do that. Should I do the Y-flip as part of the ints ->
> box
> conversion or should I keep the "origin is bottom-left" semantic?
Good question? :)
My instinct would be to have the regions in "memory layout order"
rather than "window sytem presentation order", which I guess would be
without any sort of y-flipping or rotation applied. I guess that would
make the answer "no"? But I'm not sure, I haven't read up on the spec
here.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110995] performance regression in Heroes of the Storm with Mesa 19.1.1 & Polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110995 --- Comment #3 from tempel.jul...@gmail.com --- Is there a chance to make CTS dEQP-VK.pipeline.depth_range_unrestricted.* pass without causing the performance regression? -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
Exported BOs might be imported back, then mmap()-ed to be written
too. Most drivers handle that by mmap()-ing the GEM handle after it's
been imported, but, according to [1], this is illegal. The panfrost
driver has recently switched to this generic helper (which was renamed
into drm_gem_map_offset() in the meantime) [2], and mmap()-ing
of imported BOs now fails.
Now I'm wondering how this should be solved. I guess the first question
is, is mmap()-ing of imported BOs really forbidden for all BOs? I guess
calling mmap() on a buffer that's been exported by the DRM driver itself
then re-imported shouldn't hurt, so maybe we can check that before
failing.
Now, if we really want to forbid mmap() on imported BOs, that means we
need a solution to mmap() the dmabuf object directly, and sometimes this
mapping will request RW permissions. The problem is, all function
exporting BOs in mesa are exporting them in RO-mode (resulting FD is
O_READ), thus preventing mmap()s in RW mode.
This patch modifies all
drmPrimeHandleToFD()/ioctl(DRM_IOCTL_PRIME_HANDLE_TO_FD) call sites
to pass the DRM_RDWR flag so that what's described above becomes
possible.
I'm not saying this is what we should do, it's more a way to start the
discussion. Feel free to propose alternives to this solution.
[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem.c?h=v5.2-rc7#n318
[2]https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/panfrost?id=583bbf46133c726bae277e8f4e32bfba2a528c7f
Signed-off-by: Boris Brezillon
Cc:
Cc:
Cc: Steven Price
Cc: Rob Herring
---
Cc-ing dri-devel is not a mistake, I really to have feedback from the DRM
maintainers, since this started with a kernel-side change.
---
src/etnaviv/drm/etnaviv_bo.c | 4 ++--
src/freedreno/drm/freedreno_bo.c | 4 ++--
src/freedreno/vulkan/tu_drm.c | 2 +-
src/gallium/auxiliary/renderonly/renderonly.c | 2 +-
src/gallium/drivers/iris/iris_bufmgr.c| 2 +-
src/gallium/drivers/lima/lima_bo.c| 2 +-
src/gallium/drivers/panfrost/pan_drm.c| 2 +-
src/gallium/drivers/v3d/v3d_bufmgr.c | 2 +-
src/gallium/drivers/vc4/vc4_bufmgr.c | 2 +-
src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 ++-
src/gallium/winsys/svga/drm/vmw_screen_dri.c | 5 +++--
src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 2 +-
src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 3 ++-
src/intel/vulkan/anv_gem.c| 2 +-
src/mesa/drivers/dri/i965/brw_bufmgr.c| 2 +-
15 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/src/etnaviv/drm/etnaviv_bo.c b/src/etnaviv/drm/etnaviv_bo.c
index 6e952fa47858..92634141b580 100644
--- a/src/etnaviv/drm/etnaviv_bo.c
+++ b/src/etnaviv/drm/etnaviv_bo.c
@@ -294,8 +294,8 @@ int etna_bo_dmabuf(struct etna_bo *bo)
{
int ret, prime_fd;
- ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
- &prime_fd);
+ ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
+DRM_CLOEXEC | DRM_RDWR, &prime_fd);
if (ret) {
ERROR_MSG("failed to get dmabuf fd: %d", ret);
return ret;
diff --git a/src/freedreno/drm/freedreno_bo.c b/src/freedreno/drm/freedreno_bo.c
index 7449160f1371..ba19b08d7c54 100644
--- a/src/freedreno/drm/freedreno_bo.c
+++ b/src/freedreno/drm/freedreno_bo.c
@@ -318,8 +318,8 @@ int fd_bo_dmabuf(struct fd_bo *bo)
{
int ret, prime_fd;
- ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
- &prime_fd);
+ ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
+DRM_CLOEXEC | DRM_RDWR, &prime_fd);
if (ret) {
ERROR_MSG("failed to get dmabuf fd: %d", ret);
return ret;
diff --git a/src/freedreno/vulkan/tu_drm.c b/src/freedreno/vulkan/tu_drm.c
index 9b2e6f78879e..6bef3012ddb5 100644
--- a/src/freedreno/vulkan/tu_drm.c
+++ b/src/freedreno/vulkan/tu_drm.c
@@ -147,7 +147,7 @@ tu_gem_export_dmabuf(const struct tu_device *dev, uint32_t
gem_handle)
{
int prime_fd;
int ret = drmPrimeHandleToFD(dev->physical_device->local_fd, gem_handle,
-DRM_CLOEXEC, &prime_fd);
+DRM_CLOEXEC | DRM_RDWR, &prime_fd);
return ret == 0 ? prime_fd : -1;
}
diff --git a/src/gallium/auxiliary/renderonly/renderonly.c
b/src/gallium/auxiliary/renderonly/renderonly.c
index d6a344009378..c1cc31115105 100644
--- a/src/gallium/auxiliary/renderonly/renderonly.c
+++ b/src/gallium/auxiliary/renderonly/renderonly.c
@@ -101,7 +101,7 @@ renderonly_create_kms_dumb_buffer_for_resource(struct
pipe_resource *rsc,
out_handle->type = WINSYS_HANDLE_TYPE_FD;
out_handle->stride = create_dumb.pitch;
- err = drmPrimeHandleToFD(ro->kms_fd, create_dumb.handle, O_CLOEXEC,
+ err =
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, 3 Jul 2019 07:45:32 -0600 Rob Herring wrote: > On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon > wrote: > > > > Exported BOs might be imported back, then mmap()-ed to be written > > too. Most drivers handle that by mmap()-ing the GEM handle after it's > > been imported, but, according to [1], this is illegal. > > It's not illegal, but is supposed to go thru the dmabuf mmap > functions. That's basically what I'm proposing here, just didn't post the patch skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD instead of the DRM-node one, but I have it working for panfrost. > However, none of the driver I've looked at (etnaviv, msm, > v3d, vgem) do that. It probably works because it's the same driver > doing the import and export or both drivers have essentially the same > implementations. Yes, but maybe that's something we should start fixing if mmap()-ing the dmabuf is the recommended solution. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, 3 Jul 2019 15:13:25 +0100
Steven Price wrote:
> On 03/07/2019 14:56, Boris Brezillon wrote:
> > On Wed, 3 Jul 2019 07:45:32 -0600
> > Rob Herring wrote:
> >
> >> On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
> >> wrote:
> >>>
> >>> Exported BOs might be imported back, then mmap()-ed to be written
> >>> too. Most drivers handle that by mmap()-ing the GEM handle after it's
> >>> been imported, but, according to [1], this is illegal.
> >>
> >> It's not illegal, but is supposed to go thru the dmabuf mmap
> >> functions.
> >
> > That's basically what I'm proposing here, just didn't post the patch
> > skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD
> > instead of the DRM-node one, but I have it working for panfrost.
>
> If we want to we could make the Panfrost kernel driver internally call
> dma_buf_mmap() so that mapping using the DRM-node "just works". This is
> indeed what the kbase driver does.
Well, userspace should at least skip DRM_IOCTL_PANFROST_MMAP_BO (or
ignore its return code), so calling mmap() on the dmabuf FD instead of
the DRM-node FD shouldn't be that hard.
>
> >> However, none of the driver I've looked at (etnaviv, msm,
> >> v3d, vgem) do that. It probably works because it's the same driver
> >> doing the import and export or both drivers have essentially the same
> >> implementations.
> >
> > Yes, but maybe that's something we should start fixing if mmap()-ing
> > the dmabuf is the recommended solution.
>
> I'm open to options here. User space calling mmap() on the dma_buf file
> descriptor should always be safe (the exporter can do whatever is
> necessary to make it work). If that happens then the patches I posted
> close off the DRM node version which could be broken if the exporter
> needs to do anything to prepare the buffer for CPU access (i.e. cache
> maintenance).
Talking about CPU <-> GPU syncs, I was wondering if the
mmap(gem_handle) step was providing any guarantee that would
allow us to ignore all the cache maintenance operations that are
required when mmap()-ing a dmabuf directly. Note that in both cases the
dmabuf is imported.
>
> Alternatively if user space wants/needs to use the DMA node then we can
> take a look at what needs to change in the kernel. From a quick look at
> the code it seems we'd need to split drm_gem_mmap() into a helper so
> that it can return whether the exporter is handling everything or if the
> caller needs to do some more work (e.g. drm_gem_shmem_mmap() needs to
> allocate backing pages). But because drm_gem_mmap() is used as the
> direct callback for some drivers we'd need to preserve the interface.
>
> The below (completely untested) patch demonstrates the idea.
>
> Steve
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index a8c4468f03d9..df661e24cadf 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
> * If the caller is not granted access to the buffer object, the mmap
> will fail
> * with EACCES. Please see the vma manager for more information.
> */
> -int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +int drm_gem_mmap_helper(struct file *filp, struct vm_area_struct *vma)
> {
> struct drm_file *priv = filp->private_data;
> struct drm_device *dev = priv->minor->dev;
> @@ -1189,6 +1189,11 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> vma->vm_flags &= ~VM_MAYWRITE;
> }
>
> + if (obj->import_attach) {
> + ret = dma_buf_mmap(obj->dma_buf, vma, 0);
> + return ret?:1;
> + }
> +
> ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
> vma);
>
> @@ -1196,6 +1201,16 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
>
> return ret;
> }
> +
> +int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> + int ret;
> +
> + ret = drm_gem_mmap_helper(filp, vma);
> + if (ret == 1)
> + return 0;
> + return ret;
> +}
> EXPORT_SYMBOL(drm_gem_mmap);
>
> void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 472ea5d81f82..b85d84e4d4a8 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -466,8 +466,10 @@ int drm_gem_shmem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> struct drm_gem_shmem_object *shmem;
> int ret;
>
> - ret = drm_gem_mmap(filp, vma);
> - if (ret)
> + ret = drm_gem_mmap_helper(filp, vma);
> + if (ret == 1)
> + return 0; /* Exporter handles the mapping */
> + else if (ret)
> return ret;
>
> shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
___
mesa-dev mailing list
mesa-dev@lis
[Mesa-dev] [Bug 111051] Mesa crashed on KDE Plasma Wayland session
https://bugs.freedesktop.org/show_bug.cgi?id=111051 Bug ID: 111051 Summary: Mesa crashed on KDE Plasma Wayland session Product: Mesa Version: 19.0 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: EGL/Wayland Assignee: wayland-b...@lists.freedesktop.org Reporter: bugsefor...@gmx.com QA Contact: mesa-dev@lists.freedesktop.org I already reported this crash to KDE devs. Read comment 1 in the following link https://bugs.kde.org/show_bug.cgi?id=408695#c1 I can't reproduce consistently, the crash occurred just once. If I'm not wrong, the crash occured with mesa 19.0.6. Distribution (Platform): Archlinux Packages Operating System: Linux 5.1.9-arch1-1-ARCH x86_64 Qt Version: 5.12.3 Frameworks Version: 5.59.0 Application: Plasma (plasmashell), signal: Segmentation fault Using host libthread_db library "/usr/lib/libthread_db.so.1". [Current thread is 1 (Thread 0x7fe3c46de880 (LWP 9806))] Thread 13 (Thread 0x7fe37b3fe700 (LWP 9884)): #0 0x7fe3c8947bac in pthread_cond_wait@@GLIBC_2.3.2 () at /usr/lib/libpthread.so.0 #1 0x7fe3c94c991c in QWaitConditionPrivate::wait(QDeadlineTimer) (deadline=..., this=0x55f6621b0430) at thread/qwaitcondition_unix.cpp:146 #2 0x7fe3c94c991c in QWaitCondition::wait(QMutex*, QDeadlineTimer) (this=, mutex=0x55f6621b4eb0, deadline=...) at thread/qwaitcondition_unix.cpp:225 #3 0x7fe3c94c9a0a in QWaitCondition::wait(QMutex*, unsigned long) (this=this@entry=0x55f6621b4eb8, mutex=mutex@entry=0x55f6621b4eb0, time=time@entry=18446744073709551615) at ../../include/QtCore/../../src/corelib/kernel/qdeadlinetimer.h:68 #4 0x7fe3cb24e6ed in QSGRenderThreadEventQueue::takeEvent(bool) (wait=true, this=0x55f6621b4ea8) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/qsgthreadedrenderloop.cpp:245 #5 0x7fe3cb24e6ed in QSGRenderThread::processEventsAndWaitForMore() (this=this@entry=0x55f6621b4e30) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/qsgthreadedrenderloop.cpp:710 #6 0x7fe3cb24e95c in QSGRenderThread::run() (this=0x55f6621b4e30) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/qsgthreadedrenderloop.cpp:739 #7 0x7fe3c94c363c in QThreadPrivate::start(void*) (arg=0x55f6621b4e30) at thread/qthread_unix.cpp:361 #8 0x7fe3c8941a92 in start_thread () at /usr/lib/libpthread.so.0 #9 0x7fe3c91a8cd3 in clone () at /usr/lib/libc.so.6 Thread 12 (Thread 0x7fe37bfff700 (LWP 9883)): [KCrash Handler] #6 0x7fe3c13aacfb in () at /usr/lib/libEGL_mesa.so.0 #7 0x7fe3c13aadf4 in () at /usr/lib/libEGL_mesa.so.0 #8 0x7fe3c07f2c80 in () at /usr/lib/dri/i965_dri.so #9 0x7fe3c07f3425 in () at /usr/lib/dri/i965_dri.so #10 0x7fe3c07ee7c3 in () at /usr/lib/dri/i965_dri.so #11 0x7fe3cb20b9cd in QSGBatchRenderer::Renderer::renderBatches() (this=this@entry=0x7fe370006fa0) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/include/QtQuick/5.12.3/QtQuick/private/../../../../../src/quick/scenegraph/coreapi/qsgrenderer_p.h:103 #12 0x7fe3cb21129f in QSGBatchRenderer::Renderer::render() (this=) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/coreapi/qsgbatchrenderer.cpp:2735 #13 0x7fe3cb201c7e in QSGRenderer::renderScene(QSGBindable const&) (bindable=..., this=0x7fe370006fa0) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/coreapi/qsgrenderer.cpp:244 #14 0x7fe3cb201c7e in QSGRenderer::renderScene(QSGBindable const&) (this=0x7fe370006fa0, bindable=...) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/coreapi/qsgrenderer.cpp:204 #15 0x7fe3cb20214c in QSGRenderer::renderScene(unsigned int) (this=, fboId=) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/coreapi/qsgrenderer.cpp:197 #16 0x7fe3cb23f3f0 in QSGDefaultRenderContext::renderNextFrame(QSGRenderer*, unsigned int) (this=0x55f66072b230, renderer=0x7fe370006fa0, fboId=) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/qsgdefaultrendercontext.cpp:182 #17 0x7fe3cb2a3b65 in QQuickWindowPrivate::renderSceneGraph(QSize const&) (this=this@entry=0x55f6608436f0, size=...) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/items/qquickwindow.cpp:486 #18 0x7fe3cb24ab6b in QSGRenderThread::syncAndRender() (this=this@entry=0x55f66072b660) at /tmp/makepkg/qt5-declarative-debug/src/qtdeclarative-everywhere-src-5.12.3/src/quick/scenegraph/qsgthreadedrenderloop.cpp:646 #19 0x7fe3cb24e918 in QSGRenderThread::run() (this=0x55f66072b660) at /tmp/makepkg/qt5-dec
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, 3 Jul 2019 15:50:08 +0100 Steven Price wrote: > On 03/07/2019 15:33, Boris Brezillon wrote: > > On Wed, 3 Jul 2019 15:13:25 +0100 > > Steven Price wrote: > > > >> On 03/07/2019 14:56, Boris Brezillon wrote: > >>> On Wed, 3 Jul 2019 07:45:32 -0600 > >>> Rob Herring wrote: > >>> > On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon > wrote: > > > > Exported BOs might be imported back, then mmap()-ed to be written > > too. Most drivers handle that by mmap()-ing the GEM handle after it's > > been imported, but, according to [1], this is illegal. > > It's not illegal, but is supposed to go thru the dmabuf mmap > functions. > >>> > >>> That's basically what I'm proposing here, just didn't post the patch > >>> skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD > >>> instead of the DRM-node one, but I have it working for panfrost. > >> > >> If we want to we could make the Panfrost kernel driver internally call > >> dma_buf_mmap() so that mapping using the DRM-node "just works". This is > >> indeed what the kbase driver does. > > > > Well, userspace should at least skip DRM_IOCTL_PANFROST_MMAP_BO (or > > ignore its return code), so calling mmap() on the dmabuf FD instead of > > the DRM-node FD shouldn't be that hard. > > What I was suggesting is that user space would still call > DRM_IOCTL_PANFROST_MMAP_BO to get an offset which uses in a call to > mmap(..., drm_node_fd, offset). The kernel could detect that the buffer > is imported and call the exporter for the actual mmap() functionality. Oops, sorry, brain fart. I thought it was DRM_IOCTL_PANFROST_MMAP_BO that was failing, but it's actually the mmap() call, so providing this wrapper kernel-side should work. > > The alternative is that user space 'simply' remembers that a buffer is > imported and keeps the file descriptor around so that it can instead > directly mmap() the dma_buf fd. Which is certainly easiest from the > kernel's perspective (and was what I assumed panfrost was doing - I > should have checked more closely!). > > However, none of the driver I've looked at (etnaviv, msm, > v3d, vgem) do that. It probably works because it's the same driver > doing the import and export or both drivers have essentially the same > implementations. > >>> > >>> Yes, but maybe that's something we should start fixing if mmap()-ing > >>> the dmabuf is the recommended solution. > >> > >> I'm open to options here. User space calling mmap() on the dma_buf file > >> descriptor should always be safe (the exporter can do whatever is > >> necessary to make it work). If that happens then the patches I posted > >> close off the DRM node version which could be broken if the exporter > >> needs to do anything to prepare the buffer for CPU access (i.e. cache > >> maintenance). > > > > Talking about CPU <-> GPU syncs, I was wondering if the > > mmap(gem_handle) step was providing any guarantee that would > > allow us to ignore all the cache maintenance operations that are > > required when mmap()-ing a dmabuf directly. Note that in both cases the > > dmabuf is imported. > > In theory the exporter should do whatever is required to ensure that the > CPU is synchronised when a user space mapping exists. There are some > issues here though: > > * In theory the kernel driver should map the dma_buf purely for the > duration that a job is using the buffer (and unmap immediately after). > This gives the exporter the knowledge of when the GPU is using the > memory and allows the exporter to page out of the memory if necessary. > In practise this map/unmap operation is expensive (updating the GPU's > page tables) so most drivers don't actually bother and keep the memory > mapped. This means the exporter cannot tell when the buffer is used or > move the pages. > > * The CPU mappings can be faulted on demand (performing the necessary > CPU cache invalidate if needed) and shot-down to allow moving the > memory. In theory when the GPU needs the memory it should map the buffer > and the exporter can then shoot down the mappings, perform the CPU cache > clean and then allow the GPU to use the memory. A subsequent CPU access > would then refault the page, ensuring a CPU cache invalidate so the > latest data is visible. > > * The majority of exporters are simple and deal with uncached memory > (e.g. frame buffers) or are actually exporting back to the same driver > (e.g. window surfaces). In these situations either the driver already > has the necessary "magic" to deal with caches (e.g. kbase provides > explicit cache maintenance operations), or it's "uncached" anyway so it > doesn't matter. This means that hardly anyone tests with the complex > cases... > > From a user space ABI, my understanding is that a dma_buf mmap() mapping > should be coherent, and user space isn't expected to do anything to make > it work. Obviously any importing device might have it's own
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, Jul 03, 2019 at 07:45:32AM -0600, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
> wrote:
> >
> > Exported BOs might be imported back, then mmap()-ed to be written
> > too. Most drivers handle that by mmap()-ing the GEM handle after it's
> > been imported, but, according to [1], this is illegal.
>
> It's not illegal, but is supposed to go thru the dmabuf mmap
> functions. However, none of the driver I've looked at (etnaviv, msm,
> v3d, vgem) do that. It probably works because it's the same driver
> doing the import and export or both drivers have essentially the same
> implementations.
For self-import we short-circuit out the underlying dma-buf and directly
go to the gem_bo again.
> > The panfrost
> > driver has recently switched to this generic helper (which was renamed
> > into drm_gem_map_offset() in the meantime) [2], and mmap()-ing
> > of imported BOs now fails.
>
> I think I'm going to revert this.
Can't we change the helper to force the rw mode to make this work?
Aside: The reason I'm not a huge fan of forwarding gem mmap to dma-buf
mmap is that generally the begin/end_cpu_access stuff gets lots on the way
there. Which mostly doesn't matter since everyone prefers to share
coherent buffers, except when it totally does matter. So by forwarding
mmap and pretending it all keeps working as-is we're digging ourselves a
bit into a hole I think :-/
Since the same discussion is happening with etnaviv: Why exactly does mesa
need to mmap imported buffers?
-Daniel
>
> > Now I'm wondering how this should be solved. I guess the first question
> > is, is mmap()-ing of imported BOs really forbidden for all BOs? I guess
> > calling mmap() on a buffer that's been exported by the DRM driver itself
> > then re-imported shouldn't hurt, so maybe we can check that before
> > failing.
> >
> > Now, if we really want to forbid mmap() on imported BOs, that means we
> > need a solution to mmap() the dmabuf object directly, and sometimes this
> > mapping will request RW permissions. The problem is, all function
> > exporting BOs in mesa are exporting them in RO-mode (resulting FD is
> > O_READ), thus preventing mmap()s in RW mode.
> >
> > This patch modifies all
> > drmPrimeHandleToFD()/ioctl(DRM_IOCTL_PRIME_HANDLE_TO_FD) call sites
> > to pass the DRM_RDWR flag so that what's described above becomes
> > possible.
> >
> > I'm not saying this is what we should do, it's more a way to start the
> > discussion. Feel free to propose alternives to this solution.
> >
> > [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem.c?h=v5.2-rc7#n318
> > [2]https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/panfrost?id=583bbf46133c726bae277e8f4e32bfba2a528c7f
> >
> > Signed-off-by: Boris Brezillon
> > Cc:
> > Cc:
> > Cc: Steven Price
> > Cc: Rob Herring
> > ---
> > Cc-ing dri-devel is not a mistake, I really to have feedback from the DRM
> > maintainers, since this started with a kernel-side change.
> > ---
> > src/etnaviv/drm/etnaviv_bo.c | 4 ++--
> > src/freedreno/drm/freedreno_bo.c | 4 ++--
> > src/freedreno/vulkan/tu_drm.c | 2 +-
> > src/gallium/auxiliary/renderonly/renderonly.c | 2 +-
> > src/gallium/drivers/iris/iris_bufmgr.c| 2 +-
> > src/gallium/drivers/lima/lima_bo.c| 2 +-
> > src/gallium/drivers/panfrost/pan_drm.c| 2 +-
> > src/gallium/drivers/v3d/v3d_bufmgr.c | 2 +-
> > src/gallium/drivers/vc4/vc4_bufmgr.c | 2 +-
> > src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 ++-
> > src/gallium/winsys/svga/drm/vmw_screen_dri.c | 5 +++--
> > src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 2 +-
> > src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 3 ++-
> > src/intel/vulkan/anv_gem.c| 2 +-
> > src/mesa/drivers/dri/i965/brw_bufmgr.c| 2 +-
> > 15 files changed, 21 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/etnaviv/drm/etnaviv_bo.c b/src/etnaviv/drm/etnaviv_bo.c
> > index 6e952fa47858..92634141b580 100644
> > --- a/src/etnaviv/drm/etnaviv_bo.c
> > +++ b/src/etnaviv/drm/etnaviv_bo.c
> > @@ -294,8 +294,8 @@ int etna_bo_dmabuf(struct etna_bo *bo)
> > {
> > int ret, prime_fd;
> >
> > - ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
> > - &prime_fd);
> > + ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
> > +DRM_CLOEXEC | DRM_RDWR, &prime_fd);
> > if (ret) {
> > ERROR_MSG("failed to get dmabuf fd: %d", ret);
> > return ret;
> > diff --git a/src/freedreno/drm/freedreno_bo.c
> > b/src/freedreno/drm/freedreno_bo.c
> > index 7449160f1371..ba19b08d7c54 100644
> > --- a/src/freedreno/drm/freedreno_bo.c
> > +++ b/src/freedreno/drm/freedreno_bo.c
> > @@ -318,8 +318,8 @@ int fd_bo_dmabuf(struct fd_bo *bo)
> > {
> >
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, Jul 03, 2019 at 05:47:24PM +0200, Daniel Vetter wrote:
> On Wed, Jul 03, 2019 at 07:45:32AM -0600, Rob Herring wrote:
> > On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
> > wrote:
> > >
> > > Exported BOs might be imported back, then mmap()-ed to be written
> > > too. Most drivers handle that by mmap()-ing the GEM handle after it's
> > > been imported, but, according to [1], this is illegal.
> >
> > It's not illegal, but is supposed to go thru the dmabuf mmap
> > functions. However, none of the driver I've looked at (etnaviv, msm,
> > v3d, vgem) do that. It probably works because it's the same driver
> > doing the import and export or both drivers have essentially the same
> > implementations.
>
> For self-import we short-circuit out the underlying dma-buf and directly
> go to the gem_bo again.
>
> > > The panfrost
> > > driver has recently switched to this generic helper (which was renamed
> > > into drm_gem_map_offset() in the meantime) [2], and mmap()-ing
> > > of imported BOs now fails.
> >
> > I think I'm going to revert this.
>
> Can't we change the helper to force the rw mode to make this work?
>
> Aside: The reason I'm not a huge fan of forwarding gem mmap to dma-buf
> mmap is that generally the begin/end_cpu_access stuff gets lots on the way
> there. Which mostly doesn't matter since everyone prefers to share
> coherent buffers, except when it totally does matter. So by forwarding
> mmap and pretending it all keeps working as-is we're digging ourselves a
> bit into a hole I think :-/
>
> Since the same discussion is happening with etnaviv: Why exactly does mesa
> need to mmap imported buffers?
Also, for dumb mmap I very much want to keep this requirement. If you
import a dma-buf and then mmap it through the dumb buffer interfaces,
you're just doing all the things wrong :-)
-Daniel
> -Daniel
>
> >
> > > Now I'm wondering how this should be solved. I guess the first question
> > > is, is mmap()-ing of imported BOs really forbidden for all BOs? I guess
> > > calling mmap() on a buffer that's been exported by the DRM driver itself
> > > then re-imported shouldn't hurt, so maybe we can check that before
> > > failing.
> > >
> > > Now, if we really want to forbid mmap() on imported BOs, that means we
> > > need a solution to mmap() the dmabuf object directly, and sometimes this
> > > mapping will request RW permissions. The problem is, all function
> > > exporting BOs in mesa are exporting them in RO-mode (resulting FD is
> > > O_READ), thus preventing mmap()s in RW mode.
> > >
> > > This patch modifies all
> > > drmPrimeHandleToFD()/ioctl(DRM_IOCTL_PRIME_HANDLE_TO_FD) call sites
> > > to pass the DRM_RDWR flag so that what's described above becomes
> > > possible.
> > >
> > > I'm not saying this is what we should do, it's more a way to start the
> > > discussion. Feel free to propose alternives to this solution.
> > >
> > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem.c?h=v5.2-rc7#n318
> > > [2]https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/panfrost?id=583bbf46133c726bae277e8f4e32bfba2a528c7f
> > >
> > > Signed-off-by: Boris Brezillon
> > > Cc:
> > > Cc:
> > > Cc: Steven Price
> > > Cc: Rob Herring
> > > ---
> > > Cc-ing dri-devel is not a mistake, I really to have feedback from the DRM
> > > maintainers, since this started with a kernel-side change.
> > > ---
> > > src/etnaviv/drm/etnaviv_bo.c | 4 ++--
> > > src/freedreno/drm/freedreno_bo.c | 4 ++--
> > > src/freedreno/vulkan/tu_drm.c | 2 +-
> > > src/gallium/auxiliary/renderonly/renderonly.c | 2 +-
> > > src/gallium/drivers/iris/iris_bufmgr.c| 2 +-
> > > src/gallium/drivers/lima/lima_bo.c| 2 +-
> > > src/gallium/drivers/panfrost/pan_drm.c| 2 +-
> > > src/gallium/drivers/v3d/v3d_bufmgr.c | 2 +-
> > > src/gallium/drivers/vc4/vc4_bufmgr.c | 2 +-
> > > src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 ++-
> > > src/gallium/winsys/svga/drm/vmw_screen_dri.c | 5 +++--
> > > src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 2 +-
> > > src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 3 ++-
> > > src/intel/vulkan/anv_gem.c| 2 +-
> > > src/mesa/drivers/dri/i965/brw_bufmgr.c| 2 +-
> > > 15 files changed, 21 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/src/etnaviv/drm/etnaviv_bo.c b/src/etnaviv/drm/etnaviv_bo.c
> > > index 6e952fa47858..92634141b580 100644
> > > --- a/src/etnaviv/drm/etnaviv_bo.c
> > > +++ b/src/etnaviv/drm/etnaviv_bo.c
> > > @@ -294,8 +294,8 @@ int etna_bo_dmabuf(struct etna_bo *bo)
> > > {
> > > int ret, prime_fd;
> > >
> > > - ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
> > > - &prime_fd);
> > > + ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
> > > +
[Mesa-dev] [Bug 111043] PBO unpacking is not accelerated
https://bugs.freedesktop.org/show_bug.cgi?id=111043 John changed: What|Removed |Added CC||john.etted...@gmail.com -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
> Since the same discussion is happening with etnaviv: Why exactly does mesa > need to mmap imported buffers? The golden question! Seemingly, (one of the) reasons is that the state tracker does colourspace conversion in software if the winsys wants a format that the driver doesn't advertise. Unrelated to this, that's almost certainly a performance issue and might explain quite a bit... In this case, the winsys seems to want RGB10_A2 but we settle for RGBA8. I thought that was just changing EGL negotiation stuff.. I had no idea it was covering up in software... not good... signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On 03/07/2019 15:33, Boris Brezillon wrote: > On Wed, 3 Jul 2019 15:13:25 +0100 > Steven Price wrote: > >> On 03/07/2019 14:56, Boris Brezillon wrote: >>> On Wed, 3 Jul 2019 07:45:32 -0600 >>> Rob Herring wrote: >>> On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon wrote: > > Exported BOs might be imported back, then mmap()-ed to be written > too. Most drivers handle that by mmap()-ing the GEM handle after it's > been imported, but, according to [1], this is illegal. It's not illegal, but is supposed to go thru the dmabuf mmap functions. >>> >>> That's basically what I'm proposing here, just didn't post the patch >>> skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD >>> instead of the DRM-node one, but I have it working for panfrost. >> >> If we want to we could make the Panfrost kernel driver internally call >> dma_buf_mmap() so that mapping using the DRM-node "just works". This is >> indeed what the kbase driver does. > > Well, userspace should at least skip DRM_IOCTL_PANFROST_MMAP_BO (or > ignore its return code), so calling mmap() on the dmabuf FD instead of > the DRM-node FD shouldn't be that hard. What I was suggesting is that user space would still call DRM_IOCTL_PANFROST_MMAP_BO to get an offset which uses in a call to mmap(..., drm_node_fd, offset). The kernel could detect that the buffer is imported and call the exporter for the actual mmap() functionality. The alternative is that user space 'simply' remembers that a buffer is imported and keeps the file descriptor around so that it can instead directly mmap() the dma_buf fd. Which is certainly easiest from the kernel's perspective (and was what I assumed panfrost was doing - I should have checked more closely!). However, none of the driver I've looked at (etnaviv, msm, v3d, vgem) do that. It probably works because it's the same driver doing the import and export or both drivers have essentially the same implementations. >>> >>> Yes, but maybe that's something we should start fixing if mmap()-ing >>> the dmabuf is the recommended solution. >> >> I'm open to options here. User space calling mmap() on the dma_buf file >> descriptor should always be safe (the exporter can do whatever is >> necessary to make it work). If that happens then the patches I posted >> close off the DRM node version which could be broken if the exporter >> needs to do anything to prepare the buffer for CPU access (i.e. cache >> maintenance). > > Talking about CPU <-> GPU syncs, I was wondering if the > mmap(gem_handle) step was providing any guarantee that would > allow us to ignore all the cache maintenance operations that are > required when mmap()-ing a dmabuf directly. Note that in both cases the > dmabuf is imported. In theory the exporter should do whatever is required to ensure that the CPU is synchronised when a user space mapping exists. There are some issues here though: * In theory the kernel driver should map the dma_buf purely for the duration that a job is using the buffer (and unmap immediately after). This gives the exporter the knowledge of when the GPU is using the memory and allows the exporter to page out of the memory if necessary. In practise this map/unmap operation is expensive (updating the GPU's page tables) so most drivers don't actually bother and keep the memory mapped. This means the exporter cannot tell when the buffer is used or move the pages. * The CPU mappings can be faulted on demand (performing the necessary CPU cache invalidate if needed) and shot-down to allow moving the memory. In theory when the GPU needs the memory it should map the buffer and the exporter can then shoot down the mappings, perform the CPU cache clean and then allow the GPU to use the memory. A subsequent CPU access would then refault the page, ensuring a CPU cache invalidate so the latest data is visible. * The majority of exporters are simple and deal with uncached memory (e.g. frame buffers) or are actually exporting back to the same driver (e.g. window surfaces). In these situations either the driver already has the necessary "magic" to deal with caches (e.g. kbase provides explicit cache maintenance operations), or it's "uncached" anyway so it doesn't matter. This means that hardly anyone tests with the complex cases... From a user space ABI, my understanding is that a dma_buf mmap() mapping should be coherent, and user space isn't expected to do anything to make it work. Obviously any importing device might have it's own coherency details which will be up to the ABI of that device (e.g. Mali has caches which may need to be flushed - this is usually done at the start/end of a job chain, so coherency is not guaranteed while the job chain is running). Steve ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On 03/07/2019 14:56, Boris Brezillon wrote:
> On Wed, 3 Jul 2019 07:45:32 -0600
> Rob Herring wrote:
>
>> On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
>> wrote:
>>>
>>> Exported BOs might be imported back, then mmap()-ed to be written
>>> too. Most drivers handle that by mmap()-ing the GEM handle after it's
>>> been imported, but, according to [1], this is illegal.
>>
>> It's not illegal, but is supposed to go thru the dmabuf mmap
>> functions.
>
> That's basically what I'm proposing here, just didn't post the patch
> skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD
> instead of the DRM-node one, but I have it working for panfrost.
If we want to we could make the Panfrost kernel driver internally call
dma_buf_mmap() so that mapping using the DRM-node "just works". This is
indeed what the kbase driver does.
>> However, none of the driver I've looked at (etnaviv, msm,
>> v3d, vgem) do that. It probably works because it's the same driver
>> doing the import and export or both drivers have essentially the same
>> implementations.
>
> Yes, but maybe that's something we should start fixing if mmap()-ing
> the dmabuf is the recommended solution.
I'm open to options here. User space calling mmap() on the dma_buf file
descriptor should always be safe (the exporter can do whatever is
necessary to make it work). If that happens then the patches I posted
close off the DRM node version which could be broken if the exporter
needs to do anything to prepare the buffer for CPU access (i.e. cache
maintenance).
Alternatively if user space wants/needs to use the DMA node then we can
take a look at what needs to change in the kernel. From a quick look at
the code it seems we'd need to split drm_gem_mmap() into a helper so
that it can return whether the exporter is handling everything or if the
caller needs to do some more work (e.g. drm_gem_shmem_mmap() needs to
allocate backing pages). But because drm_gem_mmap() is used as the
direct callback for some drivers we'd need to preserve the interface.
The below (completely untested) patch demonstrates the idea.
Steve
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index a8c4468f03d9..df661e24cadf 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
* If the caller is not granted access to the buffer object, the mmap
will fail
* with EACCES. Please see the vma manager for more information.
*/
-int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+int drm_gem_mmap_helper(struct file *filp, struct vm_area_struct *vma)
{
struct drm_file *priv = filp->private_data;
struct drm_device *dev = priv->minor->dev;
@@ -1189,6 +1189,11 @@ int drm_gem_mmap(struct file *filp, struct
vm_area_struct *vma)
vma->vm_flags &= ~VM_MAYWRITE;
}
+ if (obj->import_attach) {
+ ret = dma_buf_mmap(obj->dma_buf, vma, 0);
+ return ret?:1;
+ }
+
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
vma);
@@ -1196,6 +1201,16 @@ int drm_gem_mmap(struct file *filp, struct
vm_area_struct *vma)
return ret;
}
+
+int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ int ret;
+
+ ret = drm_gem_mmap_helper(filp, vma);
+ if (ret == 1)
+ return 0;
+ return ret;
+}
EXPORT_SYMBOL(drm_gem_mmap);
void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 472ea5d81f82..b85d84e4d4a8 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -466,8 +466,10 @@ int drm_gem_shmem_mmap(struct file *filp, struct
vm_area_struct *vma)
struct drm_gem_shmem_object *shmem;
int ret;
- ret = drm_gem_mmap(filp, vma);
- if (ret)
+ ret = drm_gem_mmap_helper(filp, vma);
+ if (ret == 1)
+ return 0; /* Exporter handles the mapping */
+ else if (ret)
return ret;
shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
wrote:
>
> Exported BOs might be imported back, then mmap()-ed to be written
> too. Most drivers handle that by mmap()-ing the GEM handle after it's
> been imported, but, according to [1], this is illegal.
It's not illegal, but is supposed to go thru the dmabuf mmap
functions. However, none of the driver I've looked at (etnaviv, msm,
v3d, vgem) do that. It probably works because it's the same driver
doing the import and export or both drivers have essentially the same
implementations.
> The panfrost
> driver has recently switched to this generic helper (which was renamed
> into drm_gem_map_offset() in the meantime) [2], and mmap()-ing
> of imported BOs now fails.
I think I'm going to revert this.
> Now I'm wondering how this should be solved. I guess the first question
> is, is mmap()-ing of imported BOs really forbidden for all BOs? I guess
> calling mmap() on a buffer that's been exported by the DRM driver itself
> then re-imported shouldn't hurt, so maybe we can check that before
> failing.
>
> Now, if we really want to forbid mmap() on imported BOs, that means we
> need a solution to mmap() the dmabuf object directly, and sometimes this
> mapping will request RW permissions. The problem is, all function
> exporting BOs in mesa are exporting them in RO-mode (resulting FD is
> O_READ), thus preventing mmap()s in RW mode.
>
> This patch modifies all
> drmPrimeHandleToFD()/ioctl(DRM_IOCTL_PRIME_HANDLE_TO_FD) call sites
> to pass the DRM_RDWR flag so that what's described above becomes
> possible.
>
> I'm not saying this is what we should do, it's more a way to start the
> discussion. Feel free to propose alternives to this solution.
>
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem.c?h=v5.2-rc7#n318
> [2]https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/panfrost?id=583bbf46133c726bae277e8f4e32bfba2a528c7f
>
> Signed-off-by: Boris Brezillon
> Cc:
> Cc:
> Cc: Steven Price
> Cc: Rob Herring
> ---
> Cc-ing dri-devel is not a mistake, I really to have feedback from the DRM
> maintainers, since this started with a kernel-side change.
> ---
> src/etnaviv/drm/etnaviv_bo.c | 4 ++--
> src/freedreno/drm/freedreno_bo.c | 4 ++--
> src/freedreno/vulkan/tu_drm.c | 2 +-
> src/gallium/auxiliary/renderonly/renderonly.c | 2 +-
> src/gallium/drivers/iris/iris_bufmgr.c| 2 +-
> src/gallium/drivers/lima/lima_bo.c| 2 +-
> src/gallium/drivers/panfrost/pan_drm.c| 2 +-
> src/gallium/drivers/v3d/v3d_bufmgr.c | 2 +-
> src/gallium/drivers/vc4/vc4_bufmgr.c | 2 +-
> src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 ++-
> src/gallium/winsys/svga/drm/vmw_screen_dri.c | 5 +++--
> src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 2 +-
> src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 3 ++-
> src/intel/vulkan/anv_gem.c| 2 +-
> src/mesa/drivers/dri/i965/brw_bufmgr.c| 2 +-
> 15 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/src/etnaviv/drm/etnaviv_bo.c b/src/etnaviv/drm/etnaviv_bo.c
> index 6e952fa47858..92634141b580 100644
> --- a/src/etnaviv/drm/etnaviv_bo.c
> +++ b/src/etnaviv/drm/etnaviv_bo.c
> @@ -294,8 +294,8 @@ int etna_bo_dmabuf(struct etna_bo *bo)
> {
> int ret, prime_fd;
>
> - ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
> - &prime_fd);
> + ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
> +DRM_CLOEXEC | DRM_RDWR, &prime_fd);
> if (ret) {
> ERROR_MSG("failed to get dmabuf fd: %d", ret);
> return ret;
> diff --git a/src/freedreno/drm/freedreno_bo.c
> b/src/freedreno/drm/freedreno_bo.c
> index 7449160f1371..ba19b08d7c54 100644
> --- a/src/freedreno/drm/freedreno_bo.c
> +++ b/src/freedreno/drm/freedreno_bo.c
> @@ -318,8 +318,8 @@ int fd_bo_dmabuf(struct fd_bo *bo)
> {
> int ret, prime_fd;
>
> - ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
> - &prime_fd);
> + ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
> +DRM_CLOEXEC | DRM_RDWR, &prime_fd);
> if (ret) {
> ERROR_MSG("failed to get dmabuf fd: %d", ret);
> return ret;
> diff --git a/src/freedreno/vulkan/tu_drm.c b/src/freedreno/vulkan/tu_drm.c
> index 9b2e6f78879e..6bef3012ddb5 100644
> --- a/src/freedreno/vulkan/tu_drm.c
> +++ b/src/freedreno/vulkan/tu_drm.c
> @@ -147,7 +147,7 @@ tu_gem_export_dmabuf(const struct tu_device *dev,
> uint32_t gem_handle)
> {
> int prime_fd;
> int ret = drmPrimeHandleToFD(dev->physical_device->local_fd, gem_handle,
> -DRM_CLOEXEC, &prime_fd);
> +DRM_CLOEXEC | DRM_RDWR, &prime_fd
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, Jul 3, 2019 at 8:13 AM Steven Price wrote:
>
> On 03/07/2019 14:56, Boris Brezillon wrote:
> > On Wed, 3 Jul 2019 07:45:32 -0600
> > Rob Herring wrote:
> >
> >> On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
> >> wrote:
> >>>
> >>> Exported BOs might be imported back, then mmap()-ed to be written
> >>> too. Most drivers handle that by mmap()-ing the GEM handle after it's
> >>> been imported, but, according to [1], this is illegal.
> >>
> >> It's not illegal, but is supposed to go thru the dmabuf mmap
> >> functions.
> >
> > That's basically what I'm proposing here, just didn't post the patch
> > skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD
> > instead of the DRM-node one, but I have it working for panfrost.
>
> If we want to we could make the Panfrost kernel driver internally call
> dma_buf_mmap() so that mapping using the DRM-node "just works". This is
> indeed what the kbase driver does.
>
> >> However, none of the driver I've looked at (etnaviv, msm,
> >> v3d, vgem) do that. It probably works because it's the same driver
> >> doing the import and export or both drivers have essentially the same
> >> implementations.
> >
> > Yes, but maybe that's something we should start fixing if mmap()-ing
> > the dmabuf is the recommended solution.
>
> I'm open to options here. User space calling mmap() on the dma_buf file
> descriptor should always be safe (the exporter can do whatever is
> necessary to make it work). If that happens then the patches I posted
> close off the DRM node version which could be broken if the exporter
> needs to do anything to prepare the buffer for CPU access (i.e. cache
> maintenance).
>
> Alternatively if user space wants/needs to use the DMA node then we can
> take a look at what needs to change in the kernel. From a quick look at
> the code it seems we'd need to split drm_gem_mmap() into a helper so
> that it can return whether the exporter is handling everything or if the
> caller needs to do some more work (e.g. drm_gem_shmem_mmap() needs to
> allocate backing pages). But because drm_gem_mmap() is used as the
> direct callback for some drivers we'd need to preserve the interface.
>
> The below (completely untested) patch demonstrates the idea.
>
> Steve
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index a8c4468f03d9..df661e24cadf 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
> * If the caller is not granted access to the buffer object, the mmap
> will fail
> * with EACCES. Please see the vma manager for more information.
> */
> -int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +int drm_gem_mmap_helper(struct file *filp, struct vm_area_struct *vma)
> {
> struct drm_file *priv = filp->private_data;
> struct drm_device *dev = priv->minor->dev;
> @@ -1189,6 +1189,11 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> vma->vm_flags &= ~VM_MAYWRITE;
> }
>
> + if (obj->import_attach) {
> + ret = dma_buf_mmap(obj->dma_buf, vma, 0);
I don't see how calling dma_buf_mmap() would work. Looking at etnaviv
which is the only GPU driver calling it, it looks to me like there a
recursive loop:
driver->fops.mmap -> obj->mmap -> dma_buf_mmap -> .gem_prime_mmap ->
etnaviv_gem_prime_mmap -> obj->mmap -> dma_buf_mmap -> ...
Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On Wed, Jul 3, 2019 at 6:11 PM Steven Price wrote: > > On 03/07/2019 15:33, Boris Brezillon wrote: > > On Wed, 3 Jul 2019 15:13:25 +0100 > > Steven Price wrote: > > > >> On 03/07/2019 14:56, Boris Brezillon wrote: > >>> On Wed, 3 Jul 2019 07:45:32 -0600 > >>> Rob Herring wrote: > >>> > On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon > wrote: > > > > Exported BOs might be imported back, then mmap()-ed to be written > > too. Most drivers handle that by mmap()-ing the GEM handle after it's > > been imported, but, according to [1], this is illegal. > > It's not illegal, but is supposed to go thru the dmabuf mmap > functions. > >>> > >>> That's basically what I'm proposing here, just didn't post the patch > >>> skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD > >>> instead of the DRM-node one, but I have it working for panfrost. > >> > >> If we want to we could make the Panfrost kernel driver internally call > >> dma_buf_mmap() so that mapping using the DRM-node "just works". This is > >> indeed what the kbase driver does. > > > > Well, userspace should at least skip DRM_IOCTL_PANFROST_MMAP_BO (or > > ignore its return code), so calling mmap() on the dmabuf FD instead of > > the DRM-node FD shouldn't be that hard. > > What I was suggesting is that user space would still call > DRM_IOCTL_PANFROST_MMAP_BO to get an offset which uses in a call to > mmap(..., drm_node_fd, offset). The kernel could detect that the buffer > is imported and call the exporter for the actual mmap() functionality. > > The alternative is that user space 'simply' remembers that a buffer is > imported and keeps the file descriptor around so that it can instead > directly mmap() the dma_buf fd. Which is certainly easiest from the > kernel's perspective (and was what I assumed panfrost was doing - I > should have checked more closely!). > > However, none of the driver I've looked at (etnaviv, msm, > v3d, vgem) do that. It probably works because it's the same driver > doing the import and export or both drivers have essentially the same > implementations. > >>> > >>> Yes, but maybe that's something we should start fixing if mmap()-ing > >>> the dmabuf is the recommended solution. > >> > >> I'm open to options here. User space calling mmap() on the dma_buf file > >> descriptor should always be safe (the exporter can do whatever is > >> necessary to make it work). If that happens then the patches I posted > >> close off the DRM node version which could be broken if the exporter > >> needs to do anything to prepare the buffer for CPU access (i.e. cache > >> maintenance). > > > > Talking about CPU <-> GPU syncs, I was wondering if the > > mmap(gem_handle) step was providing any guarantee that would > > allow us to ignore all the cache maintenance operations that are > > required when mmap()-ing a dmabuf directly. Note that in both cases the > > dmabuf is imported. > > In theory the exporter should do whatever is required to ensure that the > CPU is synchronised when a user space mapping exists. There are some > issues here though: > > * In theory the kernel driver should map the dma_buf purely for the > duration that a job is using the buffer (and unmap immediately after). > This gives the exporter the knowledge of when the GPU is using the > memory and allows the exporter to page out of the memory if necessary. > In practise this map/unmap operation is expensive (updating the GPU's > page tables) so most drivers don't actually bother and keep the memory > mapped. This means the exporter cannot tell when the buffer is used or > move the pages. Yeah refaulting is too slow if you care the least about performance. > * The CPU mappings can be faulted on demand (performing the necessary > CPU cache invalidate if needed) and shot-down to allow moving the > memory. In theory when the GPU needs the memory it should map the buffer > and the exporter can then shoot down the mappings, perform the CPU cache > clean and then allow the GPU to use the memory. A subsequent CPU access > would then refault the page, ensuring a CPU cache invalidate so the > latest data is visible. We thought that was the answer, until it was clear its not. dma-buf mmap isn't coherent, you need to call the begin/end ioctls. > * The majority of exporters are simple and deal with uncached memory > (e.g. frame buffers) or are actually exporting back to the same driver > (e.g. window surfaces). In these situations either the driver already > has the necessary "magic" to deal with caches (e.g. kbase provides > explicit cache maintenance operations), or it's "uncached" anyway so it > doesn't matter. This means that hardly anyone tests with the complex > cases... > > From a user space ABI, my understanding is that a dma_buf mmap() mapping > should be coherent, and user space isn't expected to do anything to make > it work. Obviously any importing device might have it's own coherency > details which will be
Re: [Mesa-dev] [RFC PATCH] mesa: Export BOs in RW mode
On 03/07/2019 16:59, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 8:13 AM Steven Price wrote:
>>
>> On 03/07/2019 14:56, Boris Brezillon wrote:
>>> On Wed, 3 Jul 2019 07:45:32 -0600
>>> Rob Herring wrote:
>>>
On Wed, Jul 3, 2019 at 7:34 AM Boris Brezillon
wrote:
>
> Exported BOs might be imported back, then mmap()-ed to be written
> too. Most drivers handle that by mmap()-ing the GEM handle after it's
> been imported, but, according to [1], this is illegal.
It's not illegal, but is supposed to go thru the dmabuf mmap
functions.
>>>
>>> That's basically what I'm proposing here, just didn't post the patch
>>> skipping the GET_OFFSET step and doing the mmap() on the dmabuf FD
>>> instead of the DRM-node one, but I have it working for panfrost.
>>
>> If we want to we could make the Panfrost kernel driver internally call
>> dma_buf_mmap() so that mapping using the DRM-node "just works". This is
>> indeed what the kbase driver does.
>>
However, none of the driver I've looked at (etnaviv, msm,
v3d, vgem) do that. It probably works because it's the same driver
doing the import and export or both drivers have essentially the same
implementations.
>>>
>>> Yes, but maybe that's something we should start fixing if mmap()-ing
>>> the dmabuf is the recommended solution.
>>
>> I'm open to options here. User space calling mmap() on the dma_buf file
>> descriptor should always be safe (the exporter can do whatever is
>> necessary to make it work). If that happens then the patches I posted
>> close off the DRM node version which could be broken if the exporter
>> needs to do anything to prepare the buffer for CPU access (i.e. cache
>> maintenance).
>>
>> Alternatively if user space wants/needs to use the DMA node then we can
>> take a look at what needs to change in the kernel. From a quick look at
>> the code it seems we'd need to split drm_gem_mmap() into a helper so
>> that it can return whether the exporter is handling everything or if the
>> caller needs to do some more work (e.g. drm_gem_shmem_mmap() needs to
>> allocate backing pages). But because drm_gem_mmap() is used as the
>> direct callback for some drivers we'd need to preserve the interface.
>>
>> The below (completely untested) patch demonstrates the idea.
>>
>> Steve
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index a8c4468f03d9..df661e24cadf 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -1140,7 +1140,7 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
>> * If the caller is not granted access to the buffer object, the mmap
>> will fail
>> * with EACCES. Please see the vma manager for more information.
>> */
>> -int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> +int drm_gem_mmap_helper(struct file *filp, struct vm_area_struct *vma)
>> {
>> struct drm_file *priv = filp->private_data;
>> struct drm_device *dev = priv->minor->dev;
>> @@ -1189,6 +1189,11 @@ int drm_gem_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> vma->vm_flags &= ~VM_MAYWRITE;
>> }
>>
>> + if (obj->import_attach) {
>> + ret = dma_buf_mmap(obj->dma_buf, vma, 0);
>
> I don't see how calling dma_buf_mmap() would work. Looking at etnaviv
> which is the only GPU driver calling it, it looks to me like there a
> recursive loop:
>
> driver->fops.mmap -> obj->mmap -> dma_buf_mmap -> .gem_prime_mmap ->
> etnaviv_gem_prime_mmap -> obj->mmap -> dma_buf_mmap -> ...
I did say it was untested :)
The call to dma_buf_mmap should only be in the drivers->fops.mmap
callback, not in obj->mmap. I have to admit I didn't actually bother to
look where drm_gem_mmap() was being used. So drm_gem_shmem_mmap() needs
to call a function which does do the dma_buf_mmap(), but the callback in
obj->mmap shouldn't.
There's a reason I initially went for simply preventing user space
mmap()ing imported objects (through the DRM node) rather than trying to
make it work ;)
Steve
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111053] Elite Dangerous shadows render as white.
https://bugs.freedesktop.org/show_bug.cgi?id=111053 --- Comment #1 from Haxk20 --- Created attachment 144695 --> https://bugs.freedesktop.org/attachment.cgi?id=144695&action=edit How its supposed to look -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111053] Elite Dangerous shadows render as white.
https://bugs.freedesktop.org/show_bug.cgi?id=111053 Bug ID: 111053 Summary: Elite Dangerous shadows render as white. Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: major Priority: medium Component: Drivers/Vulkan/radeon Assignee: mesa-dev@lists.freedesktop.org Reporter: haxk...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org Created attachment 144694 --> https://bugs.freedesktop.org/attachment.cgi?id=144694&action=edit bug picture I reported this bug originally as LLVM bug because after changing LLVM to older version it worked totally OK but i have been told in the bug report to report it here. The issue: After few days of not playing ED i started it to find out that shadows look white and basically entire game looks like if brightness was set to 1000% Basically unplayable. Im using LLVM from git but im using mesa-git repo in Arch linux and the last working packaged version of LLVM is r319790.e8da65c698e. Im happy to provide any logs you would need. Im attaching pictures of how to game is supposed to look like and how it looks like with the bug. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] ACO: A New Compiler Backend for RADV
Hello everyone, as some of you already know, for a little over one year I have been working on an alternate compiler backend for the RADV driver. At the beginning, Bas Nieuwenhuizen helped out a lot, and since last December Rhys Perry has also helped tremendously working full-time on ACO. In this RFC, I'd like to share with you our motivation for this work as well as some implementation details and the current state. The current development branch of ACO with full commit history can be found at https://github.com/daniel-schuermann/mesa/tree/backend while a slightly more stable branch is (until upstream) maintained at https://github.com/daniel-schuermann/mesa/tree/master/ For initial results, I'd like to refer to this post: https://steamcommunity.com/games/221410/announcements/detail/1602634609636894200 Feel free to ask questions or just add your thoughts. Motivation: The RADV driver currently uses LLVM as backend for shader compilation. There are some shortcomings regarding LLVM's compilation of graphic shaders which need to be addressed. The idea and motivation of ACO is the expectation that it would be less work long-term to re-write the backend than to fix LLVM. Without going to much into detail here, the main shortcomings of LLVM are compile times and the handling of control flow, which has lead to some serious bugs in the past. Additionally, we were able to implement a more aggressive divergence analysis and having more precise control over register pressure which can ultimately lead to more efficient binaries. A welcome side-effect is an integrated development process without having to deal with LLVM's release cycles. Implementation: What started as a proof-of-concept and interesting experimental platform advanced quickly to a full-featured backend capable of replacing LLVM in the RADV driver in the near future. ACO is based on principles from recent compiler research results and tries to avoid the issues we are experiencing with LLVM. The IR is fully SSA-based and also does register allocation on SSA which allows to precisely pre-calculate the register demand of a shader. We implemented the notion of a logical and linear (or physical) control flow graph which let us quickly and easily add horizontal reductions (thx Connor Abbott) - a problem which took almost two years and various attempts to solve in LLVM, still being far slower than our solution. ACO is written with just-in-time compilation in mind and uses data structures which are quick to iterate. Avoiding pointer-based data structures like linked lists and the common def-use chains leads to much faster compile times. ACO is fully written in C++. Current State: Currently, ACO only supports FS and CS, only on VI+ and only on 32bit and some 64bit operations. It misses VGPR spilling (we didn't need it on any tested game so far) and has a theoretical issue (in case a divergent/uniform memory write is followed by a memory read of the other kind) which needs a proper alias analysis to resolve. Nevertheless, ACO is able to correctly compile the shaders of (almost?) all games including complex ones like Shadow of the Tomb Raider and Wolfenstein II. We'd like to upstream ACO as experimental driver option to ease development synchronization, get more feedback, but ultimately also to give access to the performance enhancements we achieved. To ease the upstreaming efforts, we created MRs for all changes to the NIR infrastructure. After these MRs have went through the reviewing process and landed, we are going to create a single MR for ACO. Meanwhile, we will refactor the coding style and squash the commits. Please review! nir: lowering shared memory derefs with nir_lower_io_explicit(): https://gitlab.freedesktop.org/mesa/mesa/merge_requests/622 radv/radeonsi: Use NIR barycentric interpolation intrinsics: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/906 WIP: nir: add divergence analysis pass: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/918 WIP: nir: lower int64 in a single pass: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1224 nir: A Couple of Comparison Optimizations: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1228 radv: disable lower_sub: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1236 nir: change nir_lower_io_to_vector() so that it can always vectorize FS outputs: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1238 nir/lower_idiv: add new urcp path: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1239 nir: add a memory load/store vectorization and combining pass: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1240 nir: replace nir_move_load_const() with nir_opt_sink(): https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1241 We welcome any testing feedback and bug reports at https://github.com/daniel-schuermann/mesa/issues Thanks, Daniel Schürmann Rhys Perry Bas Nieuwenhuizen _
Re: [Mesa-dev] [RFC] ACO: A New Compiler Backend for RADV
Congratulations on this monumental achievement! Bringing up a whole back-end compiler is a huge amount of work and doing it in a year with only a couple of people is pretty impressive. It's great to see this work finally see the light of day and I look forward to seeing how it progresses going forward. --Jason On Wed, Jul 3, 2019 at 12:23 PM Daniel Schürmann wrote: > Hello everyone, > as some of you already know, for a little over one year I have been > working on > an alternate compiler backend for the RADV driver. At the beginning, Bas > Nieuwenhuizen > helped out a lot, and since last December Rhys Perry has also helped > tremendously > working full-time on ACO. > > In this RFC, I'd like to share with you our motivation for this work as > well as some > implementation details and the current state. > > The current development branch of ACO with full commit history can be > found at > https://github.com/daniel-schuermann/mesa/tree/backend > while a slightly more stable branch is (until upstream) maintained at > https://github.com/daniel-schuermann/mesa/tree/master/ > > For initial results, I'd like to refer to this post: > > https://steamcommunity.com/games/221410/announcements/detail/1602634609636894200 > > Feel free to ask questions or just add your thoughts. > > Motivation: > The RADV driver currently uses LLVM as backend for shader compilation. > There are some > shortcomings regarding LLVM's compilation of graphic shaders which need > to be addressed. > The idea and motivation of ACO is the expectation that it would be less > work long-term to > re-write the backend than to fix LLVM. > Without going to much into detail here, the main shortcomings of LLVM > are compile times and > the handling of control flow, which has lead to some serious bugs in the > past. > Additionally, we were able to implement a more aggressive divergence > analysis and having more > precise control over register pressure which can ultimately lead to more > efficient binaries. > A welcome side-effect is an integrated development process without > having to deal with LLVM's > release cycles. > > Implementation: > What started as a proof-of-concept and interesting experimental platform > advanced quickly to > a full-featured backend capable of replacing LLVM in the RADV driver in > the near future. > ACO is based on principles from recent compiler research results and > tries to avoid the issues > we are experiencing with LLVM. The IR is fully SSA-based and also does > register allocation on > SSA which allows to precisely pre-calculate the register demand of a > shader. > We implemented the notion of a logical and linear (or physical) control > flow graph which let us > quickly and easily add horizontal reductions (thx Connor Abbott) - a > problem which took almost > two years and various attempts to solve in LLVM, still being far slower > than our solution. > ACO is written with just-in-time compilation in mind and uses data > structures which are quick to > iterate. Avoiding pointer-based data structures like linked lists and > the common def-use chains > leads to much faster compile times. ACO is fully written in C++. > > Current State: > Currently, ACO only supports FS and CS, only on VI+ and only on 32bit > and some 64bit operations. > It misses VGPR spilling (we didn't need it on any tested game so far) > and has a theoretical > issue (in case a divergent/uniform memory write is followed by a memory > read of the other kind) > which needs a proper alias analysis to resolve. > Nevertheless, ACO is able to correctly compile the shaders of (almost?) > all games including > complex ones like Shadow of the Tomb Raider and Wolfenstein II. > We'd like to upstream ACO as experimental driver option to ease > development synchronization, > get more feedback, but ultimately also to give access to the performance > enhancements we achieved. > > > To ease the upstreaming efforts, we created MRs for all changes to the > NIR infrastructure. > After these MRs have went through the reviewing process and landed, we > are going to create a > single MR for ACO. Meanwhile, we will refactor the coding style and > squash the commits. > Please review! > > nir: lowering shared memory derefs with nir_lower_io_explicit(): > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/622 > > radv/radeonsi: Use NIR barycentric interpolation intrinsics: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/906 > > WIP: nir: add divergence analysis pass: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/918 > > WIP: nir: lower int64 in a single pass: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1224 > > nir: A Couple of Comparison Optimizations: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1228 > > radv: disable lower_sub: > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1236 > > nir: change nir_lower_io_to_vector() so that it can always vectorize FS > outputs: > https://gitlab.freedes
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 106224, which changed state. Bug 106224 Summary: "Medieval 2 Total War" will sometimes crash system on a R9 270 card https://bugs.freedesktop.org/show_bug.cgi?id=106224 What|Removed |Added Status|NEEDINFO|RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] radeonsi: glmark2 - regression (GL_INVALID_OPERATION in glFramebufferTexture2D) - bisected
NDEBUG is set = release NDEBUG is not set = debugoptimized NDEBUG is not set and DEBUG is set = debug NDEBUG is not set ---> enable assertions and cheap debug code, NIR enables expensive validations DEBUG is set ---> enable possibly very expensive debug code, may be replaced by !NDEBUG in the future Marek On Wed, Jul 3, 2019 at 4:00 AM Michel Dänzer wrote: > On 2019-07-02 7:29 p.m., Marek Olšák wrote: > > If you don't wanna see the messages, don't use debugoptimized. > > Makes sense, but means it needs to be guarded by DEBUG, not NDEBUG. > > > -- > Earthling Michel Dänzer | https://www.amd.com > Libre software enthusiast | Mesa and X developer > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: Add ir3/ir3_nir_imul.c generation to Android.mk
On Wed, Jul 3, 2019 at 4:10 PM John Stultz wrote: > > With current master we're seeing build failures with AOSP: > error: undefined symbol: ir3_nir_lower_imul > > This is due to the ir3_nir_imul.c file not being generated > in the Android.mk files. > > This patch simply adds it to the Android build, after which > thigns build and boot ok on db410c. > > Cc: Rob Clark > Cc: Emil Velikov > Cc: Amit Pundir > Cc: Sumit Semwal > Cc: Alistair Strachan > Cc: Greg Hartman > Cc: Tapani Pälli > Signed-off-by: John Stultz Reviewed-by: Rob Clark > --- > src/freedreno/Makefile.sources | 3 ++- > src/gallium/drivers/freedreno/Android.gen.mk | 7 +++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/freedreno/Makefile.sources b/src/freedreno/Makefile.sources > index d8aaf2caecc..75ec361663b 100644 > --- a/src/freedreno/Makefile.sources > +++ b/src/freedreno/Makefile.sources > @@ -48,7 +48,8 @@ ir3_SOURCES := \ > ir3/ir3_sun.c > > ir3_GENERATED_FILES := \ > - ir3/ir3_nir_trig.c > + ir3/ir3_nir_trig.c \ > + ir3/ir3_nir_imul.c > > registers_FILES := \ > registers/a2xx.xml.h \ > diff --git a/src/gallium/drivers/freedreno/Android.gen.mk > b/src/gallium/drivers/freedreno/Android.gen.mk > index d29ba159d5c..1d3ee5ff856 100644 > --- a/src/gallium/drivers/freedreno/Android.gen.mk > +++ b/src/gallium/drivers/freedreno/Android.gen.mk > @@ -28,11 +28,18 @@ ir3_nir_trig_deps := \ > $(MESA_TOP)/src/freedreno/ir3/ir3_nir_trig.py \ > $(MESA_TOP)/src/compiler/nir/nir_algebraic.py > > +ir3_nir_imul_deps := \ > + $(MESA_TOP)/src/freedreno/ir3/ir3_nir_imul.py > + > intermediates := $(call local-generated-sources-dir) > > $(intermediates)/ir3/ir3_nir_trig.c: $(ir3_nir_trig_deps) > @mkdir -p $(dir $@) > $(hide) $(MESA_PYTHON2) $< -p $(MESA_TOP)/src/compiler/nir > $@ > > +$(intermediates)/ir3/ir3_nir_imul.c: $(ir3_nir_imul_deps) > + @mkdir -p $(dir $@) > + $(hide) $(MESA_PYTHON2) $< -p $(MESA_TOP)/src/compiler/nir > $@ > + > LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, \ > $(ir3_GENERATED_FILES)) > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 68527, which changed state. Bug 68527 Summary: Planetary Annihilation Alpha: translation from TGSI failed ! https://bugs.freedesktop.org/show_bug.cgi?id=68527 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 79417, which changed state. Bug 79417 Summary: r600-cayman: GPU lockup in Planetary Annihilation (PTE 66705) on HD6950 https://bugs.freedesktop.org/show_bug.cgi?id=79417 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 86720, which changed state. Bug 86720 Summary: [radeon] Europa Universalis 4 freezing during game start (10.3.3+, still broken on 11.0.2) https://bugs.freedesktop.org/show_bug.cgi?id=86720 What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 75911, which changed state. Bug 75911 Summary: Ravensword: Shadowlands graphics lag (GM45) https://bugs.freedesktop.org/show_bug.cgi?id=75911 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 92557, which changed state. Bug 92557 Summary: [3D Games] [HSW/SKL/BXT] Euro Truck simulator 2 stops unexpectedly when the game is loading https://bugs.freedesktop.org/show_bug.cgi?id=92557 What|Removed |Added Status|NEEDINFO|RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 92999, which changed state. Bug 92999 Summary: [APITRACE] Shadow of Mordor missing fonts in menu https://bugs.freedesktop.org/show_bug.cgi?id=92999 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 93475, which changed state. Bug 93475 Summary: Saints Row IV causes GPU lockup on Linux https://bugs.freedesktop.org/show_bug.cgi?id=93475 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 Bug ID: 111054 Summary: Vulkan overlay doesn't work with X4: Foundations Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Vulkan/Common Assignee: mesa-dev@lists.freedesktop.org Reporter: shtetl...@gmail.com CC: airl...@freedesktop.org, chadvers...@chromium.org, ja...@jlekstrand.net For some reason when used with X4: Foundations, the overlay doesn't show up. Mesa master (git-75d8b4e795) Driver: radv GPU: Sapphire Pulse AMD Vega 56. X4: Foundations 2.50 Hotfix 1 (GOG version). -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 --- Comment #1 from Shmerl --- Just for the reference, the game is using Vulkan. -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 93840, which changed state. Bug 93840 Summary: [i965] Compiler backend uses too much stack with Alien: Isolation https://bugs.freedesktop.org/show_bug.cgi?id=93840 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 98693, which changed state. Bug 98693 Summary: Mad Max: Long lags when VRAM full https://bugs.freedesktop.org/show_bug.cgi?id=98693 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 100019, which changed state. Bug 100019 Summary: [bdw] GPU hang and application crash in Pillars of Eternity (steam) https://bugs.freedesktop.org/show_bug.cgi?id=100019 What|Removed |Added Status|NEEDINFO|RESOLVED Resolution|--- |WORKSFORME -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 --- Comment #2 from Lionel Landwerlin --- Any other information? I don't have the game, but I was able to use the overlay with games like Dota 2. -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 --- Comment #3 from Shmerl --- It works fine in other cases for me (like when using with Wine+dxvk in Witcher 3). So something strange is going with X4 specifically. Is there some way to narrow down why it's not loading? -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 --- Comment #4 from Shmerl --- Interesting, I just analyzed loaded shared libraries for case when the layer is showing up (vkcube), and I see libVkLayer_MESA_overlay.so listed. While in case of X4 it's not present. So something does prevent it from loading. -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 106875] The game Anima Gate of Memories has artefects
https://bugs.freedesktop.org/show_bug.cgi?id=106875 Timothy Arceri changed: What|Removed |Added Component|Drivers/Gallium/radeonsi|Mesa core Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org Summary|The game Anima Gate of |The game Anima Gate of |Memories has artefects on |Memories has artefects |Vega 56 | --- Comment #2 from Timothy Arceri --- I can confirm that my skylake machine running i965 has the same issues with this game so moving this bug to the "Mesa core" component for now. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] radv: do not crash when generating binning state for unknown chips
These values are only useful if binning is disabled.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_pipeline.c | 44 +-
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 71d3be240b2..49687405705 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -2691,29 +2691,29 @@ radv_pipeline_generate_binning_state(struct
radeon_cmdbuf *ctx_cs,
VkExtent2D bin_size = radv_compute_bin_size(pipeline, pCreateInfo);
- unsigned context_states_per_bin; /* allowed range: [1, 6] */
- unsigned persistent_states_per_bin; /* allowed range: [1, 32] */
- unsigned fpovs_per_batch; /* allowed range: [0, 255], 0 = unlimited */
-
- switch (pipeline->device->physical_device->rad_info.family) {
- case CHIP_VEGA10:
- case CHIP_VEGA12:
- case CHIP_VEGA20:
- context_states_per_bin = 1;
- persistent_states_per_bin = 1;
- fpovs_per_batch = 63;
- break;
- case CHIP_RAVEN:
- case CHIP_RAVEN2:
- context_states_per_bin = 6;
- persistent_states_per_bin = 32;
- fpovs_per_batch = 63;
- break;
- default:
- unreachable("unhandled family while determining binning
state.");
- }
-
if (pipeline->device->pbb_allowed && bin_size.width && bin_size.height)
{
+ unsigned context_states_per_bin; /* allowed range: [1, 6] */
+ unsigned persistent_states_per_bin; /* allowed range: [1, 32] */
+ unsigned fpovs_per_batch; /* allowed range: [0, 255], 0 =
unlimited */
+
+ switch (pipeline->device->physical_device->rad_info.family) {
+ case CHIP_VEGA10:
+ case CHIP_VEGA12:
+ case CHIP_VEGA20:
+ context_states_per_bin = 1;
+ persistent_states_per_bin = 1;
+ fpovs_per_batch = 63;
+ break;
+ case CHIP_RAVEN:
+ case CHIP_RAVEN2:
+ context_states_per_bin = 6;
+ persistent_states_per_bin = 32;
+ fpovs_per_batch = 63;
+ break;
+ default:
+ unreachable("unhandled family while determining binning
state.");
+ }
+
pa_sc_binner_cntl_0 =
S_028C44_BINNING_MODE(V_028C44_BINNING_ALLOWED) |
S_028C44_BIN_SIZE_X(bin_size.width == 16) |
--
2.22.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] radv: fix potential crash in the compute resolve path
If the destination attachment is UNUSED.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_meta_resolve_cs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c
b/src/amd/vulkan/radv_meta_resolve_cs.c
index 7d3cc166e0d..13c61509b21 100644
--- a/src/amd/vulkan/radv_meta_resolve_cs.c
+++ b/src/amd/vulkan/radv_meta_resolve_cs.c
@@ -917,12 +917,13 @@ radv_cmd_buffer_resolve_subpass_cs(struct radv_cmd_buffer
*cmd_buffer)
for (uint32_t i = 0; i < subpass->color_count; ++i) {
struct radv_subpass_attachment src_att =
subpass->color_attachments[i];
struct radv_subpass_attachment dst_att =
subpass->resolve_attachments[i];
- struct radv_image_view *src_iview =
fb->attachments[src_att.attachment].attachment;
- struct radv_image_view *dst_iview =
fb->attachments[dst_att.attachment].attachment;
if (dst_att.attachment == VK_ATTACHMENT_UNUSED)
continue;
+ struct radv_image_view *src_iview =
fb->attachments[src_att.attachment].attachment;
+ struct radv_image_view *dst_iview =
fb->attachments[dst_att.attachment].attachment;
+
VkImageResolve region = {
.extent = (VkExtent3D){ fb->width, fb->height, 0 },
.srcSubresource = (VkImageSubresourceLayers) {
--
2.22.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 111054] Vulkan overlay doesn't work with X4: Foundations
https://bugs.freedesktop.org/show_bug.cgi?id=111054 --- Comment #5 from Lionel Landwerlin --- Running with VK_LOADER_DEBUG=all might help figuring out what's going wrong. -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
