Re: [PATCH] drm/amd: Add DMCU firmware loading on raven

[Felix Kuehling]

On 2018-09-07 01:49 PM, David Francis wrote:
> [Why]
> DMCU (Display MicroController Unit) is an on-GPU microcontroller
> in AMD graphics cards that is used in features for
> embedded displays such as Panel Self-Refresh
>
> DMCU is part of the DM IP block
>
> [How]
> DMCU is added as an option in the enum AMDGPU_UCODE_ID
>
> DMCU needs two pieces of firmware - the initial eram and the
> interrupt vectors.  These are treated as seperate pieces of
> firmware and loaded by PSP
>
> The loading occurs in the sw_init hook of DM
>
> If the firmware is not found, the sw_init hook returns without error.
> DMCU is not a requirement for DM to run.

Doesn't request_fw have a one-minute timeout? So if the "optional"
firmware is not found during boot, you'll sit there with a blank screen
for a minute and lead many users to believe that their system is hanging.

Regards,
  Felix

>
> Signed-off-by: David Francis 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 98 +++
>  3 files changed, 106 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index b358e7519987..38d3af317aa2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
>   AMDGPU_UCODE_ID_UVD1,
>   AMDGPU_UCODE_ID_VCE,
>   AMDGPU_UCODE_ID_VCN,
> + AMDGPU_UCODE_ID_DMCU_ERAM,
> + AMDGPU_UCODE_ID_DMCU_INTV,
>   AMDGPU_UCODE_ID_MAXIMUM,
>  };
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 02be34e72ed9..240dc8c85867 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
> enum psp_gfx_fw_type *
>   case AMDGPU_UCODE_ID_VCN:
>   *type = GFX_FW_TYPE_VCN;
>   break;
> + case AMDGPU_UCODE_ID_DMCU_ERAM:
> + *type = GFX_FW_TYPE_DMCU_ERAM;
> + break;
> + case AMDGPU_UCODE_ID_DMCU_INTV:
> + *type = GFX_FW_TYPE_DMCU_ISR;
> + break;
>   case AMDGPU_UCODE_ID_MAXIMUM:
>   default:
>   return -EINVAL;
> 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 5103eba75cb3..8ad0ee359ef8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -30,6 +30,7 @@
>  #include "vid.h"
>  #include "amdgpu.h"
>  #include "amdgpu_display.h"
> +#include "amdgpu_ucode.h"
>  #include "atom.h"
>  #include "amdgpu_dm.h"
>  #include "amdgpu_pm.h"
> @@ -50,6 +51,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -71,6 +73,12 @@
>  
>  #include "modules/inc/mod_freesync.h"
>  
> +#define FIRMWARE_RAVEN_DMCU_ERAM "amdgpu/raven_dmcu_eram.bin"
> +MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
> +
> +#define FIRMWARE_RAVEN_DMCU_INTV "amdgpu/raven_dmcu_intv.bin"
> +MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
> +
>  /* basic init/fini API */
>  static int amdgpu_dm_init(struct amdgpu_device *adev);
>  static void amdgpu_dm_fini(struct amdgpu_device *adev);
> @@ -516,6 +524,96 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
>  
>  static int dm_sw_init(void *handle)
>  {
> + const struct firmware *fw;
> + const char *fw_name_dmcu_eram;
> + const char *fw_name_dmcu_intv;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int r;
> +
> + switch(adev->asic_type) {
> + case CHIP_BONAIRE:
> + case CHIP_HAWAII:
> + case CHIP_KAVERI:
> + case CHIP_KABINI:
> + case CHIP_MULLINS:
> + case CHIP_TONGA:
> + case CHIP_FIJI:
> + case CHIP_CARRIZO:
> + case CHIP_STONEY:
> + case CHIP_POLARIS11:
> + case CHIP_POLARIS10:
> + case CHIP_POLARIS12:
> + case CHIP_VEGAM:
> + case CHIP_VEGA10:
> + case CHIP_VEGA12:
> + case CHIP_VEGA20:
> + return 0;
> + case CHIP_RAVEN:
> + fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
> + fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
> + break;
> + default:
> + DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> + return -1;
> + }
> +
> + r = request_firmware(, fw_name_dmcu_eram, adev->dev);
> + if (r == -ENOENT)
> + {
> + /* DMCU firmware is not necessary, so don't raise a fuss if 
> it's missing */
> + return 0;
> + }
> + if (r) {
> + dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
> + fw_name_dmcu_eram);
> + return r;
> + }
> +
> + r = amdgpu_ucode_validate(fw);
> + 

Re: [PATCH] drm/amdkfd: Only add bi-directional iolink on GPU with XGMI or largebar

[Felix Kuehling]

[+Eric, see my comment below about iolink atomic flags]

On 2018-09-07 05:37 PM, shaoyunl wrote:
> Change-Id: Ibb6a89ed878fffccb9a8bb4032b07a10ee298a99
> Signed-off-by: shaoyunl 

