Re: iMac 10,1 with Ubuntu 16.04: black screen after suspend

[Lukas Wunner]

[cc += dri-devel, amd-gfx]

On Fri, May 26, 2017 at 02:13:29PM +0200, Florian Echtler wrote:
> I'm running Ubuntu 16.04.2 on a 27" unibody iMac 10,1 from 2009. However, even
> with the most recent HWE stack (kernel 4.8), the display stays black after
> suspend. I can ssh into the machine, so wakeup is OK, but the display is
> entirely black (backlight stays off).
> 
> Note: this appears to be entirely unrelated to the TDM stuff I posted about
> earlier. Interestingly, when I enable TDM, even after suspend, the display 
> works
> while TDM is active. Once I disable TDM, it's back to black (unintentional 
> song
> reference there).

Sounds like a radeon issue to me.

The LCD_PANEL_PWR and LCD_BKL_ON_MUX pins are coming from a mux.

In TDM mode, the pins are muxed to the SMC, so it can turn the panel on
and off.  If TDM is not in use, the pins are muxed to the ATI card.

The ATI card has MXM_PNL_PWR_EN and MXM_PNL_BL_EN pins.  Those must be
enabled for the panel to light up.  Perhaps radeon needs to be extended
to do that.  One theory is that this is done via ACPI, but perhaps
only if a certain operating system is running.  Dump the ACPI tables
and search DSDT and SSDT* tables for methods relating to the ATI card
and/or backlight.  If you find checks for OSDW there (e.g. in _PS0,
i.e. on resume), it means the AML code is only executed on Darwin.
Conversely !OSDW means the code is only executed on Windows (Boot Camp).
Linux masquerades as Darwin, but this can be disabled with
acpi_osi=!Darwin on the command line.  It's worth trying if that changes
the behaviour.  If it does, then macOS likely sets those pins on resume
and we need to do the same in radeon.

HTH,

Lukas

> 
> I've noted that in /sys/class/backlight, there are two different control
> interfaces, apple_backlight and radeon_bl0. Before suspend, I can control the
> screen brightness from system settings, even if only one of them is loaded, so
> both brightness interfaces work.
> 
> What I've already tried without success:
> 
> - Blindly switching to the console and back.
> - Calling DISPLAY=:0.0 xrandr --output eDP1 --auto via ssh.
> - Restarting X via ssh.
> - Blacklist apple_bl on the kernel cmdline (modprobe.blacklist=apple_bl).
> - Disabling the radeon backlight on the kernel cmdline (radeon.backlight=0).
> - Using radeontool to turn the backlight back on after suspend.
> - Directly using the interface in /sys/class/backlight/ to turn it back on.
> 
> I'm starting to run out of ideas... any hints?
> 
> Regards, Florian
> -- 
> SENT FROM MY DEC VT50 TERMINAL
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

[Alex Deucher]

