[PATCH] drm/amd/powerplay: hint and error out when prerequisite not meet

[Evan Quan]

Do not ignore the error silently. Otherwise user may thought it
was executed successfully.

Change-Id: I4ac0cff5918d53d1d737f1c2fb1a4f81917dbf56
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 14 ++
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c |  4 +++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 5e071917594f..e0a9f02d68d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -756,8 +756,11 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
if (ret)
return ret;
 
-   if (adev->powerplay.pp_funcs->force_clock_level)
-   amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
+   if (adev->powerplay.pp_funcs->force_clock_level) {
+   ret = amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
+   if (ret)
+   return ret;
+   }
 
return count;
 }
@@ -789,8 +792,11 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
if (ret)
return ret;
 
-   if (adev->powerplay.pp_funcs->force_clock_level)
-   amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);
+   if (adev->powerplay.pp_funcs->force_clock_level) {
+   ret = amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);
+   if (ret)
+   return ret;
+   }
 
return count;
 }
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c 
b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 27215e4b488b..285375b96ea4 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -727,8 +727,10 @@ static int pp_dpm_force_clock_level(void *handle,
mutex_lock(&hwmgr->smu_lock);
if (hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
ret = hwmgr->hwmgr_func->force_clock_level(hwmgr, type, mask);
-   else
+   else {
+   pr_info_ratelimited("Force clock level setting is for manual 
dpm mode only.\n");
ret = -EINVAL;
+   }
mutex_unlock(&hwmgr->smu_lock);
return ret;
 }
-- 
2.20.1

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

[PATCH] drm/amdgpu/sriov: For finishing routine send rel event after init failed

[Emily Deng]

When init fail, sendsend rel init, req_fini and rel_fini to host for the
finishing routine.

Signed-off-by: Emily Deng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 3c57ffc..ccd2e83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1652,7 +1652,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
if (r) {
DRM_ERROR("sw_init of IP block <%s> failed %d\n",
  adev->ip_blocks[i].version->funcs->name, r);
-   return r;
+   goto init_failed;
}
adev->ip_blocks[i].status.sw = true;
 
@@ -1661,17 +1661,17 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
r = amdgpu_device_vram_scratch_init(adev);
if (r) {
DRM_ERROR("amdgpu_vram_scratch_init failed 
%d\n", r);
-   return r;
+   goto init_failed;
}
r = adev->ip_blocks[i].version->funcs->hw_init((void 
*)adev);
if (r) {
DRM_ERROR("hw_init %d failed %d\n", i, r);
-   return r;
+   goto init_failed;
}
r = amdgpu_device_wb_init(adev);
if (r) {
DRM_ERROR("amdgpu_device_wb_init failed %d\n", 
r);
-   return r;
+   goto init_failed;
}
adev->ip_blocks[i].status.hw = true;
 
@@ -1682,7 +1682,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)

AMDGPU_CSA_SIZE);
if (r) {
DRM_ERROR("allocate CSA failed %d\n", 
r);
-   return r;
+   goto init_failed;
}
}
}
@@ -1690,30 +1690,32 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
 
r = amdgpu_ucode_create_bo(adev); /* create ucode bo when sw_init 
complete*/
if (r)
-   return r;
+   goto init_failed;
 
r = amdgpu_device_ip_hw_init_phase1(adev);
if (r)
-   return r;
+   goto init_failed;
 
r = amdgpu_device_fw_loading(adev);
if (r)
-   return r;
+   goto init_failed;
 
r = amdgpu_device_ip_hw_init_phase2(adev);
if (r)
-   return r;
+   goto init_failed;
 
if (adev->gmc.xgmi.num_physical_nodes > 1)
amdgpu_xgmi_add_device(adev);
amdgpu_amdkfd_device_init(adev);
 
+init_failed:
if (amdgpu_sriov_vf(adev)) {
-   amdgpu_virt_init_data_exchange(adev);
+   if (!r)
+   amdgpu_virt_init_data_exchange(adev);
amdgpu_virt_release_full_gpu(adev, true);
}
 
-   return 0;
+   return r;
 }
 
 /**
@@ -2621,6 +2623,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
}
dev_err(adev->dev, "amdgpu_device_ip_init failed\n");
amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_AMDGPU_INIT_FAIL, 0, 
0);
+   if (amdgpu_virt_request_full_gpu(adev, false))
+   amdgpu_virt_release_full_gpu(adev, false);
goto failed;
}
 
-- 
2.7.4

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

Re: [PATCH] drm/amdkfd: Allow building KFD on ARM64

[Alex Deucher]

On Wed, Jan 2, 2019 at 5:26 PM Kuehling, Felix  wrote:
>
> ifdef x86_64 specific code.
> Allow enabling CONFIG_HSA_AMD on ARM64.
>
> CC: Mark Nutter 
> Signed-off-by: Felix Kuehling 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdkfd/Kconfig| 4 ++--
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 8 
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 8 +---
>  3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig 
> b/drivers/gpu/drm/amd/amdkfd/Kconfig
> index fbf0ee5..c361360 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
> @@ -4,8 +4,8 @@
>
>  config HSA_AMD
> bool "HSA kernel driver for AMD GPU devices"
> -   depends on DRM_AMDGPU && X86_64
> -   imply AMD_IOMMU_V2
> +   depends on DRM_AMDGPU && (X86_64 || ARM64)
> +   imply AMD_IOMMU_V2 if X86_64
> select MMU_NOTIFIER
> help
>   Enable this if you want to use HSA features on AMD GPU devices.
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index c02adbb..d7ddb35 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -863,6 +863,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, 
> int *avail_size,
> return 0;
>  }
>
> +#if CONFIG_X86_64
>  static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
> uint32_t *num_entries,
> struct crat_subtype_iolink *sub_type_hdr)
> @@ -905,6 +906,7 @@ static int kfd_fill_iolink_info_for_cpu(int numa_node_id, 
> int *avail_size,
>
> return 0;
>  }
> +#endif
>
>  /* kfd_create_vcrat_image_cpu - Create Virtual CRAT for CPU
>   *
> @@ -920,7 +922,9 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
> size_t *size)
> struct crat_subtype_generic *sub_type_hdr;
> int avail_size = *size;
> int numa_node_id;
> +#ifdef CONFIG_X86_64
> uint32_t entries = 0;
> +#endif
> int ret = 0;
>
> if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_CPU)
> @@ -982,6 +986,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
> size_t *size)
> sub_type_hdr->length);
>
> /* Fill in Subtype: IO Link */
> +#ifdef CONFIG_X86_64
> ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
> &entries,
> (struct crat_subtype_iolink *)sub_type_hdr);
> @@ -992,6 +997,9 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
> size_t *size)
>
> sub_type_hdr = (typeof(sub_type_hdr))((char *)sub_type_hdr +
> sub_type_hdr->length * entries);
> +#else
> +   pr_info("IO link not available for non x86 platforms\n");
> +#endif
>
> crat_table->num_domains++;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 5f5b2ac..8afc592 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1400,9 +1400,11 @@ static int kfd_cpumask_to_apic_id(const struct cpumask 
> *cpumask)
> first_cpu_of_numa_node = cpumask_first(cpumask);
> if (first_cpu_of_numa_node >= nr_cpu_ids)
> return -1;
> -   cpuinfo = &cpu_data(first_cpu_of_numa_node);
> -
> -   return cpuinfo->apicid;
> +#ifdef CONFIG_X86_64
> +   return cpu_data(first_cpu_of_numa_node).apicid;
> +#else
> +   return first_cpu_of_numa_node;
> +#endif
>  }
>
>  /* kfd_numa_node_to_apic_id - Returns the APIC ID of the first logical 
> processor
> --
> 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

[PATCH v3 15/16] drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()

[Lyude Paul]

It occurred to me that we never actually check this! So let's start
doing that.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index c33c4a3aec34..fc9bcbb55417 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3647,7 +3647,7 @@ drm_dp_mst_atomic_check_topology_state(struct 
drm_dp_mst_topology_mgr *mgr,
   struct drm_dp_mst_topology_state 
*mst_state)
 {
struct drm_dp_vcpi_allocation *vcpi;
-   int avail_slots = 63;
+   int avail_slots = 63, payload_count = 0;
 
list_for_each_entry(vcpi, &mst_state->vcpis, next) {
/* Releasing VCPI is always OK-even if the port is gone */
@@ -3667,6 +3667,12 @@ drm_dp_mst_atomic_check_topology_state(struct 
drm_dp_mst_topology_mgr *mgr,
 avail_slots + vcpi->vcpi);
return -ENOSPC;
}
+
+   if (++payload_count > mgr->max_payloads) {
+   DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many 
payloads (max=%d)\n",
+mgr, mst_state, mgr->max_payloads);
+   return -EINVAL;
+   }
}
DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n",
 mgr, mst_state, avail_slots,
-- 
2.20.1

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

[PATCH v3 14/16] drm/dp_mst: Start tracking per-port VCPI allocations

[Lyude Paul]

There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:

/* We cannot rely on port->vcpi.num_slots to update
 * topology_state->avail_slots as the port may not exist if the parent
 * branch device was unplugged. This should be fixed by tracking
 * per-port slot allocation in drm_dp_mst_topology_state instead of
 * depending on the caller to tell us how many slots to release.
 */

That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.

So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.

Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.

Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.

Changes since v8:
 * Fix compile errors, whoops!

Changes since v7:
 - Don't check for mixed stale/valid VCPI allocations, just rely on
 connector registration to stop such erroneous modesets

Changes since v6:
 - Keep a kref to all of the ports we have allocations on. This required
   a good bit of changing to when we call drm_dp_find_vcpi_slots(),
   mainly that we need to ensure that we only redo VCPI allocations on
   actual mode or CRTC changes, not crtc_state->active changes.
   Additionally, we no longer take the registration of the DRM connector
   for each port into account because so long as we have a kref to the
   port in the new or previous atomic state, the connector will stay
   registered.
 - Use the small changes to drm_dp_put_port() to add even more error
   checking to make misusage of the helpers more obvious. I added this
   after having to chase down various use-after-free conditions that
   started popping up from the new helpers so no one else has to
   troubleshoot that.
 - Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
 - Update documentation again, note that find/release() should both not be
   called on the same port in a single atomic check phase (but multiple
   calls to one or the other is OK)

Changes since v4:
 - Don't skip the atomic checks for VCPI allocations if no new VCPI
   allocations happen in a state. This makes the next change I'm about
   to list here a lot easier to implement.
 - Don't ignore VCPI allocations on destroyed ports, instead ensure that
   when ports are destroyed and still have VCPI allocations in the
   topology state, the only state changes allowed are releasing said
   ports' VCPI. This prevents a state with a mix of VCPI allocations
   from destroyed ports, and allocations from valid ports.

Changes since v3:
 - Don't release VCPI allocations in the topology state immediately in
   drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
   over them in drm_dp_mst_duplicate_state(). This makes it so
   drm_dp_atomic_release_vcpi_slots() is still idempotent while also
   throwing warnings if the driver messes up it's book keeping and tries
   to release VCPI slots on a port that doesn't have any pre-existing
   VCPI allocation - danvet
 - Change mst_state/state in some debugging messages to "mst state"

Changes since v2:
 - Use kmemdup() for duplicating MST state - danvet
 - Move port validation out of duplicate state callback - danvet
 - Handle looping through MST topology states in
   drm_dp_mst_atomic_check() so the driver doesn't have to do it
 - Fix documentation in drm_dp_atomic_find_vcpi_slots()
 - Move the atomic check for each individual topology state into it's
   own function, reduces indenting
 - Don't consider "stale" MST ports when calculating the bandwidth
   requirements. This is needed because originally we relied on the
   state duplication functions to prune any stale ports from the new
   state, which would prevent us from incorrectly considering their
   bandwidth requirements alongside legitimate new payloads.
 - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
 - Annotate atomic VCPI and atomic check functions with __must_check
   - danvet

Changes since 

[PATCH v3 16/16] drm/nouveau: Use atomic VCPI helpers for MST

[Lyude Paul]

Currently, nouveau uses the yolo method of setting up MST displays: it
uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the
display configuration. These helpers don't take care to make sure they
take a reference to the mstb port that they're checking, and
additionally don't actually check whether or not the topology still has
enough bandwidth to provide the VCPI tokens required.

So, drop usage of the old helpers and move entirely over to the atomic
helpers.

Changes since v5:
 - Update nv50_msto_atomic_check() and nv50_mstc_atomic_check() to the
   new requirements for drm_dp_atomic_find_vcpi_slots() and
   drm_dp_atomic_release_vcpi_slots()

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 52 ++---
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 67f7bf97e5d9..df696008d205 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -762,16 +762,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
   struct drm_crtc_state *crtc_state,
   struct drm_connector_state *conn_state)
 {
-   struct nv50_mstc *mstc = nv50_mstc(conn_state->connector);
+   struct drm_atomic_state *state = crtc_state->state;
+   struct drm_connector *connector = conn_state->connector;
+   struct nv50_mstc *mstc = nv50_mstc(connector);
struct nv50_mstm *mstm = mstc->mstm;
-   int bpp = conn_state->connector->display_info.bpc * 3;
+   int bpp = connector->display_info.bpc * 3;
int slots;
 
-   mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp);
+   mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
+bpp);
 
-   slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn);
-   if (slots < 0)
-   return slots;
+   if (crtc_state->connectors_changed || crtc_state->mode_changed) {
+   slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
+ mstc->port, mstc->pbn);
+   if (slots < 0)
+   return slots;
+   }
 
return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
   mstc->native);
@@ -934,12 +940,42 @@ nv50_mstc_get_modes(struct drm_connector *connector)
return ret;
 }
 
+static int
+nv50_mstc_atomic_check(struct drm_connector *connector,
+  struct drm_connector_state *new_conn_state)
+{
+   struct drm_atomic_state *state = new_conn_state->state;
+   struct nv50_mstc *mstc = nv50_mstc(connector);
+   struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr;
+   struct drm_connector_state *old_conn_state =
+   drm_atomic_get_old_connector_state(state, connector);
+   struct drm_crtc_state *old_crtc_state;
+   struct drm_crtc *new_crtc = new_conn_state->crtc,
+   *old_crtc = old_conn_state->crtc;
+
+   if (!old_crtc)
+   return 0;
+
+   old_crtc_state = drm_atomic_get_old_crtc_state(state, old_crtc);
+   if (!old_crtc_state || !old_crtc_state->enable)
+   return 0;
+
+   if (new_crtc)
+   return 0;
+
+   /* This connector will be left without an enabled CRTC, so its VCPI
+* must be released here
+*/
+   return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port);
+}
+
 static const struct drm_connector_helper_funcs
 nv50_mstc_help = {
.get_modes = nv50_mstc_get_modes,
.mode_valid = nv50_mstc_mode_valid,
.best_encoder = nv50_mstc_best_encoder,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
+   .atomic_check = nv50_mstc_atomic_check,
 };
 
 static enum drm_connector_status
@@ -2121,6 +2157,10 @@ nv50_disp_atomic_check(struct drm_device *dev, struct 
drm_atomic_state *state)
return ret;
}
 
+   ret = drm_dp_mst_atomic_check(state);
+   if (ret)
+   return ret;
+
return 0;
 }
 
-- 
2.20.1

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

[PATCH v3 00/16] MST refcounting/atomic helpers cleanup

[Lyude Paul]

This is the series I've been working on for a while now to get all of
the atomic DRM drivers in the tree to use the atomic MST helpers, and to
make the atomic MST helpers actually idempotent. Turns out it's a lot
more difficult to do that without also fixing how port and branch device
refcounting works so that it actually makes sense, since the current
upstream implementation requires a ton of magic in the atomic helpers to
work around properly and in many situations just plain doesn't work as
intended.

There's still more cleanup that can be done, but I think this is a good
place to start off for now :).

Lyude Paul (16):
  drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and
friends
  drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
  drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref
fails
  drm/dp_mst: Stop releasing VCPI when removing ports from topology
  drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs
  drm/i915: Keep malloc references to MST ports
  drm/amdgpu/display: Keep malloc ref to MST port
  drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()
  drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()
  drm/nouveau: Keep malloc references to MST ports
  drm/nouveau: Stop unsetting mstc->port, use malloc refs
  drm/nouveau: Grab payload lock in nv50_msto_payload()
  drm/dp_mst: Add some atomic state iterator macros
  drm/dp_mst: Start tracking per-port VCPI allocations
  drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()
  drm/nouveau: Use atomic VCPI helpers for MST

 .../gpu/dp-mst/topology-figure-1.dot  |  52 +
 .../gpu/dp-mst/topology-figure-2.dot  |  56 ++
 .../gpu/dp-mst/topology-figure-3.dot  |  59 ++
 Documentation/gpu/drm-kms-helpers.rst |  26 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  11 +-
 drivers/gpu/drm/drm_dp_mst_topology.c | 938 ++
 drivers/gpu/drm/i915/intel_connector.c|   4 +
 drivers/gpu/drm/i915/intel_display.c  |   4 +
 drivers/gpu/drm/i915/intel_dp_mst.c   |  65 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  94 +-
 include/drm/drm_dp_mst_helper.h   | 151 ++-
 11 files changed, 1208 insertions(+), 252 deletions(-)
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-2.dot
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-3.dot

-- 
2.20.1

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

[PATCH v3 10/16] drm/nouveau: Keep malloc references to MST ports

[Lyude Paul]

Now that we finally have a sane way to keep port allocations around, use
it to fix the potential unchecked ->port accesses that nouveau makes by
making sure we keep the mst port allocated for as long as it's
drm_connector is accessible.