See one comment inline. It's not directly related to this change so this
is Reviewed-by: Felix Kuehling 

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 15 +--
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.h |  3 ++-
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  1 +
>  3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 130db4d..d4560f1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -353,8 +353,8 @@ static int kfd_parse_subtype_iolink(struct 
> crat_subtype_iolink *iolink,
>   id_from = iolink->proximity_domain_from;
>   id_to = iolink->proximity_domain_to;
>  
> - pr_debug("Found IO link entry in CRAT table with id_from=%d\n",
> - id_from);
> + pr_debug("Found IO link entry in CRAT table with id_from=%d, id_to 
> %d\n",
> + id_from, id_to);
>   list_for_each_entry(dev, device_list, list) {
>   if (id_from == dev->proximity_domain) {
>   props = kfd_alloc_struct(props);
> @@ -391,12 +391,12 @@ static int kfd_parse_subtype_iolink(struct 
> crat_subtype_iolink *iolink,
>   /* CPU topology is created before GPUs are detected, so CPU->GPU
>* links are not built at that time. If a PCIe type is discovered, it
>* means a GPU is detected and we are adding GPU->CPU to the topology.
> -  * At this time, also add the corresponded CPU->GPU link.
> +  * At this time, also add the corresponded CPU->GPU link if GPU
> +  * is large bar.
>* For xGMI, we only added the link with one direction in the crat
>* table, add corresponded reversed direction link now.
>*/
> - if (props && (props->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS ||
> -   props->iolink_type == CRAT_IOLINK_TYPE_XGMI)) {
> + if (props && (iolink->flags & CRAT_IOLINK_FLAGS_BI_DIRECTIONAL)) {

Hmm, looks like we're not populating props->flags at the moment. That's
probably an oversight we should correct. Then we could also move the
atomic flag code from kfd_fill_iolink_non_crat_info into
kfd_fill_gpu_direct_io_link for the GPU->CPU link and this function for
the reverse.

Regards,
  Felix

>   to_dev = kfd_topology_device_by_proximity_domain(id_to);
>   if (!to_dev)
>   return -ENODEV;
> @@ -1057,6 +1057,8 @@ static int kfd_fill_gpu_direct_io_link_to_cpu(int 
> *avail_size,
>   sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
>   sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
>   sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
> + if (kfd_dev_is_large_bar(kdev))
> + sub_type_hdr->flags |= CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
>  
>   /* Fill in IOLINK subtype.
>* TODO: Fill-in other fields of iolink subtype
> @@ -1088,7 +1090,8 @@ static int kfd_fill_gpu_xgmi_link_to_gpu(int 
> *avail_size,
>  
>   sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
>   sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
> - sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
> + sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED |
> +CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
>  
>   sub_type_hdr->io_interface_type = CRAT_IOLINK_TYPE_XGMI;
>   sub_type_hdr->proximity_domain_from = proximity_domain_from;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> index 7a93aeb..7c3f192 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> @@ -232,7 +232,8 @@ struct crat_subtype_ccompute {
>  #define CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT  (1 << 2)
>  #define CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT  (1 << 3)
>  #define CRAT_IOLINK_FLAGS_NO_PEER_TO_PEER_DMA(1 << 4)
> -#define CRAT_IOLINK_FLAGS_RESERVED_MASK  0xffe0
> +#define CRAT_IOLINK_FLAGS_BI_DIRECTIONAL (1 << 31)
> +#define CRAT_IOLINK_FLAGS_RESERVED_MASK  0x7fe0
>  
>  /*
>   * IO interface types
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 6a5418f..05283c9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -696,6 +696,7 @@ struct amdkfd_ioctl_desc {
>   unsigned int cmd_drv;
>   const char *name;
>  };
> +bool kfd_dev_is_large_bar(struct kfd_dev *dev);
>  
>  int kfd_process_create_wq(void);
>  void kfd_process_destroy_wq(void);

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

Re: [RFC] drm/amdgpu: Add macros and documentation for format modifiers.

[Marek Olšák]

On Fri, Sep 7, 2018 at 5:55 AM, Bas Nieuwenhuizen
 wrote:
> On Fri, Sep 7, 2018 at 6:51 AM Marek Olšák  wrote:
>>
>> Hopefully this answers some questions.
>>
>> Other parameters that affect tiling layouts are GB_ADDR_CONFIG (all
>> chips) and MC_ARB_RAMCFG (GFX6-8 only), and those vary with each chip.
>
> For GFX6-GFX8:
> From GB_ADDR_CONFIG addrlib only uses the pipe interleave bytes which
> are 0 (=256 bytes) for all AMDGPU HW (and on GFX9 addrlib even asserts
> on that).  From MC_ARB_RAMCFG addrlib reads the number of banks and
> ranks, calculates the number of logical banks from it, but then does
> not use it. (Presumably because it is the same number as the number of
> banks in the tiling table entry?) Some bits gets used by the kernel
> (memory row size), but those get encoded in the tile split of the
> tiling table, i.e. we do not need the separate bits.
>
> for GFX9, only the DCC meta surface seems to depend on GB_ADDR_CONFIG
> (except the aforementioned pipe interleave bytes) which are constant.

On GFX9, addrlib in Mesa uses most fields from GB_ADDR_CONFIG.
GB_ADDR_CONFIG defines the tiling formats.

On older chips, addrlib reads some fields from GB_ADDR_CONFIG and uses
the chip identification for others like the number of pipes, even
though GB_ADDR_CONFIG has the information too.

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

RE: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Zeng, Oak]

Hi Christian,

See comments inline

Thanks,
Oak

From: Koenig, Christian
Sent: Monday, September 10, 2018 7:44 AM
To: Zeng, Oak ; Oak Zeng ; 
amd-gfx@lists.freedesktop.org; Yang, Philip 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

>The VM doesn't know that information either.

Yes, VM doesn’t know the information. More specifically I meant we need VM data 
structure to update PTs during page fault handling. For example, we walk the 4 
level page table (the first level is pointed by vm->root) to figure out the 
entries need to be filled, allocate page table entry Bos if necessary, allocate 
physical page for the faulty address, setup 4 level page table for the faulty 
virtual address to pointing to the allocated physical page.

>In other words when you request the address of a PT you get the address where 
>memory management has moved it, not the address where the hardware is 
>processing the fault from.

Do you mean the address in the PT is what vm has mapped and the faulty address 
need to be mapped in the PT?


I see only a few possible solutions for that:
>1. We use an extra structure in the kernel driver which holds the current 
>address of the PTs and is feed by memory management when buffers move around.
The amdgpu_vm and amdgpu_vm_pt data structure are invented for the purpose of 
management of page table BO. Why we need invent extra structure? Think about 
the normal case of the page fault handling when the process and vm are still 
alive. In the normal case, the amdgpu_vm structure can meet exactly purpose of 
page table update.

The said extra data structure must look very similar to the amdgpu_vm/vm_pt 
structure. So the best way is to reuse the existing vm structure, either in 
normal case or in process termination case. Is it possible to delay the destroy 
of vm during process termination?

>2. We change the SDMA firmware to do the walk for us and update the PTEs based 
>on the information in the page directory.
As the page table BO need to be freed later. You still need to manage them in 
the driver. SDMA FW can walk and update but it need information from driver 
side. Driver still need a similar struct like amdgpu_vm.

>3. We use the UTCL walker to get us to the correct address which is then 
>updated with the SDMA.
I don’t quite understand your point here. What do you mean by “correct 
address”? Do you mean the physical address to be filled to the page table? If 
this is your meaning, how UTCL2 knows this information – the physical address 
has to be allocated/validated through driver.


Regards,
Christian.

Am 07.09.2018 um 17:30 schrieb Zeng, Oak:
Without a VM, how can you get the physical address of the faulty virtual 
address? Where this information should be hold?

Thanks,
Oak

From: Koenig, Christian
Sent: Monday, September 10, 2018 7:20 AM
To: Zeng, Oak ; Oak Zeng 
; 
amd-gfx@lists.freedesktop.org; Yang, 
Philip 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Am I right that the handling of page fault need a valid VM?
No, exactly that view is incorrect.

The VM is meant to be a memory management structure of page tables and is 
completely pointless fault processing because it represent the future state of 
the page tables and not the current one.

What we need for fault processing is a new structure build around PASIDs which 
is feed by the with addresses when page tables are moved around.

Alternatively I hope that we can use the SDMA to walk the page tables and 
update the required entries by just using the address.

Regards,
Christian.

Am 07.09.2018 um 16:55 schrieb Zeng, Oak:
Hi Christian,

What is the value of delay the destroy of hash to after vm is destroyed? Since 
vm is destroyed, you basically don’t have enough information to paging in the 
correct page to gpuvm. Am I right that the handling of page fault need a valid 
VM? For example, you need the VM to get the corresponding physical address of 
the faulty page.

After vm is destroyed, the retry interrupt can be directly discard as you can’t 
find vm through pasid. You can think this is also one kind of prescreen.

So I am stand for the point that, there is no need to delay the destroy of hash 
to after vm is destroyed. Prescreening hash can be destroyed at the time of 
vm_fini.

Thanks,
Oak

From: Christian König 

Sent: Friday, September 07, 2018 4:39 AM
To: Zeng, Oak ; Oak Zeng 
; 
amd-gfx@lists.freedesktop.org
Cc: Zeng, Oak 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a suggestion 
but rather a must have.

The VM structures must be destroyed while the processing is still ongoing, or 

gim and amdgpu

[Alexander Frolov]

Hi!

I want to use sriov functionality of S75150x2 with some debian-based 
distro with pretty up-to-date kernel (4.15.3-1). Of course, this lead me 
to some issues with gim and amdgpu.


Is it possible to use amdgpu with gim instead of amdgpu-pro? Are there 
any plans to support gim for newer kernels? or support sysfs in the amdgpu?


Thank you!

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

[PATCH] drm/amdkfd: Only add bi-directional iolink on GPU with XGMI or largebar

[shaoyunl]

Change-Id: Ibb6a89ed878fffccb9a8bb4032b07a10ee298a99
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 15 +--
 drivers/gpu/drm/amd/amdkfd/kfd_crat.h |  3 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 130db4d..d4560f1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -353,8 +353,8 @@ static int kfd_parse_subtype_iolink(struct 
crat_subtype_iolink *iolink,
id_from = iolink->proximity_domain_from;
id_to = iolink->proximity_domain_to;
 
-   pr_debug("Found IO link entry in CRAT table with id_from=%d\n",
-   id_from);
+   pr_debug("Found IO link entry in CRAT table with id_from=%d, id_to 
%d\n",
+   id_from, id_to);
list_for_each_entry(dev, device_list, list) {
if (id_from == dev->proximity_domain) {
props = kfd_alloc_struct(props);
@@ -391,12 +391,12 @@ static int kfd_parse_subtype_iolink(struct 
crat_subtype_iolink *iolink,
/* CPU topology is created before GPUs are detected, so CPU->GPU
 * links are not built at that time. If a PCIe type is discovered, it
 * means a GPU is detected and we are adding GPU->CPU to the topology.
-* At this time, also add the corresponded CPU->GPU link.
+* At this time, also add the corresponded CPU->GPU link if GPU
+* is large bar.
 * For xGMI, we only added the link with one direction in the crat
 * table, add corresponded reversed direction link now.
 */
-   if (props && (props->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS ||
- props->iolink_type == CRAT_IOLINK_TYPE_XGMI)) {
+   if (props && (iolink->flags & CRAT_IOLINK_FLAGS_BI_DIRECTIONAL)) {
to_dev = kfd_topology_device_by_proximity_domain(id_to);
if (!to_dev)
return -ENODEV;
@@ -1057,6 +1057,8 @@ static int kfd_fill_gpu_direct_io_link_to_cpu(int 
*avail_size,
sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
+   if (kfd_dev_is_large_bar(kdev))
+   sub_type_hdr->flags |= CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
 
/* Fill in IOLINK subtype.
 * TODO: Fill-in other fields of iolink subtype
@@ -1088,7 +1090,8 @@ static int kfd_fill_gpu_xgmi_link_to_gpu(int *avail_size,
 
sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
-   sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
+   sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED |
+  CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
 
sub_type_hdr->io_interface_type = CRAT_IOLINK_TYPE_XGMI;
sub_type_hdr->proximity_domain_from = proximity_domain_from;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
index 7a93aeb..7c3f192 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
@@ -232,7 +232,8 @@ struct crat_subtype_ccompute {
 #define CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT(1 << 2)
 #define CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT(1 << 3)
 #define CRAT_IOLINK_FLAGS_NO_PEER_TO_PEER_DMA  (1 << 4)
-#define CRAT_IOLINK_FLAGS_RESERVED_MASK0xffe0
+#define CRAT_IOLINK_FLAGS_BI_DIRECTIONAL   (1 << 31)
+#define CRAT_IOLINK_FLAGS_RESERVED_MASK0x7fe0
 
 /*
  * IO interface types
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 6a5418f..05283c9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -696,6 +696,7 @@ struct amdkfd_ioctl_desc {
unsigned int cmd_drv;
const char *name;
 };
+bool kfd_dev_is_large_bar(struct kfd_dev *dev);
 
 int kfd_process_create_wq(void);
 void kfd_process_destroy_wq(void);
-- 
2.7.4

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

[PATCH 06/12] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[shaoyunl]

From: Shaoyun Liu 

Add dummy function for xgmi function interface with psp

Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
Signed-off-by: Shaoyun Liu 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index b70cfa3..a0c2d54 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -548,6 +548,32 @@ static int psp_v11_0_mode1_reset(struct psp_context *psp)
return 0;
 }
 
+/* TODO: Fill in follow functions once PSP firmware interface for XGMI is 
ready.
+ * For now, return success and hack the hive_id so high level code can
+ * start testing */
+static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
+   int number_devices, struct psp_xgmi_topology_info *topology)
+{
+   return 0;
+}
+
+static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
+   int number_devices, struct psp_xgmi_topology_info *topology)
+{
+   return 0;
+}
+
+static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
+{
+   u64 hive_id = 0;
+
+   /* Remove me when we can get correct hive_id through PSP */
+   if (psp->adev->gmc.xgmi.num_physical_nodes)
+   hive_id = 0x123456789abcdef;
+
+   return hive_id;
+}
+
 static const struct psp_funcs psp_v11_0_funcs = {
.init_microcode = psp_v11_0_init_microcode,
.bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
@@ -560,6 +586,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
.cmd_submit = psp_v11_0_cmd_submit,
.compare_sram_data = psp_v11_0_compare_sram_data,
.mode1_reset = psp_v11_0_mode1_reset,
+   .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
+   .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
+   .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
 };
 
 void psp_v11_0_set_psp_funcs(struct psp_context *psp)
-- 
2.7.4

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

[PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset

[shaoyunl]

From: Alex Deucher 

On hives with xgmi enabled, the fb_location aperture is a size
which defines the total framebuffer size of all nodes in the
hive.  Each GPU in the hive has the same view via the fb_location
aperture.  GPU0 starts at offset (0 * segment size),
GPU1 starts at offset (1 * segment size), etc.

For access to local vram on each GPU, we need to take this offset into
account. This including on setting up GPUVM page table and GART table

Change-Id: I9efd510bed68fdb9afdfbdc76e1046792471ee78
Acked-by: Huang Rui 
Acked-by: Slava Abramov 
Signed-off-by: Shaoyun Liu 
Signed-off-by: Alex Deucher 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  | 20 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c |  3 +++
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c|  6 ++
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  |  7 +++
 5 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 6acdeeb..cf97c1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -147,8 +147,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, 
struct amdgpu_gmc *mc)
/* VCE doesn't like it when BOs cross a 4GB segment, so align
 * the GART base on a 4GB boundary as well.
 */
-   size_bf = mc->vram_start;
-   size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->vram_end + 1, four_gb);
+   size_bf = mc->fb_start;
+   size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->fb_end + 1, four_gb);
 
if (mc->gart_size > max(size_bf, size_af)) {
dev_warn(adev->dev, "limiting GART\n");
@@ -184,23 +184,23 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev, 
struct amdgpu_gmc *mc)
const uint64_t sixteen_gb_mask = ~(sixteen_gb - 1);
u64 size_af, size_bf;
 
-   if (mc->vram_start > mc->gart_start) {
-   size_bf = (mc->vram_start & sixteen_gb_mask) -
+   if (mc->fb_start > mc->gart_start) {
+   size_bf = (mc->fb_start & sixteen_gb_mask) -
ALIGN(mc->gart_end + 1, sixteen_gb);
-   size_af = mc->mc_mask + 1 - ALIGN(mc->vram_end + 1, sixteen_gb);
+   size_af = mc->mc_mask + 1 - ALIGN(mc->fb_end + 1, sixteen_gb);
} else {
-   size_bf = mc->vram_start & sixteen_gb_mask;
+   size_bf = mc->fb_start & sixteen_gb_mask;
size_af = (mc->gart_start & sixteen_gb_mask) -
-   ALIGN(mc->vram_end + 1, sixteen_gb);
+   ALIGN(mc->fb_end + 1, sixteen_gb);
}
 
if (size_bf > size_af) {
-   mc->agp_start = mc->vram_start > mc->gart_start ?
+   mc->agp_start = mc->fb_start > mc->gart_start ?
mc->gart_end + 1 : 0;
mc->agp_size = size_bf;
} else {
-   mc->agp_start = (mc->vram_start > mc->gart_start ?
-   mc->vram_end : mc->gart_end) + 1,
+   mc->agp_start = (mc->fb_start > mc->gart_start ?
+   mc->fb_end : mc->gart_end) + 1,
mc->agp_size = size_af;
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index a929a55..df96dfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -114,6 +114,13 @@ struct amdgpu_gmc {
u64 gart_end;
u64 vram_start;
u64 vram_end;
+   /* FB region , it's same as local vram region in single GPU, in XGMI
+* configuration, this region covers all GPUs in the same hive ,
+* each GPU in the hive has the same view of this FB region .
+* GPU0's vram starts at offset (0 * segment size) ,
+* GPU1 starts at offset (1 * segment size), etc.   */
+   u64 fb_start;
+   u64 fb_end;
unsignedvram_width;
u64 real_vram_size;
int vram_mtrr;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
index d4170cb..5e9ab8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
@@ -44,6 +44,9 @@ int gfxhub_v1_1_get_xgmi_info(struct amdgpu_device *adev)
REG_GET_FIELD(xgmi_lfb_cntl, MC_VM_XGMI_LFB_CNTL, 
PF_LFB_REGION);
if (adev->gmc.xgmi.physical_node_id > 3)
return -EINVAL;
+   adev->gmc.xgmi.node_segment_size = REG_GET_FIELD(
+   RREG32_SOC15(GC, 0, mmMC_VM_XGMI_LFB_SIZE),
+   MC_VM_XGMI_LFB_SIZE, PF_LFB_SIZE) << 24;
}
 
return 0;
diff --git 

[PATCH v2] drm/amd: Add DMCU firmware loading on raven

[David Francis]

[Why]
DMCU (Display MicroController Unit) is an on-GPU microcontroller
in AMD graphics cards that is used in features for
embedded displays such as Panel Self-Refresh

DMCU is part of the DM IP block

[How]
DMCU is added as an option in the enum AMDGPU_UCODE_ID

DMCU needs two pieces of firmware - the initial eram and the
interrupt vectors.  These are treated as seperate pieces of
firmware and loaded by PSP

The loading occurs in the sw_init hook of DM

If the firmware is not found, the sw_init hook returns without error.
DMCU is not a requirement for DM to run.

v2: Move dmcu firmware loading into its own function, properly
release firmware, add debug messages

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |   2 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|   6 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 114 +-
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index b358e7519987..38d3af317aa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
AMDGPU_UCODE_ID_UVD1,
AMDGPU_UCODE_ID_VCE,
AMDGPU_UCODE_ID_VCN,
+   AMDGPU_UCODE_ID_DMCU_ERAM,
+   AMDGPU_UCODE_ID_DMCU_INTV,
AMDGPU_UCODE_ID_MAXIMUM,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 02be34e72ed9..240dc8c85867 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
enum psp_gfx_fw_type *
case AMDGPU_UCODE_ID_VCN:
*type = GFX_FW_TYPE_VCN;
break;
+   case AMDGPU_UCODE_ID_DMCU_ERAM:
+   *type = GFX_FW_TYPE_DMCU_ERAM;
+   break;
+   case AMDGPU_UCODE_ID_DMCU_INTV:
+   *type = GFX_FW_TYPE_DMCU_ISR;
+   break;
case AMDGPU_UCODE_ID_MAXIMUM:
default:
return -EINVAL;
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 5103eba75cb3..016da243dde8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -30,6 +30,7 @@
 #include "vid.h"
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_ucode.h"
 #include "atom.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_pm.h"
@@ -50,6 +51,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -71,6 +73,12 @@
 
 #include "modules/inc/mod_freesync.h"
 
+#define FIRMWARE_RAVEN_DMCU_ERAM   "amdgpu/raven_dmcu_eram.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
+
+#define FIRMWARE_RAVEN_DMCU_INTV   "amdgpu/raven_dmcu_intv.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
+
 /* basic init/fini API */
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
@@ -514,11 +522,115 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
return;
 }
 
-static int dm_sw_init(void *handle)
+static int load_dmcu_fw(struct amdgpu_device *adev)
 {
+   const struct firmware *fw;
+   const char *fw_name_dmcu_eram;
+   const char *fw_name_dmcu_intv;
+   int r;
+   const struct common_firmware_header *hdr;
+
+   switch(adev->asic_type) {
+   case CHIP_BONAIRE:
+   case CHIP_HAWAII:
+   case CHIP_KAVERI:
+   case CHIP_KABINI:
+   case CHIP_MULLINS:
+   case CHIP_TONGA:
+   case CHIP_FIJI:
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   case CHIP_POLARIS11:
+   case CHIP_POLARIS10:
+   case CHIP_POLARIS12:
+   case CHIP_VEGAM:
+   case CHIP_VEGA10:
+   case CHIP_VEGA12:
+   case CHIP_VEGA20:
+   return 0;
+   case CHIP_RAVEN:
+   fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
+   fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
+   break;
+   default:
+   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
+   return -1;
+   }
+
+   if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
+   {
+   DRM_DEBUG_KMS("dm: DMCU firmware not supported on direct or SMU 
loading\n");
+   return 0;
+   }
+
+   r = request_firmware(, fw_name_dmcu_eram, adev->dev);
+   if (r == -ENOENT)
+   {
+   /* DMCU firmware is not necessary, so don't raise a fuss if 
it's missing */
+   DRM_DEBUG_KMS("dm: DMCU firmware not found\n");
+   return 0;
+   }
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
+   fw_name_dmcu_eram);
+   return r;
+   }
+
+   r = amdgpu_ucode_validate(fw);
+  

Re: [PATCH] drm/amd: Add DMCU firmware loading on raven

[Harry Wentland]

On 2018-09-07 01:49 PM, David Francis wrote:
> [Why]
> DMCU (Display MicroController Unit) is an on-GPU microcontroller
> in AMD graphics cards that is used in features for
> embedded displays such as Panel Self-Refresh
> 
> DMCU is part of the DM IP block
> 
> [How]
> DMCU is added as an option in the enum AMDGPU_UCODE_ID
> 
> DMCU needs two pieces of firmware - the initial eram and the
> interrupt vectors.  These are treated as seperate pieces of
> firmware and loaded by PSP
> 
> The loading occurs in the sw_init hook of DM
> 
> If the firmware is not found, the sw_init hook returns without error.
> DMCU is not a requirement for DM to run.
> 
> Signed-off-by: David Francis 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 98 +++
>  3 files changed, 106 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index b358e7519987..38d3af317aa2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
>   AMDGPU_UCODE_ID_UVD1,
>   AMDGPU_UCODE_ID_VCE,
>   AMDGPU_UCODE_ID_VCN,
> + AMDGPU_UCODE_ID_DMCU_ERAM,
> + AMDGPU_UCODE_ID_DMCU_INTV,
>   AMDGPU_UCODE_ID_MAXIMUM,
>  };
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 02be34e72ed9..240dc8c85867 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
> enum psp_gfx_fw_type *
>   case AMDGPU_UCODE_ID_VCN:
>   *type = GFX_FW_TYPE_VCN;
>   break;
> + case AMDGPU_UCODE_ID_DMCU_ERAM:
> + *type = GFX_FW_TYPE_DMCU_ERAM;
> + break;
> + case AMDGPU_UCODE_ID_DMCU_INTV:
> + *type = GFX_FW_TYPE_DMCU_ISR;
> + break;
>   case AMDGPU_UCODE_ID_MAXIMUM:
>   default:
>   return -EINVAL;
> 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 5103eba75cb3..8ad0ee359ef8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -30,6 +30,7 @@
>  #include "vid.h"
>  #include "amdgpu.h"
>  #include "amdgpu_display.h"
> +#include "amdgpu_ucode.h"
>  #include "atom.h"
>  #include "amdgpu_dm.h"
>  #include "amdgpu_pm.h"
> @@ -50,6 +51,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -71,6 +73,12 @@
>  
>  #include "modules/inc/mod_freesync.h"
>  
> +#define FIRMWARE_RAVEN_DMCU_ERAM "amdgpu/raven_dmcu_eram.bin"
> +MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
> +
> +#define FIRMWARE_RAVEN_DMCU_INTV "amdgpu/raven_dmcu_intv.bin"
> +MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
> +
>  /* basic init/fini API */
>  static int amdgpu_dm_init(struct amdgpu_device *adev);
>  static void amdgpu_dm_fini(struct amdgpu_device *adev);
> @@ -516,6 +524,96 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
>  
>  static int dm_sw_init(void *handle)
>  {

Move the DMCU loading code into a separate function, even though it's the only 
one called here.

> + const struct firmware *fw;
> + const char *fw_name_dmcu_eram;
> + const char *fw_name_dmcu_intv;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int r;
> +
> + switch(adev->asic_type) {
> + case CHIP_BONAIRE:
> + case CHIP_HAWAII:
> + case CHIP_KAVERI:
> + case CHIP_KABINI:
> + case CHIP_MULLINS:
> + case CHIP_TONGA:
> + case CHIP_FIJI:
> + case CHIP_CARRIZO:
> + case CHIP_STONEY:
> + case CHIP_POLARIS11:
> + case CHIP_POLARIS10:
> + case CHIP_POLARIS12:
> + case CHIP_VEGAM:
> + case CHIP_VEGA10:
> + case CHIP_VEGA12:
> + case CHIP_VEGA20:
> + return 0;
> + case CHIP_RAVEN:
> + fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
> + fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
> + break;
> + default:
> + DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> + return -1;
> + }
> +
> + r = request_firmware(, fw_name_dmcu_eram, adev->dev);
> + if (r == -ENOENT)
> + {
> + /* DMCU firmware is not necessary, so don't raise a fuss if 
> it's missing */

Maybe add a DRM_DEBUG_KMS here. For debug purposes we might still want to know 
whether firmware is available or not when testing features supported by DMCU.

> + return 0;
> + }
> + if (r) {
> + dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
> + fw_name_dmcu_eram);
> + return r;
> + }
> +
> + r = 

Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[Christian König]

Ok in this case feel free to add my Acked-by.

Christian.

Am 07.09.2018 um 20:10 schrieb Felix Kuehling:

On 2018-09-07 05:05 AM, Christian König wrote:

Well, exactly that's my point. How certain are we that the PSP
firmware interface is not going to change?

The interface has been defined and agreed on and is being implemented at
the moment. The placeholders here are based on that design. Of course
that's not a guarantee that it won't change, but it's the best we can do
to be prepared and allow us to make progress in the mean time.


At bare minimum we should add big "/* TODOD: .. */" markers all around
explaining why those functions aren't implemented yet.

OK.

Regards,
   Felix


Thanks,
Christian.

Am 07.09.2018 um 08:03 schrieb Kuehling, Felix:

This is on purpose. These functions for now are place holders because
the PSP firmware interface is not ready yet, and we want to start
testing XGMI with higher level code with some hard-coded topology.
Once we have proper PSP firmware, these place holders will be filled in.

Regards,
    Felix


From: amd-gfx  on behalf of
Christian König 
Sent: Thursday, September 6, 2018 3:42:33 AM
To: Liu, Shaoyun; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for
xgmi topology interface with psp

Am 05.09.2018 um 17:30 schrieb shaoyunl:

From: Shaoyun Liu 

Add dummy function for xgmi function interface with psp

Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
Signed-off-by: Shaoyun Liu 
Reviewed-by: Felix Kuehling 

Well NAK. We usually don't add dummy functions and that completely
circumvents the error handling added in patch #6.

Either we should implement the functions or not call them in the first
place.

Christian.


---
    drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 26
++
    1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index b70cfa3..b1c0b33 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -548,6 +548,29 @@ static int psp_v11_0_mode1_reset(struct
psp_context *psp)
    return 0;
    }

+static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
+ int number_devices, struct psp_xgmi_topology_info *topology)
+{
+ return 0;
+}
+
+static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
+ int number_devices, struct psp_xgmi_topology_info *topology)
+{
+ return 0;
+}
+
+static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
+{
+ u64 hive_id = 0;
+
+ /* Remove me when normal psp interface is ready */
+ if (psp->adev->gmc.xgmi.num_physical_nodes)
+ hive_id = 0x123456789abcdef;
+
+ return hive_id;
+}
+
    static const struct psp_funcs psp_v11_0_funcs = {
    .init_microcode = psp_v11_0_init_microcode,
    .bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
@@ -560,6 +583,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
    .cmd_submit = psp_v11_0_cmd_submit,
    .compare_sram_data = psp_v11_0_compare_sram_data,
    .mode1_reset = psp_v11_0_mode1_reset,
+ .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
+ .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
+ .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
    };

    void psp_v11_0_set_psp_funcs(struct psp_context *psp)

___
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 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[Felix Kuehling]

On 2018-09-07 05:05 AM, Christian König wrote:
> Well, exactly that's my point. How certain are we that the PSP
> firmware interface is not going to change?

The interface has been defined and agreed on and is being implemented at
the moment. The placeholders here are based on that design. Of course
that's not a guarantee that it won't change, but it's the best we can do
to be prepared and allow us to make progress in the mean time.

>
> At bare minimum we should add big "/* TODOD: .. */" markers all around
> explaining why those functions aren't implemented yet.

OK.

Regards,
  Felix

>
> Thanks,
> Christian.
>
> Am 07.09.2018 um 08:03 schrieb Kuehling, Felix:
>> This is on purpose. These functions for now are place holders because
>> the PSP firmware interface is not ready yet, and we want to start
>> testing XGMI with higher level code with some hard-coded topology.
>> Once we have proper PSP firmware, these place holders will be filled in.
>>
>> Regards,
>>    Felix
>>
>> 
>> From: amd-gfx  on behalf of
>> Christian König 
>> Sent: Thursday, September 6, 2018 3:42:33 AM
>> To: Liu, Shaoyun; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for
>> xgmi topology interface with psp
>>
>> Am 05.09.2018 um 17:30 schrieb shaoyunl:
>>> From: Shaoyun Liu 
>>>
>>> Add dummy function for xgmi function interface with psp
>>>
>>> Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
>>> Signed-off-by: Shaoyun Liu 
>>> Reviewed-by: Felix Kuehling 
>> Well NAK. We usually don't add dummy functions and that completely
>> circumvents the error handling added in patch #6.
>>
>> Either we should implement the functions or not call them in the first
>> place.
>>
>> Christian.
>>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 26
>>> ++
>>>    1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>> index b70cfa3..b1c0b33 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>> @@ -548,6 +548,29 @@ static int psp_v11_0_mode1_reset(struct
>>> psp_context *psp)
>>>    return 0;
>>>    }
>>>
>>> +static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
>>> + int number_devices, struct psp_xgmi_topology_info *topology)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
>>> + int number_devices, struct psp_xgmi_topology_info *topology)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
>>> +{
>>> + u64 hive_id = 0;
>>> +
>>> + /* Remove me when normal psp interface is ready */
>>> + if (psp->adev->gmc.xgmi.num_physical_nodes)
>>> + hive_id = 0x123456789abcdef;
>>> +
>>> + return hive_id;
>>> +}
>>> +
>>>    static const struct psp_funcs psp_v11_0_funcs = {
>>>    .init_microcode = psp_v11_0_init_microcode,
>>>    .bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
>>> @@ -560,6 +583,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
>>>    .cmd_submit = psp_v11_0_cmd_submit,
>>>    .compare_sram_data = psp_v11_0_compare_sram_data,
>>>    .mode1_reset = psp_v11_0_mode1_reset,
>>> + .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
>>> + .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
>>> + .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
>>>    };
>>>
>>>    void psp_v11_0_set_psp_funcs(struct psp_context *psp)
>> ___
>> 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] drm/amd: Add DMCU firmware loading on raven