On Fri, May 26, 2017 at 3:23 PM, Samuel Li  wrote:
> From: Xiaojie Yuan 
>
> v2: fix an off by one error and leading white spaces
> v3: use thread safe strtok_r(); initialize len before calling getline();
> change printf() to drmMsg(); add initial amdgpu.ids
>
> Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
> Reviewed-by: Junwei Zhang 
> Signed-off-by: Samuel Li 
> ---
>  Makefile.am  |   3 +
>  amdgpu/Makefile.am   |   2 +
>  amdgpu/Makefile.sources  |   2 +-
>  amdgpu/amdgpu_asic_id.c  | 199 
> +++
>  amdgpu/amdgpu_asic_id.h  | 165 ---
>  amdgpu/amdgpu_device.c   |  28 +--
>  amdgpu/amdgpu_internal.h |  10 +++
>  include/drm/amdgpu.ids   | 154 
>  8 files changed, 390 insertions(+), 173 deletions(-)
>  create mode 100644 amdgpu/amdgpu_asic_id.c
>  delete mode 100644 amdgpu/amdgpu_asic_id.h
>  create mode 100644 include/drm/amdgpu.ids
>
> diff --git a/Makefile.am b/Makefile.am
> index dfb8fcd..8de8f6c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
>
>  pkgconfigdir = @pkgconfigdir@
>  pkgconfig_DATA = libdrm.pc
> +libdrmdatadir = $(datadir)/libdrm
> +dist_libdrmdata_DATA = include/drm/amdgpu.ids
> +export libdrmdatadir
>
>  if HAVE_LIBKMS
>  LIBKMS_SUBDIR = libkms
> diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
> index cf7bc1b..da71c1c 100644
> --- a/amdgpu/Makefile.am
> +++ b/amdgpu/Makefile.am
> @@ -30,6 +30,8 @@ AM_CFLAGS = \
> $(PTHREADSTUBS_CFLAGS) \
> -I$(top_srcdir)/include/drm
>
> +AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
> +
>  libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
>  libdrm_amdgpu_ladir = $(libdir)
>  libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 -no-undefined
> diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
> index 487b9e0..bc3abaa 100644
> --- a/amdgpu/Makefile.sources
> +++ b/amdgpu/Makefile.sources
> @@ -1,5 +1,5 @@
>  LIBDRM_AMDGPU_FILES := \
> -   amdgpu_asic_id.h \
> +   amdgpu_asic_id.c \
> amdgpu_bo.c \
> amdgpu_cs.c \
> amdgpu_device.c \
> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
> new file mode 100644
> index 000..5b415e3
> --- /dev/null
> +++ b/amdgpu/amdgpu_asic_id.c
> @@ -0,0 +1,199 @@
> +/*
> + * Copyright © 2017 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "xf86drm.h"
> +#include "amdgpu_drm.h"
> +#include "amdgpu_internal.h"
> +
> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> +{
> +   char *buf, *saveptr;
> +   char *s_did;
> +   char *s_rid;
> +   char *s_name;
> +   char *endptr;
> +   int r = 0;
> +
> +   buf = strdup(line);
> +   if (!buf)
> +   return -ENOMEM;
> +
> +   /* ignore empty line and commented line */
> +   if (strlen(line) == 0 || line[0] == '#') {
> +   r = -EAGAIN;
> +   goto out;
> +   }
> +
> +   /* device id */
> +   s_did = strtok_r(buf, ",", &saveptr);
> +   if (!s_did) {
> +   r = -EINVAL;
> +   goto out;
> +   }
> +
> +   id->did = strtol(s_did, &endptr, 16);
> +   if (*endptr) {
> +   r = -EINVAL;
> +   goto out;
> +   }
> +
> +   /* revision id */
> +   s_rid = strtok_r(NULL, ",", &saveptr);
> +   if (!s_rid) {
> +   r = -EINVAL;
> +   goto out;
> +   }
> +
> +   id->rid = strtol(s_rid, &endptr, 16);
> +   if (*endptr) {
> +   r = -EINVAL;
> +   goto out;
> +   }
> +
> +   

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Christian König]

Hi David,

the idea is that the compositor (which is DRM master) can change the 
priority of it's clients.


So using dev->fd is pointless because that is the fd of the DRM master 
process.


Regards,
Christian.

Am 26.05.2017 um 11:02 schrieb Mao, David:

Hi Andres,
Why the fd is needed for this interface?
Why not just using the dev->fd instead of fd?
IIRC, if there are more than one fds opened in the process upon the 
same device, they will share the same amdgpu_device_handle which is 
guaranteed by amdgpu_device_initialize.
In other word, we should not run into the case that user creates more 
contexts with newly opened fd after tuning the priority of existing 
context in the same process unless the previous fd is closed.


Thanks.
Best Regards,
David

On 25 May 2017, at 8:00 AM, Andres Rodriguez > wrote:


When multiple environments are running simultaneously on a system, e.g.
an X desktop + a SteamVR game session, it may be useful to sacrifice
performance in one environment in order to boost it on the other.

This series provides a mechanism for a DRM_MASTER to provide exclusive
gpu access to a group of processes.

Note: This series is built on the assumption that the drm lease patch 
series

will extend DRM_MASTER status to lesees.

The libdrm we intend to provide is as follows:

/**
* Set the priority of all contexts in a process
*
* This function will change the priority of all contexts owned by
* the process identified by fd.
*
* \param dev - \c [in] device handle
* \param fd  - \c [in] fd from target process
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes @fd can be *any* file descriptor from the target process.
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
 int fd, int32_t priority);

/**
* Request to raise the minimum required priority to schedule a gpu job
*
* Submit a request to increase the minimum required priority to schedule
* a gpu job. Once this function returns, the gpu scheduler will no longer
* consider jobs from contexts with priority lower than @priority.
*
* The minimum priority considered by the scheduler will be the 
highest from

* all currently active requests.
*
* Requests are refcounted, and must be balanced using
* amdgpu_sched_min_priority_put()
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
 int32_t priority);

/**
* Drop a request to raise the minimum required scheduler priority
*
* This call balances amdgpu_sched_min_priority_get()
*
* If no other active requests exists for @priority, the minimum required
* priority will decay to a lower level until one is reached with an 
active

* request or the lowest priority is reached.
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
 int32_t priority);

Using this app, VRComposer can raise the priority of the VRapp and 
itself. Then
it can restrict the minimum scheduler priority in order to become 
exclusive gpu

clients.

One of the areas I'd like feedback is the following scenario. If a 
VRapp opens
a new fd and creates a new context after a call to set_priority, this 
specific
context will be lower priority than the rest. If the minimum required 
priority

is then raised, it is possible that this new context will be starved and
deadlock the VRapp.

One solution I had in mind to address this situation, is to make 
set_priority
also raise the priority of future contexts created by the VRapp. 
However, that
would require keeping track of the requested priority on a 
per-process data
structure. The current design appears to steer clean of keeping any 
process
specific data, and everything instead of stored on a per-file basis. 
Which is
why I did not pursue this approach. But if this is something you'd 
like me to

implement let me know.

One could also argue that preventing an application deadlock should 
be handled
between the VRComposer and the VRApp. It is not the kernel's 
responsibility to
babysit userspace applications and prevent themselves from shooting 
themselves
in the foot. The same could be achieved by improper usage of shared 
fences

between processes.

Thoughts/feedback/comments on this issue, or others, are appreciated.

Regards,
Andres


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org 
https://lists.freedesk

[PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

[Samuel Li]

From: Xiaojie Yuan 

v2: fix an off by one error and leading white spaces
v3: use thread safe strtok_r(); initialize len before calling getline();
change printf() to drmMsg(); add initial amdgpu.ids

Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
Reviewed-by: Junwei Zhang 
Signed-off-by: Samuel Li 
---
 Makefile.am  |   3 +
 amdgpu/Makefile.am   |   2 +
 amdgpu/Makefile.sources  |   2 +-
 amdgpu/amdgpu_asic_id.c  | 199 +++
 amdgpu/amdgpu_asic_id.h  | 165 ---
 amdgpu/amdgpu_device.c   |  28 +--
 amdgpu/amdgpu_internal.h |  10 +++
 include/drm/amdgpu.ids   | 154 
 8 files changed, 390 insertions(+), 173 deletions(-)
 create mode 100644 amdgpu/amdgpu_asic_id.c
 delete mode 100644 amdgpu/amdgpu_asic_id.h
 create mode 100644 include/drm/amdgpu.ids

diff --git a/Makefile.am b/Makefile.am
index dfb8fcd..8de8f6c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
 
 pkgconfigdir = @pkgconfigdir@
 pkgconfig_DATA = libdrm.pc
+libdrmdatadir = $(datadir)/libdrm
+dist_libdrmdata_DATA = include/drm/amdgpu.ids
+export libdrmdatadir
 
 if HAVE_LIBKMS
 LIBKMS_SUBDIR = libkms
diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
index cf7bc1b..da71c1c 100644
--- a/amdgpu/Makefile.am
+++ b/amdgpu/Makefile.am
@@ -30,6 +30,8 @@ AM_CFLAGS = \
$(PTHREADSTUBS_CFLAGS) \
-I$(top_srcdir)/include/drm
 
+AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
+
 libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
 libdrm_amdgpu_ladir = $(libdir)
 libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 -no-undefined
diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index 487b9e0..bc3abaa 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -1,5 +1,5 @@
 LIBDRM_AMDGPU_FILES := \
-   amdgpu_asic_id.h \
+   amdgpu_asic_id.c \
amdgpu_bo.c \
amdgpu_cs.c \
amdgpu_device.c \
diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
new file mode 100644
index 000..5b415e3
--- /dev/null
+++ b/amdgpu/amdgpu_asic_id.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright © 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "xf86drm.h"
+#include "amdgpu_drm.h"
+#include "amdgpu_internal.h"
+
+static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
+{
+   char *buf, *saveptr;
+   char *s_did;
+   char *s_rid;
+   char *s_name;
+   char *endptr;
+   int r = 0;
+
+   buf = strdup(line);
+   if (!buf)
+   return -ENOMEM;
+
+   /* ignore empty line and commented line */
+   if (strlen(line) == 0 || line[0] == '#') {
+   r = -EAGAIN;
+   goto out;
+   }
+
+   /* device id */
+   s_did = strtok_r(buf, ",", &saveptr);
+   if (!s_did) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   id->did = strtol(s_did, &endptr, 16);
+   if (*endptr) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   /* revision id */
+   s_rid = strtok_r(NULL, ",", &saveptr);
+   if (!s_rid) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   id->rid = strtol(s_rid, &endptr, 16);
+   if (*endptr) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   /* marketing name */
+   s_name = strtok_r(NULL, ",", &saveptr);
+   if (!s_name) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   id->marketing_name = strdup(s_name);
+   if (id->marketing_name == NULL) {
+   r = -EINVAL;
+   goto out;
+   }
+
+out:
+   free(buf);
+
+

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Andres Rodriguez]