Additionally, now that we've guaranteed that mstc->port is allocated for
as long as we keep mstc around we can remove the connector registration
checks for codepaths which release payloads, allowing us to release
payloads on active topologies properly. These registration checks were
only required before in order to avoid situations where mstc->port could
technically be pointing at freed memory.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 0f7d72518604..982054bbcc8b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -964,7 +964,11 @@ static void
 nv50_mstc_destroy(struct drm_connector *connector)
 {
struct nv50_mstc *mstc = nv50_mstc(connector);
+
drm_connector_cleanup(&mstc->connector);
+   if (mstc->port)
+   drm_dp_mst_put_port_malloc(mstc->port);
+
kfree(mstc);
 }
 
@@ -1012,6 +1016,7 @@ nv50_mstc_new(struct nv50_mstm *mstm, struct 
drm_dp_mst_port *port,
drm_object_attach_property(&mstc->connector.base, 
dev->mode_config.path_property, 0);
drm_object_attach_property(&mstc->connector.base, 
dev->mode_config.tile_property, 0);
drm_connector_set_path_property(&mstc->connector, path);
+   drm_dp_mst_get_port_malloc(port);
return 0;
 }
 
@@ -1077,6 +1082,7 @@ nv50_mstm_destroy_connector(struct 
drm_dp_mst_topology_mgr *mgr,
drm_fb_helper_remove_one_connector(&drm->fbcon->helper, 
&mstc->connector);
 
drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
+   drm_dp_mst_put_port_malloc(mstc->port);
mstc->port = NULL;
drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
 
-- 
2.20.1

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

[PATCH v3 11/16] drm/nouveau: Stop unsetting mstc->port, use malloc refs

[Lyude Paul]

Same as we did for i915, but for nouveau this time. Additionally, we
grab a malloc reference to the port that lasts for the entire lifetime
of nv50_mstc, which gives us the guarantee that mstc->port will always
point to valid memory for as long as the mstc stays around.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 982054bbcc8b..157d208d37b5 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -709,8 +709,7 @@ nv50_msto_cleanup(struct nv50_msto *msto)
 
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
 
-   if (mstc->port)
-   drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+   drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
 
msto->mstc = NULL;
msto->head = NULL;
@@ -735,7 +734,7 @@ nv50_msto_prepare(struct nv50_msto *msto)
};
 
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
-   if (mstc->port && mstc->port->vcpi.vcpi > 0) {
+   if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
if (payload) {
args.vcpi.start_slot = payload->start_slot;
@@ -832,8 +831,7 @@ nv50_msto_disable(struct drm_encoder *encoder)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
 
-   if (mstc->port)
-   drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
+   drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
 
mstm->outp->update(mstm->outp, msto->head->base.index, NULL, 0, 0);
mstm->modified = true;
@@ -945,7 +943,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool 
force)
enum drm_connector_status conn_status;
int ret;
 
-   if (!mstc->port)
+   if (drm_connector_is_unregistered(connector))
return connector_status_disconnected;
 
ret = pm_runtime_get_sync(connector->dev->dev);
@@ -966,8 +964,7 @@ nv50_mstc_destroy(struct drm_connector *connector)
struct nv50_mstc *mstc = nv50_mstc(connector);
 
drm_connector_cleanup(&mstc->connector);
-   if (mstc->port)
-   drm_dp_mst_put_port_malloc(mstc->port);
+   drm_dp_mst_put_port_malloc(mstc->port);
 
kfree(mstc);
 }
@@ -1081,11 +1078,6 @@ nv50_mstm_destroy_connector(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_fb_helper_remove_one_connector(&drm->fbcon->helper, 
&mstc->connector);
 
-   drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
-   drm_dp_mst_put_port_malloc(mstc->port);
-   mstc->port = NULL;
-   drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
-
drm_connector_put(&mstc->connector);
 }
 
-- 
2.20.1

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

[PATCH v3 05/16] drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs

[Lyude Paul]

Up until now, freeing payloads on remote MST hubs that just had ports
removed has almost never worked because we've been relying on port
validation in order to stop us from accessing ports that have already
been freed from memory, but ports which need their payloads released due
to being removed will never be a valid part of the topology after
they've been removed.

Since we've introduced malloc refs, we can replace all of the validation
logic in payload helpers which are used for deallocation with some
well-placed malloc krefs. This ensures that regardless of whether or not
the ports are still valid and in the topology, any port which has an
allocated payload will remain allocated in memory until it's payloads
have been removed - finally allowing us to actually release said
payloads correctly.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 54 +++
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f10a7edb401e..769e2c0419c2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2102,10 +2102,6 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
u8 sinks[DRM_DP_MAX_SDP_STREAMS];
int i;
 
-   port = drm_dp_mst_topology_get_port_validated(mgr, port);
-   if (!port)
-   return -EINVAL;
-
port_num = port->port_num;
mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
if (!mstb) {
@@ -2113,10 +2109,8 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
   port->parent,
   &port_num);
 
-   if (!mstb) {
-   drm_dp_mst_topology_put_port(port);
+   if (!mstb)
return -EINVAL;
-   }
}
 
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -2153,7 +2147,6 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
kfree(txmsg);
 fail_put:
drm_dp_mst_topology_put_mstb(mstb);
-   drm_dp_mst_topology_put_port(port);
return ret;
 }
 
@@ -2258,15 +2251,16 @@ static int drm_dp_destroy_payload_step2(struct 
drm_dp_mst_topology_mgr *mgr,
  */
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 {
-   int i, j;
-   int cur_slots = 1;
struct drm_dp_payload req_payload;
struct drm_dp_mst_port *port;
+   int i, j;
+   int cur_slots = 1;
 
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
struct drm_dp_payload *payload = &mgr->payloads[i];
+   bool put_port = false;
 
/* solve the current payloads - compare to the hw ones
   - update the hw view */
@@ -2274,12 +2268,20 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
if (vcpi) {
port = container_of(vcpi, struct drm_dp_mst_port,
vcpi);
-   port = drm_dp_mst_topology_get_port_validated(mgr,
- port);
-   if (!port) {
-   mutex_unlock(&mgr->payload_lock);
-   return -EINVAL;
+
+   /* Validated ports don't matter if we're releasing
+* VCPI
+*/
+   if (vcpi->num_slots) {
+   port = drm_dp_mst_topology_get_port_validated(
+   mgr, port);
+   if (!port) {
+   mutex_unlock(&mgr->payload_lock);
+   return -EINVAL;
+   }
+   put_port = true;
}
+
req_payload.num_slots = vcpi->num_slots;
req_payload.vcpi = vcpi->vcpi;
} else {
@@ -2311,7 +2313,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
}
cur_slots += req_payload.num_slots;
 
-   if (port)
+   if (put_port)
drm_dp_mst_topology_put_port(port);
}
 
@@ -3126,6 +3128,8 @@ bool drm_dp_mst_allocate_vcpi(struct 
drm_dp_mst_topology_mgr *mgr,
DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
  pbn, port->vcpi.num_slots);
 
+   /* Keep port allocated until it's payload has been removed *

[PATCH v3 06/16] drm/i915: Keep malloc references to MST ports

[Lyude Paul]

So that the ports stay around until we've destroyed the connectors, in
order to ensure that we don't pass an invalid pointer to any MST helpers
once we introduce the new MST VCPI helpers.

Changes since v1:
* Move drm_dp_mst_get_port_malloc() to where we assign
  intel_connector->port - danvet

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/i915/intel_connector.c | 4 
 drivers/gpu/drm/i915/intel_dp_mst.c| 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_connector.c 
b/drivers/gpu/drm/i915/intel_connector.c
index 18e370f607bc..37d2c644f4b8 100644
--- a/drivers/gpu/drm/i915/intel_connector.c
+++ b/drivers/gpu/drm/i915/intel_connector.c
@@ -95,6 +95,10 @@ void intel_connector_destroy(struct drm_connector *connector)
intel_panel_fini(&intel_connector->panel);
 
drm_connector_cleanup(connector);
+
+   if (intel_connector->port)
+   drm_dp_mst_put_port_malloc(intel_connector->port);
+
kfree(connector);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index f05427b74e34..631fd1537252 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -457,6 +457,7 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
intel_connector->mst_port = intel_dp;
intel_connector->port = port;
+   drm_dp_mst_get_port_malloc(port);
 
connector = &intel_connector->base;
ret = drm_connector_init(dev, connector, &intel_dp_mst_connector_funcs,
-- 
2.20.1

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

[PATCH v3 01/16] drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends

[Lyude Paul]

s/drm_dp_get_validated_port_ref/drm_dp_mst_topology_get_port_validated/
s/drm_dp_put_port/drm_dp_mst_topology_put_port/
s/drm_dp_get_validated_mstb_ref/drm_dp_mst_topology_get_mstb_validated/
s/drm_dp_put_mst_branch_device/drm_dp_mst_topology_put_mstb/

This is a much more consistent naming scheme, and will make even more
sense once we redesign how the current refcounting scheme here works.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 114 ++
 1 file changed, 62 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2ab16c9e6243..6f9b211069a7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -46,7 +46,7 @@ static bool dump_dp_payload_table(struct 
drm_dp_mst_topology_mgr *mgr,
  char *buf);
 static int test_calc_pbn_mode(void);
 
-static void drm_dp_put_port(struct drm_dp_mst_port *port);
+static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port);
 
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 int id,
@@ -888,7 +888,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref 
*kref)
 */
list_for_each_entry_safe(port, tmp, &mstb->ports, next) {
list_del(&port->next);
-   drm_dp_put_port(port);
+   drm_dp_mst_topology_put_port(port);
}
 
/* drop any tx slots msg */
@@ -911,7 +911,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref 
*kref)
kref_put(kref, drm_dp_free_mst_branch_device);
 }
 
-static void drm_dp_put_mst_branch_device(struct drm_dp_mst_branch *mstb)
+static void drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb)
 {
kref_put(&mstb->kref, drm_dp_destroy_mst_branch_device);
 }
@@ -930,7 +930,7 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port 
*port, int old_pdt)
case DP_PEER_DEVICE_MST_BRANCHING:
mstb = port->mstb;
port->mstb = NULL;
-   drm_dp_put_mst_branch_device(mstb);
+   drm_dp_mst_topology_put_mstb(mstb);
break;
}
 }
@@ -970,12 +970,14 @@ static void drm_dp_destroy_port(struct kref *kref)
kfree(port);
 }
 
-static void drm_dp_put_port(struct drm_dp_mst_port *port)
+static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port)
 {
kref_put(&port->kref, drm_dp_destroy_port);
 }
 
-static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct drm_dp_mst_branch *mstb, 
struct drm_dp_mst_branch *to_find)
+static struct drm_dp_mst_branch *
+drm_dp_mst_topology_get_mstb_validated_locked(struct drm_dp_mst_branch *mstb,
+ struct drm_dp_mst_branch *to_find)
 {
struct drm_dp_mst_port *port;
struct drm_dp_mst_branch *rmstb;
@@ -985,7 +987,8 @@ static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct
}
list_for_each_entry(port, &mstb->ports, next) {
if (port->mstb) {
-   rmstb = 
drm_dp_mst_get_validated_mstb_ref_locked(port->mstb, to_find);
+   rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
+   port->mstb, to_find);
if (rmstb)
return rmstb;
}
@@ -993,12 +996,15 @@ static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct
return NULL;
 }
 
-static struct drm_dp_mst_branch *drm_dp_get_validated_mstb_ref(struct 
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb)
+static struct drm_dp_mst_branch *
+drm_dp_mst_topology_get_mstb_validated(struct drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_mst_branch *mstb)
 {
struct drm_dp_mst_branch *rmstb = NULL;
mutex_lock(&mgr->lock);
if (mgr->mst_primary)
-   rmstb = 
drm_dp_mst_get_validated_mstb_ref_locked(mgr->mst_primary, mstb);
+   rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
+   mgr->mst_primary, mstb);
mutex_unlock(&mgr->lock);
return rmstb;
 }
@@ -1021,7 +1027,9 @@ static struct drm_dp_mst_port 
*drm_dp_mst_get_port_ref_locked(struct drm_dp_mst_
return NULL;
 }
 
