[PATCH xf86-video-amdgpu 1/3] xf86-video-modesetting: Record non-desktop kernel property at PreInit time

[Keith Packard]

Save any value of the kernel non-desktop property in the xf86Output
structure to avoid non-desktop outputs in the default configuration.

[Also bump randrproto requirement to a version that defines
RR_PROPERTY_NON_DESKTOP - ajax]

Signed-off-by: Keith Packard 
Reviewed-by: Adam Jackson 
---
 src/drmmode_display.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 746e52a..b0fc7ea 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1980,6 +1980,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
drmModePropertyBlobPtr path_blob = NULL;
char name[32];
int i;
+   Bool nonDesktop = FALSE;
const char *s;
 
koutput =
@@ -1989,6 +1990,10 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
return 0;
 
path_blob = koutput_get_prop_blob(pAMDGPUEnt->fd, koutput, "PATH");
+   i = koutput_get_prop_idx(pAMDGPUEnt->fd, koutput, DRM_MODE_PROP_RANGE, 
RR_PROPERTY_NON_DESKTOP);
+   if (i >= 0)
+   nonDesktop = koutput->prop_values[i] != 0;
+
 
kencoders = calloc(sizeof(drmModeEncoderPtr), koutput->count_encoders);
if (!kencoders) {
@@ -2021,6 +2026,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
drmmode_output = output->driver_private;
drmmode_output->output_id = mode_res->connectors[num];
drmmode_output->mode_output = koutput;
+   output->non_desktop = nonDesktop;
for (i = 0; i < koutput->count_encoders; i++) {
drmModeFreeEncoder(kencoders[i]);
}
@@ -2064,6 +2070,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
output->interlaceAllowed = TRUE;
output->doubleScanAllowed = TRUE;
output->driver_private = drmmode_output;
+   output->non_desktop = nonDesktop;
 
output->possible_crtcs = 0x;
for (i = 0; i < koutput->count_encoders; i++) {
-- 
2.16.2

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

[no subject]

[Keith Packard]

Here are the patches to the modesetting driver amended for the amdgpu
driver.

-keith

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

[PATCH xf86-video-amdgpu 2/3] xf86-video-modesetting: Create CONNECTOR_ID properties for outputs [v2]

[Keith Packard]

This lets a DRM client map between X outputs and kernel connectors.

v2:
Change CONNECTOR_ID to enum -- Adam Jackson 

Signed-off-by: Keith Packard 
Reviewed-by: Adam Jackson 
---
 src/drmmode_display.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index b0fc7ea..fe319eb 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1662,6 +1662,29 @@ static void 
drmmode_output_create_resources(xf86OutputPtr output)
drmmode_output->tear_free = info->tear_free;
drmmode_output->num_props++;
 
+   /* Create CONNECTOR_ID property */
+   {
+   Atomname = MakeAtom("CONNECTOR_ID", 12, TRUE);
+   INT32   value = mode_output->connector_id;
+
+   if (name != BAD_RESOURCE) {
+   err = RRConfigureOutputProperty(output->randr_output, 
name,
+   FALSE, FALSE, TRUE,
+   1, );
+   if (err != 0) {
+   xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
+  "RRConfigureOutputProperty error, 
%d\n", err);
+   }
+   err = RRChangeOutputProperty(output->randr_output, name,
+XA_INTEGER, 32, 
PropModeReplace, 1,
+, FALSE, FALSE);
+   if (err != 0) {
+   xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
+  "RRChangeOutputProperty error, 
%d\n", err);
+   }
+   }
+   }
+
for (i = 0; i < drmmode_output->num_props; i++) {
drmmode_prop_ptr p = _output->props[i];
drmmode_prop = p->mode_prop;
-- 
2.16.2

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

[PATCH xf86-video-amdgpu 3/3] Add RandR leases with modesetting driver support [v6]

[Keith Packard]

This adds support for RandR CRTC/Output leases through the modesetting
driver, creating a lease using new kernel infrastructure and returning
that to a client through an fd which will have access to only those
resources.

v2: Restore CRTC mode when leases terminate

When a lease terminates for a crtc we have saved data for, go
ahead and restore the saved mode.

v3: Report RR_Rotate_0 rotations for leased crtcs.

Ignore leased CRTCs when selecting screen size.

Stop leasing encoders, the kernel doesn't do that anymore.

Turn off crtc->enabled while leased so that modesetting
ignores them.

Check lease status before calling any driver mode functions

When starting a lease, mark leased CRTCs as disabled and hide
their cursors. Also, check to see if there are other
non-leased CRTCs which are driving leased Outputs and mark
them as disabled as well. Sometimes an application will lease
an idle crtc instead of the one already associated with the
leased output.

When terminating a lease, reset any CRTCs which are driving
outputs that are no longer leased so that they start working
again.

This required splitting the DIX level lease termination code
into two pieces, one to remove the lease from the system
(RRLeaseTerminated) and a new function that frees the lease
data structure (RRLeaseFree).

v4: Report RR_Rotate_0 rotation for leased crtcs.

v5: Terminate all leases on server reset.

Leases hang around after the associated client exits so that
the client doesn't need to occupy an X server client slot and
consume a file descriptor once it has gotten the output
resources necessary.

Any leases still hanging around when the X server resets or
shuts down need to be cleaned up by calling the kernel to
terminate the lease and freeing any DIX structures.

Note that we cannot simply use the existing
drmmode_terminate_lease function on each lease as that wants
to also reset the video mode, and during server shut down that

   modesetting: Validate leases on VT enter

The kernel doesn't allow any master ioctls to run when another
VT is active, including simple things like listing the active
leases. To deal with that, we check the list of leases
whenever the X server VT is activated.

   xfree86: hide disabled cursors when resetting after lease termination

The lessee may well have played with cursors and left one
active on our screen. Just tell the kernel to turn it off.

v6: Add meson build infrastructure

[Also bumped libdrm requirement - ajax]

Signed-off-by: Keith Packard 
Reviewed-by: Adam Jackson 
---
 configure.ac  |  12 +
 src/amdgpu_kms.c  |   2 +
 src/drmmode_display.c | 147 +-
 src/drmmode_display.h |   5 ++
 4 files changed, 165 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index e7cc933..ff4a965 100644
--- a/configure.ac
+++ b/configure.ac
@@ -54,6 +54,14 @@ if test "x$GCC" = "xyes"; then
CPPFLAGS="$CPPFLAGS -Wall"
 fi
 
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+#include 
+#ifndef __GLIBC__
+#error not glibc
+#endif
+]], [])], [AC_DEFINE(_GNU_SOURCE, 1,
+   [ Enable GNU and other extensions to the C environment for glibc])])
+
 AH_TOP([#include "xorg-server.h"])
 
 # Define a configure option for an alternate module directory
@@ -203,6 +211,8 @@ AC_CHECK_HEADERS([dri3.h], [], [],
 
 CPPFLAGS="$SAVE_CPPFLAGS"
 
+AC_CHECK_LIB([bsd], [arc4random_buf])
+
 # Checks for headers/macros for byte swapping
 # Known variants:
 #   bswap_16, bswap_32, bswap_64  (glibc)
@@ -213,6 +223,8 @@ CPPFLAGS="$SAVE_CPPFLAGS"
 # if  is found, assume it's the correct version
 AC_CHECK_HEADERS([byteswap.h])
 
+AC_REPLACE_FUNCS([reallocarray])
+
 # if  is found, have to check which version
 AC_CHECK_HEADER([sys/endian.h], [HAVE_SYS_ENDIAN_H="yes"], 
[HAVE_SYS_ENDIAN_H="no"])
 
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index e1aae99..037dda0 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1708,6 +1708,8 @@ static Bool AMDGPUCloseScreen_KMS(ScreenPtr pScreen)
/* Clear mask of assigned crtc's in this generation */
pAMDGPUEnt->assigned_crtcs = 0;
 
+   drmmode_terminate_leases(pScrn, >drmmode);
+
drmmode_uevent_fini(pScrn, >drmmode);
amdgpu_drm_queue_close(pScrn);
 
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index fe319eb..de52f2f 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2303,8 +2303,150 @@ fail:
return FALSE;
 }
 
+static void
+drmmode_validate_leases(ScrnInfoPtr scrn)
+{
+   ScreenPtr screen = scrn->pScreen;
+   rrScrPrivPtr scr_priv = rrGetScrPriv(screen);
+   AMDGPUEntPtr pAMDGPUEnt = 

Re: [PATCH] drm/amdgpu: used cached pcie gen info for SI (v2)

[Zhu, Rex]

Reviewed-by: Rex Zhu

Best Regards
Rex



From: amd-gfx  on behalf of Alex Deucher 

Sent: Thursday, March 1, 2018 10:04 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander; sta...@vger.kernel.org
Subject: [PATCH] drm/amdgpu: used cached pcie gen info for SI (v2)

Rather than querying it every time we need it.
Also fixes a crash in VM pass through if there is no
root bridge because the cached value fetch already checks
this properly.

v2: fix includes

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=105244
Signed-off-by: Alex Deucher 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/si.c | 22 
 drivers/gpu/drm/amd/amdgpu/si_dpm.c | 50 ++---
 2 files changed, 23 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
index f20c4b7414e8..6e61b56bfbfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/si.c
+++ b/drivers/gpu/drm/amd/amdgpu/si.c
@@ -31,6 +31,7 @@
 #include "amdgpu_uvd.h"
 #include "amdgpu_vce.h"
 #include "atom.h"
+#include "amd_pcie.h"
 #include "amdgpu_powerplay.h"
 #include "sid.h"
 #include "si_ih.h"
@@ -1484,8 +1485,8 @@ static void si_pcie_gen3_enable(struct amdgpu_device 
*adev)
 {
 struct pci_dev *root = adev->pdev->bus->self;
 int bridge_pos, gpu_pos;
-   u32 speed_cntl, mask, current_data_rate;
-   int ret, i;
+   u32 speed_cntl, current_data_rate;
+   int i;
 u16 tmp16;

 if (pci_is_root_bus(adev->pdev->bus))
@@ -1497,23 +1498,20 @@ static void si_pcie_gen3_enable(struct amdgpu_device 
*adev)
 if (adev->flags & AMD_IS_APU)
 return;

-   ret = drm_pcie_get_speed_cap_mask(adev->ddev, );
-   if (ret != 0)
-   return;
-
-   if (!(mask & (DRM_PCIE_SPEED_50 | DRM_PCIE_SPEED_80)))
+   if (!(adev->pm.pcie_gen_mask & (CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2 |
+   CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)))
 return;

 speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
 current_data_rate = (speed_cntl & LC_CURRENT_DATA_RATE_MASK) >>
 LC_CURRENT_DATA_RATE_SHIFT;
-   if (mask & DRM_PCIE_SPEED_80) {
+   if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) {
 if (current_data_rate == 2) {
 DRM_INFO("PCIE gen 3 link speeds already enabled\n");
 return;
 }
 DRM_INFO("enabling PCIE gen 3 link speeds, disable with 
amdgpu.pcie_gen2=0\n");
-   } else if (mask & DRM_PCIE_SPEED_50) {
+   } else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) {
 if (current_data_rate == 1) {
 DRM_INFO("PCIE gen 2 link speeds already enabled\n");
 return;
@@ -1529,7 +1527,7 @@ static void si_pcie_gen3_enable(struct amdgpu_device 
*adev)
 if (!gpu_pos)
 return;

-   if (mask & DRM_PCIE_SPEED_80) {
+   if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) {
 if (current_data_rate != 2) {
 u16 bridge_cfg, gpu_cfg;
 u16 bridge_cfg2, gpu_cfg2;
@@ -1612,9 +1610,9 @@ static void si_pcie_gen3_enable(struct amdgpu_device 
*adev)

 pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, );
 tmp16 &= ~0xf;
-   if (mask & DRM_PCIE_SPEED_80)
+   if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
 tmp16 |= 3;
-   else if (mask & DRM_PCIE_SPEED_50)
+   else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
 tmp16 |= 2;
 else
 tmp16 |= 1;
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dpm.c 
b/drivers/gpu/drm/amd/amdgpu/si_dpm.c
index 8138053fcef1..8137c02fd16a 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_dpm.c
@@ -26,6 +26,7 @@
 #include "amdgpu_pm.h"
 #include "amdgpu_dpm.h"
 #include "amdgpu_atombios.h"
+#include "amd_pcie.h"
 #include "sid.h"
 #include "r600_dpm.h"
 #include "si_dpm.h"
@@ -3331,29 +3332,6 @@ static void btc_apply_voltage_delta_rules(struct 
amdgpu_device *adev,
 }
 }

-static enum amdgpu_pcie_gen r600_get_pcie_gen_support(struct amdgpu_device 
*adev,
-  u32 sys_mask,
-  enum amdgpu_pcie_gen asic_gen,
-  enum amdgpu_pcie_gen default_gen)
-{
-   switch (asic_gen) {
-   case AMDGPU_PCIE_GEN1:
-   return AMDGPU_PCIE_GEN1;
-   case AMDGPU_PCIE_GEN2:
-   return AMDGPU_PCIE_GEN2;
-   case AMDGPU_PCIE_GEN3:
-   return AMDGPU_PCIE_GEN3;
-   default:
- 

[PATCH 1/1] amdgpu: add scatter gather display flag

[Samuel Li]

---
 include/drm/amdgpu_drm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index d9aa4a3..dc4d8bd 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -526,6 +526,7 @@ struct drm_amdgpu_cs_chunk_data {
  */
 #define AMDGPU_IDS_FLAGS_FUSION 0x1
 #define AMDGPU_IDS_FLAGS_PREEMPTION 0x2
+#define AMDGPU_IDS_FLAGS_SGDISPLAY  0x4
 
 /* indicate if acceleration can be working */
 #define AMDGPU_INFO_ACCEL_WORKING  0x00
-- 
2.7.4

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

[PATCH 2/2] drm/amdgpu: Expose scatter gather display to user space.

[Samuel Li]

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++
 include/uapi/drm/amdgpu_drm.h   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 51a4b08..2cb344f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -31,6 +31,7 @@
 #include "amdgpu_sched.h"
 #include "amdgpu_uvd.h"
 #include "amdgpu_vce.h"
+#include "amdgpu_display.h"
 
 #include 
 #include 
@@ -578,6 +579,8 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
if (amdgpu_sriov_vf(adev))
dev_info.ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;
+   if (amdgpu_display_framebuffer_domains(adev) == 
AMDGPU_GEM_DOMAIN_GTT)
+   dev_info.ids_flags |= AMDGPU_IDS_FLAGS_SGDISPLAY;
 
vm_size = adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
vm_size -= AMDGPU_VA_RESERVED_SIZE;
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index fe17b67..2f30b98 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -584,6 +584,7 @@ struct drm_amdgpu_cs_chunk_data {
  */
 #define AMDGPU_IDS_FLAGS_FUSION 0x1
 #define AMDGPU_IDS_FLAGS_PREEMPTION 0x2
+#define AMDGPU_IDS_FLAGS_SGDISPLAY  0x4
 
 /* indicate if acceleration can be working */
 #define AMDGPU_INFO_ACCEL_WORKING  0x00
-- 
2.7.4

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

[PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

It's enabled by default. -1 is auto, to allow both vram and gtt
memory be available, for testing purpose only.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 5 +
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 292c7e7..6b0ee34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
 extern int amdgpu_compute_multipipe;
 extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
+extern int amdgpu_sg_display;
 
 #ifdef CONFIG_DRM_AMDGPU_SI
 extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5495b29..dfa11b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -513,8 +513,13 @@ uint32_t amdgpu_display_framebuffer_domains(struct 
amdgpu_device *adev)
 #if defined(CONFIG_DRM_AMD_DC)
if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
adev->flags & AMD_IS_APU &&
-   amdgpu_device_asic_has_dc_support(adev->asic_type))
-   domain |= AMDGPU_GEM_DOMAIN_GTT;
+   amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+   if (amdgpu_sg_display == 1) {
+   domain = AMDGPU_GEM_DOMAIN_GTT;
+   } else if (amdgpu_sg_display == -1) {
+   domain |= AMDGPU_GEM_DOMAIN_GTT;
+   }
+   }
 #endif
 
return domain;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e670936..f0ada24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
 int amdgpu_compute_multipipe = -1;
 int amdgpu_gpu_recovery = -1; /* auto */
 int amdgpu_emu_mode = 0;
+int amdgpu_sg_display = 1;
 
 MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
 module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
@@ -290,6 +291,9 @@ module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 
0444);
 MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable)");
 module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
 
+MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 = enable, 0 = 
disable, -1 = auto");
+module_param_named(sg_display, amdgpu_sg_display, int, 0444);
+
 #ifdef CONFIG_DRM_AMDGPU_SI
 
 #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 1206301..10f1f4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -138,6 +138,11 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
  fb_tiled);
domain = amdgpu_display_framebuffer_domains(adev);
+   if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+   DRM_INFO("Scatter gather display: enabled\n");
+   } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
+   DRM_INFO("Scatter gather display: auto\n");
+   }
 
height = ALIGN(mode_cmd->height, 8);
size = mode_cmd->pitches[0] * height;
-- 
2.7.4

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

RE: [PATCH] drm/amdgpu: Fail when setting negative DPM levels v2

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Kent Russell
> Sent: Friday, March 2, 2018 12:53 PM
> To: amd-...@freedesktop.org
> Cc: Russell, Kent 
> Subject: [PATCH] drm/amdgpu: Fail when setting negative DPM levels v2
> 
> kstrtol can handle negative values, which is unchecked here. While setting a
> negative value will result in the mask being 1<<0x , which the SMU 
> will
> ignore because it's out of long bits, return an error instead of silently 
> failing.
> 
> v2 Add similar check for sclk_od and mclk_od
> 
> Signed-off-by: Kent Russell 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 9e73cbc..5bdb421 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -476,7 +476,7 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -522,7 +522,7 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -567,7 +567,7 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -606,7 +606,7 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device
> *dev,
> 
>   ret = kstrtol(buf, 0, );
> 
> - if (ret) {
> + if (ret || value < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -650,7 +650,7 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device
> *dev,
> 
>   ret = kstrtol(buf, 0, );
> 
> - if (ret) {
> + if (ret || value < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> --
> 2.7.4
> 
> ___
> 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

RE: [PATCH] drm/amdgpu: Fail when setting negative DPM levels

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Kent Russell
> Sent: Friday, March 2, 2018 12:39 PM
> To: amd-...@freedesktop.org
> Cc: Russell, Kent 
> Subject: [PATCH] drm/amdgpu: Fail when setting negative DPM levels
> 
> kstrtol can handle negative values, which is unchecked here. While setting a
> negative value will result in the mask being 1<<0x , which the SMU 
> will
> ignore because it's out of long bits, return an error instead of silently 
> failing.
> 
> Signed-off-by: Kent Russell 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 9e73cbc..145ef62 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -476,7 +476,7 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -522,7 +522,7 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> @@ -567,7 +567,7 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct
> device *dev,
>   sub_str[1] = '\0';
>   ret = kstrtol(sub_str, 0, );
> 
> - if (ret) {
> + if (ret || level < 0) {
>   count = -EINVAL;
>   goto fail;
>   }
> --
> 2.7.4
> 
> ___
> 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

Re: [PATCH] drm/amdgpu: try again kiq access if not in IRQ(v2)

[Felix Kuehling]

On 2018-03-02 04:29 AM, Liu, Monk wrote:
> In_atomic() isnot encouraged to be used to judge if sleep is possible, see 
> the macros of it
>
> #define in_atomic() (preept_count() != 0)

OK. But my point is still that you're not testing the right thing when
you check in_interrupt(). The comment before the in_atomic macro
definition states the limitations and says "do not use in driver code".
Unfortunately it doesn't suggest any alternative. I think in_interrupt
is actually worse, because it misses even more cases than in_atomic.

Regards,
  Felix

>
>
> /Monk
>
> -Original Message-
> From: Kuehling, Felix 
> Sent: 2018年3月1日 23:50
> To: amd-gfx@lists.freedesktop.org; Liu, Monk 
> Subject: Re: [PATCH] drm/amdgpu: try again kiq access if not in IRQ(v2)
>
> On 2018-02-28 02:27 AM, Monk Liu wrote:
>> sometimes GPU is switched to other VFs and won't swich back soon, so 
>> the kiq reg access will not signal within a short period, instead of 
>> busy waiting a long time(MAX_KEQ_REG_WAIT) and returning TMO we can 
>> istead sleep 5ms and try again later (non irq context)
>>
>> And since the waiting in kiq_r/weg is busy wait, so MAX_KIQ_REG_WAIT 
>> shouldn't set to a long time, set it to 10ms is more appropriate.
>>
>> if gpu already in reset state, don't retry the KIQ reg access 
>> otherwise it would always hang because KIQ was already die usually.
>>
>> v2:
>> replace schedule() with msleep() for the wait
>>
>> Change-Id: I8fc807ce85a8d30d2b50153f3f3a6eda344ef994
>> Signed-off-by: Monk Liu 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 15 +--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> index b832651..1672f5b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> @@ -22,7 +22,7 @@
>>   */
>>  
>>  #include "amdgpu.h"
>> -#define MAX_KIQ_REG_WAIT1 /* in usecs */
>> +#define MAX_KIQ_REG_WAIT1 /* in usecs, 10ms */
>>  
>>  uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)  { @@ -152,9 
>> +152,14 @@ uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, 
>> uint32_t reg)
>>  amdgpu_ring_commit(ring);
>>  spin_unlock_irqrestore(>ring_lock, flags);
>>  
>> +retry_read:
>>  r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>>  if (r < 1) {
>>  DRM_ERROR("wait for kiq fence error: %ld\n", r);
>> +if (!in_interrupt() && !adev->in_gpu_reset) {
> You should check in_atomic here. Because it's invalid to sleep in atomic 
> context (e.g. while holding a spin lock) even when not in an interrupt.
> This seems to happen a lot for indirect register access, e.g.
> soc15_pcie_rreg.
>
> Regards,
>   Felix
>
>> +msleep(5);
>> +goto retry_read;
>> +}
>>  return ~0;
>>  }
>>  val = adev->wb.wb[adev->virt.reg_val_offs];
>> @@ -179,9 +184,15 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, 
>> uint32_t reg, uint32_t v)
>>  amdgpu_ring_commit(ring);
>>  spin_unlock_irqrestore(>ring_lock, flags);
>>  
>> +retry_write:
>>  r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>> -if (r < 1)
>> +if (r < 1) {
>>  DRM_ERROR("wait for kiq fence error: %ld\n", r);
>> +if (!in_interrupt() && !adev->in_gpu_reset) {
>> +msleep(5);
>> +goto retry_write;
>> +}
>> +}
>>  }
>>  
>>  /**

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

Re: Changes to let KFD use render node VMs

[Felix Kuehling]

On 2018-03-02 01:07 PM, Christian König wrote:
> Am 02.03.2018 um 17:38 schrieb Felix Kuehling:
>> On 2018-03-02 02:43 AM, Christian König wrote:
>>> Hi Felix,
>>>
>>> patch #3 looks good to me in general, but why do you need to cache the
>>> pd_phys_addr?
>>>
>>> The BO already does that anyway, so you can just use
>>> amdgpu_bo_gpu_addr() for this which also makes some additional checks
>>> on debug builds.
>> Do you mean amdgpu_bo_gpu_offset? That one requires the reservation to
>> be locked unless it's pinned.
>
> Correct, well that's why I suggested it.
>
>> We are caching the PD physical address here to get around a tricky
>> circular locking issue we ran into under memory pressure with evictions.
>> I don't remember all the details, but I do remember that the deadlock
>> involved fences, which aren't tracked by the lock dependency checker. So
>> it was particularly tricky to nail down. Avoiding the need to lock the
>> page directory reservation for finding out its physical address breaks
>> the lock cycle.
>
> OK, so what you do is you get the pd address after you validated the
> BOs and cache it until you need it in the hardware setup?

Correct. There is a KFD-KGD interface that queries the PD address from
the VM. The address gets put into the MAP_PROCESS packet in the HWS
runlist. After that the runlist effectively caches the same address
until an eviction causes a preemption.

>
> If yes than that would be valid because we do exactly the same in the
> job during command submission.
>
>>> patch #5 well mixing power management into the VM functions is a clear
>>> NAK.
>> This part won't be in my initial upstream push for GPUVM.
>>
>>> That certainly doesn't belong there, but we can have a counter how
>>> many compute VMs we have in the manager. amdgpu_vm_make_compute() can
>>> then return if this was the first VM to became a compute VM or not.
>> We currently trigger compute power profile switching based on the
>> existence of compute VMs. It was a convenient hook where amdgpu finds
>> out about the existence of compute work. If that's not acceptable we can
>> look into moving it elsewhere, possibly using a new KFD2KGD interface.
>
> As I said the VM code can still count how many compute VM there are,
> the issue is it should not make power management decisions based on that.
>
> The caller which triggers the switch of the VM to be a compute VM
> should make that decision.

Makes sense.

Regards,
  Felix

>
> Christian.
>
>>
>>> The rest of the patch looks good to me.
>> Thanks,
>>    Felix
>>
>>> Regards,
>>> Christian.
>>>
>>> Am 01.03.2018 um 23:58 schrieb Felix Kuehling:
 Hi Christian,

 I have a working patch series against amd-kfg-staging that lets KFD
 use
 VMs from render node FDs, as we discussed. There are two patches in
 that
 series that touch amdgpu_vm.[ch] that I'd like your feedback on
 before I
 commit the changes to amd-kfd-staging and include them in my upstream
 patch series for KFD GPUVM support. See attached.

 Thanks,
     Felix

>

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

Re: [PATCH] drm/amdgpu: Fail when setting negative DPM levels v2

[Eric Huang]

Reviewed-by: Eric Huang 


On 2018-03-02 12:53 PM, Kent Russell wrote:

kstrtol can handle negative values, which is unchecked here. While
setting a negative value will result in the mask being 1<<0x ,
which the SMU will ignore because it's out of long bits, return an
error instead of silently failing.

v2 Add similar check for sclk_od and mclk_od

Signed-off-by: Kent Russell 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..5bdb421 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -476,7 +476,7 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
  
-		if (ret) {

+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -522,7 +522,7 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
  
-		if (ret) {

+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -567,7 +567,7 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
  
-		if (ret) {

+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -606,7 +606,7 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
  
  	ret = kstrtol(buf, 0, );
  
-	if (ret) {

+   if (ret || value < 0) {
count = -EINVAL;
goto fail;
}
@@ -650,7 +650,7 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
  
  	ret = kstrtol(buf, 0, );
  
-	if (ret) {

+   if (ret || value < 0) {
count = -EINVAL;
goto fail;
}


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

Re: Changes to let KFD use render node VMs

[Christian König]

Am 02.03.2018 um 17:38 schrieb Felix Kuehling:

On 2018-03-02 02:43 AM, Christian König wrote:

Hi Felix,

patch #3 looks good to me in general, but why do you need to cache the
pd_phys_addr?

The BO already does that anyway, so you can just use
amdgpu_bo_gpu_addr() for this which also makes some additional checks
on debug builds.

Do you mean amdgpu_bo_gpu_offset? That one requires the reservation to
be locked unless it's pinned.


Correct, well that's why I suggested it.


We are caching the PD physical address here to get around a tricky
circular locking issue we ran into under memory pressure with evictions.
I don't remember all the details, but I do remember that the deadlock
involved fences, which aren't tracked by the lock dependency checker. So
it was particularly tricky to nail down. Avoiding the need to lock the
page directory reservation for finding out its physical address breaks
the lock cycle.


OK, so what you do is you get the pd address after you validated the BOs 
and cache it until you need it in the hardware setup?


If yes than that would be valid because we do exactly the same in the 
job during command submission.



patch #5 well mixing power management into the VM functions is a clear
NAK.

This part won't be in my initial upstream push for GPUVM.


That certainly doesn't belong there, but we can have a counter how
many compute VMs we have in the manager. amdgpu_vm_make_compute() can
then return if this was the first VM to became a compute VM or not.

We currently trigger compute power profile switching based on the
existence of compute VMs. It was a convenient hook where amdgpu finds
out about the existence of compute work. If that's not acceptable we can
look into moving it elsewhere, possibly using a new KFD2KGD interface.


As I said the VM code can still count how many compute VM there are, the 
issue is it should not make power management decisions based on that.


The caller which triggers the switch of the VM to be a compute VM should 
make that decision.


Christian.




The rest of the patch looks good to me.

Thanks,
   Felix


Regards,
Christian.

Am 01.03.2018 um 23:58 schrieb Felix Kuehling:

Hi Christian,

I have a working patch series against amd-kfg-staging that lets KFD use
VMs from render node FDs, as we discussed. There are two patches in that
series that touch amdgpu_vm.[ch] that I'd like your feedback on before I
commit the changes to amd-kfd-staging and include them in my upstream
patch series for KFD GPUVM support. See attached.

Thanks,
    Felix



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

[PATCH] drm/amdgpu: Fail when setting negative DPM levels v2

[Kent Russell]

kstrtol can handle negative values, which is unchecked here. While
setting a negative value will result in the mask being 1<<0x ,
which the SMU will ignore because it's out of long bits, return an
error instead of silently failing.

v2 Add similar check for sclk_od and mclk_od

Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..5bdb421 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -476,7 +476,7 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -522,7 +522,7 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -567,7 +567,7 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -606,7 +606,7 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
 
ret = kstrtol(buf, 0, );
 
-   if (ret) {
+   if (ret || value < 0) {
count = -EINVAL;
goto fail;
}
@@ -650,7 +650,7 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
 
ret = kstrtol(buf, 0, );
 
-   if (ret) {
+   if (ret || value < 0) {
count = -EINVAL;
goto fail;
}
-- 
2.7.4

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

[PATCH] drm/amdgpu: Fail when setting negative DPM levels

[Kent Russell]

kstrtol can handle negative values, which is unchecked here. While
setting a negative value will result in the mask being 1<<0x ,
which the SMU will ignore because it's out of long bits, return an
error instead of silently failing.

Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..145ef62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -476,7 +476,7 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -522,7 +522,7 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
@@ -567,7 +567,7 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
sub_str[1] = '\0';
ret = kstrtol(sub_str, 0, );
 
-   if (ret) {
+   if (ret || level < 0) {
count = -EINVAL;
goto fail;
}
-- 
2.7.4

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

[bug report] drm/amd/display: Implement interface for CRC on CRTC

[Dan Carpenter]

Hello Leo (Sunpeng) Li,

This is a semi-automatic email about new static checker warnings.

The patch 31aec354f92c: "drm/amd/display: Implement interface for CRC 
on CRTC" from Dec 18, 2017, leads to the following Smatch complaint:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:323 
dm_crtc_high_irq()
error: we previously assumed 'acrtc' could be null (see line 319)

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
   318  
   319  if (acrtc)
^
Old code checked for NULL

   320  crtc_index = acrtc->crtc_id;
   321  
   322  drm_handle_vblank(adev->ddev, crtc_index);
   323  amdgpu_dm_crtc_handle_crc_irq(>base);
   ^^^
The new code dereferences without checking (inside the function call).

   324  }
   325  

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

Re: Changes to let KFD use render node VMs

[Felix Kuehling]

On 2018-03-02 02:43 AM, Christian König wrote:
> Hi Felix,
>
> patch #3 looks good to me in general, but why do you need to cache the
> pd_phys_addr?
>
> The BO already does that anyway, so you can just use
> amdgpu_bo_gpu_addr() for this which also makes some additional checks
> on debug builds.

Do you mean amdgpu_bo_gpu_offset? That one requires the reservation to
be locked unless it's pinned.

We are caching the PD physical address here to get around a tricky
circular locking issue we ran into under memory pressure with evictions.
I don't remember all the details, but I do remember that the deadlock
involved fences, which aren't tracked by the lock dependency checker. So
it was particularly tricky to nail down. Avoiding the need to lock the
page directory reservation for finding out its physical address breaks
the lock cycle.

>
> patch #5 well mixing power management into the VM functions is a clear
> NAK.

This part won't be in my initial upstream push for GPUVM.

>
> That certainly doesn't belong there, but we can have a counter how
> many compute VMs we have in the manager. amdgpu_vm_make_compute() can
> then return if this was the first VM to became a compute VM or not.

We currently trigger compute power profile switching based on the
existence of compute VMs. It was a convenient hook where amdgpu finds
out about the existence of compute work. If that's not acceptable we can
look into moving it elsewhere, possibly using a new KFD2KGD interface.

>
> The rest of the patch looks good to me.

Thanks,
  Felix

>
> Regards,
> Christian.
>
> Am 01.03.2018 um 23:58 schrieb Felix Kuehling:
>> Hi Christian,
>>
>> I have a working patch series against amd-kfg-staging that lets KFD use
>> VMs from render node FDs, as we discussed. There are two patches in that
>> series that touch amdgpu_vm.[ch] that I'd like your feedback on before I
>> commit the changes to amd-kfd-staging and include them in my upstream
>> patch series for KFD GPUVM support. See attached.
>>
>> Thanks,
>>    Felix
>>
>

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

[PATCH 2/2] drm/amd/pp: Add PCC feature support on Vega

[Rex Zhu]

This features controls vega peak current protection to allow
for a wider compatibility with power supplies.

Change-Id: Ic3fe4871fdef1f7372fe09430e97a639b2e28abf
Reviewed-by: Alex Deucher 
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 34 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h |  1 +
 drivers/gpu/drm/amd/powerplay/inc/vega10_ppsmc.h   |  1 +
 3 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 547f57b..25165b4 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -299,6 +299,8 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
 {
struct vega10_hwmgr *data = (struct vega10_hwmgr *)(hwmgr->backend);
int i;
+   uint32_t sub_vendor_id, hw_revision;
+   struct amdgpu_device *adev = hwmgr->adev;
 
vega10_initialize_power_tune_defaults(hwmgr);
 
@@ -363,6 +365,7 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
FEATURE_FAN_CONTROL_BIT;
data->smu_features[GNLD_ACG].smu_feature_id = FEATURE_ACG_BIT;
data->smu_features[GNLD_DIDT].smu_feature_id = FEATURE_GFX_EDC_BIT;
+   data->smu_features[GNLD_PCC_LIMIT].smu_feature_id = 
FEATURE_PCC_LIMIT_CONTROL_BIT;
 
if (!data->registry_data.prefetcher_dpm_key_disabled)
data->smu_features[GNLD_DPM_PREFETCHER].supported = true;
@@ -432,6 +435,15 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr 
*hwmgr)
if (data->registry_data.didt_support)
data->smu_features[GNLD_DIDT].supported = true;
 
+   hw_revision = adev->pdev->revision;
+   sub_vendor_id = adev->pdev->subsystem_vendor;
+
+   if ((hwmgr->chip_id == 0x6862 ||
+   hwmgr->chip_id == 0x6861 ||
+   hwmgr->chip_id == 0x6868) &&
+   (hw_revision == 0) &&
+   (sub_vendor_id != 0x1002))
+   data->smu_features[GNLD_PCC_LIMIT].supported = true;
 }
 
 #ifdef PPLIB_VEGA10_EVV_SUPPORT
@@ -2814,12 +2826,32 @@ static int vega10_start_dpm(struct pp_hwmgr *hwmgr, 
uint32_t bitmap)
return 0;
 }
 
+static int vega10_enable_disable_PCC_limit_feature(struct pp_hwmgr *hwmgr, 
bool enable)
+{
+   struct vega10_hwmgr *data =
+   (struct vega10_hwmgr *)(hwmgr->backend);
+
+   if (data->smu_features[GNLD_PCC_LIMIT].supported) {
+   if (enable == data->smu_features[GNLD_PCC_LIMIT].enabled)
+   pr_info("GNLD_PCC_LIMIT has been %s \n", enable ? 
"enabled" : "disabled");
+   PP_ASSERT_WITH_CODE(!vega10_enable_smc_features(hwmgr,
+   enable, 
data->smu_features[GNLD_PCC_LIMIT].smu_feature_bitmap),
+   "Attempt to Enable PCC Limit feature Failed!",
+   return -EINVAL);
+   data->smu_features[GNLD_PCC_LIMIT].enabled = enable;
+   }
+
+   return 0;
+}
+
 static int vega10_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
 {
struct vega10_hwmgr *data =
(struct vega10_hwmgr *)(hwmgr->backend);
int tmp_result, result = 0;
 
+   vega10_enable_disable_PCC_limit_feature(hwmgr, true);
+
if ((hwmgr->smu_version == 0x001c2c00) ||
(hwmgr->smu_version == 0x001c2d00))
smum_send_msg_to_smc_with_parameter(hwmgr,
@@ -4668,6 +4700,8 @@ static int vega10_disable_dpm_tasks(struct pp_hwmgr 
*hwmgr)
tmp_result =  vega10_acg_disable(hwmgr);
PP_ASSERT_WITH_CODE((tmp_result == 0),
"Failed to disable acg!", result = tmp_result);
+
+   vega10_enable_disable_PCC_limit_feature(hwmgr, false);
return result;
 }
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h
index dafc821..8f6c2cb 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h
@@ -66,6 +66,7 @@ enum {
GNLD_FEATURE_FAST_PPT_BIT,
GNLD_DIDT,
GNLD_ACG,
+   GNLD_PCC_LIMIT,
GNLD_FEATURES_MAX
 };
 
diff --git a/drivers/gpu/drm/amd/powerplay/inc/vega10_ppsmc.h 
b/drivers/gpu/drm/amd/powerplay/inc/vega10_ppsmc.h
index 247c973..c3ed737 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/vega10_ppsmc.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/vega10_ppsmc.h
@@ -131,6 +131,7 @@
 #define PPSMC_MSG_RunAcgInOpenLoop   0x5E
 #define PPSMC_MSG_InitializeAcg  0x5F
 #define PPSMC_MSG_GetCurrPkgPwr  0x61
+#define PPSMC_MSG_SetPccThrottleLevel0x67
 #define PPSMC_MSG_UpdatePkgPwrPidAlpha   0x68
 #define PPSMC_Message_Count  0x69
 
-- 
1.9.1

[PATCH 1/2] drm/amd/pp: Export new smu message for PCC feature on Vega10

[Rex Zhu]

used to set PccThrottleLevel and PccResidencyThreshold

Change-Id: I2a5871de2ad18984022b790bde3a86404b9afd8c
Reviewed-by: Alex Deucher 
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/inc/smu9.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu9.h 
b/drivers/gpu/drm/amd/powerplay/inc/smu9.h
index 550ed67..70ac4d4 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu9.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu9.h
@@ -58,7 +58,7 @@
 #define FEATURE_FAST_PPT_BIT26
 #define FEATURE_GFX_EDC_BIT 27
 #define FEATURE_ACG_BIT 28
-#define FEATURE_SPARE_29_BIT29
+#define FEATURE_PCC_LIMIT_CONTROL_BIT   29
 #define FEATURE_SPARE_30_BIT30
 #define FEATURE_SPARE_31_BIT31
 
@@ -94,7 +94,7 @@
 #define FEATURE_FAST_PPT_MASK(1 << FAST_PPT_BIT   )
 #define FEATURE_GFX_EDC_MASK (1 << FEATURE_GFX_EDC_BIT)
 #define FEATURE_ACG_MASK (1 << FEATURE_ACG_BIT)
-#define FFEATURE_SPARE_29_MASK   (1 << FEATURE_SPARE_29_BIT   )
+#define FEATURE_PCC_LIMIT_CONTROL_MASK   (1 << FEATURE_PCC_LIMIT_CONTROL_BIT  )
 #define FFEATURE_SPARE_30_MASK   (1 << FEATURE_SPARE_30_BIT   )
 #define FFEATURE_SPARE_31_MASK   (1 << FEATURE_SPARE_31_BIT   )
 /* Workload types */
-- 
1.9.1

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

[PATCH 3/4] drm/amd/pp: Revert gfx/compute profile switch sysfs

[Rex Zhu]

The gfx/compute profiling mode switch is only for internally
test. Not a complete solution and unexpectly upstream.
so revert it.

Change-Id: I1af1b64a63b6fc12c24cf73df03b083b3661ca02
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|   8 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 180 ---
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 256 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.h|   7 -
 drivers/gpu/drm/amd/include/kgd_pp_interface.h |   7 -
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 114 -
 .../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c  |  17 --
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   |   2 -
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  77 ---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  86 ---
 .../gpu/drm/amd/powerplay/inc/hardwaremanager.h|   1 -
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |   9 -
 drivers/gpu/drm/amd/powerplay/inc/smumgr.h |   3 -
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  27 ---
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  66 --
 .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|  64 --
 drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c  |  10 -
 .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c|  64 --
 18 files changed, 998 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index bd745a4..9c373f8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -341,14 +341,6 @@ enum amdgpu_pcie_gen {
((adev)->powerplay.pp_funcs->reset_power_profile_state(\
(adev)->powerplay.pp_handle, request))
 
-#define amdgpu_dpm_get_power_profile_state(adev, query) \
-   ((adev)->powerplay.pp_funcs->get_power_profile_state(\
-   (adev)->powerplay.pp_handle, query))
-
-#define amdgpu_dpm_set_power_profile_state(adev, request) \
-   ((adev)->powerplay.pp_funcs->set_power_profile_state(\
-   (adev)->powerplay.pp_handle, request))
-
 #define amdgpu_dpm_switch_power_profile(adev, type) \
((adev)->powerplay.pp_funcs->switch_power_profile(\
(adev)->powerplay.pp_handle, type))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 9e73cbc..632b186 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -734,161 +734,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
 }
 
-static ssize_t amdgpu_get_pp_power_profile(struct device *dev,
-   char *buf, struct amd_pp_profile *query)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   int ret = 0xff;
-
-   if (adev->powerplay.pp_funcs->get_power_profile_state)
-   ret = amdgpu_dpm_get_power_profile_state(
-   adev, query);
-
-   if (ret)
-   return ret;
-
-   return snprintf(buf, PAGE_SIZE,
-   "%d %d %d %d %d\n",
-   query->min_sclk / 100,
-   query->min_mclk / 100,
-   query->activity_threshold,
-   query->up_hyst,
-   query->down_hyst);
-}
-
-static ssize_t amdgpu_get_pp_gfx_power_profile(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct amd_pp_profile query = {0};
-
-   query.type = AMD_PP_GFX_PROFILE;
-
-   return amdgpu_get_pp_power_profile(dev, buf, );
-}
-
-static ssize_t amdgpu_get_pp_compute_power_profile(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   struct amd_pp_profile query = {0};
-
-   query.type = AMD_PP_COMPUTE_PROFILE;
-
-   return amdgpu_get_pp_power_profile(dev, buf, );
-}
-
-static ssize_t amdgpu_set_pp_power_profile(struct device *dev,
-   const char *buf,
-   size_t count,
-   struct amd_pp_profile *request)
-{
-   struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = ddev->dev_private;
-   uint32_t loop = 0;
-   char *sub_str, buf_cpy[128], *tmp_str;
-   const char delimiter[3] = {' ', '\n', '\0'};
-   long int value;
-   int ret = 0xff;
-
-   if (strncmp("reset", buf, strlen("reset")) == 0) {
-   if (adev->powerplay.pp_funcs->reset_power_profile_state)
-   ret = amdgpu_dpm_reset_power_profile_state(
-   adev, request);
-   if (ret) {
-   count = -EINVAL;
-   goto fail;
-   }
-   return count;
-   

[PATCH 4/4] drm/amd/pp: Add auto power profilng switch based on workloads

[Rex Zhu]

Add power profiling mode dynamic switch based on the workloads.
Currently, support Cumpute, VR, Video, 3D,power saving with Cumpute
have highest prority, power saving have lowest prority.

in manual dpm mode, driver will stop auto switch, just save the client's
requests. user can set power profiling mode through sysfs.

when exit manual dpm mode, driver will response the client's requests.
switch based on the client's prority.

Change-Id: Ia97b679f4f11a7c3cabd350462101897909c8115
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h|  4 +--
 drivers/gpu/drm/amd/include/kgd_pp_interface.h | 18 +---
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 32 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c| 16 +++
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c   | 11 
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  6 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  8 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.h |  3 --
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |  5 
 9 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index 9c373f8f..643d008 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -341,9 +341,9 @@ enum amdgpu_pcie_gen {
((adev)->powerplay.pp_funcs->reset_power_profile_state(\
(adev)->powerplay.pp_handle, request))
 
-#define amdgpu_dpm_switch_power_profile(adev, type) \
+#define amdgpu_dpm_switch_power_profile(adev, type, en) \
((adev)->powerplay.pp_funcs->switch_power_profile(\
-   (adev)->powerplay.pp_handle, type))
+   (adev)->powerplay.pp_handle, type, en))
 
 #define amdgpu_dpm_set_clockgating_by_smu(adev, msg_id) \
((adev)->powerplay.pp_funcs->set_clockgating_by_smu(\
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 7dfba2d..ec54584 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -83,20 +83,6 @@ enum amd_vce_level {
AMD_VCE_LEVEL_DC_GP_HIGH = 5, /* DC, general purpose queue, 1080 >= res 
> 720 */
 };
 
-enum amd_pp_profile_type {
-   AMD_PP_GFX_PROFILE,
-   AMD_PP_COMPUTE_PROFILE,
-};
-
-struct amd_pp_profile {
-   enum amd_pp_profile_type type;
-   uint32_t min_sclk;
-   uint32_t min_mclk;
-   uint16_t activity_threshold;
-   uint8_t up_hyst;
-   uint8_t down_hyst;
-};
-
 enum amd_fan_ctrl_mode {
AMD_FAN_CTRL_NONE = 0,
AMD_FAN_CTRL_MANUAL = 1,
@@ -151,7 +137,6 @@ enum PP_SMC_POWER_PROFILE {
PP_SMC_POWER_PROFILE_VR   = 0x3,
PP_SMC_POWER_PROFILE_COMPUTE  = 0x4,
PP_SMC_POWER_PROFILE_CUSTOM   = 0x5,
-   PP_SMC_POWER_PROFILE_AUTO = 0x6,
 };
 
 enum {
@@ -260,8 +245,7 @@ struct amd_pm_funcs {
int (*get_pp_table)(void *handle, char **table);
int (*set_pp_table)(void *handle, const char *buf, size_t size);
void (*debugfs_print_current_performance_level)(void *handle, struct 
seq_file *m);
-   int (*switch_power_profile)(void *handle,
-   enum amd_pp_profile_type type);
+   int (*switch_power_profile)(void *handle, enum PP_SMC_POWER_PROFILE 
type, bool en);
 /* export to amdgpu */
void (*powergate_uvd)(void *handle, bool gate);
void (*powergate_vce)(void *handle, bool gate);
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c 
b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 4876871..3bd2fad 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -1073,22 +1073,44 @@ static int pp_odn_edit_dpm_table(void *handle, uint32_t 
type, long *input, uint3
 }
 
 static int pp_dpm_switch_power_profile(void *handle,
-   enum amd_pp_profile_type type)
+   enum PP_SMC_POWER_PROFILE type, bool en)
 {
struct pp_hwmgr *hwmgr;
-   struct amd_pp_profile request = {0};
struct pp_instance *pp_handle = (struct pp_instance *)handle;
+   long *workload;
+   uint32_t index;
 
if (pp_check(pp_handle))
return -EINVAL;
 
hwmgr = pp_handle->hwmgr;
 
-   if (hwmgr->current_power_profile != type) {
-   request.type = type;
-   pp_dpm_set_power_profile_state(handle, );
+   if (hwmgr->hwmgr_func->set_power_profile_mode == NULL) {
+   pr_info("%s was not implemented.\n", __func__);
+   return -EINVAL;
+   }
+
+   if (!(type < PP_SMC_POWER_PROFILE_CUSTOM))
+   return -EINVAL;
+
+   mutex_lock(_handle->pp_lock);
+
+   if (!en) {
+   hwmgr->workload_mask &= ~(1 < 

[PATCH 2/4] drm/amd/pp: Fix sclk in highest two levels in compute mode on smu7

[Rex Zhu]

Compute workload tends to be "bursty", Only tune the behavior of
nature dpm don't work well for most of such workloads. From tests
result, Fix sclk in highest two levels can get better performance.
so add min sclk setting into the default cumpute workload policy on
smu7.

user still can change sclk range through sysfs pp_dpm_sclk
for better perf/watt.

Change-Id: I0ffd19a5d3c9e57f22aebd9ff1e18b980a111384
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index c1e32f4..ba114482 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -5005,6 +5005,26 @@ static int smu7_get_power_profile_mode(struct pp_hwmgr 
*hwmgr, char *buf)
return size;
 }
 
+static void smu7_patch_compute_profile_mode(struct pp_hwmgr *hwmgr,
+   enum PP_SMC_POWER_PROFILE requst)
+{
+   struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
+   uint32_t tmp, level;
+
+   if (requst == PP_SMC_POWER_PROFILE_COMPUTE) {
+   if (data->dpm_level_enable_mask.sclk_dpm_enable_mask) {
+   level = 0;
+   tmp = data->dpm_level_enable_mask.sclk_dpm_enable_mask;
+   while (tmp >>= 1)
+   level++;
+   if (level > 0)
+   smu7_force_clock_level(hwmgr, PP_SCLK, 3 << 
(level-1));
+   }
+   } else if (hwmgr->power_profile_mode == PP_SMC_POWER_PROFILE_COMPUTE) {
+   smu7_force_clock_level(hwmgr, PP_SCLK, 
data->dpm_level_enable_mask.sclk_dpm_enable_mask);
+   }
+}
+
 static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, 
uint32_t size)
 {
struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
@@ -5055,6 +5075,7 @@ static int smu7_set_power_profile_mode(struct pp_hwmgr 
*hwmgr, long *input, uint
data->current_profile_setting.mclk_down_hyst = 
tmp.mclk_down_hyst;
data->current_profile_setting.mclk_activity = 
tmp.mclk_activity;
}
+   smu7_patch_compute_profile_mode(hwmgr, mode);
hwmgr->power_profile_mode = mode;
}
break;
-- 
1.9.1

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

[PATCH 1/4] drm/amd/pp: Implement get/set_power_profile_mode on smu7

[Rex Zhu]

It show what parameters can be configured to tune
the behavior of natural dpm for perf/watt on smu7.

user can select the mode per workload, but even the default per
workload settings are not bulletproof.

user can configure custom settings per different use case
for better perf or better perf/watt.

cat pp_power_profile_mode
NUMMODE_NAME SCLK_UP_HYST   SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL 
MCLK_UP_HYST   MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL
  0   3D_FULL_SCREEN:0  100   30
0  100   10
  1 POWER_SAVING:   100   30
---
  2VIDEO:---   
10   16   31
  3   VR:0   11   50
0  100   10
  4  COMPUTE:05   30
---
  5   CUSTOM:000
000
  *  CURRENT:0  100   30
0  100   10

Under manual dpm level,

user can echo "0/1/2/3/4">pp_power_profile_mode
to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
mode.

echo "5 * * * * * * * *">pp_power_profile_mode
to set custom settings.
"5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"

if the parameter enable_sclk/enable_mclk is true,
driver will update the following parameters to dpm table.
if false, ignore the following parameters.

Reviewed-by: Alex Deucher 
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 143 +++
 1 file changed, 143 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 731475b..c1e32f4 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -81,6 +81,13 @@
 #define PCIE_BUS_CLK1
 #define TCLK(PCIE_BUS_CLK / 10)
 
+static const struct profile_mode_setting smu7_profiling[5] =
+   {{1, 0, 100, 30, 1, 0, 100, 10},
+{1, 10, 0, 30, 0, 0, 0, 0},
+{0, 0, 0, 0, 1, 10, 16, 31},
+{1, 0, 11, 50, 1, 0, 100, 10},
+{1, 0, 5, 30, 0, 0, 0, 0},
+   };
 
 /** Values for the CG_THERMAL_CTRL::DPM_EVENT_SRC field. */
 enum DPM_EVENT_SRC {
@@ -2475,6 +2482,9 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
smu7_patch_voltage_workaround(hwmgr);
smu7_init_dpm_defaults(hwmgr);
 
+   hwmgr->power_profile_mode = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
+   hwmgr->default_power_profile_mode = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
+
/* Get leakage voltage based on leakage ID. */
if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_EVV)) {
@@ -4923,6 +4933,137 @@ static int smu7_odn_edit_dpm_table(struct pp_hwmgr 
*hwmgr,
return 0;
 }
 
+static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
+{
+   struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
+   uint32_t i, size = 0;
+   uint32_t len;
+
+   static const char *profile_name[6] = {"3D_FULL_SCREEN",
+   "POWER_SAVING",
+   "VIDEO",
+   "VR",
+   "COMPUTE",
+   "CUSTOM"};
+
+   static const char *title[8] = {"NUM",
+   "MODE_NAME",
+   "SCLK_UP_HYST",
+   "SCLK_DOWN_HYST",
+   "SCLK_ACTIVE_LEVEL",
+   "MCLK_UP_HYST",
+   "MCLK_DOWN_HYST",
+   "MCLK_ACTIVE_LEVEL"};
+
+   if (!buf)
+   return -EINVAL;
+
+   size += sprintf(buf + size, "%s %16s %16s %16s %16s %16s %16s %16s\n",
+   title[0], title[1], title[2], title[3],
+   title[4], title[5], title[6], title[7]);
+
+   len = sizeof(smu7_profiling) / sizeof(struct profile_mode_setting);
+
+   for (i = 0; i < len; i++) {
+   if (smu7_profiling[i].bupdate_sclk)
+   size += sprintf(buf + size, "%3d %16s: %8d %16d %16d ",
+   i, profile_name[i], smu7_profiling[i].sclk_up_hyst,
+   

[PATCH xf86-video-ati 2/3] modesetting: Reset output_id if drmModeGetConnector failed

[Michel Dänzer]

From: Daniel Martin 

If drmModeGetConnector() fails in drmmode_output_detect(), we have to
reset the output_id to -1 too.

Yet another spot leading to a potential NULL dereference when handling
the mode_output member as output_id was != -1. Though, this case should
be very hard to hit.

Signed-off-by: Daniel Martin 

(Ported from amdgpu commit 10054b6c3d9a755b30abb43020121b9631fa296d)

Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 606f3f900..525ded256 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1424,9 +1424,12 @@ drmmode_output_detect(xf86OutputPtr output)
xf86OutputStatus status;
drmModeFreeConnector(drmmode_output->mode_output);
 
-   drmmode_output->mode_output = drmModeGetConnector(pRADEONEnt->fd, 
drmmode_output->output_id);
-   if (!drmmode_output->mode_output)
+   drmmode_output->mode_output =
+   drmModeGetConnector(pRADEONEnt->fd, drmmode_output->output_id);
+   if (!drmmode_output->mode_output) {
+   drmmode_output->output_id = -1;
return XF86OutputStatusDisconnected;
+   }
 
switch (drmmode_output->mode_output->connection) {
case DRM_MODE_CONNECTED:
-- 
2.16.2

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

[PATCH xf86-video-ati 3/3] modesetting: Update property values at detect and uevent time

[Michel Dänzer]

From: Keith Packard 

We were updating the link-status property when a uevent came in, but
we also want to update the non-desktop property, and potentially
others as well. We also want to check at detect time in case we don't
get a hotplug event.

This patch updates every property provided by the kernel, sending
changes to DIX so it can track things as well.

Signed-off-by: Keith Packard 

(Ported from amdgpu commit 374cb8fef4fdbb648af089ee80803ec78321f1b2)

Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.c | 104 +-
 1 file changed, 86 insertions(+), 18 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 525ded256..b1f5c4888 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1415,6 +1415,72 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_res
return 1;
 }
 
+/*
+ * Update all of the property values for an output
+ */
+static void
+drmmode_output_update_properties(xf86OutputPtr output)
+{
+   drmmode_output_private_ptr drmmode_output = output->driver_private;
+   int i, j, k;
+   int err;
+   drmModeConnectorPtr koutput;
+
+   /* Use the most recently fetched values from the kernel */
+   koutput = drmmode_output->mode_output;
+
+   if (!koutput)
+   return;
+
+   for (i = 0; i < drmmode_output->num_props; i++) {
+   drmmode_prop_ptr p = _output->props[i];
+
+   for (j = 0; j < koutput->count_props; j++) {
+   if (koutput->props[j] != p->mode_prop->prop_id)
+   continue;
+
+   /* Check to see if the property value has changed */
+   if (koutput->prop_values[j] == p->value)
+   break;
+
+   p->value = koutput->prop_values[j];
+
+   if (p->mode_prop->flags & DRM_MODE_PROP_RANGE) {
+   INT32 value = p->value;
+
+   err = 
RRChangeOutputProperty(output->randr_output,
+p->atoms[0], 
XA_INTEGER,
+32, 
PropModeReplace, 1,
+, FALSE, 
TRUE);
+   if (err != 0) {
+   xf86DrvMsg(output->scrn->scrnIndex, 
X_ERROR,
+  "RRChangeOutputProperty 
error, %d\n",
+  err);
+   }
+   } else if (p->mode_prop->flags & DRM_MODE_PROP_ENUM) {
+   for (k = 0; k < p->mode_prop->count_enums; k++) 
{
+   if (p->mode_prop->enums[k].value == 
p->value)
+   break;
+   }
+   if (k < p->mode_prop->count_enums) {
+   err = 
RRChangeOutputProperty(output->randr_output,
+
p->atoms[0], XA_ATOM,
+32, 
PropModeReplace, 1,
+
>atoms[k + 1], FALSE,
+TRUE);
+   if (err != 0) {
+   
xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
+  
"RRChangeOutputProperty error, %d\n",
+  err);
+   }
+   }
+   }
+
+   break;
+   }
+}
+}
+
 static xf86OutputStatus
 drmmode_output_detect(xf86OutputPtr output)
 {
@@ -1431,6 +1497,8 @@ drmmode_output_detect(xf86OutputPtr output)
return XF86OutputStatusDisconnected;
}
 
+   drmmode_output_update_properties(output);
+
switch (drmmode_output->mode_output->connection) {
case DRM_MODE_CONNECTED:
status = XF86OutputStatusConnected;
@@ -2909,35 +2977,35 @@ radeon_mode_hotplug(ScrnInfoPtr scrn, drmmode_ptr 
drmmode)
xf86OutputPtr output = config->output[i];
xf86CrtcPtr crtc = output->crtc;
drmmode_output_private_ptr drmmode_output = 
output->driver_private;
-   uint32_t con_id, idx;
-   drmModeConnectorPtr koutput;
+
+   drmmode_output_detect(output);
 
if (!crtc || !drmmode_output->mode_output)
continue;
 
-   

[PATCH xf86-video-ati 1/3] modesetting: Use helper to fetch drmModeProperty(Blob)s

[Michel Dänzer]

From: Daniel Martin 

Replace the various loops to lookup drmModeProperty(Blob)s by
introducing helper functions.

Signed-off-by: Daniel Martin 

(Ported from amdgpu commit fb58e06acd6c6bd59de2dbdadbca27eb1dd0025b)

Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.c | 121 --
 1 file changed, 69 insertions(+), 52 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index d8bb9cc9a..606f3f900 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1449,6 +1449,51 @@ drmmode_output_mode_valid(xf86OutputPtr output, 
DisplayModePtr pModes)
return MODE_OK;
 }
 
+static int
+koutput_get_prop_idx(int fd, drmModeConnectorPtr koutput,
+int type, const char *name)
+{
+int idx = -1;
+
+for (int i = 0; i < koutput->count_props; i++) {
+drmModePropertyPtr prop = drmModeGetProperty(fd, koutput->props[i]);
+
+if (!prop)
+continue;
+
+if (drm_property_type_is(prop, type) && !strcmp(prop->name, name))
+idx = i;
+
+drmModeFreeProperty(prop);
+
+if (idx > -1)
+break;
+}
+
+return idx;
+}
+
+static int
+koutput_get_prop_id(int fd, drmModeConnectorPtr koutput,
+int type, const char *name)
+{
+int idx = koutput_get_prop_idx(fd, koutput, type, name);
+
+return (idx > -1) ? koutput->props[idx] : -1;
+}
+
+static drmModePropertyBlobPtr
+koutput_get_prop_blob(int fd, drmModeConnectorPtr koutput, const char *name)
+{
+drmModePropertyBlobPtr blob = NULL;
+int idx = koutput_get_prop_idx(fd, koutput, DRM_MODE_PROP_BLOB, name);
+
+if (idx > -1)
+blob = drmModeGetPropertyBlob(fd, koutput->prop_values[idx]);
+
+return blob;
+}
+
 static DisplayModePtr
 drmmode_output_get_modes(xf86OutputPtr output)
 {
@@ -1457,25 +1502,16 @@ drmmode_output_get_modes(xf86OutputPtr output)
RADEONEntPtr pRADEONEnt = RADEONEntPriv(output->scrn);
int i;
DisplayModePtr Modes = NULL, Mode;
-   drmModePropertyPtr props;
xf86MonPtr mon = NULL;
 
if (!koutput)
return NULL;
 
+   drmModeFreePropertyBlob(drmmode_output->edid_blob);
+
/* look for an EDID property */
-   for (i = 0; i < koutput->count_props; i++) {
-   props = drmModeGetProperty(pRADEONEnt->fd, koutput->props[i]);
-   if (props && (props->flags & DRM_MODE_PROP_BLOB)) {
-   if (!strcmp(props->name, "EDID")) {
-   if (drmmode_output->edid_blob)
-   
drmModeFreePropertyBlob(drmmode_output->edid_blob);
-   drmmode_output->edid_blob = 
drmModeGetPropertyBlob(pRADEONEnt->fd, koutput->prop_values[i]);
-   }
-   }
-   if (props)
-   drmModeFreeProperty(props);
-   }
+   drmmode_output->edid_blob =
+   koutput_get_prop_blob(pRADEONEnt->fd, koutput, "EDID");
 
if (drmmode_output->edid_blob) {
mon = xf86InterpretEDID(output->scrn->scrnIndex,
@@ -1896,7 +1932,6 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
drmModeConnectorPtr koutput;
drmModeEncoderPtr *kencoders = NULL;
drmmode_output_private_ptr drmmode_output;
-   drmModePropertyPtr props;
drmModePropertyBlobPtr path_blob = NULL;
char name[32];
int i;
@@ -1906,17 +1941,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
if (!koutput)
return 0;
 
-   for (i = 0; i < koutput->count_props; i++) {
-   props = drmModeGetProperty(pRADEONEnt->fd, koutput->props[i]);
-   if (props && (props->flags & DRM_MODE_PROP_BLOB)) {
-   if (!strcmp(props->name, "PATH")) {
-   path_blob = 
drmModeGetPropertyBlob(pRADEONEnt->fd, koutput->prop_values[i]);
-   drmModeFreeProperty(props);
-   break;
-   }
-   }
-   drmModeFreeProperty(props);
-   }
+   path_blob = koutput_get_prop_blob(pRADEONEnt->fd, koutput, "PATH");
 
kencoders = calloc(sizeof(drmModeEncoderPtr), koutput->count_encoders);
if (!kencoders) {
@@ -1996,17 +2021,9 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, drmModeResPtr mode_r
/* work out the possible clones later */
output->possible_clones = 0;
 
-   for (i = 0; i < koutput->count_props; i++) {
-   props = drmModeGetProperty(pRADEONEnt->fd, koutput->props[i]);
-   if (props && (props->flags & DRM_MODE_PROP_ENUM)) {
-   if (!strcmp(props->name, "DPMS")) {
-   drmmode_output->dpms_enum_id = 

[PATCH] drm/amd/pp: Fix incorrect return value in smu7_check_clk_voltage_valid

[Rex Zhu]

Change-Id: I248982c5e0100b7a975d1a19781749baeadd71f4
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index dbfe139..10fb3ec 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -4692,7 +4692,7 @@ static bool smu7_check_clk_voltage_valid(struct pp_hwmgr 
*hwmgr,
struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table;
 
if (table_info == NULL)
-   return -EINVAL;
+   return false;
 
dep_sclk_table = table_info->vdd_dep_on_sclk;
min_vddc = dep_sclk_table->entries[0].vddc;
-- 
1.9.1

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

RE: [PATCH] drm/amdgpu: try again kiq access if not in IRQ(v2)

[Liu, Monk]

In_atomic() isnot encouraged to be used to judge if sleep is possible, see the 
macros of it

#define in_atomic() (preept_count() != 0)


/Monk

-Original Message-
From: Kuehling, Felix 
Sent: 2018年3月1日 23:50
To: amd-gfx@lists.freedesktop.org; Liu, Monk 
Subject: Re: [PATCH] drm/amdgpu: try again kiq access if not in IRQ(v2)

On 2018-02-28 02:27 AM, Monk Liu wrote:
> sometimes GPU is switched to other VFs and won't swich back soon, so 
> the kiq reg access will not signal within a short period, instead of 
> busy waiting a long time(MAX_KEQ_REG_WAIT) and returning TMO we can 
> istead sleep 5ms and try again later (non irq context)
>
> And since the waiting in kiq_r/weg is busy wait, so MAX_KIQ_REG_WAIT 
> shouldn't set to a long time, set it to 10ms is more appropriate.
>
> if gpu already in reset state, don't retry the KIQ reg access 
> otherwise it would always hang because KIQ was already die usually.
>
> v2:
> replace schedule() with msleep() for the wait
>
> Change-Id: I8fc807ce85a8d30d2b50153f3f3a6eda344ef994
> Signed-off-by: Monk Liu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b832651..1672f5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -22,7 +22,7 @@
>   */
>  
>  #include "amdgpu.h"
> -#define MAX_KIQ_REG_WAIT 1 /* in usecs */
> +#define MAX_KIQ_REG_WAIT 1 /* in usecs, 10ms */
>  
>  uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)  { @@ -152,9 
> +152,14 @@ uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t 
> reg)
>   amdgpu_ring_commit(ring);
>   spin_unlock_irqrestore(>ring_lock, flags);
>  
> +retry_read:
>   r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>   if (r < 1) {
>   DRM_ERROR("wait for kiq fence error: %ld\n", r);
> + if (!in_interrupt() && !adev->in_gpu_reset) {

You should check in_atomic here. Because it's invalid to sleep in atomic 
context (e.g. while holding a spin lock) even when not in an interrupt.
This seems to happen a lot for indirect register access, e.g.
soc15_pcie_rreg.

Regards,
  Felix

> + msleep(5);
> + goto retry_read;
> + }
>   return ~0;
>   }
>   val = adev->wb.wb[adev->virt.reg_val_offs];
> @@ -179,9 +184,15 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, 
> uint32_t reg, uint32_t v)
>   amdgpu_ring_commit(ring);
>   spin_unlock_irqrestore(>ring_lock, flags);
>  
> +retry_write:
>   r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> - if (r < 1)
> + if (r < 1) {
>   DRM_ERROR("wait for kiq fence error: %ld\n", r);
> + if (!in_interrupt() && !adev->in_gpu_reset) {
> + msleep(5);
> + goto retry_write;
> + }
> + }
>  }
>  
>  /**

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