[Francis, David]

Ignore this thread - new, fixed patch is up


From: Francis, David
Sent: September 7, 2018 10:26:59 AM
To: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd: Add DMCU firmware loading on raven


This will cause amdgpu startup to fail if the firmware does not exist: this 
should either wait until the firmware is available or not return an error value 
if -ENOENT is returned from request_firmware


From: David Francis 
Sent: September 7, 2018 10:16:56 AM
To: amd-gfx@lists.freedesktop.org
Cc: Francis, David
Subject: [PATCH] drm/amd: Add DMCU firmware loading on raven

[Why]
DMCU (Display MicroController Unit) is an on-GPU microcontroller
in AMD graphics cards that is used in features for
embedded displays such as Panel Self-Refresh

DMCU is part of the DM IP block

[How]
DMCU is added as an option in the enum AMDGPU_UCODE_ID

DMCU needs two pieces of firmware - the initial eram and the
interrupt vectors.  These are treated as seperate pieces of
firmware and loaded by PSP

The loading occurs in the sw_init hook of DM

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 93 +++
 3 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index b358e7519987..38d3af317aa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
 AMDGPU_UCODE_ID_UVD1,
 AMDGPU_UCODE_ID_VCE,
 AMDGPU_UCODE_ID_VCN,
+   AMDGPU_UCODE_ID_DMCU_ERAM,
+   AMDGPU_UCODE_ID_DMCU_INTV,
 AMDGPU_UCODE_ID_MAXIMUM,
 };

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 02be34e72ed9..240dc8c85867 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
enum psp_gfx_fw_type *
 case AMDGPU_UCODE_ID_VCN:
 *type = GFX_FW_TYPE_VCN;
 break;