On 2017-05-26 05:02 AM, Mao, David wrote:

Hi Andres,
Why the fd is needed for this interface?


The fd is used to identify the process for which we wish to raise the 
priority. It can be any fd from the target process, it doesn't have to 
be a drm file descriptor at all.


The fd is used to retrieve the (struct pid*) of the target process on 
the kernel side. In effect, it is a replacement for passing a pid number 
across process boundaries.


For reference, amdgpu_sched_process_priority_set() in patch 3


Why not just using the dev->fd instead of
IIRC, if there are more than one fds opened in the process upon the same 
device, they will share the same amdgpu_device_handle which is 
guaranteed by amdgpu_device_initialize.


Thanks for pointing that out. I wasn't aware that the amdgpu drm layer 
would always perform all command submission through the same fd 
(dev->fd) for the same amdgpu_device.


Your suggestion actually makes it a lot simpler to deal with this issue 
at a file level instead of at a process level. Since only one fd per 
device is used for command submission.


For a multi-gpu setup we would still need to share multiple fds, across 
the process boundaries.


This also helped me realize that my current implementation doesn't deal 
with multi-gpu cases correctly. As I iterate over the fds belonging to

a single drm device.

In other word, we should not run into the case that user creates more 
contexts with newly opened fd after tuning the priority of existing 
context in the same process unless the previous fd is closed.


Thanks.
Best Regards,
David

On 25 May 2017, at 8:00 AM, Andres Rodriguez > wrote:


When multiple environments are running simultaneously on a system, e.g.
an X desktop + a SteamVR game session, it may be useful to sacrifice
performance in one environment in order to boost it on the other.

This series provides a mechanism for a DRM_MASTER to provide exclusive
gpu access to a group of processes.

Note: This series is built on the assumption that the drm lease patch 
series

will extend DRM_MASTER status to lesees.

The libdrm we intend to provide is as follows:

/**
* Set the priority of all contexts in a process
*
* This function will change the priority of all contexts owned by
* the process identified by fd.
*
* \param dev - \c [in] device handle
* \param fd  - \c [in] fd from target process
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes @fd can be *any* file descriptor from the target process.
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
 int fd, int32_t priority);

/**
* Request to raise the minimum required priority to schedule a gpu job
*
* Submit a request to increase the minimum required priority to schedule
* a gpu job. Once this function returns, the gpu scheduler will no longer
* consider jobs from contexts with priority lower than @priority.
*
* The minimum priority considered by the scheduler will be the highest 
from

* all currently active requests.
*
* Requests are refcounted, and must be balanced using
* amdgpu_sched_min_priority_put()
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
 int32_t priority);

/**
* Drop a request to raise the minimum required scheduler priority
*
* This call balances amdgpu_sched_min_priority_get()
*
* If no other active requests exists for @priority, the minimum required
* priority will decay to a lower level until one is reached with an active
* request or the lowest priority is reached.
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
 int32_t priority);

Using this app, VRComposer can raise the priority of the VRapp and 
itself. Then
it can restrict the minimum scheduler priority in order to become 
exclusive gpu

clients.

One of the areas I'd like feedback is the following scenario. If a 
VRapp opens
a new fd and creates a new context after a call to set_priority, this 
specific
context will be lower priority than the rest. If the minimum required 
priority

is then raised, it is possible that this new context will be starved and
deadlock the VRapp.

One solution I had in mind to address this situation, is to make 
set_priority
also raise the priority of future contexts created by the VRapp. 
However, that
would require keeping track of the requested priority on a per-process 
data
structure. The cu

Re: GART write flush error on SI w/ amdgpu

[Marek Olšák]

On Tue, May 9, 2017 at 2:13 PM, Nicolai Hähnle  wrote:
> Hi all,
>
> I'm seeing some very strange errors on Verde with CPU readback from GART,
> and am pretty much out of ideas. Some help would be very much appreciated.
>
> The error manifests with the
> GL45-CTS.gtf32.GL3Tests.packed_pixels.packed_pixels_pbo test on amdgpu,
but
> *not* on radeon. Here's what the test does:
>
> 1. Upload a texture.
> 2. Read the texture back via a shader that uses shader buffer writes to
> write data to a buffer that is allocated in GART.
> 3. The CPU then reads from the buffer -- and sometimes gets stale data.
>
> This sequence is repeated for many sub-tests. There are some sub-tests
where
> the CPU reads stale data from the buffer, i.e. the shader writes simply
> don't make it to the CPU. The tests vary superficially, e.g. the first
> failing test is (almost?) always one where data is written in 16-bit words
> (but there are succeeding sub-tests with 16-bit writes as well).
>
> The bug is *not* a timing issue. Adding even a 1sec delay (sleep(1);)
> between the fence wait and the return of glMapBuffer does not fix the
> problem. The data must be stuck in a cache somewhere.
>
> Since the test runs okay with the radeon module, I tried some changes
based
> on comparing the IB submit between radeon and amdgpu, and based on
comparing
> register settings via scans obtained from umr. Some of the things I've
> tried:
>
> - Set HDP_MISC_CNTL.FLUSH_INVALIDATE_CACHE to 1 (both radeon and
amdgpu/gfx9
> set this)
> - Add SURFACE_SYNC packets preceded by setting CP_COHER_CNTL2 to the vmid
> (radeon does this)
> - Change gfx_v6_0_ring_emit_hdp_invalidate: select ME engine instead of
PFP
> (which seems more logical, and is done by gfx7+), or remove the
> corresponding WRITE_DATA entirely
>
> None of these changes helped.
>
> What *does* help is adding an artificial wait. Specifically, I'm adding a
> sequence of
>
> - WRITE_DATA
> - CACHE_FLUSH_AND_INV_TS_EVENT (BOTTOM_OF_PIPE_TS has same behavior)
> - WAIT_REG_MEM
>
> as can be seen in the attached patch. This works around the problem, but
it
> makes no sense:
>
> Adding the wait sequence *before* the SURFACE_SYNC in ring_emit_fence
works
> around the problem. However(!) it does not actually cause the UMD to wait
> any longer than before. Without this change, the UMD immediately sees a
> signaled user fence (and never uses an ioctl to wait), and with this
change,
> it *still* sees a signaled user fence.
>
> Also, note that the way I've hacked the change, the wait sequence is only
> added for the user fence emit (and I'm using a modified UMD to ensure that
> there is enough memory to be used by the added wait sequence).
>
> Adding the wait sequence *after* the SURFACE_SYNC *doesn't* work around
the
> problem.
>
> So for whatever reason, the added wait sequence *before* the SURFACE_SYNC
> encourages some part of the GPU to flush the data from wherever it's
stuck,
> and that's just really bizarre. There must be something really simple I'm
> missing, and any pointers would be appreciated.

Have you tried this?

diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c
b/src/gallium/drivers/radeonsi/si_hw_context.c
index 92c09cb..e6ac0ba 100644
--- a/src/gallium/drivers/radeonsi/si_hw_context.c
+++ b/src/gallium/drivers/radeonsi/si_hw_context.c
@@ -133,7 +133,8 @@ void si_context_gfx_flush(void *context, unsigned flags,
SI_CONTEXT_PS_PARTIAL_FLUSH;

/* DRM 3.1.0 doesn't flush TC for VI correctly. */
-   if (ctx->b.chip_class == VI && ctx->b.screen->info.drm_minor <= 1)
+   if ((ctx->b.chip_class == VI && ctx->b.screen->info.drm_minor <= 1)
||
+   (ctx->b.chip_class == SI && ctx->b.screen->info.drm_major == 3))
ctx->b.flags |= SI_CONTEXT_INV_GLOBAL_L2 |
SI_CONTEXT_INV_VMEM_L1;