-static struct drm_dp_mst_port *drm_dp_get_validated_port_ref(struct 
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
+static struct drm_dp_mst_port *
+drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *rport = NULL;
mutex_lock(&mgr->lock);
@@ -1210,7 +1218,7 @@ static void drm_dp_add_port(struct drm

[PATCH v3 09/16] drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()

[Lyude Paul]

There is no need to look at the port's VCPI allocation before calling
drm_dp_mst_deallocate_vcpi(), as we already have msto->disabled to let
us avoid cleaning up an msto more then once. The DP MST core will never
call drm_dp_mst_deallocate_vcpi() on it's own, which is presumably what
these checks are meant to protect against.

More importantly though, we're about to stop clearing mstc->port in the
next commit, which means if we could potentially hit a use-after-free
error if we tried to check mstc->port->vcpi here. So to make life easier
for anyone who bisects this code in the future, use msto->disabled
instead to check whether or not we need to deallocate VCPI instead.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 641252208e67..0f7d72518604 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -704,14 +704,17 @@ nv50_msto_cleanup(struct nv50_msto *msto)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
 
+   if (!msto->disabled)
+   return;
+
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
-   if (mstc->port && mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
+
+   if (mstc->port)
drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
-   if (msto->disabled) {
-   msto->mstc = NULL;
-   msto->head = NULL;
-   msto->disabled = false;
-   }
+
+   msto->mstc = NULL;
+   msto->head = NULL;
+   msto->disabled = false;
 }
 
 static void
-- 
2.20.1

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

[PATCH v3 13/16] drm/dp_mst: Add some atomic state iterator macros

[Lyude Paul]

Changes since v6:
 - Move EXPORT_SYMBOL() for drm_dp_mst_topology_state_funcs to this
   commit
 - Document __drm_dp_mst_state_iter_get() and note that it shouldn't be
   called directly

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c |  5 +-
 include/drm/drm_dp_mst_helper.h   | 96 +++
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 769e2c0419c2..f79962759bc4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3527,10 +3527,11 @@ static void drm_dp_mst_destroy_state(struct 
drm_private_obj *obj,
kfree(mst_state);
 }
 
-static const struct drm_private_state_funcs mst_state_funcs = {
+const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
.atomic_duplicate_state = drm_dp_mst_duplicate_state,
.atomic_destroy_state = drm_dp_mst_destroy_state,
 };
+EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
 
 /**
  * drm_atomic_get_mst_topology_state: get MST topology state
@@ -3614,7 +3615,7 @@ int drm_dp_mst_topology_mgr_init(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_atomic_private_obj_init(dev, &mgr->base,
&mst_state->base,
-   &mst_state_funcs);
+   &drm_dp_mst_topology_state_funcs);
 
return 0;
 }
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8eca5f29242c..581163c8d7d7 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -650,4 +650,100 @@ int drm_dp_send_power_updown_phy(struct 
drm_dp_mst_topology_mgr *mgr,
 void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port);
 void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);
 
+extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
+
+/**
+ * __drm_dp_mst_state_iter_get - private atomic state iterator function for
+ * macro-internal use
+ * @state: &struct drm_atomic_state pointer
+ * @mgr: pointer to the &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: optional pointer to the old &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @new_state: optional pointer to the new &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @i: int iteration cursor, for macro-internal use
+ *
+ * Used by for_each_oldnew_mst_mgr_in_state(),
+ * for_each_old_mst_mgr_in_state(), and for_each_new_mst_mgr_in_state(). Don't
+ * call this directly.
+ *
+ * Returns:
+ * True if the current &struct drm_private_obj is a &struct
+ * drm_dp_mst_topology_mgr, false otherwise.
+ */
+static inline bool
+__drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
+   struct drm_dp_mst_topology_mgr **mgr,
+   struct drm_dp_mst_topology_state **old_state,
+   struct drm_dp_mst_topology_state **new_state,
+   int i)
+{
+   struct __drm_private_objs_state *objs_state = &state->private_objs[i];
+
+   if (objs_state->ptr->funcs != &drm_dp_mst_topology_state_funcs)
+   return false;
+
+   *mgr = to_dp_mst_topology_mgr(objs_state->ptr);
+   if (old_state)
+   *old_state = to_dp_mst_topology_state(objs_state->old_state);
+   if (new_state)
+   *new_state = to_dp_mst_topology_state(objs_state->new_state);
+
+   return true;
+}
+
+/**
+ * for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology
+ * managers in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking both old and new state. This is useful in places where the state
+ * delta needs to be considered, for example in atomic check functions.
+ */
+#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state, 
__i) \
+   for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+   for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), 
&(old_state), &(new_state), (__i)))
+
+/**
+ * for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in

[PATCH v3 02/16] drm/dp_mst: Introduce new refcounting scheme for mstbs and ports

[Lyude Paul]

The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.

To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:

commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")

Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;

commit 263efde31f97 ("drm/dp/mst: Get validated port ref in 
drm_dp_update_payload_part1()")

But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.

Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:

commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, 
just ref")

But then that introduced use-after-free errors, so I quickly reverted
it:

commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during 
destruction, just ref"")

And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortuntely, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.

After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.

To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.

Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.

Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:

- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)

Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.

Changes since v2:
* Fix commit message - checkpatch
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
  into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
  the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
  docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
  and payloads - danvet
* Make suggested documentation cha

[PATCH v3 12/16] drm/nouveau: Grab payload lock in nv50_msto_payload()

[Lyude Paul]

Going through the currently programmed payloads isn't safe without
holding mgr->payload_lock, so actually do that and warn if anyone tries
calling nv50_msto_payload() in the future without grabbing the right
locks.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 157d208d37b5..67f7bf97e5d9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -680,6 +680,8 @@ nv50_msto_payload(struct nv50_msto *msto)
struct nv50_mstm *mstm = mstc->mstm;
int vcpi = mstc->port->vcpi.vcpi, i;
 
+   WARN_ON(!mutex_is_locked(&mstm->mgr.payload_lock));
+
NV_ATOMIC(drm, "%s: vcpi %d\n", msto->encoder.name, vcpi);
for (i = 0; i < mstm->mgr.max_payloads; i++) {
struct drm_dp_payload *payload = &mstm->mgr.payloads[i];
@@ -733,6 +735,8 @@ nv50_msto_prepare(struct nv50_msto *msto)
   (0x0100 << msto->head->base.index),
};
 
+   mutex_lock(&mstm->mgr.payload_lock);
+
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
@@ -748,7 +752,9 @@ nv50_msto_prepare(struct nv50_msto *msto)
  msto->encoder.name, msto->head->base.base.name,
  args.vcpi.start_slot, args.vcpi.num_slots,
  args.vcpi.pbn, args.vcpi.aligned_pbn);
+
nvif_mthd(&drm->display->disp.object, 0, &args, sizeof(args));
+   mutex_unlock(&mstm->mgr.payload_lock);
 }
 
 static int
-- 
2.20.1

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

[PATCH v3 08/16] drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()

[Lyude Paul]

Trying to destroy the connector using mstc->connector.funcs->destroy()
if connector initialization fails is wrong: there is no possible
codepath in nv50_mstc_new where nv50_mstm_add_connector() would return
<0 and mstc would be non-NULL.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 26af45785939..641252208e67 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1099,11 +1099,8 @@ nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr 
*mgr,
int ret;
 
ret = nv50_mstc_new(mstm, port, path, &mstc);
-   if (ret) {
-   if (mstc)
-   mstc->connector.funcs->destroy(&mstc->connector);
+   if (ret)
return NULL;
-   }
 
return &mstc->connector;
 }
-- 
2.20.1

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

[PATCH v3 07/16] drm/amdgpu/display: Keep malloc ref to MST port

[Lyude Paul]

Just like i915 and nouveau, it's a good idea for us to hold a malloc
reference to the port here so that we never pass a freed pointer to any
of the DP MST helper functions.

Also, we stop unsetting aconnector->port in
dm_dp_destroy_mst_connector(). There's literally no point to that
assignment that I can see anyway.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5e7ca1f3a8d1..24632727e127 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -191,6 +191,7 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
drm_encoder_cleanup(&amdgpu_encoder->base);
kfree(amdgpu_encoder);
drm_connector_cleanup(connector);
+   drm_dp_mst_put_port_malloc(amdgpu_dm_connector->port);
kfree(amdgpu_dm_connector);
 }
 
@@ -363,7 +364,9 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
amdgpu_dm_connector_funcs_reset(connector);
 
DRM_INFO("DM_MST: added connector: %p [id: %d] [master: %p]\n",
-   aconnector, connector->base.id, aconnector->mst_port);
+aconnector, connector->base.id, aconnector->mst_port);
+
+   drm_dp_mst_get_port_malloc(port);
 
DRM_DEBUG_KMS(":%d\n", connector->base.id);
 
@@ -379,12 +382,12 @@ static void dm_dp_destroy_mst_connector(struct 
drm_dp_mst_topology_mgr *mgr,
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
 
DRM_INFO("DM_MST: Disabling connector: %p [id: %d] [master: %p]\n",
-   aconnector, connector->base.id, 
aconnector->mst_port);
+aconnector, connector->base.id, aconnector->mst_port);
 
-   aconnector->port = NULL;
if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(connector, NULL);
-   dc_link_remove_remote_sink(aconnector->dc_link, 
aconnector->dc_sink);
+   dc_link_remove_remote_sink(aconnector->dc_link,
+  aconnector->dc_sink);
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
}
-- 
2.20.1

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

[PATCH v3 04/16] drm/dp_mst: Stop releasing VCPI when removing ports from topology

[Lyude Paul]

This has never actually worked, and isn't needed anyway: the driver's
always going to try to deallocate VCPI when it tears down the display
that the VCPI belongs to.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b8a47c795fa9..f10a7edb401e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1176,8 +1176,6 @@ static void drm_dp_destroy_port(struct kref *kref)
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
 
if (!port->input) {
-   port->vcpi.num_slots = 0;
-
kfree(port->cached_edid);
 
/*
@@ -3493,12 +3491,6 @@ static void drm_dp_destroy_connector_work(struct 
work_struct *work)
drm_dp_port_teardown_pdt(port, port->pdt);
port->pdt = DP_PEER_DEVICE_NONE;
 
-   if (!port->input && port->vcpi.vcpi > 0) {
-   drm_dp_mst_reset_vcpi_slots(mgr, port);
-   drm_dp_update_payload_part1(mgr);
-   drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
-   }
-
drm_dp_mst_put_port_malloc(port);
send_hotplug = true;
}
-- 
2.20.1

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

[PATCH v3 03/16] drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref fails

[Lyude Paul]

While this isn't a complete fix, this will improve the reliability of
drm_dp_get_last_connected_port_and_mstb() pretty significantly during
hotplug events, since there's a chance that the in-memory topology tree
may not be fully updated when drm_dp_get_last_connected_port_and_mstb()
is called and thus might end up causing our search to fail on an mstb
whose topology refcount has reached 0, but has not yet been removed from
it's parent.

Ideally, we should further fix this problem by ensuring that we deal
with the potential for racing with a hotplug event, which would look
like this:

* drm_dp_payload_send_msg() retrieves the last living relative of mstb
  with drm_dp_get_last_connected_port_and_mstb()
* drm_dp_payload_send_msg() starts building payload message
  At the same time, mstb gets unplugged from the topology and is no
  longer the actual last living relative of the original mstb
* drm_dp_payload_send_msg() tries sending the payload message, hub times
  out
* Hub timed out, we give up and run away-resulting in the payload being
  leaked

This could be fixed by restarting the
drm_dp_get_last_connected_port_and_mstb() search whenever we get a
timeout, sending the payload to the new mstb, then repeating until
either the entire topology is removed from the system or
drm_dp_get_last_connected_port_and_mstb() fails. But since the above
race condition is not terribly likely, we'll address that in a later
patch series once we've improved the recovery handling for VCPI
allocations in the rest of the DP MST helpers.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 54 ++-
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index c0dc20fbd55a..b8a47c795fa9 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2045,24 +2045,50 @@ static struct drm_dp_mst_port 
*drm_dp_get_last_connected_port_to_mstb(struct drm
return 
drm_dp_get_last_connected_port_to_mstb(mstb->port_parent->parent);
 }
 
-static struct drm_dp_mst_branch 
*drm_dp_get_last_connected_port_and_mstb(struct drm_dp_mst_topology_mgr *mgr,
-struct 
drm_dp_mst_branch *mstb,
-int 
*port_num)
+/**
+ * drm_dp_get_last_connected_port_and_mstb() - Find the last living relatives
+ * in a topology of a given branch device
+ * @mgr: The topology manager to use
+ * @mstb: The disconnected branch device
+ * @port_num: Where to store the number of the last connected port
+ *
+ * Searches upwards in the topology starting from @mstb to try to find the
+ * closest available parent of @mstb that's still connected to the rest of the
+ * topology. This can be used in order to perform operations like releasing
+ * payloads, where the branch device which owned the payload may no longer be
+ * around and thus would require that the payload on the last living relative
+ * be freed instead.
+ *
+ * Returns:
+ * The last connected &drm_dp_mst_branch in the topology that was a parent of
+ * @mstb, if there is one.
+ */
+static struct drm_dp_mst_branch *
+drm_dp_get_last_connected_port_and_mstb(struct drm_dp_mst_topology_mgr *mgr,
+   struct drm_dp_mst_branch *mstb,
+   int *port_num)
 {
struct drm_dp_mst_branch *rmstb = NULL;
struct drm_dp_mst_port *found_port;
+
mutex_lock(&mgr->lock);
-   if (mgr->mst_primary) {
+   if (!mgr->mst_primary)
+   goto out;
+
+   do {
found_port = drm_dp_get_last_connected_port_to_mstb(mstb);
+   if (!found_port)
+   break;
 
-   if (found_port) {
+   if (drm_dp_mst_topology_try_get_mstb(found_port->parent)) {
rmstb = found_port->parent;
-   if (drm_dp_mst_topology_try_get_mstb(rmstb))
-   *port_num = found_port->port_num;
-   else
-   rmstb = NULL;
+   *port_num = found_port->port_num;
+   } else {
+   /* Search again, starting from this parent */
+   mstb = found_port->parent;
}
-   }
+   } while (!rmstb);
+out:
mutex_unlock(&mgr->lock);
return rmstb;
 }
@@ -2111,6 +2137,14 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_dp_queue_down_tx(mgr, txmsg);
 
+   /*
+* FIXME: there is a small chance that between getting the last
+* connected mstb and sending the payload message, the last connected
+* mstb could also be removed from the topology. In th

[PATCH] drm/amdkfd: Allow building KFD on ARM64

[Kuehling, Felix]

ifdef x86_64 specific code.
Allow enabling CONFIG_HSA_AMD on ARM64.

CC: Mark Nutter 
Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdkfd/Kconfig| 4 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 8 
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 8 +---
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig 
b/drivers/gpu/drm/amd/amdkfd/Kconfig
index fbf0ee5..c361360 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -4,8 +4,8 @@
 
 config HSA_AMD
bool "HSA kernel driver for AMD GPU devices"
-   depends on DRM_AMDGPU && X86_64
-   imply AMD_IOMMU_V2
+   depends on DRM_AMDGPU && (X86_64 || ARM64)
+   imply AMD_IOMMU_V2 if X86_64
select MMU_NOTIFIER
help
  Enable this if you want to use HSA features on AMD GPU devices.
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index c02adbb..d7ddb35 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -863,6 +863,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, int 
*avail_size,
return 0;
 }
 
+#if CONFIG_X86_64
 static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
uint32_t *num_entries,
struct crat_subtype_iolink *sub_type_hdr)
@@ -905,6 +906,7 @@ static int kfd_fill_iolink_info_for_cpu(int numa_node_id, 
int *avail_size,
 
return 0;
 }
+#endif
 
 /* kfd_create_vcrat_image_cpu - Create Virtual CRAT for CPU
  *
@@ -920,7 +922,9 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
size_t *size)
struct crat_subtype_generic *sub_type_hdr;
int avail_size = *size;
int numa_node_id;
+#ifdef CONFIG_X86_64
uint32_t entries = 0;
+#endif
int ret = 0;
 
if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_CPU)
@@ -982,6 +986,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
size_t *size)
sub_type_hdr->length);
 
/* Fill in Subtype: IO Link */
+#ifdef CONFIG_X86_64
ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
&entries,
(struct crat_subtype_iolink *)sub_type_hdr);
@@ -992,6 +997,9 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, 
size_t *size)
 
sub_type_hdr = (typeof(sub_type_hdr))((char *)sub_type_hdr +
sub_type_hdr->length * entries);
+#else
+   pr_info("IO link not available for non x86 platforms\n");
+#endif
 
crat_table->num_domains++;
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 5f5b2ac..8afc592 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1400,9 +1400,11 @@ static int kfd_cpumask_to_apic_id(const struct cpumask 
*cpumask)
first_cpu_of_numa_node = cpumask_first(cpumask);
if (first_cpu_of_numa_node >= nr_cpu_ids)
return -1;
-   cpuinfo = &cpu_data(first_cpu_of_numa_node);
-
-   return cpuinfo->apicid;
+#ifdef CONFIG_X86_64
+   return cpu_data(first_cpu_of_numa_node).apicid;
+#else
+   return first_cpu_of_numa_node;
+#endif
 }
 
 /* kfd_numa_node_to_apic_id - Returns the APIC ID of the first logical 
processor
-- 
2.7.4

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

Re: [PATCH] drm/amd/display: Set requested plane state DCC params for GFX9

[Wentland, Harry]

On 2019-01-02 3:15 p.m., Nicholas Kazlauskas wrote:
> [Why]
> Hardware support for Delta Color Compression (DCC) decompression is
> available in DC for GFX9 but there's no way for userspace to enable
> the feature.
> 
> Enabling the feature can provide improved GFX performance and
> power savings in many situations.
> 
> [How]
> The GFX9 DCC parameters are passed to amdgpu_dm from AMDGPU via the
> amdgpu_bo tiling_flags. The plane capability is queried and the
> parameters are set accordingly.
> 
> The DCC address is given via a 256 byte aligned offset on the
> framebuffer address. The DCC address is updated whenever the buffer
> address changes.
> 
> Cc: Marek Olšák 
> Cc: Harry Wentland 
> Signed-off-by: Nicholas Kazlauskas 

Reviewed-by: Harry Wentland 

Harry

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 90 ++-
>  1 file changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 306540c36616..4839d2dc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2356,6 +2356,68 @@ static int get_fb_info(const struct amdgpu_framebuffer 
> *amdgpu_fb,
>   return r;
>  }
>  
> +static inline uint64_t get_dcc_address(uint64_t address, uint64_t 
> tiling_flags)
> +{
> + uint32_t offset = AMDGPU_TILING_GET(tiling_flags, DCC_OFFSET_256B);
> +
> + return offset ? (address + offset * 256) : 0;
> +}
> +
> +static bool fill_plane_dcc_attributes(struct amdgpu_device *adev,
> +   const struct amdgpu_framebuffer *afb,
> +   struct dc_plane_state *plane_state,
> +   uint64_t info)
> +{
> + struct dc *dc = adev->dm.dc;
> + struct dc_dcc_surface_param input = {0};
> + struct dc_surface_dcc_cap output = {0};
> + uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
> + uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
> + uint64_t dcc_address;
> +
> + if (!offset)
> + return false;
> +
> + if (!dc->cap_funcs.get_dcc_compression_cap)
> + return false;
> +
> + input.format = plane_state->format;
> + input.surface_size.width =
> + plane_state->plane_size.grph.surface_size.width;
> + input.surface_size.height =
> + plane_state->plane_size.grph.surface_size.height;
> + input.swizzle_mode = plane_state->tiling_info.gfx9.swizzle;
> +
> + if (plane_state->rotation == ROTATION_ANGLE_0 ||
> + plane_state->rotation == ROTATION_ANGLE_180)
> + input.scan = SCAN_DIRECTION_HORIZONTAL;
> + else if (plane_state->rotation == ROTATION_ANGLE_90 ||
> +  plane_state->rotation == ROTATION_ANGLE_270)
> + input.scan = SCAN_DIRECTION_VERTICAL;
> +
> + if (!dc->cap_funcs.get_dcc_compression_cap(dc, &input, &output))
> + return false;
> +
> + if (!output.capable)
> + return false;
> +
> + if (i64b == 0 && output.grph.rgb.independent_64b_blks != 0)
> + return false;
> +
> + plane_state->dcc.enable = 1;
> + plane_state->dcc.grph.meta_pitch =
> + AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1;
> + plane_state->dcc.grph.independent_64b_blks = i64b;
> +
> + dcc_address = get_dcc_address(afb->address, info);
> + plane_state->address.grph.meta_addr.low_part =
> + lower_32_bits(dcc_address);
> + plane_state->address.grph.meta_addr.high_part =
> + upper_32_bits(dcc_address);
> +
> + return true;
> +}
> +
>  static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
>struct dc_plane_state *plane_state,
>const struct amdgpu_framebuffer 
> *amdgpu_fb)
> @@ -2408,6 +2470,10 @@ static int fill_plane_attributes_from_fb(struct 
> amdgpu_device *adev,
>   return -EINVAL;
>   }
>  
> + memset(&plane_state->address, 0, sizeof(plane_state->address));
> + memset(&plane_state->tiling_info, 0, sizeof(plane_state->tiling_info));
> + memset(&plane_state->dcc, 0, sizeof(plane_state->dcc));
> +
>   if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
>   plane_state->address.type = PLN_ADDR_TYPE_GRAPHICS;
>   plane_state->plane_size.grph.surface_size.x = 0;
> @@ -2439,8 +2505,6 @@ static int fill_plane_attributes_from_fb(struct 
> amdgpu_device *adev,
>   plane_state->color_space = COLOR_SPACE_YCBCR709;
>   }
>  
> - memset(&plane_state->tiling_info, 0, sizeof(plane_state->tiling_info));
> -
>   /* Fill GFX8 params */
>   if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == 
> DC_ARRAY_2D_TILED_THIN1) {
>   unsigned int bankw, bankh, mtaspect, tile_split, num_banks;
> @@ -2489,6 +2553,9 @@ static

Re: [PATCH] drm/amd/display: Set requested plane state DCC params for GFX9

[Alex Deucher]

On Wed, Jan 2, 2019 at 3:15 PM Nicholas Kazlauskas
 wrote:
>
> [Why]
> Hardware support for Delta Color Compression (DCC) decompression is
> available in DC for GFX9 but there's no way for userspace to enable
> the feature.
>
> Enabling the feature can provide improved GFX performance and
> power savings in many situations.
>
> [How]
> The GFX9 DCC parameters are passed to amdgpu_dm from AMDGPU via the
> amdgpu_bo tiling_flags. The plane capability is queried and the
> parameters are set accordingly.
>
> The DCC address is given via a 256 byte aligned offset on the
> framebuffer address. The DCC address is updated whenever the buffer
> address changes.
>
> Cc: Marek Olšák 
> Cc: Harry Wentland 
> Signed-off-by: Nicholas Kazlauskas 

Acked-by: Alex Deucher 

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 90 ++-
>  1 file changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 306540c36616..4839d2dc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2356,6 +2356,68 @@ static int get_fb_info(const struct amdgpu_framebuffer 
> *amdgpu_fb,
> return r;
>  }
>
> +static inline uint64_t get_dcc_address(uint64_t address, uint64_t 
> tiling_flags)
> +{
> +   uint32_t offset = AMDGPU_TILING_GET(tiling_flags, DCC_OFFSET_256B);
> +
> +   return offset ? (address + offset * 256) : 0;
> +}
> +
> +static bool fill_plane_dcc_attributes(struct amdgpu_device *adev,
> + const struct amdgpu_framebuffer *afb,
> + struct dc_plane_state *plane_state,
> + uint64_t info)
> +{
> +   struct dc *dc = adev->dm.dc;
> +   struct dc_dcc_surface_param input = {0};
> +   struct dc_surface_dcc_cap output = {0};
> +   uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
> +   uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
> +   uint64_t dcc_address;
> +
> +   if (!offset)
> +   return false;
> +
> +   if (!dc->cap_funcs.get_dcc_compression_cap)
> +   return false;
> +
> +   input.format = plane_state->format;
> +   input.surface_size.width =
> +   plane_state->plane_size.grph.surface_size.width;
> +   input.surface_size.height =
> +   plane_state->plane_size.grph.surface_size.height;
> +   input.swizzle_mode = plane_state->tiling_info.gfx9.swizzle;
> +
> +   if (plane_state->rotation == ROTATION_ANGLE_0 ||
> +   plane_state->rotation == ROTATION_ANGLE_180)
> +   input.scan = SCAN_DIRECTION_HORIZONTAL;
> +   else if (plane_state->rotation == ROTATION_ANGLE_90 ||
> +plane_state->rotation == ROTATION_ANGLE_270)
> +   input.scan = SCAN_DIRECTION_VERTICAL;
> +
> +   if (!dc->cap_funcs.get_dcc_compression_cap(dc, &input, &output))
> +   return false;
> +
> +   if (!output.capable)
> +   return false;
> +
> +   if (i64b == 0 && output.grph.rgb.independent_64b_blks != 0)
> +   return false;
> +
> +   plane_state->dcc.enable = 1;
> +   plane_state->dcc.grph.meta_pitch =
> +   AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1;
> +   plane_state->dcc.grph.independent_64b_blks = i64b;
> +
> +   dcc_address = get_dcc_address(afb->address, info);
> +   plane_state->address.grph.meta_addr.low_part =
> +   lower_32_bits(dcc_address);
> +   plane_state->address.grph.meta_addr.high_part =
> +   upper_32_bits(dcc_address);
> +
> +   return true;
> +}
> +
>  static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
>  struct dc_plane_state *plane_state,
>  const struct amdgpu_framebuffer 
> *amdgpu_fb)
> @@ -2408,6 +2470,10 @@ static int fill_plane_attributes_from_fb(struct 
> amdgpu_device *adev,
> return -EINVAL;
> }
>
> +   memset(&plane_state->address, 0, sizeof(plane_state->address));
> +   memset(&plane_state->tiling_info, 0, 
> sizeof(plane_state->tiling_info));
> +   memset(&plane_state->dcc, 0, sizeof(plane_state->dcc));
> +
> if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
> plane_state->address.type = PLN_ADDR_TYPE_GRAPHICS;
> plane_state->plane_size.grph.surface_size.x = 0;
> @@ -2439,8 +2505,6 @@ static int fill_plane_attributes_from_fb(struct 
> amdgpu_device *adev,
> plane_state->color_space = COLOR_SPACE_YCBCR709;
> }
>
> -   memset(&plane_state->tiling_info, 0, 
> sizeof(plane_state->tiling_info));
> -
> /* Fill GFX8 params */
> if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == 
> DC_ARRAY_2D_TILED_THIN1

Re: [PATCH 2/2] drm/amdgpu: set the executable flag on unused Vega10 PTEs

[Kuehling, Felix]

Don't you need to update amdgpu_vm_clear_freed as well?

Regards,
  Felix

On 2019-01-02 8:35 a.m., Christian König wrote:
> Otherwise we run into a non-retry fault on access.
>
> It seems to be a hardware bug that the executable bit has
> higher priority than the valid bit.
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e73d152659a2..01a68f4e17d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -799,9 +799,15 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   addr += ats_entries * 8;
>   }
>  
> - if (entries)
> + if (entries) {
> + uint64_t value = 0;
> +
> + if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10)
> + value = AMDGPU_PTE_EXECUTABLE;
> +
>   amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
> -   entries, 0, 0);
> +   entries, 0, value);
> + }
>  
>   amdgpu_ring_pad_ib(ring, &job->ibs[0]);
>  
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/display: Set requested plane state DCC params for GFX9

[Nicholas Kazlauskas]

[Why]
Hardware support for Delta Color Compression (DCC) decompression is
available in DC for GFX9 but there's no way for userspace to enable
the feature.

Enabling the feature can provide improved GFX performance and
power savings in many situations.

[How]
The GFX9 DCC parameters are passed to amdgpu_dm from AMDGPU via the
amdgpu_bo tiling_flags. The plane capability is queried and the
parameters are set accordingly.

The DCC address is given via a 256 byte aligned offset on the
framebuffer address. The DCC address is updated whenever the buffer
address changes.

Cc: Marek Olšák 
Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 90 ++-
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 306540c36616..4839d2dc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2356,6 +2356,68 @@ static int get_fb_info(const struct amdgpu_framebuffer 
*amdgpu_fb,
return r;
 }
 
+static inline uint64_t get_dcc_address(uint64_t address, uint64_t tiling_flags)
+{
+   uint32_t offset = AMDGPU_TILING_GET(tiling_flags, DCC_OFFSET_256B);
+
+   return offset ? (address + offset * 256) : 0;
+}
+
+static bool fill_plane_dcc_attributes(struct amdgpu_device *adev,
+ const struct amdgpu_framebuffer *afb,
+ struct dc_plane_state *plane_state,
+ uint64_t info)
+{
+   struct dc *dc = adev->dm.dc;
+   struct dc_dcc_surface_param input = {0};
+   struct dc_surface_dcc_cap output = {0};
+   uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
+   uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
+   uint64_t dcc_address;
+
+   if (!offset)
+   return false;
+
+   if (!dc->cap_funcs.get_dcc_compression_cap)
+   return false;
+
+   input.format = plane_state->format;
+   input.surface_size.width =
+   plane_state->plane_size.grph.surface_size.width;
+   input.surface_size.height =
+   plane_state->plane_size.grph.surface_size.height;
+   input.swizzle_mode = plane_state->tiling_info.gfx9.swizzle;
+
+   if (plane_state->rotation == ROTATION_ANGLE_0 ||
+   plane_state->rotation == ROTATION_ANGLE_180)
+   input.scan = SCAN_DIRECTION_HORIZONTAL;
+   else if (plane_state->rotation == ROTATION_ANGLE_90 ||
+plane_state->rotation == ROTATION_ANGLE_270)
+   input.scan = SCAN_DIRECTION_VERTICAL;
+
+   if (!dc->cap_funcs.get_dcc_compression_cap(dc, &input, &output))
+   return false;
+
+   if (!output.capable)
+   return false;
+
+   if (i64b == 0 && output.grph.rgb.independent_64b_blks != 0)
+   return false;
+
+   plane_state->dcc.enable = 1;
+   plane_state->dcc.grph.meta_pitch =
+   AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1;
+   plane_state->dcc.grph.independent_64b_blks = i64b;
+
+   dcc_address = get_dcc_address(afb->address, info);
+   plane_state->address.grph.meta_addr.low_part =
+   lower_32_bits(dcc_address);
+   plane_state->address.grph.meta_addr.high_part =
+   upper_32_bits(dcc_address);
+
+   return true;
+}
+
 static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
 struct dc_plane_state *plane_state,
 const struct amdgpu_framebuffer 
*amdgpu_fb)
@@ -2408,6 +2470,10 @@ static int fill_plane_attributes_from_fb(struct 
amdgpu_device *adev,
return -EINVAL;
}
 
+   memset(&plane_state->address, 0, sizeof(plane_state->address));
+   memset(&plane_state->tiling_info, 0, sizeof(plane_state->tiling_info));
+   memset(&plane_state->dcc, 0, sizeof(plane_state->dcc));
+
if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
plane_state->address.type = PLN_ADDR_TYPE_GRAPHICS;
plane_state->plane_size.grph.surface_size.x = 0;
@@ -2439,8 +2505,6 @@ static int fill_plane_attributes_from_fb(struct 
amdgpu_device *adev,
plane_state->color_space = COLOR_SPACE_YCBCR709;
}
 
-   memset(&plane_state->tiling_info, 0, sizeof(plane_state->tiling_info));
-
/* Fill GFX8 params */
if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == 
DC_ARRAY_2D_TILED_THIN1) {
unsigned int bankw, bankh, mtaspect, tile_split, num_banks;
@@ -2489,6 +2553,9 @@ static int fill_plane_attributes_from_fb(struct 
amdgpu_device *adev,
plane_state->tiling_info.gfx9.swizzle =
AMDGPU_TILING_GET(tiling_flags, SWIZZLE_MODE);
plane_state->t

Re: [PATCH 3/3] drm/amdgpu: replace get_user_pages with HMM address mirror helpers v5

[Yang, Philip]

Hi Christian,

May you help review the CS parts related changes? I tested it using 
libdrm Userptr Test under X. Do you know other test applications which 
can stress the CS userptr path?

Thanks,
Philip

On 2018-12-14 4:25 p.m., Kuehling, Felix wrote:
> Except for the GEM and CS parts, the series is Reviewed-by: Felix
> Kuehling 
> 
> Regards,
>    Felix
> 
> On 2018-12-14 4:10 p.m., Yang, Philip wrote:
>> Use HMM helper function hmm_vma_fault() to get physical pages backing
>> userptr and start CPU page table update track of those pages. Then use
>> hmm_vma_range_done() to check if those pages are updated before
>> amdgpu_cs_submit for gfx or before user queues are resumed for kfd.
>>
>> If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart
>> from scratch, for kfd, restore worker is rescheduled to retry.
>>
>> HMM simplify the CPU page table concurrent update check, so remove
>> guptasklock, mmu_invalidations, last_set_pages fields from
>> amdgpu_ttm_tt struct.
>>
>> HMM does not pin the page (increase page ref count), so remove related
>> operations like release_pages(), put_page(), mark_page_dirty().
>>
>> Change-Id: I2e8c0c6f0d8c21e5596a32d7fc91762778bc9e67
>> Signed-off-by: Philip Yang 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|   1 -
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  95 +++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c   |   2 -
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h   |   3 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 185 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  14 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c|  25 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h|   4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 168 ++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h   |   4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|   1 -
>>   11 files changed, 209 insertions(+), 293 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> index 70429f7aa9a8..717791d4fa45 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> @@ -62,7 +62,6 @@ struct kgd_mem {
>>   
>>  atomic_t invalid;
>>  struct amdkfd_process_info *process_info;
>> -struct page **user_pages;
>>   
>>  struct amdgpu_sync sync;
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index be1ab43473c6..2897793600f7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -582,28 +582,12 @@ static int init_user_pages(struct kgd_mem *mem, struct 
>> mm_struct *mm,
>>  goto out;
>>  }
>>   
>> -/* If no restore worker is running concurrently, user_pages
>> - * should not be allocated
>> - */
>> -WARN(mem->user_pages, "Leaking user_pages array");
>> -
>> -mem->user_pages = kvmalloc_array(bo->tbo.ttm->num_pages,
>> -   sizeof(struct page *),
>> -   GFP_KERNEL | __GFP_ZERO);
>> -if (!mem->user_pages) {
>> -pr_err("%s: Failed to allocate pages array\n", __func__);
>> -ret = -ENOMEM;
>> -goto unregister_out;
>> -}
>> -
>> -ret = amdgpu_ttm_tt_get_user_pages(bo->tbo.ttm, mem->user_pages);
>> +ret = amdgpu_ttm_tt_get_user_pages(bo->tbo.ttm, bo->tbo.ttm->pages);
>>  if (ret) {
>>  pr_err("%s: Failed to get user pages: %d\n", __func__, ret);
>> -goto free_out;
>> +goto unregister_out;
>>  }
>>   
>> -amdgpu_ttm_tt_set_user_pages(bo->tbo.ttm, mem->user_pages);
>> -
>>  ret = amdgpu_bo_reserve(bo, true);
>>  if (ret) {
>>  pr_err("%s: Failed to reserve BO\n", __func__);
>> @@ -616,11 +600,7 @@ static int init_user_pages(struct kgd_mem *mem, struct 
>> mm_struct *mm,
>>  amdgpu_bo_unreserve(bo);
>>   
>>   release_out:
>> -if (ret)
>> -release_pages(mem->user_pages, bo->tbo.ttm->num_pages);
>> -free_out:
>> -kvfree(mem->user_pages);
>> -mem->user_pages = NULL;
>> +amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
>>   unregister_out:
>>  if (ret)
>>  amdgpu_mn_unregister(bo);
>> @@ -679,7 +659,6 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
>>  ctx->kfd_bo.priority = 0;
>>  ctx->kfd_bo.tv.bo = &bo->tbo;
>>  ctx->kfd_bo.tv.num_shared = 1;
>> -ctx->kfd_bo.user_pages = NULL;
>>  list_add(&ctx->kfd_bo.tv.head, &ctx->list);
>>   
>>  amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]);
>> @@ -743,7 +722,6 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
>>  ctx->kfd_bo.priority = 0;
>>  ctx->kfd_bo.tv.bo = &bo->tbo;
>>  ctx->kfd_bo.tv.num_shared = 1;
>> -ctx->kfd_bo.user_pages = NULL;
>>

Re: [PATCH 2/2] drm/amdgpu: set the executable flag on unused Vega10 PTEs

[Deucher, Alexander]

Series is:

Acked-by: Alex Deucher 


From: amd-gfx  on behalf of Christian 
König 
Sent: Wednesday, January 2, 2019 8:35:06 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH 2/2] drm/amdgpu: set the executable flag on unused Vega10 PTEs

Otherwise we run into a non-retry fault on access.

It seems to be a hardware bug that the executable bit has
higher priority than the valid bit.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e73d152659a2..01a68f4e17d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -799,9 +799,15 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
 addr += ats_entries * 8;
 }

-   if (entries)
+   if (entries) {
+   uint64_t value = 0;
+
+   if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10)
+   value = AMDGPU_PTE_EXECUTABLE;
+
 amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
- entries, 0, 0);
+ entries, 0, value);
+   }

 amdgpu_ring_pad_ib(ring, &job->ibs[0]);

--
2.17.1

___
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: distinguish early and late re-init log in sriov

[Deucher, Alexander]

Acked-by: Alex Deucher 


From: amd-gfx  on behalf of wentalou 

Sent: Wednesday, January 2, 2019 3:03:56 AM
To: amd-gfx@lists.freedesktop.org
Cc: Lou, Wentao
Subject: [PATCH] drm/amdgpu: distinguish early and late re-init log in sriov

distinguish ip_reinit_early_sriov and ip_reinit_late_sriov
by different log RE-INIT-early and RE-INIT-late

Change-Id: If4dd78cb807790e9f8daffb04d893cc7fd2b0e60
Signed-off-by: Wentao Lou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7ff3a28..03b73c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2133,7 +2133,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct 
amdgpu_device *adev)
 continue;

 r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
+   DRM_INFO("RE-INIT-early: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
 if (r)
 return r;
 }
@@ -2167,7 +2167,7 @@ static int amdgpu_device_ip_reinit_late_sriov(struct 
amdgpu_device *adev)
 continue;

 r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
+   DRM_INFO("RE-INIT-late: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
 if (r)
 return r;
 }
--
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: make gfx9 enter into rlc safe mode when set MGCG

[Huang, Ray]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of likun Gao
> Sent: Wednesday, January 02, 2019 4:57 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gao, Likun 
> Subject: [PATCH] drm/amdgpu: make gfx9 enter into rlc safe mode when set
> MGCG
> 
> From: Likun Gao 
> 
> MGCG should RLC enter into safe mode first.
> 
> Signed-off-by: Likun Gao 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 7556716..968b127 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3587,6 +3587,8 @@ static void
> gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> {
>   uint32_t data, def;
> 
> + amdgpu_gfx_rlc_enter_safe_mode(adev);
> +
>   /* It is disabled by HW by default */
>   if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
>   /* 1 - RLC_CGTT_MGCG_OVERRIDE */
> @@ -3651,6 +3653,8 @@ static void
> gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
>   WREG32_SOC15(GC, 0, mmCP_MEM_SLP_CNTL,
> data);
>   }
>   }
> +
> + amdgpu_gfx_rlc_exit_safe_mode(adev);
>  }
> 
>  static void gfx_v9_0_update_3d_clock_gating(struct amdgpu_device *adev,
> --
> 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 1/3] drm/amdgpu: Relocate kgd2kfd function declaration

[Christian König]

Am 14.12.18 um 16:22 schrieb Lin, Amber:

Since amdkfd is merged into amdgpu module and amdgpu can access amdkfd
directly, move declaration of kgd2kfd functions from kfd_priv.h to
amdgpu_amdkfd.h

Signed-off-by: Amber Lin 


Yes, please. If it's not committed yet Acked-by: Christian König 
 for the series.



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 43 
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h   | 20 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  2 ++
  drivers/gpu/drm/amd/amdkfd/kfd_module.c  |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h| 22 
  drivers/gpu/drm/amd/include/kgd_kfd_interface.h  |  3 --
  7 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 2dfaf15..358f690 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -619,4 +619,47 @@ struct kfd2kgd_calls 
*amdgpu_amdkfd_gfx_9_0_get_functions(void)
  {
return NULL;
  }
+
+struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd, struct pci_dev *pdev,
+ const struct kfd2kgd_calls *f2g)
+{
+   return NULL;
+}
+
+bool kgd2kfd_device_init(struct kfd_dev *kfd,
+const struct kgd2kfd_shared_resources *gpu_resources)
+{
+   return false;
+}
+
+void kgd2kfd_device_exit(struct kfd_dev *kfd)
+{
+}
+
+void kgd2kfd_exit(void)
+{
+}
+
+void kgd2kfd_suspend(struct kfd_dev *kfd)
+{
+}
+
+int kgd2kfd_resume(struct kfd_dev *kfd)
+{
+   return 0;
+}
+
+int kgd2kfd_pre_reset(struct kfd_dev *kfd)
+{
+   return 0;
+}
+
+int kgd2kfd_post_reset(struct kfd_dev *kfd)
+{
+   return 0;
+}
+
+void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
+{
+}
  #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 70429f7..3214d31 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -33,7 +33,6 @@
  #include "amdgpu_sync.h"
  #include "amdgpu_vm.h"
  
-extern const struct kgd2kfd_calls *kgd2kfd;

  extern uint64_t amdgpu_amdkfd_total_mem_size;
  
  struct amdgpu_device;

@@ -214,4 +213,23 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
  void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
  void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo);
  
+/* KGD2KFD callbacks */

+int kgd2kfd_init(unsigned interface_version,
+const struct kgd2kfd_calls **g2f);
+void kgd2kfd_exit(void);
+struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd, struct pci_dev *pdev,
+ const struct kfd2kgd_calls *f2g);
+bool kgd2kfd_device_init(struct kfd_dev *kfd,
+const struct kgd2kfd_shared_resources *gpu_resources);
+void kgd2kfd_device_exit(struct kfd_dev *kfd);
+void kgd2kfd_suspend(struct kfd_dev *kfd);
+int kgd2kfd_resume(struct kfd_dev *kfd);
+int kgd2kfd_pre_reset(struct kfd_dev *kfd);
+int kgd2kfd_post_reset(struct kfd_dev *kfd);
+void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry);
+int kgd2kfd_quiesce_mm(struct mm_struct *mm);
+int kgd2kfd_resume_mm(struct mm_struct *mm);
+int kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm,
+  struct dma_fence *fence);
+
  #endif /* AMDGPU_AMDKFD_H_INCLUDED */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
index 574c118..3c7055e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
@@ -31,6 +31,7 @@
  
  static const struct dma_fence_ops amdkfd_fence_ops;

  static atomic_t fence_seq = ATOMIC_INIT(0);
+extern const struct kgd2kfd_calls *kgd2kfd;
  
  /* Eviction Fence

   * Fence helper functions to deal with KFD memory eviction.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index be1ab43..3fc2618 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -44,6 +44,8 @@
   */
  #define AMDGPU_USERPTR_RESTORE_DELAY_MS 1
  
+extern const struct kgd2kfd_calls *kgd2kfd;

+
  /* Impose limit on how much memory KFD can use */
  static struct {
uint64_t max_system_mem_limit;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
index 8018163..030b39d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include "kfd_priv.h"
+#include "amdgpu_amdkfd.h"
  
  static const struct kgd2kfd_calls kgd2kfd = {

.exit   = kgd2kfd_exit,
@@ -104,7 +105,6 @@ int kgd2kfd_init(unsigned int interface_vers

[PATCH 2/2] drm/amdgpu: set the executable flag on unused Vega10 PTEs

[Christian König]

Otherwise we run into a non-retry fault on access.

It seems to be a hardware bug that the executable bit has
higher priority than the valid bit.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e73d152659a2..01a68f4e17d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -799,9 +799,15 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
addr += ats_entries * 8;
}
 
-   if (entries)
+   if (entries) {
+   uint64_t value = 0;
+
+   if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10)
+   value = AMDGPU_PTE_EXECUTABLE;
+
amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
- entries, 0, 0);
+ entries, 0, value);
+   }
 
amdgpu_ring_pad_ib(ring, &job->ibs[0]);
 
-- 
2.17.1

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

[PATCH 1/2] drm/amdgpu: improve GMC v9 page fault message

[Christian König]

Note if this is a retry fault or not and cleanup the message a bit.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 521ac8954652..9c082f9aea1a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -305,6 +305,7 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
struct amdgpu_iv_entry *entry)
 {
struct amdgpu_vmhub *hub = &adev->vmhub[entry->vmid_src];
+   bool retry_fault = !!(entry->src_data[1] & 0x80);
uint32_t status = 0;
u64 addr;
 
@@ -326,8 +327,10 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);
 
dev_err(adev->dev,
-   "[%s] VMC page fault (src_id:%u ring:%u vmid:%u 
pasid:%u, for process %s pid %d thread %s pid %d)\n",
+   "[%s] %s page fault (src_id:%u ring:%u vmid:%u "
+   "pasid:%u, for process %s pid %d thread %s pid %d)\n",
entry->vmid_src ? "mmhub" : "gfxhub",
+   retry_fault ? "retry" : "no-retry",
entry->src_id, entry->ring_id, entry->vmid,
entry->pasid, task_info.process_name, task_info.tgid,
task_info.task_name, task_info.pid);
-- 
2.17.1

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

RE: [PATCH 0/9] PSP cleanup

[Zhang, Hawking]

Yep, We are in progress to move IP specific code into common interfaces. For 
instance, psp_vXX_get_fw_type/psp_vXX_prep_cmd_buf should be common interfaces, 
instead of IP specific ones. We’ll see that soon.

And I agree with you that we should stick with the existing naming style. And 
that’s why I have concern we created some unnecessary files like 
psp_asd.c/psp_tmr.c/psp_xgmi.c/psp_cmn.c.etc. All of these are just play with 
common psp gfx cmd. The interfaces are limited and actually do similar jobs.

For ASD, it is common for all the ASICs since from vega10. Although not all the 
ASICs really used ASD fw, the fact is we’ve already upstreamed ASD fw for vega 
series and onwards. Therefore, below interfaces seems unnecessary.
+psp_load_asd(psp) \
+   ((psp)->funcs->asd_load ? (psp)->funcs->asd_load((psp)) : 0) 
#define
+psp_unload_asd(psp) \
+   ((psp)->funcs->asd_unload ? (psp)->funcs->asd_unload((psp)) : 0)
+#define psp_destory_asd(psp) \
+   ((psp)->funcs->asd_destory ? (psp)->funcs->asd_destory((psp)) : 
0)

Similar case for TMR. Therefore below interfaces seems unnecessary.
+#define psp_init_tmr(psp) \
+   ((psp)->funcs->tmr_init ? (psp)->funcs->tmr_init((psp)) : 0) 
#define
+psp_load_tmr(psp) \
+   ((psp)->funcs->tmr_load ? (psp)->funcs->tmr_load((psp)) : 0) 
#define
+psp_unload_tmr(psp) \
+   ((psp)->funcs->tmr_unload ? (psp)->funcs->tmr_unload((psp)) : 0)
+#define psp_destory_tmr(psp) \
+   ((psp)->funcs->tmr_destory ? (psp)->funcs->tmr_destory((psp)) : 
0)

For XGMI, we can use either the “supported” flags in amdgpu_xgmi or 
num_physical_nodes to decide whether to load the TA or not. Therefore, below 
interfaces seems unnecessary.
+#define psp_init_xgmi(psp) \
+   ((psp)->funcs->xgmi_init ? (psp)->funcs->xgmi_init((psp)) : 0)
+#define psp_load_xgmi(psp) \
+   ((psp)->funcs->xgmi_load ? (psp)->funcs->xgmi_load((psp)) : 0)
+#define psp_unload_xgmi(psp) \
+   ((psp)->funcs->xgmi_unload ? (psp)->funcs->xgmi_unload((psp)) : 
0)
+#define psp_destory_xgmi(psp) \
+   ((psp)->funcs->xgmi_destory ? (psp)->funcs->xgmi_destory((psp)) 
: 0)

For the upcoming functionalities that need to play with TA ucode, they share 
exactly the same TA cmd with xgmi. Do we really need to separate them into 
different psp_xxx source files?

In sum, I’d prefer to stick with the old structures: common interfaces in 
amdgpu_psp.c/.h, and IP specific ones in IP specific file.

Regards,
Hawking
From: Christian König 
Sent: 2019年1月2日 20:00
To: Zhang, Hawking ; Quan, Evan ; 
amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Xu, Feifei 
; Huang, Ray 
Subject: Re: [PATCH 0/9] PSP cleanup


I'd prefer to keep the old structures: common interfaces in amdgpu_psp.c/.h, 
and IP specific ones in IP specific file.
That works for me as well.

Key take away from the change overview is this:

>   drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
>   drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
That looks like we can move a good bunch of the per IP specific code into the 
common interface. And that is something I really like to see.

No strong opinion if the common code should go into amdgpu_psp.c/h, 
amdgpu_xgmi.c/h or amdgpu_psp_xgmi.c/h.

The only restriction I have is that we should just stick with the existing 
naming convention.

Christian.

Am 02.01.19 um 11:21 schrieb Zhang, Hawking:

I'd prefer to keep the old structures: common interfaces in amdgpu_psp.c/.h, 
and IP specific ones in IP specific file.

No matter it's something related to ASD,TMR, or XGMI.etc, all of these are just 
communication/handshake jobs between driver and psp fw. Driver plays messenger 
role with several psp cmd that are shared among ASIC generations. a unified 
amdgpu_psp.c file is good enough to hold all the common stuffs.

Regards,
Hawking
[X]
From: Christian König 

Sent: Wednesday, January 2, 2019 6:01:56 PM
To: Quan, Evan; 
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander; Xu, Feifei; Huang, Ray; Zhang, Hawking
Subject: Re: [PATCH 0/9] PSP cleanup

The general idea looks good, but can we change the file and symbol
naming a bit?

So far we have named all non-ip version related functions amdgpu_* and
ip related functions ip_version.

E.g. following this xgmi functions should go into amdgpu_xgmi.c and not
psp_xgmi.c

Christian.

Am 02.01.19 um 10:21 schrieb Evan Quan:
> *** BLURB HERE ***
>
> Evan Quan (9):
>drm/amdgpu: separate the PSP ring related APIs
>drm/amdgpu: separate commonly used PSP APIs
>drm/amdgpu: separate the xgmi related APIs
>drm/amdgpu: separate the tmr related APIs
>drm/amdgpu: separate the asd related APIs
>drm/amdgpu: drop useless PSP APIs and structures
>drm/amd

Re: Regression with kernel 4.20 on armhf

[Christian König]

Hi Luis,

mhm, sounds like a timing issue. We have probably made something faster 
during bootup in 4.20 and because of this you now see this issue more often.


If the bisection doesn't show any result can you try adding some 
msleep(10) call at critical places in the driver code to narrow this down?


Officially we don't test/support ARM with the driver code, but in this 
particular case we should probably investigate since it sounds like it 
just doesn't happen on x86 because of different timing.


Thanks,
Christian.

Am 28.12.18 um 15:05 schrieb Luís Mendes:

Hi Alex,

Before all... Have a nice holidays! Happy new year!!

- Okay, so it looks like sometimes the driver is able to enter
graphical mode with the Polaris card, but most of the time it fails
before with:
[   49.762704] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout, signaled seq=2, emitted seq=3

- This is something that is happening sporadically but in a less
intensive way in 4.17, 4.18 and 4.19 kernels, so this is actually not
a regression, but rather an existent issue, which maybe the patch
"drm/amdgpu/gfx_v8_0: Reorder the gfx, kiq and kcq ring tests
sequence" solves. I tried to backport it to 4.20, but had no
improvement. Need to try with the git version, or rc1.

- This hang happens after the console is displayed in the screen, but
before switching to graphical mode with X.

- However if X is entered then the driver is stable and can be used
for long periods.

Regards,
Luís Mendes

On Tue, Dec 18, 2018 at 11:16 PM Luís Mendes  wrote:

Hi Alex,

I am already using drm_arch_can_wc_memory() set to false.
I will try to bisect...

Regards,
Luís

On Tue, Dec 18, 2018 at 7:03 PM Alex Deucher  wrote:

On Tue, Dec 18, 2018 at 8:58 AM Luís Mendes  wrote:

Hi Christian,

I've been using a Sapphire RX 550 and a Sapphire RX 460 on a custom
armhf board that runs well with Linux 4.19.9 at least, but now
starting with Linux kernel 4.20, I'm having a gpu hang, right after
the console being displayed, but before entering in graphical mode,
when starting X session.
I'm only reporting this now, because there was a PCI commit for mvebu
that also entered for linux-4.20 that caused a kernel oops during
pci_map_rom call in amdgpu initialization code. I've reverted that
patch, but now amdgpu is hanging.

It would be useful if you could bisect.  This is the first I've heard
of amdgpu working on an ARM board without write combining (WC)
disabled.  You might check to see if disabling WC helps.  Return false
in drm_arch_can_wc_memory().

Alex



[   24.801861] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout, signaled seq=2, emitted seq=3

02:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Baffin [Polaris11] (rev ff) (prog-if 00 [VGA controller])
 Subsystem: Sapphire Technology Limited Baffin [Radeon RX 560]
 Flags: bus master, fast devsel, latency 0, IRQ 51
 Memory at d000 (64-bit, prefetchable) [size=256M]
 Memory at e000 (64-bit, prefetchable) [size=2M]
 I/O ports at 1 [size=256]
 Memory at e020 (32-bit, non-prefetchable) [size=256K]
 Expansion ROM at e024 [disabled] [size=128K]
 Capabilities: 
 Kernel driver in use: amdgpu
 Kernel modules: amdgpu

dmesg follows in attachment.

Regards,
Luís
___
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


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

Re: [PATCH 0/9] PSP cleanup

[Christian König]

I'd prefer to keep the old structures: common interfaces in 
amdgpu_psp.c/.h, and IP specific ones in IP specific file.



That works for me as well.

Key take away from the change overview is this:

>   drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
>   drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
That looks like we can move a good bunch of the per IP specific code 
into the common interface. And that is something I really like to see.


No strong opinion if the common code should go into amdgpu_psp.c/h, 
amdgpu_xgmi.c/h or amdgpu_psp_xgmi.c/h.


The only restriction I have is that we should just stick with the 
existing naming convention.


Christian.

Am 02.01.19 um 11:21 schrieb Zhang, Hawking:


I'd prefer to keep the old structures: common interfaces in 
amdgpu_psp.c/.h, and IP specific ones in IP specific file.



No matter it's something related to ASD,TMR, or XGMI.etc, all of these 
are just communication/handshake jobs between driver and psp fw. 
Driver plays messenger role with several psp cmd that are shared among 
ASIC generations. a unified amdgpu_psp.c file is good enough to hold 
all the common stuffs.



Regards,
Hawking


*From:* Christian König 
*Sent:* Wednesday, January 2, 2019 6:01:56 PM
*To:* Quan, Evan; amd-gfx@lists.freedesktop.org
*Cc:* Deucher, Alexander; Xu, Feifei; Huang, Ray; Zhang, Hawking
*Subject:* Re: [PATCH 0/9] PSP cleanup
The general idea looks good, but can we change the file and symbol
naming a bit?

So far we have named all non-ip version related functions amdgpu_* and
ip related functions ip_version.

E.g. following this xgmi functions should go into amdgpu_xgmi.c and not
psp_xgmi.c

Christian.

Am 02.01.19 um 10:21 schrieb Evan Quan:
> *** BLURB HERE ***
>
> Evan Quan (9):
>    drm/amdgpu: separate the PSP ring related APIs
>    drm/amdgpu: separate commonly used PSP APIs
>    drm/amdgpu: separate the xgmi related APIs
>    drm/amdgpu: separate the tmr related APIs
>    drm/amdgpu: separate the asd related APIs
>    drm/amdgpu: drop useless PSP APIs and structures
>    drm/amdgpu: check PSP support before adding the ip block
>    drm/amdgpu: make PSP sub modules(ASD/XGMI/TMR) support configurable
>    drm/amdgpu: move psp_funcs related to a more proper place
>
>   drivers/gpu/drm/amd/amdgpu/Makefile |   7 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 504 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  93 +---
>   drivers/gpu/drm/amd/amdgpu/psp_asd.c    |  86 
>   drivers/gpu/drm/amd/amdgpu/psp_asd.h    |  32 ++
>   drivers/gpu/drm/amd/amdgpu/psp_cmn.c    | 289 +
>   drivers/gpu/drm/amd/amdgpu/psp_cmn.h    |  84 
>   drivers/gpu/drm/amd/amdgpu/psp_funcs.h  |  98 +
>   drivers/gpu/drm/amd/amdgpu/psp_ring.c   | 354 
>   drivers/gpu/drm/amd/amdgpu/psp_ring.h   |  43 ++
>   drivers/gpu/drm/amd/amdgpu/psp_tmr.c    |  84 
>   drivers/gpu/drm/amd/amdgpu/psp_tmr.h    |  32 ++
>   drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
>   drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
>   drivers/gpu/drm/amd/amdgpu/psp_xgmi.c   | 207 +
>   drivers/gpu/drm/amd/amdgpu/psp_xgmi.h   |  33 ++
>   drivers/gpu/drm/amd/amdgpu/soc15.c  |  13 +-
>   18 files changed, 1493 insertions(+), 1866 deletions(-)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_funcs.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h
>


___
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 0/9] PSP cleanup

[Zhang, Hawking]

I'd prefer to keep the old structures: common interfaces in amdgpu_psp.c/.h, 
and IP specific ones in IP specific file.


No matter it's something related to ASD,TMR, or XGMI.etc, all of these are just 
communication/handshake jobs between driver and psp fw. Driver plays messenger 
role with several psp cmd that are shared among ASIC generations. a unified 
amdgpu_psp.c file is good enough to hold all the common stuffs.


Regards,
Hawking


From: Christian K?nig 
Sent: Wednesday, January 2, 2019 6:01:56 PM
To: Quan, Evan; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander; Xu, Feifei; Huang, Ray; Zhang, Hawking
Subject: Re: [PATCH 0/9] PSP cleanup

The general idea looks good, but can we change the file and symbol
naming a bit?

So far we have named all non-ip version related functions amdgpu_* and
ip related functions ip_version.

E.g. following this xgmi functions should go into amdgpu_xgmi.c and not
psp_xgmi.c

Christian.

Am 02.01.19 um 10:21 schrieb Evan Quan:
> *** BLURB HERE ***
>
> Evan Quan (9):
>drm/amdgpu: separate the PSP ring related APIs
>drm/amdgpu: separate commonly used PSP APIs
>drm/amdgpu: separate the xgmi related APIs
>drm/amdgpu: separate the tmr related APIs
>drm/amdgpu: separate the asd related APIs
>drm/amdgpu: drop useless PSP APIs and structures
>drm/amdgpu: check PSP support before adding the ip block
>drm/amdgpu: make PSP sub modules(ASD/XGMI/TMR) support configurable
>drm/amdgpu: move psp_funcs related to a more proper place
>
>   drivers/gpu/drm/amd/amdgpu/Makefile |   7 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 504 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  93 +---
>   drivers/gpu/drm/amd/amdgpu/psp_asd.c|  86 
>   drivers/gpu/drm/amd/amdgpu/psp_asd.h|  32 ++
>   drivers/gpu/drm/amd/amdgpu/psp_cmn.c| 289 +
>   drivers/gpu/drm/amd/amdgpu/psp_cmn.h|  84 
>   drivers/gpu/drm/amd/amdgpu/psp_funcs.h  |  98 +
>   drivers/gpu/drm/amd/amdgpu/psp_ring.c   | 354 
>   drivers/gpu/drm/amd/amdgpu/psp_ring.h   |  43 ++
>   drivers/gpu/drm/amd/amdgpu/psp_tmr.c|  84 
>   drivers/gpu/drm/amd/amdgpu/psp_tmr.h|  32 ++
>   drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
>   drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
>   drivers/gpu/drm/amd/amdgpu/psp_xgmi.c   | 207 +
>   drivers/gpu/drm/amd/amdgpu/psp_xgmi.h   |  33 ++
>   drivers/gpu/drm/amd/amdgpu/soc15.c  |  13 +-
>   18 files changed, 1493 insertions(+), 1866 deletions(-)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_funcs.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h
>

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

Re: [PATCH 0/9] PSP cleanup

[Christian König]

The general idea looks good, but can we change the file and symbol 
naming a bit?


So far we have named all non-ip version related functions amdgpu_* and 
ip related functions ip_version.


E.g. following this xgmi functions should go into amdgpu_xgmi.c and not 
psp_xgmi.c


Christian.

Am 02.01.19 um 10:21 schrieb Evan Quan:

*** BLURB HERE ***

Evan Quan (9):
   drm/amdgpu: separate the PSP ring related APIs
   drm/amdgpu: separate commonly used PSP APIs
   drm/amdgpu: separate the xgmi related APIs
   drm/amdgpu: separate the tmr related APIs
   drm/amdgpu: separate the asd related APIs
   drm/amdgpu: drop useless PSP APIs and structures
   drm/amdgpu: check PSP support before adding the ip block
   drm/amdgpu: make PSP sub modules(ASD/XGMI/TMR) support configurable
   drm/amdgpu: move psp_funcs related to a more proper place

  drivers/gpu/drm/amd/amdgpu/Makefile |   7 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 504 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  93 +---
  drivers/gpu/drm/amd/amdgpu/psp_asd.c|  86 
  drivers/gpu/drm/amd/amdgpu/psp_asd.h|  32 ++
  drivers/gpu/drm/amd/amdgpu/psp_cmn.c| 289 +
  drivers/gpu/drm/amd/amdgpu/psp_cmn.h|  84 
  drivers/gpu/drm/amd/amdgpu/psp_funcs.h  |  98 +
  drivers/gpu/drm/amd/amdgpu/psp_ring.c   | 354 
  drivers/gpu/drm/amd/amdgpu/psp_ring.h   |  43 ++
  drivers/gpu/drm/amd/amdgpu/psp_tmr.c|  84 
  drivers/gpu/drm/amd/amdgpu/psp_tmr.h|  32 ++
  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
  drivers/gpu/drm/amd/amdgpu/psp_xgmi.c   | 207 +
  drivers/gpu/drm/amd/amdgpu/psp_xgmi.h   |  33 ++
  drivers/gpu/drm/amd/amdgpu/soc15.c  |  13 +-
  18 files changed, 1493 insertions(+), 1866 deletions(-)
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.h
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.h
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_funcs.h
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.h
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.h
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h



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

Re: [PATCH 1/2] drm/amdgpu/virtual_dce: No need to pin the fb's bo

[Christian König]

Am 27.12.18 um 15:30 schrieb Michel Dänzer:

On 2018-12-24 7:08 a.m., Emily Deng wrote:

For virtual display, no need to pin the fb's bo.

Signed-off-by: Emily Deng 

[...]

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index ea89c54..4e94473 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -188,10 +188,12 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
goto cleanup;
}
  
-	r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev));

-   if (unlikely(r != 0)) {
-   DRM_ERROR("failed to pin new abo buffer before flip\n");
-   goto unreserve;
+   if (!adev->enable_virtual_display) {
+   r = amdgpu_bo_pin(new_abo, 
amdgpu_display_supported_domains(adev));
+   if (unlikely(r != 0)) {
+   DRM_ERROR("failed to pin new abo buffer before flip\n");
+   goto unreserve;
+   }
}
  
  	r = amdgpu_ttm_alloc_gart(&new_abo->tbo);

I think the amdgpu_ttm_alloc_gart call could be skipped as well?


Yup agree.





@@ -242,9 +245,10 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
goto cleanup;
}
  unpin:
-   if (unlikely(amdgpu_bo_unpin(new_abo) != 0)) {
-   DRM_ERROR("failed to unpin new abo in error path\n");
-   }
+   if (!adev->enable_virtual_display)
+   if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
+   DRM_ERROR("failed to unpin new abo in error path\n");

This should be written as either

if (!adev->enable_virtual_display &&
unlikely(amdgpu_bo_unpin(new_abo) != 0))
DRM_ERROR("failed to unpin new abo in error path\n");

or

if (!adev->enable_virtual_display) {
if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
DRM_ERROR("failed to unpin new abo in error path\n");
}


P.S. A lot of people are out for Christmas and New Year's.


Indeed and thanks for the good explanation of the background in the 
other mail. I didn't realized that this was actually a bug fix for a pin 
imbalance.


BTW: It's good practice to note information like that in the commit message.

Anyway with the comment above fixed the series is Acked-by: Christian 
König 


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

[PATCH 5/9] drm/amdgpu: separate the asd related APIs

[Evan Quan]

As they are logic independently from other APIs of amdgpu_psp.c.

Change-Id: If10aa695214f8df0744a67ee9eb0524d2c5a1ec1
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/Makefile |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 62 +-
 drivers/gpu/drm/amd/amdgpu/psp_asd.c| 86 +
 drivers/gpu/drm/amd/amdgpu/psp_asd.h| 32 +
 4 files changed, 121 insertions(+), 62 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 99895c6fcd95..ebef9d2cdd61 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -94,7 +94,8 @@ amdgpu-y += \
psp_ring.o \
psp_cmn.o \
psp_xgmi.o \
-   psp_tmr.o
+   psp_tmr.o \
+   psp_asd.o
 
 # add SMC block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 2007e8948cbc..2528fbcad275 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -34,6 +34,7 @@
 #include "psp_v11_0.h"
 #include "psp_xgmi.h"
 #include "psp_tmr.h"
+#include "psp_asd.h"
 
 #define psp_ring_init(psp, type) \
(psp)->funcs->ring_init((psp), (type))
@@ -126,67 +127,6 @@ static int psp_sw_fini(void *handle)
return 0;
 }
 
-static void psp_prep_asd_cmd_buf(struct psp_gfx_cmd_resp *cmd,
-uint64_t asd_mc, uint64_t asd_mc_shared,
-uint32_t size, uint32_t shared_size)
-{
-   cmd->cmd_id = GFX_CMD_ID_LOAD_ASD;
-   cmd->cmd.cmd_load_ta.app_phy_addr_lo = lower_32_bits(asd_mc);
-   cmd->cmd.cmd_load_ta.app_phy_addr_hi = upper_32_bits(asd_mc);
-   cmd->cmd.cmd_load_ta.app_len = size;
-
-   cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_lo = lower_32_bits(asd_mc_shared);
-   cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_hi = upper_32_bits(asd_mc_shared);
-   cmd->cmd.cmd_load_ta.cmd_buf_len = shared_size;
-}
-
-static int psp_asd_init(struct psp_context *psp)
-{
-   int ret;
-
-   /*
-* Allocate 16k memory aligned to 4k from Frame Buffer (local
-* physical) for shared ASD <-> Driver
-*/
-   ret = amdgpu_bo_create_kernel(psp->adev, PSP_ASD_SHARED_MEM_SIZE,
- PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
- &psp->asd_shared_bo,
- &psp->asd_shared_mc_addr,
- &psp->asd_shared_buf);
-
-   return ret;
-}
-
-static int psp_asd_load(struct psp_context *psp)
-{
-   int ret;
-   struct psp_gfx_cmd_resp *cmd;
-
-   /* If PSP version doesn't match ASD version, asd loading will be failed.
-* add workaround to bypass it for sriov now.
-* TODO: add version check to make it common
-*/
-   if (amdgpu_sriov_vf(psp->adev))
-   return 0;
-
-   cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
-   if (!cmd)
-   return -ENOMEM;
-
-   memset(psp->fw_pri_buf, 0, PSP_1_MEG);
-   memcpy(psp->fw_pri_buf, psp->asd_start_addr, psp->asd_ucode_size);
-
-   psp_prep_asd_cmd_buf(cmd, psp->fw_pri_mc_addr, psp->asd_shared_mc_addr,
-psp->asd_ucode_size, PSP_ASD_SHARED_MEM_SIZE);
-
-   ret = psp_submit_cmd_buf(psp, NULL, cmd,
-psp->fence_buf_mc_addr);
-
-   kfree(cmd);
-
-   return ret;
-}
-
 static int psp_hw_start(struct psp_context *psp)
 {
struct amdgpu_device *adev = psp->adev;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_asd.c 
b/drivers/gpu/drm/amd/amdgpu/psp_asd.c
new file mode 100644
index ..16263b901a2b
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/psp_asd.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * 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,
+ * 

[PATCH 4/9] drm/amdgpu: separate the tmr related APIs

[Evan Quan]

As they are logic independently from other APIs of amdgpu_psp.c.

Change-Id: I3c8014caca8250bef6439e857bea1b64b6c1930a
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/Makefile |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 60 +-
 drivers/gpu/drm/amd/amdgpu/psp_tmr.c| 84 +
 drivers/gpu/drm/amd/amdgpu/psp_tmr.h| 32 ++
 4 files changed, 119 insertions(+), 60 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 30a705db0ef5..99895c6fcd95 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -93,7 +93,8 @@ amdgpu-y += \
psp_v11_0.o \
psp_ring.o \
psp_cmn.o \
-   psp_xgmi.o
+   psp_xgmi.o \
+   psp_tmr.o
 
 # add SMC block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index c33ced8371c3..2007e8948cbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -33,6 +33,7 @@
 #include "psp_v10_0.h"
 #include "psp_v11_0.h"
 #include "psp_xgmi.h"
+#include "psp_tmr.h"
 
 #define psp_ring_init(psp, type) \
(psp)->funcs->ring_init((psp), (type))
@@ -125,65 +126,6 @@ static int psp_sw_fini(void *handle)
return 0;
 }
 
-static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
-struct psp_gfx_cmd_resp *cmd,
-uint64_t tmr_mc, uint32_t size)
-{
-   if (psp_support_vmr_ring(psp))
-   cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
-   else
-   cmd->cmd_id = GFX_CMD_ID_SETUP_TMR;
-   cmd->cmd.cmd_setup_tmr.buf_phy_addr_lo = lower_32_bits(tmr_mc);
-   cmd->cmd.cmd_setup_tmr.buf_phy_addr_hi = upper_32_bits(tmr_mc);
-   cmd->cmd.cmd_setup_tmr.buf_size = size;
-}
-
-/* Set up Trusted Memory Region */
-static int psp_tmr_init(struct psp_context *psp)
-{
-   int ret;
-
-   /*
-* Allocate 3M memory aligned to 1M from Frame Buffer (local
-* physical).
-*
-* Note: this memory need be reserved till the driver
-* uninitializes.
-*/
-   ret = amdgpu_bo_create_kernel(psp->adev, PSP_TMR_SIZE, 0x10,
- AMDGPU_GEM_DOMAIN_VRAM,
- &psp->tmr_bo, &psp->tmr_mc_addr, 
&psp->tmr_buf);
-
-   return ret;
-}
-
-static int psp_tmr_load(struct psp_context *psp)
-{
-   int ret;
-   struct psp_gfx_cmd_resp *cmd;
-
-   cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
-   if (!cmd)
-   return -ENOMEM;
-
-   psp_prep_tmr_cmd_buf(psp, cmd, psp->tmr_mc_addr, PSP_TMR_SIZE);
-   DRM_INFO("reserve 0x%x from 0x%llx for PSP TMR SIZE\n",
-   PSP_TMR_SIZE, psp->tmr_mc_addr);
-
-   ret = psp_submit_cmd_buf(psp, NULL, cmd,
-psp->fence_buf_mc_addr);
-   if (ret)
-   goto failed;
-
-   kfree(cmd);
-
-   return 0;
-
-failed:
-   kfree(cmd);
-   return ret;
-}
-
 static void psp_prep_asd_cmd_buf(struct psp_gfx_cmd_resp *cmd,
 uint64_t asd_mc, uint64_t asd_mc_shared,
 uint32_t size, uint32_t shared_size)
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_tmr.c 
b/drivers/gpu/drm/amd/amdgpu/psp_tmr.c
new file mode 100644
index ..fd5fa167a56d
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/psp_tmr.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * 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.
+ *
+ */
+
+#include "psp_tmr.h"
+#include "psp_ring.h"
+
+static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
+struct psp_

[PATCH 0/9] PSP cleanup

[Evan Quan]

*** BLURB HERE ***

Evan Quan (9):
  drm/amdgpu: separate the PSP ring related APIs
  drm/amdgpu: separate commonly used PSP APIs
  drm/amdgpu: separate the xgmi related APIs
  drm/amdgpu: separate the tmr related APIs
  drm/amdgpu: separate the asd related APIs
  drm/amdgpu: drop useless PSP APIs and structures
  drm/amdgpu: check PSP support before adding the ip block
  drm/amdgpu: make PSP sub modules(ASD/XGMI/TMR) support configurable
  drm/amdgpu: move psp_funcs related to a more proper place

 drivers/gpu/drm/amd/amdgpu/Makefile |   7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 504 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  93 +---
 drivers/gpu/drm/amd/amdgpu/psp_asd.c|  86 
 drivers/gpu/drm/amd/amdgpu/psp_asd.h|  32 ++
 drivers/gpu/drm/amd/amdgpu/psp_cmn.c| 289 +
 drivers/gpu/drm/amd/amdgpu/psp_cmn.h|  84 
 drivers/gpu/drm/amd/amdgpu/psp_funcs.h  |  98 +
 drivers/gpu/drm/amd/amdgpu/psp_ring.c   | 354 
 drivers/gpu/drm/amd/amdgpu/psp_ring.h   |  43 ++
 drivers/gpu/drm/amd/amdgpu/psp_tmr.c|  84 
 drivers/gpu/drm/amd/amdgpu/psp_tmr.h|  32 ++
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 381 +
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 539 +---
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 480 +
 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c   | 207 +
 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h   |  33 ++
 drivers/gpu/drm/amd/amdgpu/soc15.c  |  13 +-
 18 files changed, 1493 insertions(+), 1866 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_asd.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_funcs.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_tmr.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h

-- 
2.20.1

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

[PATCH 1/9] drm/amdgpu: separate the PSP ring related APIs

[Evan Quan]

So that they can be shared among different PSP generations.
And there is no need to have one copy for each PSP generation.

Change-Id: I7a97f410ef5993b25f0ec3cfac4a293073d697c3
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/Makefile |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  71 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  16 +-
 drivers/gpu/drm/amd/amdgpu/psp_ring.c   | 353 
 drivers/gpu/drm/amd/amdgpu/psp_ring.h   |  43 +++
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 247 +
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 278 +--
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 231 +---
 8 files changed, 444 insertions(+), 798 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_ring.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 007478905c7b..bdcf0d4338f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -90,7 +90,8 @@ amdgpu-y += \
amdgpu_psp.o \
psp_v3_1.o \
psp_v10_0.o \
-   psp_v11_0.o
+   psp_v11_0.o \
+   psp_ring.o
 
 # add SMC block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 8189a90637f7..38398f0c10c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -33,6 +33,21 @@
 #include "psp_v10_0.h"
 #include "psp_v11_0.h"
 
+#define psp_ring_init(psp, type) \
+   (psp)->funcs->ring_init((psp), (type))
+#define psp_ring_create(psp, type) \
+   (psp)->funcs->ring_create((psp), (type))
+#define psp_ring_stop(psp, type) \
+   (psp)->funcs->ring_stop((psp), (type))
+#define psp_ring_destroy(psp, type) \
+   ((psp)->funcs->ring_destroy((psp), (type)))
+#define psp_prep_cmd_buf(ucode, cmd) \
+   (psp)->funcs->prep_cmd_buf((ucode), (cmd))
+#define psp_submit_cmd_buf(psp, ucode, cmd, fence_mc) \
+   (psp)->funcs->submit_cmd_buf((psp), (ucode), (cmd), (fence_mc))
+#define psp_support_vmr_ring(psp) \
+   ((psp)->funcs->support_vmr_ring ? 
(psp)->funcs->support_vmr_ring((psp)) : false)
+
 static void psp_set_funcs(struct amdgpu_device *adev);
 
 static int psp_early_init(void *handle)
@@ -117,50 +132,6 @@ int psp_wait_for(struct psp_context *psp, uint32_t 
reg_index,
return -ETIME;
 }
 
-static int
-psp_cmd_submit_buf(struct psp_context *psp,
-  struct amdgpu_firmware_info *ucode,
-  struct psp_gfx_cmd_resp *cmd, uint64_t fence_mc_addr)
-{
-   int ret;
-   int index;
-
-   memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE);
-
-   memcpy(psp->cmd_buf_mem, cmd, sizeof(struct psp_gfx_cmd_resp));
-
-   index = atomic_inc_return(&psp->fence_value);
-   ret = psp_cmd_submit(psp, ucode, psp->cmd_buf_mc_addr,
-fence_mc_addr, index);
-   if (ret) {
-   atomic_dec(&psp->fence_value);
-   return ret;
-   }
-
-   while (*((unsigned int *)psp->fence_buf) != index)
-   msleep(1);
-
-   /* the status field must be 0 after psp command completion */
-   if (psp->cmd_buf_mem->resp.status) {
-   if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
- ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
- psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
-   }
-
-   /* get xGMI session id from response buffer */
-   cmd->resp.session_id = psp->cmd_buf_mem->resp.session_id;
-
-   if (ucode) {
-   ucode->tmr_mc_addr_lo = psp->cmd_buf_mem->resp.fw_addr_lo;
-   ucode->tmr_mc_addr_hi = psp->cmd_buf_mem->resp.fw_addr_hi;
-   }
-
-   return ret;
-}
-
 static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
 struct psp_gfx_cmd_resp *cmd,
 uint64_t tmr_mc, uint32_t size)
@@ -206,7 +177,7 @@ static int psp_tmr_load(struct psp_context *psp)
DRM_INFO("reserve 0x%x from 0x%llx for PSP TMR SIZE\n",
PSP_TMR_SIZE, psp->tmr_mc_addr);
 
-   ret = psp_cmd_submit_buf(psp, NULL, cmd,
+   ret = psp_submit_cmd_buf(psp, NULL, cmd,
 psp->fence_buf_mc_addr);
if (ret)
goto failed;
@@ -273,7 +244,7 @@ static int psp_asd_load(struct psp_context *psp)
psp_prep_asd_cmd_buf(cmd, psp->fw_pri_mc_addr, psp->asd_shared_mc_addr,
 psp->asd_ucode_size, PSP_ASD_SHARED_MEM_SIZE);
 
-   ret = psp_cmd_submit_buf(psp, NULL, cmd,
+   ret = psp_submit_cmd_buf(psp, NULL, cmd,
 psp->fence_buf_mc_addr);
 
kfree(cmd);
@@ -33

[PATCH 2/9] drm/amdgpu: separate commonly used PSP APIs

[Evan Quan]

So that they can be shared in PSP widely.

Change-Id: Icec5d23db2c1f8241f6bdff371e6cde65ffdb101
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/Makefile |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  36 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  16 --
 drivers/gpu/drm/amd/amdgpu/psp_cmn.c| 289 
 drivers/gpu/drm/amd/amdgpu/psp_cmn.h|  41 
 drivers/gpu/drm/amd/amdgpu/psp_ring.c   |   9 +-
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 126 +--
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 250 +---
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 243 +---
 9 files changed, 366 insertions(+), 647 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_cmn.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index bdcf0d4338f8..fe27d6038da9 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -91,7 +91,8 @@ amdgpu-y += \
psp_v3_1.o \
psp_v10_0.o \
psp_v11_0.o \
-   psp_ring.o
+   psp_ring.o \
+   psp_cmn.o
 
 # add SMC block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 38398f0c10c9..d1ccc6ea7607 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -47,6 +47,20 @@
(psp)->funcs->submit_cmd_buf((psp), (ucode), (cmd), (fence_mc))
 #define psp_support_vmr_ring(psp) \
((psp)->funcs->support_vmr_ring ? 
(psp)->funcs->support_vmr_ring((psp)) : false)
+#define psp_compare_sram_data(psp, ucode, type) \
+   (psp)->funcs->compare_sram_data((psp), (ucode), (type))
+#define psp_init_microcode(psp) \
+   ((psp)->funcs->init_microcode ? 
(psp)->funcs->init_microcode((psp)) : 0)
+#define psp_bootloader_load_sysdrv(psp) \
+   ((psp)->funcs->bootloader_load_sysdrv ? 
(psp)->funcs->bootloader_load_sysdrv((psp)) : 0)
+#define psp_bootloader_load_sos(psp) \
+   ((psp)->funcs->bootloader_load_sos ? 
(psp)->funcs->bootloader_load_sos((psp)) : 0)
+#define psp_smu_reload_quirk(psp) \
+   ((psp)->funcs->smu_reload_quirk ? 
(psp)->funcs->smu_reload_quirk((psp)) : false)
+#define psp_mode1_reset(psp) \
+   ((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : 
false)
+#define amdgpu_psp_check_fw_loading_status(adev, i) \
+   (adev)->firmware.funcs->check_fw_loading_status((adev), (i))
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
@@ -110,28 +124,6 @@ static int psp_sw_fini(void *handle)
return 0;
 }
 
-int psp_wait_for(struct psp_context *psp, uint32_t reg_index,
-uint32_t reg_val, uint32_t mask, bool check_changed)
-{
-   uint32_t val;
-   int i;
-   struct amdgpu_device *adev = psp->adev;
-
-   for (i = 0; i < adev->usec_timeout; i++) {
-   val = RREG32(reg_index);
-   if (check_changed) {
-   if (val != reg_val)
-   return 0;
-   } else {
-   if ((val & mask) == reg_val)
-   return 0;
-   }
-   udelay(1);
-   }
-
-   return -ETIME;
-}
-
 static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
 struct psp_gfx_cmd_resp *cmd,
 uint64_t tmr_mc, uint32_t size)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c94fa444f8b7..f4438a5077b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -175,18 +175,6 @@ struct psp_xgmi_topology_info {
struct psp_xgmi_node_info   nodes[AMDGPU_XGMI_MAX_CONNECTED_NODES];
 };
 
-#define psp_compare_sram_data(psp, ucode, type) \
-   (psp)->funcs->compare_sram_data((psp), (ucode), (type))
-#define psp_init_microcode(psp) \
-   ((psp)->funcs->init_microcode ? 
(psp)->funcs->init_microcode((psp)) : 0)
-#define psp_bootloader_load_sysdrv(psp) \
-   ((psp)->funcs->bootloader_load_sysdrv ? 
(psp)->funcs->bootloader_load_sysdrv((psp)) : 0)
-#define psp_bootloader_load_sos(psp) \
-   ((psp)->funcs->bootloader_load_sos ? 
(psp)->funcs->bootloader_load_sos((psp)) : 0)
-#define psp_smu_reload_quirk(psp) \
-   ((psp)->funcs->smu_reload_quirk ? 
(psp)->funcs->smu_reload_quirk((psp)) : false)
-#define psp_mode1_reset(psp) \
-   ((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : 
false)
 #define psp_xgmi_get_node_id(psp, node_id) \
((psp)->funcs->xgmi_get_node_id ? 
(psp)->funcs->xgmi_get_node_id((psp), (node_id)) : -EINVAL)
 #define psp_xgmi_get_hive_id(psp, hive_id) \
@@ -198,13 +186,9 @@ struct psp_xgmi_topology_info {
   

[PATCH 3/9] drm/amdgpu: separate the xgmi related APIs

[Evan Quan]

As they are logic independently from other APIs of amdgpu_psp.c.

Change-Id: Idb9d81e15ad4d37e93b95682a07194a94c4849d6
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/Makefile |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 182 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |   1 -
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  |   1 +
 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c   | 207 
 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h   |  33 
 6 files changed, 244 insertions(+), 183 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_xgmi.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index fe27d6038da9..30a705db0ef5 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -92,7 +92,8 @@ amdgpu-y += \
psp_v10_0.o \
psp_v11_0.o \
psp_ring.o \
-   psp_cmn.o
+   psp_cmn.o \
+   psp_xgmi.o
 
 # add SMC block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index d1ccc6ea7607..c33ced8371c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -32,6 +32,7 @@
 #include "psp_v3_1.h"
 #include "psp_v10_0.h"
 #include "psp_v11_0.h"
+#include "psp_xgmi.h"
 
 #define psp_ring_init(psp, type) \
(psp)->funcs->ring_init((psp), (type))
@@ -244,187 +245,6 @@ static int psp_asd_load(struct psp_context *psp)
return ret;
 }
 
-static void psp_prep_xgmi_ta_load_cmd_buf(struct psp_gfx_cmd_resp *cmd,
- uint64_t xgmi_ta_mc, uint64_t 
xgmi_mc_shared,
- uint32_t xgmi_ta_size, uint32_t 
shared_size)
-{
-cmd->cmd_id = GFX_CMD_ID_LOAD_TA;
-cmd->cmd.cmd_load_ta.app_phy_addr_lo = lower_32_bits(xgmi_ta_mc);
-cmd->cmd.cmd_load_ta.app_phy_addr_hi = upper_32_bits(xgmi_ta_mc);
-cmd->cmd.cmd_load_ta.app_len = xgmi_ta_size;
-
-cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_lo = 
lower_32_bits(xgmi_mc_shared);
-cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_hi = 
upper_32_bits(xgmi_mc_shared);
-cmd->cmd.cmd_load_ta.cmd_buf_len = shared_size;
-}
-
-static int psp_xgmi_init_shared_buf(struct psp_context *psp)
-{
-   int ret;
-
-   /*
-* Allocate 16k memory aligned to 4k from Frame Buffer (local
-* physical) for xgmi ta <-> Driver
-*/
-   ret = amdgpu_bo_create_kernel(psp->adev, PSP_XGMI_SHARED_MEM_SIZE,
- PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
- &psp->xgmi_context.xgmi_shared_bo,
- &psp->xgmi_context.xgmi_shared_mc_addr,
- &psp->xgmi_context.xgmi_shared_buf);
-
-   return ret;
-}
-
-static int psp_xgmi_load(struct psp_context *psp)
-{
-   int ret;
-   struct psp_gfx_cmd_resp *cmd;
-
-   /*
-* TODO: bypass the loading in sriov for now
-*/
-   if (amdgpu_sriov_vf(psp->adev))
-   return 0;
-
-   cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
-   if (!cmd)
-   return -ENOMEM;
-
-   memset(psp->fw_pri_buf, 0, PSP_1_MEG);
-   memcpy(psp->fw_pri_buf, psp->ta_xgmi_start_addr, 
psp->ta_xgmi_ucode_size);
-
-   psp_prep_xgmi_ta_load_cmd_buf(cmd, psp->fw_pri_mc_addr,
- psp->xgmi_context.xgmi_shared_mc_addr,
- psp->ta_xgmi_ucode_size, 
PSP_XGMI_SHARED_MEM_SIZE);
-
-   ret = psp_submit_cmd_buf(psp, NULL, cmd,
-psp->fence_buf_mc_addr);
-
-   if (!ret) {
-   psp->xgmi_context.initialized = 1;
-   psp->xgmi_context.session_id = cmd->resp.session_id;
-   }
-
-   kfree(cmd);
-
-   return ret;
-}
-
-static void psp_prep_xgmi_ta_unload_cmd_buf(struct psp_gfx_cmd_resp *cmd,
-   uint32_t xgmi_session_id)
-{
-   cmd->cmd_id = GFX_CMD_ID_UNLOAD_TA;
-   cmd->cmd.cmd_unload_ta.session_id = xgmi_session_id;
-}
-
-static int psp_xgmi_unload(struct psp_context *psp)
-{
-   int ret;
-   struct psp_gfx_cmd_resp *cmd;
-
-   /*
-* TODO: bypass the unloading in sriov for now
-*/
-   if (amdgpu_sriov_vf(psp->adev))
-   return 0;
-
-   cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
-   if (!cmd)
-   return -ENOMEM;
-
-   psp_prep_xgmi_ta_unload_cmd_buf(cmd, psp->xgmi_context.session_id);
-
-   ret = psp_submit_cmd_buf(psp, NULL, cmd,
-psp->fence_buf_mc_addr);
-
-   kfree(cmd);
-
-   return ret;
-}
-
-static void psp_prep_xgmi_ta_invoke_cmd_buf(struct psp_gfx_cmd_resp *cmd,
-   uint32_t ta_cmd

[PATCH 9/9] drm/amdgpu: move psp_funcs related to a more proper place

[Evan Quan]

As there is no need to expose these through amdgpu_psp.h.
So, it's better to make them PSP internal used only.

Change-Id: I571be7aae8807f11a6d594be7762306b56403f82
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 96 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 73 +++---
 drivers/gpu/drm/amd/amdgpu/psp_cmn.h| 43 +++
 drivers/gpu/drm/amd/amdgpu/psp_funcs.h  | 98 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   |  2 +-
 7 files changed, 197 insertions(+), 119 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/psp_funcs.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index ecfbf618652a..3b46334fa849 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -32,57 +32,8 @@
 #include "psp_v3_1.h"
 #include "psp_v10_0.h"
 #include "psp_v11_0.h"
-
-#define psp_ring_init(psp, type) \
-   (psp)->funcs->ring_init((psp), (type))
-#define psp_ring_create(psp, type) \
-   (psp)->funcs->ring_create((psp), (type))
-#define psp_ring_stop(psp, type) \
-   (psp)->funcs->ring_stop((psp), (type))
-#define psp_ring_destroy(psp, type) \
-   ((psp)->funcs->ring_destroy((psp), (type)))
-#define psp_prep_cmd_buf(ucode, cmd) \
-   (psp)->funcs->prep_cmd_buf((ucode), (cmd))
-#define psp_submit_cmd_buf(psp, ucode, cmd, fence_mc) \
-   (psp)->funcs->submit_cmd_buf((psp), (ucode), (cmd), (fence_mc))
-#define psp_support_vmr_ring(psp) \
-   ((psp)->funcs->support_vmr_ring ? 
(psp)->funcs->support_vmr_ring((psp)) : false)
-#define psp_init_asd(psp) \
-   ((psp)->funcs->asd_init ? (psp)->funcs->asd_init((psp)) : 0)
-#define psp_load_asd(psp) \
-   ((psp)->funcs->asd_load ? (psp)->funcs->asd_load((psp)) : 0)
-#define psp_unload_asd(psp) \
-   ((psp)->funcs->asd_unload ? (psp)->funcs->asd_unload((psp)) : 0)
-#define psp_destory_asd(psp) \
-   ((psp)->funcs->asd_destory ? (psp)->funcs->asd_destory((psp)) : 
0)
-#define psp_init_tmr(psp) \
-   ((psp)->funcs->tmr_init ? (psp)->funcs->tmr_init((psp)) : 0)
-#define psp_load_tmr(psp) \
-   ((psp)->funcs->tmr_load ? (psp)->funcs->tmr_load((psp)) : 0)
-#define psp_unload_tmr(psp) \
-   ((psp)->funcs->tmr_unload ? (psp)->funcs->tmr_unload((psp)) : 0)
-#define psp_destory_tmr(psp) \
-   ((psp)->funcs->tmr_destory ? (psp)->funcs->tmr_destory((psp)) : 
0)
-#define psp_init_xgmi(psp) \
-   ((psp)->funcs->xgmi_init ? (psp)->funcs->xgmi_init((psp)) : 0)
-#define psp_load_xgmi(psp) \
-   ((psp)->funcs->xgmi_load ? (psp)->funcs->xgmi_load((psp)) : 0)
-#define psp_unload_xgmi(psp) \
-   ((psp)->funcs->xgmi_unload ? (psp)->funcs->xgmi_unload((psp)) : 
0)
-#define psp_destory_xgmi(psp) \
-   ((psp)->funcs->xgmi_destory ? (psp)->funcs->xgmi_destory((psp)) 
: 0)
-#define psp_compare_sram_data(psp, ucode, type) \
-   (psp)->funcs->compare_sram_data((psp), (ucode), (type))
-#define psp_init_microcode(psp) \
-   ((psp)->funcs->init_microcode ? 
(psp)->funcs->init_microcode((psp)) : 0)
-#define psp_bootloader_load_sysdrv(psp) \
-   ((psp)->funcs->bootloader_load_sysdrv ? 
(psp)->funcs->bootloader_load_sysdrv((psp)) : 0)
-#define psp_bootloader_load_sos(psp) \
-   ((psp)->funcs->bootloader_load_sos ? 
(psp)->funcs->bootloader_load_sos((psp)) : 0)
-#define psp_smu_reload_quirk(psp) \
-   ((psp)->funcs->smu_reload_quirk ? 
(psp)->funcs->smu_reload_quirk((psp)) : false)
-#define psp_mode1_reset(psp) \
-   ((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : 
false)
+#include "psp_cmn.h"
+#include "psp_funcs.h"
 
 static int psp_sw_init(void *handle)
 {
@@ -417,7 +368,6 @@ int psp_gpu_reset(struct amdgpu_device *adev)
return psp_mode1_reset(&adev->psp);
 }
 
-
 static int psp_set_clockgating_state(void *handle,
 enum amd_clockgating_state state)
 {
@@ -430,6 +380,48 @@ static int psp_set_powergating_state(void *handle,
return 0;
 }
 
+int psp_xgmi_get_topology_info(struct psp_context *psp,
+   int number_devices, struct psp_xgmi_topology_info *topology)
+{
+   const struct psp_funcs *funcs = (const struct psp_funcs *)(psp->priv);
+
+   if (!funcs->xgmi_get_topology_info)
+   return -EINVAL;
+
+   return funcs->xgmi_get_topology_info(psp, number_devices, topology);
+}
+
+int psp_xgmi_set_topology_info(struct psp_context *psp,
+   int number_devices, struct psp_xgmi_topology_info *topology)
+{
+   const struct psp_funcs *funcs = (const struct psp_funcs *)(psp->priv);
+
+   if (!funcs->xgmi_set_topology_info)
+   re

[PATCH 6/9] drm/amdgpu: drop useless PSP APIs and structures

[Evan Quan]

Drop those useless APIs and structures.

Change-Id: I1350c0e7ec0b990a1178a0ce92f61e56dc8851b5
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 66 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  5 --
 2 files changed, 22 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 2528fbcad275..f1ffe04b8c0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -62,19 +62,6 @@
((psp)->funcs->smu_reload_quirk ? 
(psp)->funcs->smu_reload_quirk((psp)) : false)
 #define psp_mode1_reset(psp) \
((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : 
false)
-#define amdgpu_psp_check_fw_loading_status(adev, i) \
-   (adev)->firmware.funcs->check_fw_loading_status((adev), (i))
-
-static void psp_set_funcs(struct amdgpu_device *adev);
-
-static int psp_early_init(void *handle)
-{
-   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
-   psp_set_funcs(adev);
-
-   return 0;
-}
 
 static int psp_sw_init(void *handle)
 {
@@ -166,6 +153,26 @@ static int psp_hw_start(struct psp_context *psp)
return 0;
 }
 
+static bool psp_check_fw_loading_status(struct amdgpu_device *adev,
+   enum AMDGPU_UCODE_ID ucode_type)
+{
+   struct amdgpu_firmware_info *ucode = NULL;
+
+   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
+   DRM_INFO("firmware is not loaded by PSP\n");
+   return true;
+   }
+
+   if (!adev->firmware.fw_size)
+   return false;
+
+   ucode = &adev->firmware.ucode[ucode_type];
+   if (!ucode->fw || !ucode->ucode_size)
+   return false;
+
+   return psp_compare_sram_data(&adev->psp, ucode, ucode_type);
+}
+
 static int psp_np_fw_load(struct psp_context *psp)
 {
int i, ret;
@@ -198,7 +205,7 @@ static int psp_np_fw_load(struct psp_context *psp)
 
 #if 0
/* check if firmware loaded sucessfully */
-   if (!amdgpu_psp_check_fw_loading_status(adev, i))
+   if (!psp_check_fw_loading_status(adev, i))
return -EINVAL;
 #endif
}
@@ -413,25 +420,6 @@ int psp_gpu_reset(struct amdgpu_device *adev)
return psp_mode1_reset(&adev->psp);
 }
 
-static bool psp_check_fw_loading_status(struct amdgpu_device *adev,
-   enum AMDGPU_UCODE_ID ucode_type)
-{
-   struct amdgpu_firmware_info *ucode = NULL;
-
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
-   DRM_INFO("firmware is not loaded by PSP\n");
-   return true;
-   }
-
-   if (!adev->firmware.fw_size)
-   return false;
-
-   ucode = &adev->firmware.ucode[ucode_type];
-   if (!ucode->fw || !ucode->ucode_size)
-   return false;
-
-   return psp_compare_sram_data(&adev->psp, ucode, ucode_type);
-}
 
 static int psp_set_clockgating_state(void *handle,
 enum amd_clockgating_state state)
@@ -447,7 +435,7 @@ static int psp_set_powergating_state(void *handle,
 
 const struct amd_ip_funcs psp_ip_funcs = {
.name = "psp",
-   .early_init = psp_early_init,
+   .early_init = NULL,
.late_init = NULL,
.sw_init = psp_sw_init,
.sw_fini = psp_sw_fini,
@@ -463,16 +451,6 @@ const struct amd_ip_funcs psp_ip_funcs = {
.set_powergating_state = psp_set_powergating_state,
 };
 
-static const struct amdgpu_psp_funcs psp_funcs = {
-   .check_fw_loading_status = psp_check_fw_loading_status,
-};
-
-static void psp_set_funcs(struct amdgpu_device *adev)
-{
-   if (NULL == adev->firmware.funcs)
-   adev->firmware.funcs = &psp_funcs;
-}
-
 const struct amdgpu_ip_block_version psp_v3_1_ip_block =
 {
.type = AMD_IP_BLOCK_TYPE_PSP,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index d65691b988fa..16900d4c8034 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -157,11 +157,6 @@ struct psp_context
struct psp_xgmi_context xgmi_context;
 };
 
-struct amdgpu_psp_funcs {
-   bool (*check_fw_loading_status)(struct amdgpu_device *adev,
-   enum AMDGPU_UCODE_ID);
-};
-
 #define AMDGPU_XGMI_MAX_CONNECTED_NODES64
 struct psp_xgmi_node_info {
uint64_tnode_id;
-- 
2.20.1

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

[PATCH 7/9] drm/amdgpu: check PSP support before adding the ip block

[Evan Quan]

So that we do not need to check this in every internal function.

Change-Id: I5f2665cf60a57b6ae8d04a645f633daf377ae28c
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 24 
 drivers/gpu/drm/amd/amdgpu/soc15.c  | 13 -
 2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index f1ffe04b8c0f..bf51686bdd0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -86,9 +86,6 @@ static int psp_sw_init(void *handle)
 
psp->adev = adev;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
ret = psp_init_microcode(psp);
if (ret) {
DRM_ERROR("Failed to load psp firmware!\n");
@@ -102,9 +99,6 @@ static int psp_sw_fini(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
release_firmware(adev->psp.sos_fw);
adev->psp.sos_fw = NULL;
release_firmware(adev->psp.asd_fw);
@@ -158,11 +152,6 @@ static bool psp_check_fw_loading_status(struct 
amdgpu_device *adev,
 {
struct amdgpu_firmware_info *ucode = NULL;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
-   DRM_INFO("firmware is not loaded by PSP\n");
-   return true;
-   }
-
if (!adev->firmware.fw_size)
return false;
 
@@ -296,10 +285,6 @@ static int psp_hw_init(void *handle)
int ret;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
-
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
mutex_lock(&adev->firmware.mutex);
/*
 * This sequence is just used on hw_init only once, no need on
@@ -329,9 +314,6 @@ static int psp_hw_fini(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct psp_context *psp = &adev->psp;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
if (adev->gmc.xgmi.num_physical_nodes > 1 &&
psp->xgmi_context.initialized == 1)
 psp_xgmi_terminate(psp);
@@ -360,9 +342,6 @@ static int psp_suspend(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct psp_context *psp = &adev->psp;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
if (adev->gmc.xgmi.num_physical_nodes > 1 &&
psp->xgmi_context.initialized == 1) {
ret = psp_xgmi_terminate(psp);
@@ -387,9 +366,6 @@ static int psp_resume(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct psp_context *psp = &adev->psp;
 
-   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
-   return 0;
-
DRM_INFO("PSP is resuming...\n");
 
mutex_lock(&adev->firmware.mutex);
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index d5b3f821b8f0..9f162a8a7d9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -535,10 +535,12 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
amdgpu_device_ip_block_add(adev, &vega10_common_ip_block);
amdgpu_device_ip_block_add(adev, &gmc_v9_0_ip_block);
amdgpu_device_ip_block_add(adev, &vega10_ih_ip_block);
-   if (adev->asic_type == CHIP_VEGA20)
-   amdgpu_device_ip_block_add(adev, &psp_v11_0_ip_block);
-   else
-   amdgpu_device_ip_block_add(adev, &psp_v3_1_ip_block);
+   if (likely(adev->firmware.load_type == AMDGPU_FW_LOAD_PSP)) {
+   if (adev->asic_type == CHIP_VEGA20)
+   amdgpu_device_ip_block_add(adev, 
&psp_v11_0_ip_block);
+   else
+   amdgpu_device_ip_block_add(adev, 
&psp_v3_1_ip_block);
+   }
amdgpu_device_ip_block_add(adev, &gfx_v9_0_ip_block);
amdgpu_device_ip_block_add(adev, &sdma_v4_0_ip_block);
if (!amdgpu_sriov_vf(adev))
@@ -560,7 +562,8 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
amdgpu_device_ip_block_add(adev, &vega10_common_ip_block);
amdgpu_device_ip_block_add(adev, &gmc_v9_0_ip_block);
amdgpu_device_ip_block_add(adev, &vega10_ih_ip_block);
-   amdgpu_device_ip_block_add(adev, &psp_v10_0_ip_block);
+   if (likely(adev->firmware.load_type == AMDGPU_FW_LOAD_PSP))
+   amdgpu_device_ip_block_add(adev, &psp_v10_0_ip_block);
amdgpu_device_ip_block_add(adev, &gfx_v9_0_ip_block);
amdgpu_device_ip_block_add(ade

[PATCH 8/9] drm/amdgpu: make PSP sub modules(ASD/XGMI/TMR) support configurable

[Evan Quan]

As not every generation can support all these PSP sub modules.

Change-Id: I866884e6453a37ff844427eb2d6fd56a91058ebe
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 41 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 14 -
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  |  6 
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  |  8 +
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   |  6 
 5 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index bf51686bdd0f..ecfbf618652a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -32,9 +32,6 @@
 #include "psp_v3_1.h"
 #include "psp_v10_0.h"
 #include "psp_v11_0.h"
-#include "psp_xgmi.h"
-#include "psp_tmr.h"
-#include "psp_asd.h"
 
 #define psp_ring_init(psp, type) \
(psp)->funcs->ring_init((psp), (type))
@@ -50,6 +47,30 @@
(psp)->funcs->submit_cmd_buf((psp), (ucode), (cmd), (fence_mc))
 #define psp_support_vmr_ring(psp) \
((psp)->funcs->support_vmr_ring ? 
(psp)->funcs->support_vmr_ring((psp)) : false)
+#define psp_init_asd(psp) \
+   ((psp)->funcs->asd_init ? (psp)->funcs->asd_init((psp)) : 0)
+#define psp_load_asd(psp) \
+   ((psp)->funcs->asd_load ? (psp)->funcs->asd_load((psp)) : 0)
+#define psp_unload_asd(psp) \
+   ((psp)->funcs->asd_unload ? (psp)->funcs->asd_unload((psp)) : 0)
+#define psp_destory_asd(psp) \
+   ((psp)->funcs->asd_destory ? (psp)->funcs->asd_destory((psp)) : 
0)
+#define psp_init_tmr(psp) \
+   ((psp)->funcs->tmr_init ? (psp)->funcs->tmr_init((psp)) : 0)
+#define psp_load_tmr(psp) \
+   ((psp)->funcs->tmr_load ? (psp)->funcs->tmr_load((psp)) : 0)
+#define psp_unload_tmr(psp) \
+   ((psp)->funcs->tmr_unload ? (psp)->funcs->tmr_unload((psp)) : 0)
+#define psp_destory_tmr(psp) \
+   ((psp)->funcs->tmr_destory ? (psp)->funcs->tmr_destory((psp)) : 
0)
+#define psp_init_xgmi(psp) \
+   ((psp)->funcs->xgmi_init ? (psp)->funcs->xgmi_init((psp)) : 0)
+#define psp_load_xgmi(psp) \
+   ((psp)->funcs->xgmi_load ? (psp)->funcs->xgmi_load((psp)) : 0)
+#define psp_unload_xgmi(psp) \
+   ((psp)->funcs->xgmi_unload ? (psp)->funcs->xgmi_unload((psp)) : 
0)
+#define psp_destory_xgmi(psp) \
+   ((psp)->funcs->xgmi_destory ? (psp)->funcs->xgmi_destory((psp)) 
: 0)
 #define psp_compare_sram_data(psp, ucode, type) \
(psp)->funcs->compare_sram_data((psp), (ucode), (type))
 #define psp_init_microcode(psp) \
@@ -127,16 +148,16 @@ static int psp_hw_start(struct psp_context *psp)
if (ret)
return ret;
 
-   ret = psp_tmr_load(psp);
+   ret = psp_load_tmr(psp);
if (ret)
return ret;
 
-   ret = psp_asd_load(psp);
+   ret = psp_load_asd(psp);
if (ret)
return ret;
 
if (adev->gmc.xgmi.num_physical_nodes > 1) {
-   ret = psp_xgmi_initialize(psp);
+   ret = psp_init_xgmi(psp);
/* Warning the XGMI seesion initialize failure
 * Instead of stop driver initialization
 */
@@ -245,11 +266,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
if (ret)
goto failed_mem;
 
-   ret = psp_tmr_init(psp);
+   ret = psp_init_tmr(psp);
if (ret)
goto failed_mem;
 
-   ret = psp_asd_init(psp);
+   ret = psp_init_asd(psp);
if (ret)
goto failed_mem;
 
@@ -316,7 +337,7 @@ static int psp_hw_fini(void *handle)
 
if (adev->gmc.xgmi.num_physical_nodes > 1 &&
psp->xgmi_context.initialized == 1)
-psp_xgmi_terminate(psp);
+psp_destory_xgmi(psp);
 
psp_ring_destroy(psp, PSP_RING_TYPE__KM);
 
@@ -344,7 +365,7 @@ static int psp_suspend(void *handle)
 
if (adev->gmc.xgmi.num_physical_nodes > 1 &&
psp->xgmi_context.initialized == 1) {
-   ret = psp_xgmi_terminate(psp);
+   ret = psp_destory_xgmi(psp);
if (ret) {
DRM_ERROR("Failed to terminate xgmi ta\n");
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 16900d4c8034..4cc7fd3224f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -77,6 +77,19 @@ struct psp_funcs
int (*submit_cmd_buf)(struct psp_context *psp,
   struct amdgpu_firmware_info *ucode,
   struct psp_gfx_cmd_resp *cmd, uint64_t 
fence_mc_addr);
+   bool (*support_vmr_ring)(struct psp_context *psp);
+   int (*asd_init)(struct psp_context *psp);
+   int (*asd_load)(struct psp_context *psp);
+   int (*asd_unlo

[PATCH] drm/amdgpu: make gfx9 enter into rlc safe mode when set MGCG

[likun Gao]

From: Likun Gao 

MGCG should RLC enter into safe mode first.

Signed-off-by: Likun Gao 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 7556716..968b127 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3587,6 +3587,8 @@ static void 
gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
 {
uint32_t data, def;
 
+   amdgpu_gfx_rlc_enter_safe_mode(adev);
+
/* It is disabled by HW by default */
if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
/* 1 - RLC_CGTT_MGCG_OVERRIDE */
@@ -3651,6 +3653,8 @@ static void 
gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
WREG32_SOC15(GC, 0, mmCP_MEM_SLP_CNTL, data);
}
}
+
+   amdgpu_gfx_rlc_exit_safe_mode(adev);
 }
 
 static void gfx_v9_0_update_3d_clock_gating(struct amdgpu_device *adev,
-- 
2.7.4

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

[PATCH] drm/amdgpu: distinguish early and late re-init log in sriov

[wentalou]

distinguish ip_reinit_early_sriov and ip_reinit_late_sriov
by different log RE-INIT-early and RE-INIT-late

Change-Id: If4dd78cb807790e9f8daffb04d893cc7fd2b0e60
Signed-off-by: Wentao Lou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7ff3a28..03b73c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2133,7 +2133,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct 
amdgpu_device *adev)
continue;
 
r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
+   DRM_INFO("RE-INIT-early: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
if (r)
return r;
}
@@ -2167,7 +2167,7 @@ static int amdgpu_device_ip_reinit_late_sriov(struct 
amdgpu_device *adev)
continue;
 
r = block->version->funcs->hw_init(adev);
-   DRM_INFO("RE-INIT: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
+   DRM_INFO("RE-INIT-late: %s %s\n", 
block->version->funcs->name, r?"failed":"succeeded");
if (r)
return r;
}
-- 
2.7.4

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