+   case AMDGPU_UCODE_ID_DMCU_ERAM:
+   *type = GFX_FW_TYPE_DMCU_ERAM;
+   break;
+   case AMDGPU_UCODE_ID_DMCU_INTV:
+   *type = GFX_FW_TYPE_DMCU_ISR;
+   break;
 case AMDGPU_UCODE_ID_MAXIMUM:
 default:
 return -EINVAL;
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 5103eba75cb3..4619f624f346 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -30,6 +30,7 @@
 #include "vid.h"
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_ucode.h"
 #include "atom.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_pm.h"
@@ -50,6 +51,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -71,6 +73,12 @@

 #include "modules/inc/mod_freesync.h"

+#define FIRMWARE_RAVEN_DMCU_ERAM   "amdgpu/raven_dmcu_eram.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
+
+#define FIRMWARE_RAVEN_DMCU_INTV   "amdgpu/raven_dmcu_intv.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
+
 /* basic init/fini API */
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
@@ -516,6 +524,91 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)

 static int dm_sw_init(void *handle)
 {
+   const struct firmware *fw;
+   const char *fw_name_dmcu_eram;
+   const char *fw_name_dmcu_intv;
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   int r;
+
+   switch(adev->asic_type) {
+   case CHIP_BONAIRE:
+   case CHIP_HAWAII:
+   case CHIP_KAVERI:
+   case CHIP_KABINI:
+   case CHIP_MULLINS:
+   case CHIP_TONGA:
+   case CHIP_FIJI:
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   case CHIP_POLARIS11:
+   case CHIP_POLARIS10:
+   case CHIP_POLARIS12:
+   case CHIP_VEGAM:
+   case CHIP_VEGA10:
+   case CHIP_VEGA12:
+   case CHIP_VEGA20:
+   return 0;
+   case CHIP_RAVEN:
+   fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
+   fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
+   break;
+   default:
+   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
+   return -1;
+   }
+
+   r = request_firmware(, fw_name_dmcu_eram, adev->dev);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
+   fw_name_dmcu_eram);
+   return r;
+   }
+
+   r = amdgpu_ucode_validate(fw);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: 

[PATCH] drm/amd: Add DMCU firmware loading on raven

[David Francis]

[Why]
DMCU (Display MicroController Unit) is an on-GPU microcontroller
in AMD graphics cards that is used in features for
embedded displays such as Panel Self-Refresh

DMCU is part of the DM IP block

[How]
DMCU is added as an option in the enum AMDGPU_UCODE_ID

DMCU needs two pieces of firmware - the initial eram and the
interrupt vectors.  These are treated as seperate pieces of
firmware and loaded by PSP

The loading occurs in the sw_init hook of DM

If the firmware is not found, the sw_init hook returns without error.
DMCU is not a requirement for DM to run.

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 98 +++
 3 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index b358e7519987..38d3af317aa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
AMDGPU_UCODE_ID_UVD1,
AMDGPU_UCODE_ID_VCE,
AMDGPU_UCODE_ID_VCN,
+   AMDGPU_UCODE_ID_DMCU_ERAM,
+   AMDGPU_UCODE_ID_DMCU_INTV,
AMDGPU_UCODE_ID_MAXIMUM,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 02be34e72ed9..240dc8c85867 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
enum psp_gfx_fw_type *
case AMDGPU_UCODE_ID_VCN:
*type = GFX_FW_TYPE_VCN;
break;
+   case AMDGPU_UCODE_ID_DMCU_ERAM:
+   *type = GFX_FW_TYPE_DMCU_ERAM;
+   break;
+   case AMDGPU_UCODE_ID_DMCU_INTV:
+   *type = GFX_FW_TYPE_DMCU_ISR;
+   break;
case AMDGPU_UCODE_ID_MAXIMUM:
default:
return -EINVAL;
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 5103eba75cb3..8ad0ee359ef8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -30,6 +30,7 @@
 #include "vid.h"
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_ucode.h"
 #include "atom.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_pm.h"
@@ -50,6 +51,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -71,6 +73,12 @@
 
 #include "modules/inc/mod_freesync.h"
 
+#define FIRMWARE_RAVEN_DMCU_ERAM   "amdgpu/raven_dmcu_eram.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
+
+#define FIRMWARE_RAVEN_DMCU_INTV   "amdgpu/raven_dmcu_intv.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
+
 /* basic init/fini API */
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
@@ -516,6 +524,96 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
 
 static int dm_sw_init(void *handle)
 {
+   const struct firmware *fw;
+   const char *fw_name_dmcu_eram;
+   const char *fw_name_dmcu_intv;
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   int r;
+
+   switch(adev->asic_type) {
+   case CHIP_BONAIRE:
+   case CHIP_HAWAII:
+   case CHIP_KAVERI:
+   case CHIP_KABINI:
+   case CHIP_MULLINS:
+   case CHIP_TONGA:
+   case CHIP_FIJI:
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   case CHIP_POLARIS11:
+   case CHIP_POLARIS10:
+   case CHIP_POLARIS12:
+   case CHIP_VEGAM:
+   case CHIP_VEGA10:
+   case CHIP_VEGA12:
+   case CHIP_VEGA20:
+   return 0;
+   case CHIP_RAVEN:
+   fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
+   fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
+   break;
+   default:
+   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
+   return -1;
+   }
+
+   r = request_firmware(, fw_name_dmcu_eram, adev->dev);
+   if (r == -ENOENT)
+   {
+   /* DMCU firmware is not necessary, so don't raise a fuss if 
it's missing */
+   return 0;
+   }
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
+   fw_name_dmcu_eram);
+   return r;
+   }
+
+   r = amdgpu_ucode_validate(fw);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't validate firmware 
\"%s\"\n",
+   fw_name_dmcu_eram);
+   release_firmware(fw);
+   fw = NULL;
+   return r;
+   }
+
+   if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
+   const struct common_firmware_header *hdr;
+   hdr = (const struct common_firmware_header *)fw->data;
+  

Re: [PATCH] drm/amdgpu: Fix wornings while make xmldocs

[Michel Dänzer]

Hi Iida-san,


On 2018-09-06 4:10 a.m., Masanari Iida wrote:
> This patch fixes following wornings.
> 
> ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3011:
> warning: Excess function parameter 'dev' description
> in 'amdgpu_vm_get_task_info'
> 
> ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3012:
> warning: Function parameter or member 'adev' not
> described in 'amdgpu_vm_get_task_info'
> 
> ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3012:
> warning: Excess function parameter 'dev' description
> in 'amdgpu_vm_get_task_info'
> 
> Signed-off-by: Masanari Iida 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b17771dd5ce7..861718e2be7a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -3002,7 +3002,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
> struct drm_file *filp)
>  /**
>   * amdgpu_vm_get_task_info - Extracts task info for a PASID.
>   *
> - * @dev: drm device pointer
> + * @adev: drm device pointer
>   * @pasid: PASID identifier for VM
>   * @task_info: task_info to fill.
>   */
> 

Queued in the amdgpu tree (with fixed spelling of "warnings"), thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-ati] Bail early from drm_wait_pending_flip if there's no pending flip

[Michel Dänzer]

From: Michel Dänzer 

No need to process any events in that case.

(Ported from amdgpu commit ca5eb9894fff153c0a1df7bdc4a4745713309e27)

Signed-off-by: Michel Dänzer 
---
 src/radeon_drm_queue.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c
index bf1650ea9..ea78e8e2b 100644
--- a/src/radeon_drm_queue.c
+++ b/src/radeon_drm_queue.c
@@ -284,7 +284,8 @@ void radeon_drm_wait_pending_flip(xf86CrtcPtr crtc)
 
 drmmode_crtc->wait_flip_nesting_level++;
 
-while (!xorg_list_is_empty(_drm_flip_signalled)) {
+while (drmmode_crtc->flip_pending &&
+  !xorg_list_is_empty(_drm_flip_signalled)) {
e = xorg_list_first_entry(_drm_flip_signalled,
  struct radeon_drm_queue_entry, list);
radeon_drm_queue_handle_one(e);
-- 
2.19.0.rc1

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

Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Christian König]

The VM doesn't know that information either.

In other words when you request the address of a PT you get the address 
where memory management has moved it, not the address where the hardware 
is processing the fault from.


I see only a few possible solutions for that:
1. We use an extra structure in the kernel driver which holds the 
current address of the PTs and is feed by memory management when buffers 
move around.
2. We change the SDMA firmware to do the walk for us and update the PTEs 
based on the information in the page directory.
3. We use the UTCL walker to get us to the correct address which is then 
updated with the SDMA.


Regards,
Christian.

Am 07.09.2018 um 17:30 schrieb Zeng, Oak:


Without a VM, how can you get the physical address of the faulty 
virtual address? Where this information should be hold?


Thanks,

Oak

*From:*Koenig, Christian
*Sent:* Monday, September 10, 2018 7:20 AM
*To:* Zeng, Oak ; Oak Zeng ; 
amd-gfx@lists.freedesktop.org; Yang, Philip 

*Subject:* Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Am I right that the handling of page fault need a valid VM?

No, exactly that view is incorrect.

The VM is meant to be a memory management structure of page tables and 
is completely pointless fault processing because it represent the 
future state of the page tables and not the current one.


What we need for fault processing is a new structure build around 
PASIDs which is feed by the with addresses when page tables are moved 
around.


Alternatively I hope that we can use the SDMA to walk the page tables 
and update the required entries by just using the address.


Regards,
Christian.

Am 07.09.2018 um 16:55 schrieb Zeng, Oak:

Hi Christian,

What is the value of delay the destroy of hash to after vm is
destroyed? Since vm is destroyed, you basically don’t have enough
information to paging in the correct page to gpuvm. Am I right
that the handling of page fault need a valid VM? For example, you
need the VM to get the corresponding physical address of the
faulty page.

After vm is destroyed, the retry interrupt can be directly discard
as you can’t find vm through pasid. You can think this is also one
kind of prescreen.

So I am stand for the point that, there is no need to delay the
destroy of hash to after vm is destroyed. Prescreening hash can be
destroyed at the time of vm_fini.

Thanks,

Oak

*From:*Christian König 

*Sent:* Friday, September 07, 2018 4:39 AM
*To:* Zeng, Oak  ; Oak
Zeng  ;
amd-gfx@lists.freedesktop.org 
*Cc:* Zeng, Oak  
*Subject:* Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to
amdgpu vm

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a
suggestion but rather a must have.

The VM structures must be destroyed while the processing is still
ongoing, or otherwise we would not have a clean OOM handling.

The IDR for PASID lockup can be put into amdgpu_ids.c, you can
just replace the IDA already used there with an IDR.

Since the PASID is already freed up delayed we should have the
grace period for interrupt processing included. If that still
isn't sufficient we can still add some delayed work for this.

Regards,
Christian.

Am 07.09.2018 um 06:16 schrieb ozeng:

Hi Christian,

In this implementation, fault hash is made per vm, not per
pasid as suggested, based on below considerations:

  * Delay the destroy of hash introduces more effort like how
to set the proper grace period after which no retry
interrupt will be generated. We want to avoid those
complication if we can.
  * The  problem of the per vm implementation is that it is
hard to delay the destroy of fault hash, because when vm
is destroyed, prescreen function won't be able to retrieve
the fault hash. But in this case, the prescreen function
can either let the interrupt go through (to gmc) or ignore
it. From this perspective, it is not very necessary to
delay the destroy of hash table.
  * On the other hand, since ASICs after vega10 have the HW
CAM feature. So the SW IV prescreen is only useful for
vega10. For all the HW implemented CAM, we can consider
the delayed CAM acknowledgment.
  * Making it per pasid need to introduce another idr to
correlate pasid and hash table - where to put the idr? You
will have to make it a global variable which is not very
desirable.

The main purpose of this patch is to solve the inter-process
lock issue (when hash table is full), while still keep codes

RE: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Zeng, Oak]

Without a VM, how can you get the physical address of the faulty virtual 
address? Where this information should be hold?

Thanks,
Oak

From: Koenig, Christian
Sent: Monday, September 10, 2018 7:20 AM
To: Zeng, Oak ; Oak Zeng ; 
amd-gfx@lists.freedesktop.org; Yang, Philip 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Am I right that the handling of page fault need a valid VM?
No, exactly that view is incorrect.

The VM is meant to be a memory management structure of page tables and is 
completely pointless fault processing because it represent the future state of 
the page tables and not the current one.

What we need for fault processing is a new structure build around PASIDs which 
is feed by the with addresses when page tables are moved around.

Alternatively I hope that we can use the SDMA to walk the page tables and 
update the required entries by just using the address.

Regards,
Christian.

Am 07.09.2018 um 16:55 schrieb Zeng, Oak:
Hi Christian,

What is the value of delay the destroy of hash to after vm is destroyed? Since 
vm is destroyed, you basically don’t have enough information to paging in the 
correct page to gpuvm. Am I right that the handling of page fault need a valid 
VM? For example, you need the VM to get the corresponding physical address of 
the faulty page.

After vm is destroyed, the retry interrupt can be directly discard as you can’t 
find vm through pasid. You can think this is also one kind of prescreen.

So I am stand for the point that, there is no need to delay the destroy of hash 
to after vm is destroyed. Prescreening hash can be destroyed at the time of 
vm_fini.

Thanks,
Oak

From: Christian König 

Sent: Friday, September 07, 2018 4:39 AM
To: Zeng, Oak ; Oak Zeng 
; 
amd-gfx@lists.freedesktop.org
Cc: Zeng, Oak 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a suggestion 
but rather a must have.

The VM structures must be destroyed while the processing is still ongoing, or 
otherwise we would not have a clean OOM handling.

The IDR for PASID lockup can be put into amdgpu_ids.c, you can just replace the 
IDA already used there with an IDR.

Since the PASID is already freed up delayed we should have the grace period for 
interrupt processing included. If that still isn't sufficient we can still add 
some delayed work for this.

Regards,
Christian.

Am 07.09.2018 um 06:16 schrieb ozeng:

Hi Christian,

In this implementation, fault hash is made per vm, not per pasid as suggested, 
based on below considerations:

  *   Delay the destroy of hash introduces more effort like how to set the 
proper grace period after which no retry interrupt will be generated. We want 
to avoid those complication if we can.
  *   The  problem of the per vm implementation is that it is hard to delay the 
destroy of fault hash, because when vm is destroyed, prescreen function won't 
be able to retrieve the fault hash. But in this case, the prescreen function 
can either let the interrupt go through (to gmc) or ignore it. From this 
perspective, it is not very necessary to delay the destroy of hash table.
  *   On the other hand, since ASICs after vega10 have the HW CAM feature. So 
the SW IV prescreen is only useful for vega10. For all the HW implemented CAM, 
we can consider the delayed CAM acknowledgment.
  *   Making it per pasid need to introduce another idr to correlate pasid and 
hash table - where to put the idr? You will have to make it a global variable 
which is not very desirable.

The main purpose of this patch is to solve the inter-process lock issue (when 
hash table is full), while still keep codes simple.

Also with this patch, the faults kfifo is not necessary any more. See patch 2.

Regards,

Oak

On 09/06/2018 11:28 PM, Oak Zeng wrote:

In stead of share one fault hash table per device, make it

per vm. This can avoid inter-process lock issue when fault

hash table is full.



Change-Id: I5d1281b7c41eddc8e26113e010516557588d3708

Signed-off-by: Oak Zeng 

Suggested-by: Christian Konig 


Suggested-by: Felix Kuehling 


---

 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |  75 

 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h |  10 

 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 102 -

 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  12 

 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  38 +---

 5 files changed, 127 insertions(+), 110 deletions(-)



diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

index 06373d4..4ed8621 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Christian König]

Am I right that the handling of page fault need a valid VM? 

No, exactly that view is incorrect.

The VM is meant to be a memory management structure of page tables and 
is completely pointless fault processing because it represent the future 
state of the page tables and not the current one.


What we need for fault processing is a new structure build around PASIDs 
which is feed by the with addresses when page tables are moved around.


Alternatively I hope that we can use the SDMA to walk the page tables 
and update the required entries by just using the address.


Regards,
Christian.

Am 07.09.2018 um 16:55 schrieb Zeng, Oak:


Hi Christian,

What is the value of delay the destroy of hash to after vm is 
destroyed? Since vm is destroyed, you basically don’t have enough 
information to paging in the correct page to gpuvm. Am I right that 
the handling of page fault need a valid VM? For example, you need the 
VM to get the corresponding physical address of the faulty page.


After vm is destroyed, the retry interrupt can be directly discard as 
you can’t find vm through pasid. You can think this is also one kind 
of prescreen.


So I am stand for the point that, there is no need to delay the 
destroy of hash to after vm is destroyed. Prescreening hash can be 
destroyed at the time of vm_fini.


Thanks,

Oak

*From:*Christian König 
*Sent:* Friday, September 07, 2018 4:39 AM
*To:* Zeng, Oak ; Oak Zeng ; 
amd-gfx@lists.freedesktop.org

*Cc:* Zeng, Oak 
*Subject:* Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a 
suggestion but rather a must have.


The VM structures must be destroyed while the processing is still 
ongoing, or otherwise we would not have a clean OOM handling.


The IDR for PASID lockup can be put into amdgpu_ids.c, you can just 
replace the IDA already used there with an IDR.


Since the PASID is already freed up delayed we should have the grace 
period for interrupt processing included. If that still isn't 
sufficient we can still add some delayed work for this.


Regards,
Christian.

Am 07.09.2018 um 06:16 schrieb ozeng:

Hi Christian,

In this implementation, fault hash is made per vm, not per pasid
as suggested, based on below considerations:

  * Delay the destroy of hash introduces more effort like how to
set the proper grace period after which no retry interrupt
will be generated. We want to avoid those complication if we can.
  * The  problem of the per vm implementation is that it is hard
to delay the destroy of fault hash, because when vm is
destroyed, prescreen function won't be able to retrieve the
fault hash. But in this case, the prescreen function can
either let the interrupt go through (to gmc) or ignore it.
From this perspective, it is not very necessary to delay the
destroy of hash table.
  * On the other hand, since ASICs after vega10 have the HW CAM
feature. So the SW IV prescreen is only useful for vega10. For
all the HW implemented CAM, we can consider the delayed CAM
acknowledgment.
  * Making it per pasid need to introduce another idr to correlate
pasid and hash table - where to put the idr? You will have to
make it a global variable which is not very desirable.

The main purpose of this patch is to solve the inter-process lock
issue (when hash table is full), while still keep codes simple.

Also with this patch, the faults kfifo is not necessary any more.
See patch 2.

Regards,

Oak

On 09/06/2018 11:28 PM, Oak Zeng wrote:

In stead of share one fault hash table per device, make it

per vm. This can avoid inter-process lock issue when fault

hash table is full.

Change-Id: I5d1281b7c41eddc8e26113e010516557588d3708

Signed-off-by: Oak Zeng 

Suggested-by: Christian Konig 


Suggested-by: Felix Kuehling 


---

  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |  75 

  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h |  10 

  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 102 
-

  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  12 

  drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  38 +---

  5 files changed, 127 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

index 06373d4..4ed8621 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

@@ -197,78 +197,3 @@ int amdgpu_ih_process(struct amdgpu_device *adev)

    return IRQ_HANDLED;

  }

  


-/**

- * amdgpu_ih_add_fault - Add a page 

RE: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Zeng, Oak]

Hi Christian,

What is the value of delay the destroy of hash to after vm is destroyed? Since 
vm is destroyed, you basically don’t have enough information to paging in the 
correct page to gpuvm. Am I right that the handling of page fault need a valid 
VM? For example, you need the VM to get the corresponding physical address of 
the faulty page.

After vm is destroyed, the retry interrupt can be directly discard as you can’t 
find vm through pasid. You can think this is also one kind of prescreen.

So I am stand for the point that, there is no need to delay the destroy of hash 
to after vm is destroyed. Prescreening hash can be destroyed at the time of 
vm_fini.

Thanks,
Oak

From: Christian König 
Sent: Friday, September 07, 2018 4:39 AM
To: Zeng, Oak ; Oak Zeng ; 
amd-gfx@lists.freedesktop.org
Cc: Zeng, Oak 
Subject: Re: [PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a suggestion 
but rather a must have.

The VM structures must be destroyed while the processing is still ongoing, or 
otherwise we would not have a clean OOM handling.

The IDR for PASID lockup can be put into amdgpu_ids.c, you can just replace the 
IDA already used there with an IDR.

Since the PASID is already freed up delayed we should have the grace period for 
interrupt processing included. If that still isn't sufficient we can still add 
some delayed work for this.

Regards,
Christian.

Am 07.09.2018 um 06:16 schrieb ozeng:

Hi Christian,

In this implementation, fault hash is made per vm, not per pasid as suggested, 
based on below considerations:

  *   Delay the destroy of hash introduces more effort like how to set the 
proper grace period after which no retry interrupt will be generated. We want 
to avoid those complication if we can.
  *   The  problem of the per vm implementation is that it is hard to delay the 
destroy of fault hash, because when vm is destroyed, prescreen function won't 
be able to retrieve the fault hash. But in this case, the prescreen function 
can either let the interrupt go through (to gmc) or ignore it. From this 
perspective, it is not very necessary to delay the destroy of hash table.
  *   On the other hand, since ASICs after vega10 have the HW CAM feature. So 
the SW IV prescreen is only useful for vega10. For all the HW implemented CAM, 
we can consider the delayed CAM acknowledgment.
  *   Making it per pasid need to introduce another idr to correlate pasid and 
hash table - where to put the idr? You will have to make it a global variable 
which is not very desirable.

The main purpose of this patch is to solve the inter-process lock issue (when 
hash table is full), while still keep codes simple.

Also with this patch, the faults kfifo is not necessary any more. See patch 2.

Regards,

Oak

On 09/06/2018 11:28 PM, Oak Zeng wrote:

In stead of share one fault hash table per device, make it

per vm. This can avoid inter-process lock issue when fault

hash table is full.



Change-Id: I5d1281b7c41eddc8e26113e010516557588d3708

Signed-off-by: Oak Zeng 

Suggested-by: Christian Konig 


Suggested-by: Felix Kuehling 


---

 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |  75 

 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h |  10 

 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 102 -

 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  12 

 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  38 +---

 5 files changed, 127 insertions(+), 110 deletions(-)



diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

index 06373d4..4ed8621 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c

@@ -197,78 +197,3 @@ int amdgpu_ih_process(struct amdgpu_device *adev)

   return IRQ_HANDLED;

 }



-/**

- * amdgpu_ih_add_fault - Add a page fault record

- *

- * @adev: amdgpu device pointer

- * @key: 64-bit encoding of PASID and address

- *

- * This should be called when a retry page fault interrupt is

- * received. If this is a new page fault, it will be added to a hash

- * table. The return value indicates whether this is a new fault, or

- * a fault that was already known and is already being handled.

- *

- * If there are too many pending page faults, this will fail. Retry

- * interrupts should be ignored in this case until there is enough

- * free space.

- *

- * Returns 0 if the fault was added, 1 if the fault was already known,

- * -ENOSPC if there are too many pending faults.

- */

-int amdgpu_ih_add_fault(struct amdgpu_device *adev, u64 key)

-{

-  unsigned long flags;

-  int r = -ENOSPC;

-

-  if (WARN_ON_ONCE(!adev->irq.ih.faults))

-  /* Should be allocated in _ih_sw_init on GPUs that

-   * support retry faults and require retry filtering.

-   */

-  return r;

Re: [PATCH] drm/amd: Add DMCU firmware loading on raven

[Francis, David]

This will cause amdgpu startup to fail if the firmware does not exist: this 
should either wait until the firmware is available or not return an error value 
if -ENOENT is returned from request_firmware


From: David Francis 
Sent: September 7, 2018 10:16:56 AM
To: amd-gfx@lists.freedesktop.org
Cc: Francis, David
Subject: [PATCH] drm/amd: Add DMCU firmware loading on raven

[Why]
DMCU (Display MicroController Unit) is an on-GPU microcontroller
in AMD graphics cards that is used in features for
embedded displays such as Panel Self-Refresh

DMCU is part of the DM IP block

[How]
DMCU is added as an option in the enum AMDGPU_UCODE_ID

DMCU needs two pieces of firmware - the initial eram and the
interrupt vectors.  These are treated as seperate pieces of
firmware and loaded by PSP

The loading occurs in the sw_init hook of DM

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 93 +++
 3 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index b358e7519987..38d3af317aa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
 AMDGPU_UCODE_ID_UVD1,
 AMDGPU_UCODE_ID_VCE,
 AMDGPU_UCODE_ID_VCN,
+   AMDGPU_UCODE_ID_DMCU_ERAM,
+   AMDGPU_UCODE_ID_DMCU_INTV,
 AMDGPU_UCODE_ID_MAXIMUM,
 };

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 02be34e72ed9..240dc8c85867 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
enum psp_gfx_fw_type *
 case AMDGPU_UCODE_ID_VCN:
 *type = GFX_FW_TYPE_VCN;
 break;
+   case AMDGPU_UCODE_ID_DMCU_ERAM:
+   *type = GFX_FW_TYPE_DMCU_ERAM;
+   break;
+   case AMDGPU_UCODE_ID_DMCU_INTV:
+   *type = GFX_FW_TYPE_DMCU_ISR;
+   break;
 case AMDGPU_UCODE_ID_MAXIMUM:
 default:
 return -EINVAL;
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 5103eba75cb3..4619f624f346 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -30,6 +30,7 @@
 #include "vid.h"
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_ucode.h"
 #include "atom.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_pm.h"
@@ -50,6 +51,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -71,6 +73,12 @@

 #include "modules/inc/mod_freesync.h"

+#define FIRMWARE_RAVEN_DMCU_ERAM   "amdgpu/raven_dmcu_eram.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
+
+#define FIRMWARE_RAVEN_DMCU_INTV   "amdgpu/raven_dmcu_intv.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
+
 /* basic init/fini API */
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
@@ -516,6 +524,91 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)

 static int dm_sw_init(void *handle)
 {
+   const struct firmware *fw;
+   const char *fw_name_dmcu_eram;
+   const char *fw_name_dmcu_intv;
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   int r;
+
+   switch(adev->asic_type) {
+   case CHIP_BONAIRE:
+   case CHIP_HAWAII:
+   case CHIP_KAVERI:
+   case CHIP_KABINI:
+   case CHIP_MULLINS:
+   case CHIP_TONGA:
+   case CHIP_FIJI:
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   case CHIP_POLARIS11:
+   case CHIP_POLARIS10:
+   case CHIP_POLARIS12:
+   case CHIP_VEGAM:
+   case CHIP_VEGA10:
+   case CHIP_VEGA12:
+   case CHIP_VEGA20:
+   return 0;
+   case CHIP_RAVEN:
+   fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
+   fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
+   break;
+   default:
+   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
+   return -1;
+   }
+
+   r = request_firmware(, fw_name_dmcu_eram, adev->dev);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
+   fw_name_dmcu_eram);
+   return r;
+   }
+
+   r = amdgpu_ucode_validate(fw);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't validate firmware 
\"%s\"\n",
+   fw_name_dmcu_eram);
+   release_firmware(fw);
+   fw = NULL;
+   return r;
+   }
+
+   if (adev->firmware.load_type == 

[PATCH] drm/amd: Add DMCU firmware loading on raven

[David Francis]

[Why]
DMCU (Display MicroController Unit) is an on-GPU microcontroller
in AMD graphics cards that is used in features for
embedded displays such as Panel Self-Refresh

DMCU is part of the DM IP block

[How]
DMCU is added as an option in the enum AMDGPU_UCODE_ID

DMCU needs two pieces of firmware - the initial eram and the
interrupt vectors.  These are treated as seperate pieces of
firmware and loaded by PSP

The loading occurs in the sw_init hook of DM

Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  2 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c|  6 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 93 +++
 3 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index b358e7519987..38d3af317aa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -196,6 +196,8 @@ enum AMDGPU_UCODE_ID {
AMDGPU_UCODE_ID_UVD1,
AMDGPU_UCODE_ID_VCE,
AMDGPU_UCODE_ID_VCN,
+   AMDGPU_UCODE_ID_DMCU_ERAM,
+   AMDGPU_UCODE_ID_DMCU_INTV,
AMDGPU_UCODE_ID_MAXIMUM,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 02be34e72ed9..240dc8c85867 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -91,6 +91,12 @@ psp_v10_0_get_fw_type(struct amdgpu_firmware_info *ucode, 
enum psp_gfx_fw_type *
case AMDGPU_UCODE_ID_VCN:
*type = GFX_FW_TYPE_VCN;
break;
+   case AMDGPU_UCODE_ID_DMCU_ERAM:
+   *type = GFX_FW_TYPE_DMCU_ERAM;
+   break;
+   case AMDGPU_UCODE_ID_DMCU_INTV:
+   *type = GFX_FW_TYPE_DMCU_ISR;
+   break;
case AMDGPU_UCODE_ID_MAXIMUM:
default:
return -EINVAL;
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 5103eba75cb3..4619f624f346 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -30,6 +30,7 @@
 #include "vid.h"
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_ucode.h"
 #include "atom.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_pm.h"
@@ -50,6 +51,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -71,6 +73,12 @@
 
 #include "modules/inc/mod_freesync.h"
 
+#define FIRMWARE_RAVEN_DMCU_ERAM   "amdgpu/raven_dmcu_eram.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_ERAM);
+
+#define FIRMWARE_RAVEN_DMCU_INTV   "amdgpu/raven_dmcu_intv.bin"
+MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU_INTV);
+
 /* basic init/fini API */
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
@@ -516,6 +524,91 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
 
 static int dm_sw_init(void *handle)
 {
+   const struct firmware *fw;
+   const char *fw_name_dmcu_eram;
+   const char *fw_name_dmcu_intv;
+   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   int r;
+
+   switch(adev->asic_type) {
+   case CHIP_BONAIRE:
+   case CHIP_HAWAII:
+   case CHIP_KAVERI:
+   case CHIP_KABINI:
+   case CHIP_MULLINS:
+   case CHIP_TONGA:
+   case CHIP_FIJI:
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   case CHIP_POLARIS11:
+   case CHIP_POLARIS10:
+   case CHIP_POLARIS12:
+   case CHIP_VEGAM:
+   case CHIP_VEGA10:
+   case CHIP_VEGA12:
+   case CHIP_VEGA20:
+   return 0;
+   case CHIP_RAVEN:
+   fw_name_dmcu_eram = FIRMWARE_RAVEN_DMCU_ERAM;
+   fw_name_dmcu_intv = FIRMWARE_RAVEN_DMCU_INTV;
+   break;
+   default:
+   DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
+   return -1;
+   }
+
+   r = request_firmware(, fw_name_dmcu_eram, adev->dev);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't load firmware \"%s\"\n",
+   fw_name_dmcu_eram);
+   return r;
+   }
+
+   r = amdgpu_ucode_validate(fw);
+   if (r) {
+   dev_err(adev->dev, "amdgpu_dm: Can't validate firmware 
\"%s\"\n",
+   fw_name_dmcu_eram);
+   release_firmware(fw);
+   fw = NULL;
+   return r;
+   }
+
+   if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
+   const struct common_firmware_header *hdr;
+   hdr = (const struct common_firmware_header *)fw->data;
+   adev->firmware.ucode[AMDGPU_UCODE_ID_DMCU_ERAM].ucode_id = 
AMDGPU_UCODE_ID_DMCU_ERAM;
+   adev->firmware.ucode[AMDGPU_UCODE_ID_DMCU_ERAM].fw = fw;
+   adev->firmware.fw_size +=
+   

Re: [PATCH v2] drm/amdgpu: Fix SDMA hang in prt mode v2

[Christian König]

Acked-by: Christian König 

Am 07.09.2018 um 13:19 schrieb Zhang, Hawking:

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Tao Zhou 
Sent: 2018年9月7日 18:48
To: Zhang, Hawking ; Koenig, Christian 
; Zhang, Jerry ; 
amd-gfx@lists.freedesktop.org
Cc: Li, Yukun1 ; Zhou1, Tao 
Subject: [PATCH v2] drm/amdgpu: Fix SDMA hang in prt mode v2

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue

Affected ASICs: VEGA10 VEGA12 RV1 RV2

v2: add reg clear for SDMA1

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 
---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..ee0213e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00,
+0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100), @@ -81,7 +82,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4[] = {
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC0_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_WATERMK, 0xfc00,
+0x)
  };
  
  static const struct soc15_reg_golden golden_settings_sdma_vg10[] = { @@ -108,7 +110,8 @@ static const struct soc15_reg_golden golden_settings_sdma_4_1[] = {

SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00,
+0x)
  };
  
  static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {

--
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 v2] drm/amdgpu: Fix SDMA hang in prt mode v2

[Zhang, Hawking]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Tao Zhou  
Sent: 2018年9月7日 18:48
To: Zhang, Hawking ; Koenig, Christian 
; Zhang, Jerry ; 
amd-gfx@lists.freedesktop.org
Cc: Li, Yukun1 ; Zhou1, Tao 
Subject: [PATCH v2] drm/amdgpu: Fix SDMA hang in prt mode v2

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue

Affected ASICs: VEGA10 VEGA12 RV1 RV2

v2: add reg clear for SDMA1

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..ee0213e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
+0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100), @@ -81,7 +82,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4[] = {
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC0_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_WATERMK, 0xfc00, 
+0x)
 };
 
 static const struct soc15_reg_golden golden_settings_sdma_vg10[] = { @@ -108,7 
+110,8 @@ static const struct soc15_reg_golden golden_settings_sdma_4_1[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
+0x)
 };
 
 static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {
--
2.7.4

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

Re: [PATCH 1/3] drm/ttm: fix ttm_bo_bulk_move_helper

[Christian König]

Am 07.09.2018 um 13:02 schrieb Huang, Ray:

  Yes, that was one problem. Another was that the cutting code was buggy
   and determined one of the positions to cut at the wrong time.

I went through again about the list cutting behavior and wrote down with the 
attached picture.
After do the second list_cut_position, the list2 should be point the end of 
"before" list. And list2 won't be used anymore after list cutting. May I know 
am I something missed?


Let's take a look at the original code:

list1 = is_swap ? >last->swap : >last->lru;
list2 = is_swap ? pos->first->swap.prev : pos->first->lru.prev;

list_cut_position(, lru, list1);
list_cut_position(, , list2);


As far as I can see the problem is that the first list_cur_position 
could modify the value of pos->first->lru.prev and so make the second 
list_cut_position work on the wrong position.


Regards,
Christian.



Thanks,
Ray

From: amd-gfx  on behalf of Christian König 

Sent: Thursday, September 6, 2018 6:06 PM
To: Huang, Ray
Cc: Michel Dänzer; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/ttm: fix ttm_bo_bulk_move_helper
   


Am 06.09.2018 um 12:02 schrieb Huang Rui:

On Fri, Aug 31, 2018 at 05:17:33PM +0200, Christian König wrote:

Am 31.08.2018 um 17:15 schrieb Michel Dänzer:

On 2018-08-31 3:10 p.m., Christian König wrote:

Staring at the function for six hours, just to essentially move one line
of code.

That sucks, but the commit log should describe what the problem was and
how this patch solves it.



Signed-off-by: Christian König 
---
    drivers/gpu/drm/ttm/ttm_bo.c | 13 -
    1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 35d53d81f486..138c98902033 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -250,15 +250,18 @@ EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
    static void ttm_bo_bulk_move_helper(struct ttm_lru_bulk_move_pos *pos,
  struct list_head *lru, bool is_swap)
    {
+  struct list_head *list;
  LIST_HEAD(entries);
  LIST_HEAD(before);
-  struct list_head *list1, *list2;
-  list1 = is_swap ? >last->swap : >last->lru;
-  list2 = is_swap ? pos->first->swap.prev : pos->first->lru.prev;
+  reservation_object_assert_held(pos->last->resv);
+  list = is_swap ? >last->swap : >last->lru;
+  list_cut_position(, lru, list);
+
+  reservation_object_assert_held(pos->first->resv);
+  list = is_swap ? pos->first->swap.prev : pos->first->lru.prev;
+  list_cut_position(, , list);

So the problem was that the first list_cut_position call could result in
list2 pointing to la-la-land? Good catch!

Yes, exactly. Thought that would be obvious, but going to add that
to the commit log.

Can I get a tested-by? You where much better at reproducing that than I'm.


Michel, Christian, thanks so much to take care of this when I was on
vacation. And sorry to let you take a long time for finding the cause.

Is that because I didn't hold the resveration before cut the list from
position "first" and "last"?

Yes, that was one problem. Another was that the cutting code was buggy
and determined one of the positions to cut at the wrong time.


    May I know in which cases, we must hold the
bo's reservation firstly?

BOs are reserved to prevent moving them. E.g. when the BO isn't reserved
it can move around and so the LRU where you want to add/remove it could
change.

Christian.


Thanks,
Ray



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


amd-gfx Info Page - freedesktop.org
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx 
Archives.. Using amd-gfx: To post a message to all the list members, send email 
to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or change your 
existing subscription, in the sections below.

 



___
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 v2] drm/amdgpu: Fix SDMA hang in prt mode v2

[Tao Zhou]

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue

Affected ASICs: VEGA10 VEGA12 RV1 RV2

v2: add reg clear for SDMA1

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..ee0213e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100),
@@ -81,7 +82,8 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC0_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_UTCL1_WATERMK, 0xfc00, 
0x)
 };
 
 static const struct soc15_reg_golden golden_settings_sdma_vg10[] = {
@@ -108,7 +110,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4_1[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x)
 };
 
 static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {
-- 
2.7.4

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

Re: [RFC] drm/amdgpu: Add macros and documentation for format modifiers.

[Bas Nieuwenhuizen]

On Fri, Sep 7, 2018 at 6:51 AM Marek Olšák  wrote:
>
> Hopefully this answers some questions.
>
> Other parameters that affect tiling layouts are GB_ADDR_CONFIG (all
> chips) and MC_ARB_RAMCFG (GFX6-8 only), and those vary with each chip.

For GFX6-GFX8:
From GB_ADDR_CONFIG addrlib only uses the pipe interleave bytes which
are 0 (=256 bytes) for all AMDGPU HW (and on GFX9 addrlib even asserts
on that).  From MC_ARB_RAMCFG addrlib reads the number of banks and
ranks, calculates the number of logical banks from it, but then does
not use it. (Presumably because it is the same number as the number of
banks in the tiling table entry?) Some bits gets used by the kernel
(memory row size), but those get encoded in the tile split of the
tiling table, i.e. we do not need the separate bits.

for GFX9, only the DCC meta surface seems to depend on GB_ADDR_CONFIG
(except the aforementioned pipe interleave bytes) which are constant.

I'll do some checks to see if we can trust addrlib here.

>
> Some 32bpp 1D tiling layouts are compatible across all chips (1D
> display tiling is the same as SW_256B_D if Bpp == 4).

Cool hadn't seen that they were equivalent in some situations.

>
> On GFX9, swizzle modes <= 11 are the same on all GFX9 chips. The
> remaining modes depend on GB_ADDR_CONFIG and are also more efficient.
> Bpp, number of samples, and resource type (2D/3D) affect the layout
> too, e.g. 3D textures silently use thick tiling on GFX9.
>
> Harvesting doesn't affect tiling layouts.
>
> The layout changes between layers/slices a little. Always use the base
> address of the whole image when programming the hardware. Don't assume
> that the 2nd layer has the same layout.
>
> > + * TODO: Can scanout really not support fastclear data?
>
> It can, but only those encoded in the DCC buffer (0/1). There is no
> DAL support for DCC though.
>
>
> > + * TODO: Do some generations share DCC format?
>
> DCC mirrors the tiling layout, so the same tiling mode means the same
> DCC. Take the absolute pixel address, shift it to the right, and
> you'll get the DCC element address.
>
> I would generally consider DCC as non-shareable because of different
> meanings of TILING_INDEX between chips except maybe for common GFX9
> layouts.

I was also thinking about the compression changes on Polaris that
improved compression. I'm not sure if that added e.g. new encodings in
how we store the data of a DCC tile in the main data surface.
>
>
> > [comments about number of bits]
>
> We could certainly represent all formats as a list of enums, but then
> we would need to convert the enums to the full description in drivers.
> GFX6-8 can use TILING_INDEX (except for stencil, let's ignore
> stencil). The tiling tables shouldn't change anymore because they are
> optimized for the hardware, and later hw doesn't have any tables.
>
> Marek
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Fix SDMA hang in prt mode

[Zhang, Jerry (Junwei)]

On 09/07/2018 03:41 PM, Tao Zhou wrote:

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue


What test case for that? new case?
Previously we have passed Vulkan CTS for that.

IIRC, NACK is required to reply, what's that meaning to clear that? no reply?

Regards,
Jerry



Affected ASIC: VEGA10 VEGA12 RV1 RV2

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 
---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..13bf8ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100),
@@ -108,7 +109,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4_1[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x)
  };

  static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {


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

Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[Christian König]

Well, exactly that's my point. How certain are we that the PSP firmware 
interface is not going to change?


At bare minimum we should add big "/* TODOD: .. */" markers all around 
explaining why those functions aren't implemented yet.


Thanks,
Christian.

Am 07.09.2018 um 08:03 schrieb Kuehling, Felix:

This is on purpose. These functions for now are place holders because the PSP 
firmware interface is not ready yet, and we want to start testing XGMI with 
higher level code with some hard-coded topology. Once we have proper PSP 
firmware, these place holders will be filled in.

Regards,
   Felix


From: amd-gfx  on behalf of Christian König 

Sent: Thursday, September 6, 2018 3:42:33 AM
To: Liu, Shaoyun; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi 
topology interface with psp

Am 05.09.2018 um 17:30 schrieb shaoyunl:

From: Shaoyun Liu 

Add dummy function for xgmi function interface with psp

Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
Signed-off-by: Shaoyun Liu 
Reviewed-by: Felix Kuehling 

Well NAK. We usually don't add dummy functions and that completely
circumvents the error handling added in patch #6.

Either we should implement the functions or not call them in the first
place.

Christian.


---
   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 26 ++
   1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index b70cfa3..b1c0b33 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -548,6 +548,29 @@ static int psp_v11_0_mode1_reset(struct psp_context *psp)
   return 0;
   }

+static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
+ int number_devices, struct psp_xgmi_topology_info *topology)
+{
+ return 0;
+}
+
+static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
+ int number_devices, struct psp_xgmi_topology_info *topology)
+{
+ return 0;
+}
+
+static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
+{
+ u64 hive_id = 0;
+
+ /* Remove me when normal psp interface is ready */
+ if (psp->adev->gmc.xgmi.num_physical_nodes)
+ hive_id = 0x123456789abcdef;
+
+ return hive_id;
+}
+
   static const struct psp_funcs psp_v11_0_funcs = {
   .init_microcode = psp_v11_0_init_microcode,
   .bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
@@ -560,6 +583,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
   .cmd_submit = psp_v11_0_cmd_submit,
   .compare_sram_data = psp_v11_0_compare_sram_data,
   .mode1_reset = psp_v11_0_mode1_reset,
+ .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
+ .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
+ .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
   };

   void psp_v11_0_set_psp_funcs(struct psp_context *psp)

___
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] drm/amdgpu: Fix SDMA hang in prt mode

[Christian König]

Am 07.09.2018 um 09:41 schrieb Tao Zhou:

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue

Affected ASIC: VEGA10 VEGA12 RV1 RV2

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 


Well, looks like a good catch to me. But don't we want to program the 
same register for SDMA1 as well?


Christian.


---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..13bf8ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100),
@@ -108,7 +109,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4_1[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x)
  };
  
  static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {


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

Re: two KASANs in TTM logic

[Christian König]

Hi Ray,

in the meantime can we disable the feature once more in the kernel until 
we have hammered out all possible corner cases?


As Tom figured out commenting out setting "bulk_moveable" to true should 
be enough.


Thanks,
Christian.

Am 07.09.2018 um 08:51 schrieb Huang, Ray:

Hi Tom,

Thanks to trace this issue.  I am trying to reproduce it on 
amd-staging-drm-next with piglit.
May I know the steps/configurations to repro it?

Thanks,
Ray

-Original Message-
From: amd-gfx  On Behalf Of Tom St Denis
Sent: Wednesday, September 5, 2018 9:27 PM
To: Koenig, Christian ; Daenzer, Michel 
; amd-gfx@lists.freedesktop.org; Deucher, Alexander 

Subject: Re: two KASANs in TTM logic

Logs attached.

Tom



On 09/05/2018 08:02 AM, Christian König wrote:

Still not the slightest idea what is causing this and the patch
definitely fixes things a lot.

Can you try to enable list debugging in your kernel?

Thanks,
Christian.

Am 04.09.2018 um 19:18 schrieb Tom St Denis:

Sure:

d2917f399e0b250f47d07da551a335843a24f835 is the first bad commit
commit d2917f399e0b250f47d07da551a335843a24f835
Author: Christian König 
Date:   Thu Aug 30 10:04:53 2018 +0200

     drm/amdgpu: fix "use bulk moves for efficient VM LRU handling" v2

     First step to fix the LRU corruption, we accidentially tried to
move things
     on the LRU after dropping the lock.

     Signed-off-by: Christian König 
     Tested-by: Michel Dänzer 

:04 04 ed5be1ad4da129c4154b2b43acf7ef349a470700
0008c4e2fb56512f41559618dd474c916fc09a37 M  drivers


The commit before that I can run xonotic-glx and piglit on my Carrizo
without a KASAN.

Tom

On 09/04/2018 10:05 AM, Christian König wrote:

The first one should already be fixed.

Not sure where the second comes from. Can you narrow that down further?

Christian.

Am 04.09.2018 um 15:46 schrieb Tom St Denis:

First is caused by this commit while running a GL heavy application.

d78c1fa0c9f815fe951fd57001acca3d35262a17 is the first bad commit
commit d78c1fa0c9f815fe951fd57001acca3d35262a17
Author: Michel Dänzer 
Date:   Wed Aug 29 11:59:38 2018 +0200

     Revert "drm/amdgpu: move PD/PT bos on LRU again"

     This reverts commit 31625ccae4464b61ec8cdb9740df848bbc857a5b.

     It triggered various badness on my development machine when
running the
     piglit gpu profile with radeonsi on Bonaire, looks like memory
     corruption due to insufficiently protected list manipulations.

     Signed-off-by: Michel Dänzer 
     Signed-off-by: Alex Deucher 

:04 04 b7169f0cf0c7decec631751a9896a92badb67f9d
42ea58f43199d26fc0c7ddcc655e6d0964b81817 M  drivers

The second is caused by something between that and the tip of the
4.19-rc1 amd-staging-drm-next (I haven't pinned it down yet) while
loading GNOME.

Tom



___
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 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Christian König]

Hi Oak,

yeah, but this is still a NAK. Making the hash per PASID was not a 
suggestion but rather a must have.


The VM structures must be destroyed while the processing is still 
ongoing, or otherwise we would not have a clean OOM handling.


The IDR for PASID lockup can be put into amdgpu_ids.c, you can just 
replace the IDA already used there with an IDR.


Since the PASID is already freed up delayed we should have the grace 
period for interrupt processing included. If that still isn't sufficient 
we can still add some delayed work for this.


Regards,
Christian.

Am 07.09.2018 um 06:16 schrieb ozeng:


Hi Christian,

In this implementation, fault hash is made per vm, not per pasid as 
suggested, based on below considerations:


  * Delay the destroy of hash introduces more effort like how to set
the proper grace period after which no retry interrupt will be
generated. We want to avoid those complication if we can.
  * The  problem of the per vm implementation is that it is hard to
delay the destroy of fault hash, because when vm is destroyed,
prescreen function won't be able to retrieve the fault hash. But
in this case, the prescreen function can either let the interrupt
go through (to gmc) or ignore it. From this perspective, it is not
very necessary to delay the destroy of hash table.
  * On the other hand, since ASICs after vega10 have the HW CAM
feature. So the SW IV prescreen is only useful for vega10. For all
the HW implemented CAM, we can consider the delayed CAM
acknowledgment.
  * Making it per pasid need to introduce another idr to correlate
pasid and hash table - where to put the idr? You will have to make
it a global variable which is not very desirable.

The main purpose of this patch is to solve the inter-process lock 
issue (when hash table is full), while still keep codes simple.


Also with this patch, the faults kfifo is not necessary any more. See 
patch 2.


Regards,

Oak


On 09/06/2018 11:28 PM, Oak Zeng wrote:

In stead of share one fault hash table per device, make it
per vm. This can avoid inter-process lock issue when fault
hash table is full.

Change-Id: I5d1281b7c41eddc8e26113e010516557588d3708
Signed-off-by: Oak Zeng
Suggested-by: Christian Konig
Suggested-by: Felix Kuehling
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |  75 
  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h |  10 
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 102 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  12 
  drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  38 +---
  5 files changed, 127 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index 06373d4..4ed8621 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -197,78 +197,3 @@ int amdgpu_ih_process(struct amdgpu_device *adev)
return IRQ_HANDLED;
  }
  
-/**

- * amdgpu_ih_add_fault - Add a page fault record
- *
- * @adev: amdgpu device pointer
- * @key: 64-bit encoding of PASID and address
- *
- * This should be called when a retry page fault interrupt is
- * received. If this is a new page fault, it will be added to a hash
- * table. The return value indicates whether this is a new fault, or
- * a fault that was already known and is already being handled.
- *
- * If there are too many pending page faults, this will fail. Retry
- * interrupts should be ignored in this case until there is enough
- * free space.
- *
- * Returns 0 if the fault was added, 1 if the fault was already known,
- * -ENOSPC if there are too many pending faults.
- */
-int amdgpu_ih_add_fault(struct amdgpu_device *adev, u64 key)
-{
-   unsigned long flags;
-   int r = -ENOSPC;
-
-   if (WARN_ON_ONCE(!adev->irq.ih.faults))
-   /* Should be allocated in _ih_sw_init on GPUs that
-* support retry faults and require retry filtering.
-*/
-   return r;
-
-   spin_lock_irqsave(>irq.ih.faults->lock, flags);
-
-   /* Only let the hash table fill up to 50% for best performance */
-   if (adev->irq.ih.faults->count >= (1 << (AMDGPU_PAGEFAULT_HASH_BITS-1)))
-   goto unlock_out;
-
-   r = chash_table_copy_in(>irq.ih.faults->hash, key, NULL);
-   if (!r)
-   adev->irq.ih.faults->count++;
-
-   /* chash_table_copy_in should never fail unless we're losing count */
-   WARN_ON_ONCE(r < 0);
-
-unlock_out:
-   spin_unlock_irqrestore(>irq.ih.faults->lock, flags);
-   return r;
-}
-
-/**
- * amdgpu_ih_clear_fault - Remove a page fault record
- *
- * @adev: amdgpu device pointer
- * @key: 64-bit encoding of PASID and address
- *
- * This should be called when a page fault has been handled. Any
- * future interrupt with this key will be processed as a new
- * page fault.
- */
-void amdgpu_ih_clear_fault(struct amdgpu_device *adev, u64 key)
-{
- 

[PATCH] drm/amdgpu: Fix SDMA hang in prt mode

[Tao Zhou]

Fix SDMA hang in prt mode, clear XNACK_WATERMARK in reg SDMA0_UTCL1_WATERMK to 
avoid the issue

Affected ASIC: VEGA10 VEGA12 RV1 RV2

Change-Id: I2261b8e753600731d0d8ee8bbdfc08d01eeb428e
Signed-off-by: Tao Zhou 
Tested-by: Yukun Li 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df13840..13bf8ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -70,6 +70,7 @@ static const struct soc15_reg_golden golden_settings_sdma_4[] 
= {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0100, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff0, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CHICKEN_BITS, 0xfe931f07, 
0x02831f07),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_CLK_CTRL, 0x, 
0x3f000100),
SOC15_REG_GOLDEN_VALUE(SDMA1, 0, mmSDMA1_GFX_IB_CNTL, 0x800f0100, 
0x0100),
@@ -108,7 +109,8 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4_1[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_IB_CNTL, 0x800f0111, 
0x0100),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
-   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0)
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x)
 };
 
 static const struct soc15_reg_golden golden_settings_sdma0_4_2_init[] = {
-- 
2.7.4

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

[PATCH 2/2] drm/amdgpu: Deleted faults kfifo

[Oak Zeng]

With the change of making fault hash per vm, this is not needed
anymore.

Change-Id: I9427ff1786deaf1f5b7d121a6f7f75f00acfc9b7
Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 9 -
 3 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8b220e9..5767a95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2802,7 +2802,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
goto error_free_root;
}
 
-   INIT_KFIFO(vm->faults);
vm->fault_credit = 16;
 
return 0;
@@ -2993,10 +2992,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
 
-   /* Clear pending page faults from IH when the VM is destroyed */
-   while (kfifo_get(>faults, ))
-   amdgpu_vm_clear_fault(vm->fault_hash, fault);
-
if (vm->pasid) {
unsigned long flags;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 6eb1da1..75842f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -225,9 +225,6 @@ struct amdgpu_vm {
/* Flag to indicate ATS support from PTE for GFX9 */
boolpte_support_ats;
 
-   /* Up to 128 pending retry page faults */
-   DECLARE_KFIFO(faults, u64, 128);
-
/* Limit non-retry fault storms */
unsigned intfault_credit;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index acbe5a7..7f0ed91 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -265,7 +265,6 @@ static bool vega10_ih_prescreen_iv(struct amdgpu_device 
*adev)
return true;
}
 
-   /* Track retry faults in per-VM fault FIFO. */
spin_lock(>vm_manager.pasid_lock);
vm = idr_find(>vm_manager.pasid_idr, pasid);
addr = ((u64)(dw5 & 0xf) << 44) | ((u64)dw4 << 12);
@@ -285,14 +284,6 @@ static bool vega10_ih_prescreen_iv(struct amdgpu_device 
*adev)
goto ignore_iv;
}
}
-   /* No locking required with single writer and single reader */
-   r = kfifo_put(>faults, key);
-   if (!r) {
-   /* FIFO is full. Ignore it until there is space */
-   amdgpu_vm_clear_fault(vm->fault_hash, key);
-   spin_unlock(>vm_manager.pasid_lock);
-   goto ignore_iv;
-   }
 
spin_unlock(>vm_manager.pasid_lock);
/* It's the first fault for this address, process it normally */
-- 
2.7.4

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

Re: [PATCH] mm, oom: distinguish blockable mode for mmu notifiers

[Tetsuo Handa]

On 2018/08/27 16:41, Christian König wrote:
> Am 26.08.2018 um 10:40 schrieb Tetsuo Handa:
>> I'm not following. Why don't we need to do like below (given that
>> nobody except amdgpu_mn_read_lock() holds ->read_lock) because e.g.
>> drm_sched_fence_create() from drm_sched_job_init() from amdgpu_cs_submit()
>> is doing GFP_KERNEL memory allocation with ->lock held for write?
> 
> That's a bug which needs to be fixed separately.
> 
> Allocating memory with GFP_KERNEL while holding a lock which is also taken in 
> the reclaim code path is illegal not matter what you do.
> 
> Patches to fix this are already on the appropriate mailing list and will be 
> pushed upstream today.
> 
> Regards,
> Christian.

Commit 4a2de54dc1d7668f ("drm/amdgpu: fix holding mn_lock while allocating 
memory")
seems to be calling amdgpu_mn_unlock() without amdgpu_mn_lock() when
drm_sched_job_init() failed... 



Michal, you are asking me to fix all bugs (including out of tree code) and 
prevent
future bugs just because you want to avoid using timeout in order to avoid OOM 
lockup
( https://marc.info/?i=55a3fb37-3246-73d7-0f45-5835a3f48...@i-love.sakura.ne.jp 
).
That is a too much request which is impossible for even you. More you count on
the OOM reaper, we exponentially complicates dependency and more likely to 
stumble
over unreviewed/untested code...
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] drm/amdgpu: Moved fault hash table to amdgpu vm

[Oak Zeng]

In stead of share one fault hash table per device, make it
per vm. This can avoid inter-process lock issue when fault
hash table is full.

Change-Id: I5d1281b7c41eddc8e26113e010516557588d3708
Signed-off-by: Oak Zeng 
Suggested-by: Christian Konig 
Suggested-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |  75 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h |  10 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 102 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  12 
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  38 +---
 5 files changed, 127 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index 06373d4..4ed8621 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -197,78 +197,3 @@ int amdgpu_ih_process(struct amdgpu_device *adev)
return IRQ_HANDLED;
 }
 
-/**
- * amdgpu_ih_add_fault - Add a page fault record
- *
- * @adev: amdgpu device pointer
- * @key: 64-bit encoding of PASID and address
- *
- * This should be called when a retry page fault interrupt is
- * received. If this is a new page fault, it will be added to a hash
- * table. The return value indicates whether this is a new fault, or
- * a fault that was already known and is already being handled.
- *
- * If there are too many pending page faults, this will fail. Retry
- * interrupts should be ignored in this case until there is enough
- * free space.
- *
- * Returns 0 if the fault was added, 1 if the fault was already known,
- * -ENOSPC if there are too many pending faults.
- */
-int amdgpu_ih_add_fault(struct amdgpu_device *adev, u64 key)
-{
-   unsigned long flags;
-   int r = -ENOSPC;
-
-   if (WARN_ON_ONCE(!adev->irq.ih.faults))
-   /* Should be allocated in _ih_sw_init on GPUs that
-* support retry faults and require retry filtering.
-*/
-   return r;
-
-   spin_lock_irqsave(>irq.ih.faults->lock, flags);
-
-   /* Only let the hash table fill up to 50% for best performance */
-   if (adev->irq.ih.faults->count >= (1 << (AMDGPU_PAGEFAULT_HASH_BITS-1)))
-   goto unlock_out;
-
-   r = chash_table_copy_in(>irq.ih.faults->hash, key, NULL);
-   if (!r)
-   adev->irq.ih.faults->count++;
-
-   /* chash_table_copy_in should never fail unless we're losing count */
-   WARN_ON_ONCE(r < 0);
-
-unlock_out:
-   spin_unlock_irqrestore(>irq.ih.faults->lock, flags);
-   return r;
-}
-
-/**
- * amdgpu_ih_clear_fault - Remove a page fault record
- *
- * @adev: amdgpu device pointer
- * @key: 64-bit encoding of PASID and address
- *
- * This should be called when a page fault has been handled. Any
- * future interrupt with this key will be processed as a new
- * page fault.
- */
-void amdgpu_ih_clear_fault(struct amdgpu_device *adev, u64 key)
-{
-   unsigned long flags;
-   int r;
-
-   if (!adev->irq.ih.faults)
-   return;
-
-   spin_lock_irqsave(>irq.ih.faults->lock, flags);
-
-   r = chash_table_remove(>irq.ih.faults->hash, key, NULL);
-   if (!WARN_ON_ONCE(r < 0)) {
-   adev->irq.ih.faults->count--;
-   WARN_ON_ONCE(adev->irq.ih.faults->count < 0);
-   }
-
-   spin_unlock_irqrestore(>irq.ih.faults->lock, flags);
-}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
index a23e1c0..f411ffb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
@@ -32,13 +32,6 @@ struct amdgpu_device;
 #define AMDGPU_IH_CLIENTID_LEGACY 0
 #define AMDGPU_IH_CLIENTID_MAX SOC15_IH_CLIENTID_MAX
 
-#define AMDGPU_PAGEFAULT_HASH_BITS 8
-struct amdgpu_retryfault_hashtable {
-   DECLARE_CHASH_TABLE(hash, AMDGPU_PAGEFAULT_HASH_BITS, 8, 0);
-   spinlock_t  lock;
-   int count;
-};
-
 /*
  * R6xx+ IH ring
  */
@@ -57,7 +50,6 @@ struct amdgpu_ih_ring {
booluse_doorbell;
booluse_bus_addr;
dma_addr_t  rb_dma_addr; /* only used when use_bus_addr = 
true */
-   struct amdgpu_retryfault_hashtable *faults;
 };
 
 #define AMDGPU_IH_SRC_DATA_MAX_SIZE_DW 4
@@ -95,7 +87,5 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, unsigned 
ring_size,
bool use_bus_addr);
 void amdgpu_ih_ring_fini(struct amdgpu_device *adev);
 int amdgpu_ih_process(struct amdgpu_device *adev);
-int amdgpu_ih_add_fault(struct amdgpu_device *adev, u64 key);
-void amdgpu_ih_clear_fault(struct amdgpu_device *adev, u64 key);
 
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1d7e3c1..8b220e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2692,6 +2692,22 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, 

RE: two KASANs in TTM logic

[Huang, Ray]

Hi Tom,

Thanks to trace this issue.  I am trying to reproduce it on 
amd-staging-drm-next with piglit.
May I know the steps/configurations to repro it?

Thanks,
Ray

-Original Message-
From: amd-gfx  On Behalf Of Tom St Denis
Sent: Wednesday, September 5, 2018 9:27 PM
To: Koenig, Christian ; Daenzer, Michel 
; amd-gfx@lists.freedesktop.org; Deucher, Alexander 

Subject: Re: two KASANs in TTM logic

Logs attached.

Tom



On 09/05/2018 08:02 AM, Christian König wrote:
> Still not the slightest idea what is causing this and the patch 
> definitely fixes things a lot.
> 
> Can you try to enable list debugging in your kernel?
> 
> Thanks,
> Christian.
> 
> Am 04.09.2018 um 19:18 schrieb Tom St Denis:
>> Sure:
>>
>> d2917f399e0b250f47d07da551a335843a24f835 is the first bad commit 
>> commit d2917f399e0b250f47d07da551a335843a24f835
>> Author: Christian König 
>> Date:   Thu Aug 30 10:04:53 2018 +0200
>>
>>     drm/amdgpu: fix "use bulk moves for efficient VM LRU handling" v2
>>
>>     First step to fix the LRU corruption, we accidentially tried to 
>> move things
>>     on the LRU after dropping the lock.
>>
>>     Signed-off-by: Christian König 
>>     Tested-by: Michel Dänzer 
>>
>> :04 04 ed5be1ad4da129c4154b2b43acf7ef349a470700
>> 0008c4e2fb56512f41559618dd474c916fc09a37 M  drivers
>>
>>
>> The commit before that I can run xonotic-glx and piglit on my Carrizo 
>> without a KASAN.
>>
>> Tom
>>
>> On 09/04/2018 10:05 AM, Christian König wrote:
>>> The first one should already be fixed.
>>>
>>> Not sure where the second comes from. Can you narrow that down further?
>>>
>>> Christian.
>>>
>>> Am 04.09.2018 um 15:46 schrieb Tom St Denis:
 First is caused by this commit while running a GL heavy application.

 d78c1fa0c9f815fe951fd57001acca3d35262a17 is the first bad commit 
 commit d78c1fa0c9f815fe951fd57001acca3d35262a17
 Author: Michel Dänzer 
 Date:   Wed Aug 29 11:59:38 2018 +0200

     Revert "drm/amdgpu: move PD/PT bos on LRU again"

     This reverts commit 31625ccae4464b61ec8cdb9740df848bbc857a5b.

     It triggered various badness on my development machine when 
 running the
     piglit gpu profile with radeonsi on Bonaire, looks like memory
     corruption due to insufficiently protected list manipulations.

     Signed-off-by: Michel Dänzer 
     Signed-off-by: Alex Deucher 

 :04 04 b7169f0cf0c7decec631751a9896a92badb67f9d
 42ea58f43199d26fc0c7ddcc655e6d0964b81817 M  drivers

 The second is caused by something between that and the tip of the
 4.19-rc1 amd-staging-drm-next (I haven't pinned it down yet) while 
 loading GNOME.

 Tom



 ___
 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 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[Kuehling, Felix]

This is on purpose. These functions for now are place holders because the PSP 
firmware interface is not ready yet, and we want to start testing XGMI with 
higher level code with some hard-coded topology. Once we have proper PSP 
firmware, these place holders will be filled in.

Regards,
  Felix


From: amd-gfx  on behalf of Christian 
König 
Sent: Thursday, September 6, 2018 3:42:33 AM
To: Liu, Shaoyun; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi 
topology interface with psp

Am 05.09.2018 um 17:30 schrieb shaoyunl:
> From: Shaoyun Liu 
>
> Add dummy function for xgmi function interface with psp
>
> Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
> Signed-off-by: Shaoyun Liu 
> Reviewed-by: Felix Kuehling 

Well NAK. We usually don't add dummy functions and that completely
circumvents the error handling added in patch #6.

Either we should implement the functions or not call them in the first
place.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 26 ++
>   1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index b70cfa3..b1c0b33 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -548,6 +548,29 @@ static int psp_v11_0_mode1_reset(struct psp_context *psp)
>   return 0;
>   }
>
> +static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
> + int number_devices, struct psp_xgmi_topology_info *topology)
> +{
> + return 0;
> +}
> +
> +static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
> + int number_devices, struct psp_xgmi_topology_info *topology)
> +{
> + return 0;
> +}
> +
> +static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
> +{
> + u64 hive_id = 0;
> +
> + /* Remove me when normal psp interface is ready */
> + if (psp->adev->gmc.xgmi.num_physical_nodes)
> + hive_id = 0x123456789abcdef;
> +
> + return hive_id;
> +}
> +
>   static const struct psp_funcs psp_v11_0_funcs = {
>   .init_microcode = psp_v11_0_init_microcode,
>   .bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
> @@ -560,6 +583,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
>   .cmd_submit = psp_v11_0_cmd_submit,
>   .compare_sram_data = psp_v11_0_compare_sram_data,
>   .mode1_reset = psp_v11_0_mode1_reset,
> + .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> + .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> + .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
>   };
>
>   void psp_v11_0_set_psp_funcs(struct psp_context *psp)

___
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