One more cache flush there shouldn't hurt.

Also, Mesa uses PFP_SYNC_ME. It shouldn't be necessary, but it's worth a
try.

Marek
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Mao, David]

Hi Andres,
Why the fd is needed for this interface?
Why not just using the dev->fd instead of fd?
IIRC, if there are more than one fds opened in the process upon the same 
device, they will share the same amdgpu_device_handle which is guaranteed by 
amdgpu_device_initialize.
In other word, we should not run into the case that user creates more contexts 
with newly opened fd after tuning the priority of existing context in the same 
process unless the previous fd is closed.

Thanks.
Best Regards,
David

On 25 May 2017, at 8:00 AM, Andres Rodriguez 
mailto:andre...@gmail.com>> wrote:

When multiple environments are running simultaneously on a system, e.g.
an X desktop + a SteamVR game session, it may be useful to sacrifice
performance in one environment in order to boost it on the other.

This series provides a mechanism for a DRM_MASTER to provide exclusive
gpu access to a group of processes.

Note: This series is built on the assumption that the drm lease patch series
will extend DRM_MASTER status to lesees.

The libdrm we intend to provide is as follows:

/**
* Set the priority of all contexts in a process
*
* This function will change the priority of all contexts owned by
* the process identified by fd.
*
* \param dev - \c [in] device handle
* \param fd  - \c [in] fd from target process
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes @fd can be *any* file descriptor from the target process.
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
 int fd, int32_t priority);

/**
* Request to raise the minimum required priority to schedule a gpu job
*
* Submit a request to increase the minimum required priority to schedule
* a gpu job. Once this function returns, the gpu scheduler will no longer
* consider jobs from contexts with priority lower than @priority.
*
* The minimum priority considered by the scheduler will be the highest from
* all currently active requests.
*
* Requests are refcounted, and must be balanced using
* amdgpu_sched_min_priority_put()
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
 int32_t priority);

/**
* Drop a request to raise the minimum required scheduler priority
*
* This call balances amdgpu_sched_min_priority_get()
*
* If no other active requests exists for @priority, the minimum required
* priority will decay to a lower level until one is reached with an active
* request or the lowest priority is reached.
*
* \param dev - \c [in] device handle
* \param priority- \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
* <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
 int32_t priority);

Using this app, VRComposer can raise the priority of the VRapp and itself. Then
it can restrict the minimum scheduler priority in order to become exclusive gpu
clients.

One of the areas I'd like feedback is the following scenario. If a VRapp opens
a new fd and creates a new context after a call to set_priority, this specific
context will be lower priority than the rest. If the minimum required priority
is then raised, it is possible that this new context will be starved and
deadlock the VRapp.

One solution I had in mind to address this situation, is to make set_priority
also raise the priority of future contexts created by the VRapp. However, that
would require keeping track of the requested priority on a per-process data
structure. The current design appears to steer clean of keeping any process
specific data, and everything instead of stored on a per-file basis. Which is
why I did not pursue this approach. But if this is something you'd like me to
implement let me know.

One could also argue that preventing an application deadlock should be handled
between the VRComposer and the VRApp. It is not the kernel's responsibility to
babysit userspace applications and prevent themselves from shooting themselves
in the foot. The same could be achieved by improper usage of shared fences
between processes.

Thoughts/feedback/comments on this issue, or others, are appreciated.

Regards,
Andres


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx