Re: [PATCH] drm/amdgpu: Unify the dm resume calls into one

2018-02-05 Thread Andrey Grodzovsky 

Reviewed-by: Andrey Grodzovsky

Andrey


On 02/05/2018 04:14 PM, mikita.lip...@amd.com wrote:

From: Mikita Lipski 

amdgpu_dm_display_resume is now called from dm_resume to
unify DAL resume call into a single function call

There is no more need to separately call 2 resume functions
for DM.

Initially they were separated to resume display state after
cursor is pinned. But because there is no longer any corruption
with the cursor - the calls can be merged into one function hook.

Signed-off-by: Mikita Lipski 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 10 +-
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +++-
  2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 850453e..0aaba27 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2287,14 +2287,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
resume, bool fbcon)
drm_helper_connector_dpms(connector, 
DRM_MODE_DPMS_ON);
}
drm_modeset_unlock_all(dev);
-   } else {
-   /*
-* There is no equivalent atomic helper to turn on
-* display, so we defined our own function for this,
-* once suspend resume is supported by the atomic
-* framework this will be reworked
-*/
-   amdgpu_dm_display_resume(adev);
}
}
  
@@ -2515,6 +2507,7 @@ static int amdgpu_device_reset(struct amdgpu_device *adev,

goto out;
  
  			r = amdgpu_device_ip_resume_phase2(adev);

+
if (r)
goto out;
  
@@ -2729,7 +2722,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,

if (amdgpu_device_has_dc_support(adev)) {
if (drm_atomic_helper_resume(adev->ddev, state))
dev_info(adev->dev, "drm resume failed:%d\n", r);
-   amdgpu_dm_display_resume(adev);
} else {
drm_helper_resume_force_mode(adev->ddev);
}
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 ac82382..8e6e60e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -659,11 +659,13 @@ static int dm_resume(void *handle)
  {
struct amdgpu_device *adev = handle;
struct amdgpu_display_manager *dm = &adev->dm;
+   int ret = 0;
  
  	/* power on hardware */

dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
  
-	return 0;

+   ret = amdgpu_dm_display_resume(adev);
+   return ret;
  }
  
  int amdgpu_dm_display_resume(struct amdgpu_device *adev)


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

Re: [PATCH] drm/amdgpu: Unify the dm resume calls into one

2018-02-05 Thread Harry Wentland 
On 2018-02-05 04:14 PM, mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> amdgpu_dm_display_resume is now called from dm_resume to
> unify DAL resume call into a single function call
> 
> There is no more need to separately call 2 resume functions
> for DM.
> 
> Initially they were separated to resume display state after
> cursor is pinned. But because there is no longer any corruption
> with the cursor - the calls can be merged into one function hook.
> 
> Signed-off-by: Mikita Lipski 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 10 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +++-
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 850453e..0aaba27 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2287,14 +2287,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
> resume, bool fbcon)
>   drm_helper_connector_dpms(connector, 
> DRM_MODE_DPMS_ON);
>   }
>   drm_modeset_unlock_all(dev);
> - } else {
> - /*
> -  * There is no equivalent atomic helper to turn on
> -  * display, so we defined our own function for this,
> -  * once suspend resume is supported by the atomic
> -  * framework this will be reworked
> -  */
> - amdgpu_dm_display_resume(adev);
>   }
>   }
>  
> @@ -2515,6 +2507,7 @@ static int amdgpu_device_reset(struct amdgpu_device 
> *adev,
>   goto out;
>  
>   r = amdgpu_device_ip_resume_phase2(adev);
> +

Nitpick: remove the unrelated newline change.

With this fixed patch is
Reviewed-by: Harry Wentland 

Harry

>   if (r)
>   goto out;
>  
> @@ -2729,7 +2722,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device 
> *adev,
>   if (amdgpu_device_has_dc_support(adev)) {
>   if (drm_atomic_helper_resume(adev->ddev, state))
>   dev_info(adev->dev, "drm resume failed:%d\n", r);
> - amdgpu_dm_display_resume(adev);
>   } else {
>   drm_helper_resume_force_mode(adev->ddev);
>   }
> 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 ac82382..8e6e60e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -659,11 +659,13 @@ static int dm_resume(void *handle)
>  {
>   struct amdgpu_device *adev = handle;
>   struct amdgpu_display_manager *dm = &adev->dm;
> + int ret = 0;
>  
>   /* power on hardware */
>   dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
>  
> - return 0;
> + ret = amdgpu_dm_display_resume(adev);
> + return ret;
>  }
>  
>  int amdgpu_dm_display_resume(struct amdgpu_device *adev)
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Unify the dm resume calls into one

2018-02-05 Thread mikita.lipski 
From: Mikita Lipski 

amdgpu_dm_display_resume is now called from dm_resume to
unify DAL resume call into a single function call

There is no more need to separately call 2 resume functions
for DM.

Initially they were separated to resume display state after
cursor is pinned. But because there is no longer any corruption
with the cursor - the calls can be merged into one function hook.

Signed-off-by: Mikita Lipski 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 10 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +++-
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 850453e..0aaba27 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2287,14 +2287,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
resume, bool fbcon)
drm_helper_connector_dpms(connector, 
DRM_MODE_DPMS_ON);
}
drm_modeset_unlock_all(dev);
-   } else {
-   /*
-* There is no equivalent atomic helper to turn on
-* display, so we defined our own function for this,
-* once suspend resume is supported by the atomic
-* framework this will be reworked
-*/
-   amdgpu_dm_display_resume(adev);
}
}
 
@@ -2515,6 +2507,7 @@ static int amdgpu_device_reset(struct amdgpu_device *adev,
goto out;
 
r = amdgpu_device_ip_resume_phase2(adev);
+
if (r)
goto out;
 
@@ -2729,7 +2722,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
if (amdgpu_device_has_dc_support(adev)) {
if (drm_atomic_helper_resume(adev->ddev, state))
dev_info(adev->dev, "drm resume failed:%d\n", r);
-   amdgpu_dm_display_resume(adev);
} else {
drm_helper_resume_force_mode(adev->ddev);
}
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 ac82382..8e6e60e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -659,11 +659,13 @@ static int dm_resume(void *handle)
 {
struct amdgpu_device *adev = handle;
struct amdgpu_display_manager *dm = &adev->dm;
+   int ret = 0;
 
/* power on hardware */
dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
 
-   return 0;
+   ret = amdgpu_dm_display_resume(adev);
+   return ret;
 }
 
 int amdgpu_dm_display_resume(struct amdgpu_device *adev)
-- 
2.7.4

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

[PATCH umr] use correct MM hub for SDMA IB packets

2018-02-05 Thread Tom St Denis 
Signed-off-by: Tom St Denis 
---
 src/lib/ring_decode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index f87d43e077a2..b18948d26b5f 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -1331,6 +1331,8 @@ static void print_decode_sdma(struct umr_asic *asic, 
struct umr_ring_decoder *de
break;
case 4: // INDIRECT
decoder->sdma.next_ib_state.ib_vmid = 
(ib >> 16) & 0xF;
+   if (asic->family >= FAMILY_AI)
+   
decoder->sdma.next_ib_state.ib_vmid |= UMR_MM_HUB;
printf(", VMID: %s%u%s", BLUE, 
decoder->sdma.next_ib_state.ib_vmid, RST);
decoder->sdma.n_words = 6;
break;
-- 
2.14.3

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

Re: [PATCH 3/9] drm/amdkfd: Make IOMMUv2 code conditional

2018-02-05 Thread Christian König 

Looks good to me on first glance.

You probably don't mind that I'm going to pull a good part of that into 
amdgpu as next step?


Regards,
Christian.

Am 03.02.2018 um 03:29 schrieb Felix Kuehling:

The attached patch is my attempt to keep most of the IOMMU code in one
place (new kfd_iommu.c) to avoid #ifdefs all over the place. This way I
can still conditionally compile a bunch of KFD code that is only needed
for IOMMU handling, with stub functions for kernel configs without IOMMU
support. About 300 lines of conditionally compiled code got moved to
kfd_iommu.c.

The only piece I didn't move into kfd_iommu.c is
kfd_signal_iommu_event. I prefer to keep that in kfd_events.c because it
doesn't call any IOMMU driver functions, and because it's closely
related to the rest of the event handling logic. It could be compiled
unconditionally, but it would be dead code without IOMMU support.

And I moved pdd->bound to a place where it doesn't consume extra space
(on 64-bit systems due to structure alignment) instead of making it
conditional.

This is only compile-tested for now.

If you like this approach, I'll do more testing and squash it with "Make
IOMMUv2 code conditional".

Regards,
   Felix


On 2018-01-31 10:00 AM, Oded Gabbay wrote:

On Wed, Jan 31, 2018 at 4:56 PM, Oded Gabbay  wrote:

Hi Felix,
Please don't spread 19 #ifdefs throughout the code.
I suggest to put one #ifdef in linux/amd-iommu.h itself around all the
functions declarations and in the #else section put macros with empty
implementations. This is much more readable and maintainable.

Oded

To emphasize my point, there is a call to amd_iommu_bind_pasid in
kfd_bind_processes_to_device() which isn't wrapped with the #ifdef so
the compliation breaks. Putting the #ifdefs around the calls is simply
not scalable.

Oded


On Fri, Jan 5, 2018 at 12:17 AM, Felix Kuehling  wrote:

dGPUs work without IOMMUv2. Make IOMMUv2 initialization dependent on
ASIC information. Also allow building KFD without IOMMUv2 support.
This is still useful for dGPUs and prepares for enabling KFD on
architectures that don't support AMD IOMMUv2.

Signed-off-by: Felix Kuehling 
---
  drivers/gpu/drm/amd/amdkfd/Kconfig|  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_crat.c |  8 +++-
  drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 62 +--
  drivers/gpu/drm/amd/amdkfd/kfd_events.c   |  2 +
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  5 +++
  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 17 ++---
  drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  2 +
  drivers/gpu/drm/amd/amdkfd/kfd_topology.h |  2 +
  8 files changed, 74 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig 
b/drivers/gpu/drm/amd/amdkfd/Kconfig
index bc5a294..5bbeb95 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -4,6 +4,6 @@

  config HSA_AMD
 tristate "HSA kernel driver for AMD GPU devices"
-   depends on DRM_AMDGPU && AMD_IOMMU_V2 && X86_64
+   depends on DRM_AMDGPU && X86_64
 help
   Enable this if you want to use HSA features on AMD GPU devices.
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 2bc2816..3478270 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -22,7 +22,9 @@

  #include 
  #include 
+#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
  #include 
+#endif
  #include "kfd_crat.h"
  #include "kfd_priv.h"
  #include "kfd_topology.h"
@@ -1037,15 +1039,17 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
 struct crat_subtype_generic *sub_type_hdr;
 struct crat_subtype_computeunit *cu;
 struct kfd_cu_info cu_info;
-   struct amd_iommu_device_info iommu_info;
 int avail_size = *size;
 uint32_t total_num_of_cu;
 int num_of_cache_entries = 0;
 int cache_mem_filled = 0;
 int ret = 0;
+#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
+   struct amd_iommu_device_info iommu_info;
 const u32 required_iommu_flags = AMD_IOMMU_DEVICE_FLAG_ATS_SUP |
  AMD_IOMMU_DEVICE_FLAG_PRI_SUP |
  AMD_IOMMU_DEVICE_FLAG_PASID_SUP;
+#endif
 struct kfd_local_mem_info local_mem_info;

 if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_GPU)
@@ -1106,12 +1110,14 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
 /* Check if this node supports IOMMU. During parsing this flag will
  * translate to HSA_CAP_ATS_PRESENT
  */
+#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
 iommu_info.flags = 0;
 if (amd_iommu_device_info(kdev->pdev, &iommu_info) == 0) {
 if ((iommu_info.flags & required_iommu_flags) ==
 required_iommu_flags)
 cu->hsa_capability

RE: [PATCH] drm/amdgpu: Basic emulation support

2018-02-05 Thread Liu, Shaoyun 
-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Monday, February 05, 2018 12:53 PM
To: Bridgman, John
Cc: Koenig, Christian; Liu, Shaoyun; amd-gfx list
Subject: Re: [PATCH] drm/amdgpu: Basic emulation support

On Mon, Feb 5, 2018 at 12:34 PM, Bridgman, John  wrote:
>
>
>>-Original Message-
>>From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf 
>>Of Christian König
>>Sent: Monday, February 05, 2018 11:49 AM
>>To: Alex Deucher; Liu, Shaoyun
>>Cc: amd-gfx list
>>Subject: Re: [PATCH] drm/amdgpu: Basic emulation support
>>
>>Am 05.02.2018 um 17:45 schrieb Alex Deucher:
>>> On Thu, Feb 1, 2018 at 6:16 PM, Shaoyun Liu 
>>wrote:
 Add amdgpu_emu_mode module parameter to control the emulation
>>mode
 Avoid vbios operation on emulation since there is no vbios post 
 duirng emulation, use the common hw_init to simulate the post

 Change-Id: Iba32fa16e735490e7401e471219797b83c6c2a58
 Signed-off-by: Shaoyun Liu 
>>> Acked-by: Alex Deucher 
>>
>>Acked-by: Christian König  as well.
>
> Maybe add a comment to the following change indicating that we might have 
> done early HW init either due to emulation or early init of GMC during normal 
> operation ?
>
> Otherwise change is Reviewed-by: John Bridgman 


Actually, thinking about this a bit more, is there any reason to not always 
just do the common soc hw init first?  It doesn't seem like it would hurt 
anything.

Alex


Ye ,  seems  the common hw init used to program the golden setting and  some 
PCIE registers , it should works even  in   none-emulation  mode .

Shaoyun.liu


>
>>
>>>
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26
>>+++---
   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 
   3 files changed, 28 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 index ab10295..4c9c320 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 @@ -129,6 +129,7 @@
   extern int amdgpu_lbpw;
   extern int amdgpu_compute_multipipe;
   extern int amdgpu_gpu_recovery;
 +extern int amdgpu_emu_mode;

   #ifdef CONFIG_DRM_AMDGPU_SI
   extern int amdgpu_si_support;
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index 6adb6e8..fe7a941 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -1339,6 +1339,20 @@ static int amdgpu_device_ip_init(struct
>>amdgpu_device *adev)
  return r;
  }
  adev->ip_blocks[i].status.sw = true;
 +
 +   if (amdgpu_emu_mode == 1) {
 +   /* Need to do common hw init first on emulation  */
 +   if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_COMMON) {
 +   r = 
 + adev->ip_blocks[i].version->funcs->hw_init((void
>>*)adev);
 +   if (r) {
 +   DRM_ERROR("hw_init of IP block 
 <%s> failed %d\n",
 +   
 adev->ip_blocks[i].version->funcs->name, r);
 +   return r;
 +   }
 +   adev->ip_blocks[i].status.hw = true;
 +   }
 +   }
 +
  /* need to do gmc hw init early so we can allocate gpu 
 mem */
  if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_GMC) {
  r = amdgpu_device_vram_scratch_init(adev);
 @@ -1372,8 +1386,7 @@ static int amdgpu_device_ip_init(struct
>>amdgpu_device *adev)
  for (i = 0; i < adev->num_ip_blocks; i++) {
  if (!adev->ip_blocks[i].status.sw)
  continue;
 -   /* gmc hw init is done early */
 -   if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_GMC)
 +   if (adev->ip_blocks[i].status.hw)
  continue;
  r = adev->ip_blocks[i].version->funcs->hw_init((void 
 *)adev);
  if (r) {
 @@ -1914,6 +1927,9 @@ int amdgpu_device_init(struct amdgpu_device
>>*adev,
  if (runtime)
  vga_switcheroo_init_domain_pm_ops(adev->dev,
 &adev->vga_pm_domain);

 +   if (amdgpu_emu_mode == 1)
 +   goto fence_driver_init;
 +
  /* Read BIOS */
  if (!amdgpu_get_bios(adev)) {
  r = -EINVAL;
 @@ -1966,6 +1982,7 @@ int amdgpu_device_init(struct amdgpu_

Re: [PATCH] drm/amdgpu: Basic emulation support

2018-02-05 Thread Alex Deucher 
On Mon, Feb 5, 2018 at 12:34 PM, Bridgman, John  wrote:
>
>
>>-Original Message-
>>From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of
>>Christian König
>>Sent: Monday, February 05, 2018 11:49 AM
>>To: Alex Deucher; Liu, Shaoyun
>>Cc: amd-gfx list
>>Subject: Re: [PATCH] drm/amdgpu: Basic emulation support
>>
>>Am 05.02.2018 um 17:45 schrieb Alex Deucher:
>>> On Thu, Feb 1, 2018 at 6:16 PM, Shaoyun Liu 
>>wrote:
 Add amdgpu_emu_mode module parameter to control the emulation
>>mode
 Avoid vbios operation on emulation since there is no vbios post
 duirng emulation, use the common hw_init to simulate the post

 Change-Id: Iba32fa16e735490e7401e471219797b83c6c2a58
 Signed-off-by: Shaoyun Liu 
>>> Acked-by: Alex Deucher 
>>
>>Acked-by: Christian König  as well.
>
> Maybe add a comment to the following change indicating that we might have 
> done early HW init either due to emulation or early init of GMC during normal 
> operation ?
>
> Otherwise change is Reviewed-by: John Bridgman 


Actually, thinking about this a bit more, is there any reason to not
always just do the common soc hw init first?  It doesn't seem like it
would hurt anything.

Alex

>
>>
>>>
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26
>>+++---
   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 
   3 files changed, 28 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 index ab10295..4c9c320 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 @@ -129,6 +129,7 @@
   extern int amdgpu_lbpw;
   extern int amdgpu_compute_multipipe;
   extern int amdgpu_gpu_recovery;
 +extern int amdgpu_emu_mode;

   #ifdef CONFIG_DRM_AMDGPU_SI
   extern int amdgpu_si_support;
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index 6adb6e8..fe7a941 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -1339,6 +1339,20 @@ static int amdgpu_device_ip_init(struct
>>amdgpu_device *adev)
  return r;
  }
  adev->ip_blocks[i].status.sw = true;
 +
 +   if (amdgpu_emu_mode == 1) {
 +   /* Need to do common hw init first on emulation  */
 +   if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_COMMON) {
 +   r = 
 adev->ip_blocks[i].version->funcs->hw_init((void
>>*)adev);
 +   if (r) {
 +   DRM_ERROR("hw_init of IP block 
 <%s> failed %d\n",
 +   
 adev->ip_blocks[i].version->funcs->name, r);
 +   return r;
 +   }
 +   adev->ip_blocks[i].status.hw = true;
 +   }
 +   }
 +
  /* need to do gmc hw init early so we can allocate gpu 
 mem */
  if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_GMC) {
  r = amdgpu_device_vram_scratch_init(adev);
 @@ -1372,8 +1386,7 @@ static int amdgpu_device_ip_init(struct
>>amdgpu_device *adev)
  for (i = 0; i < adev->num_ip_blocks; i++) {
  if (!adev->ip_blocks[i].status.sw)
  continue;
 -   /* gmc hw init is done early */
 -   if (adev->ip_blocks[i].version->type ==
>>AMD_IP_BLOCK_TYPE_GMC)
 +   if (adev->ip_blocks[i].status.hw)
  continue;
  r = adev->ip_blocks[i].version->funcs->hw_init((void 
 *)adev);
  if (r) {
 @@ -1914,6 +1927,9 @@ int amdgpu_device_init(struct amdgpu_device
>>*adev,
  if (runtime)
  vga_switcheroo_init_domain_pm_ops(adev->dev,
 &adev->vga_pm_domain);

 +   if (amdgpu_emu_mode == 1)
 +   goto fence_driver_init;
 +
  /* Read BIOS */
  if (!amdgpu_get_bios(adev)) {
  r = -EINVAL;
 @@ -1966,6 +1982,7 @@ int amdgpu_device_init(struct amdgpu_device
>>*adev,
  amdgpu_atombios_i2c_init(adev);
  }

 +fence_driver_init:
  /* Fence driver */
  r = amdgpu_fence_driver_init(adev);
  if (r) {
 @@ -2108,7 +2125,10 @@ void amdgpu_device_fini(struct amdgpu_device
>>*adev)
  /* free i2c buses */
  if (!amdgpu_device_has_dc_support(adev))

RE: [PATCH] drm/amdgpu: Basic emulation support

2018-02-05 Thread Bridgman, John 


>-Original Message-
>From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Christian König
>Sent: Monday, February 05, 2018 11:49 AM
>To: Alex Deucher; Liu, Shaoyun
>Cc: amd-gfx list
>Subject: Re: [PATCH] drm/amdgpu: Basic emulation support
>
>Am 05.02.2018 um 17:45 schrieb Alex Deucher:
>> On Thu, Feb 1, 2018 at 6:16 PM, Shaoyun Liu 
>wrote:
>>> Add amdgpu_emu_mode module parameter to control the emulation
>mode
>>> Avoid vbios operation on emulation since there is no vbios post
>>> duirng emulation, use the common hw_init to simulate the post
>>>
>>> Change-Id: Iba32fa16e735490e7401e471219797b83c6c2a58
>>> Signed-off-by: Shaoyun Liu 
>> Acked-by: Alex Deucher 
>
>Acked-by: Christian König  as well.

Maybe add a comment to the following change indicating that we might have done 
early HW init either due to emulation or early init of GMC during normal 
operation ? 

Otherwise change is Reviewed-by: John Bridgman 

>
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26
>+++---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 
>>>   3 files changed, 28 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index ab10295..4c9c320 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -129,6 +129,7 @@
>>>   extern int amdgpu_lbpw;
>>>   extern int amdgpu_compute_multipipe;
>>>   extern int amdgpu_gpu_recovery;
>>> +extern int amdgpu_emu_mode;
>>>
>>>   #ifdef CONFIG_DRM_AMDGPU_SI
>>>   extern int amdgpu_si_support;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 6adb6e8..fe7a941 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -1339,6 +1339,20 @@ static int amdgpu_device_ip_init(struct
>amdgpu_device *adev)
>>>  return r;
>>>  }
>>>  adev->ip_blocks[i].status.sw = true;
>>> +
>>> +   if (amdgpu_emu_mode == 1) {
>>> +   /* Need to do common hw init first on emulation  */
>>> +   if (adev->ip_blocks[i].version->type ==
>AMD_IP_BLOCK_TYPE_COMMON) {
>>> +   r = 
>>> adev->ip_blocks[i].version->funcs->hw_init((void
>*)adev);
>>> +   if (r) {
>>> +   DRM_ERROR("hw_init of IP block <%s> 
>>> failed %d\n",
>>> +   
>>> adev->ip_blocks[i].version->funcs->name, r);
>>> +   return r;
>>> +   }
>>> +   adev->ip_blocks[i].status.hw = true;
>>> +   }
>>> +   }
>>> +
>>>  /* need to do gmc hw init early so we can allocate gpu mem 
>>> */
>>>  if (adev->ip_blocks[i].version->type ==
>AMD_IP_BLOCK_TYPE_GMC) {
>>>  r = amdgpu_device_vram_scratch_init(adev);
>>> @@ -1372,8 +1386,7 @@ static int amdgpu_device_ip_init(struct
>amdgpu_device *adev)
>>>  for (i = 0; i < adev->num_ip_blocks; i++) {
>>>  if (!adev->ip_blocks[i].status.sw)
>>>  continue;
>>> -   /* gmc hw init is done early */
>>> -   if (adev->ip_blocks[i].version->type ==
>AMD_IP_BLOCK_TYPE_GMC)
>>> +   if (adev->ip_blocks[i].status.hw)
>>>  continue;
>>>  r = adev->ip_blocks[i].version->funcs->hw_init((void 
>>> *)adev);
>>>  if (r) {
>>> @@ -1914,6 +1927,9 @@ int amdgpu_device_init(struct amdgpu_device
>*adev,
>>>  if (runtime)
>>>  vga_switcheroo_init_domain_pm_ops(adev->dev,
>>> &adev->vga_pm_domain);
>>>
>>> +   if (amdgpu_emu_mode == 1)
>>> +   goto fence_driver_init;
>>> +
>>>  /* Read BIOS */
>>>  if (!amdgpu_get_bios(adev)) {
>>>  r = -EINVAL;
>>> @@ -1966,6 +1982,7 @@ int amdgpu_device_init(struct amdgpu_device
>*adev,
>>>  amdgpu_atombios_i2c_init(adev);
>>>  }
>>>
>>> +fence_driver_init:
>>>  /* Fence driver */
>>>  r = amdgpu_fence_driver_init(adev);
>>>  if (r) {
>>> @@ -2108,7 +2125,10 @@ void amdgpu_device_fini(struct amdgpu_device
>*adev)
>>>  /* free i2c buses */
>>>  if (!amdgpu_device_has_dc_support(adev))
>>>  amdgpu_i2c_fini(adev);
>>> -   amdgpu_atombios_fini(adev);
>>> +
>>> +   if (amdgpu_emu_mode != 1)
>>> +   amdgpu_atombios_fini(adev);
>>> +
>>>  kfree(adev->bios);
>>>  adev->bios = NULL;
>>>  if (!pci_is_thunderbolt_attached(adev->pdev))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>

Re: [PATCH] drm/amdgpu: Basic emulation support

2018-02-05 Thread Christian König 

Am 05.02.2018 um 17:45 schrieb Alex Deucher:

On Thu, Feb 1, 2018 at 6:16 PM, Shaoyun Liu  wrote:

Add amdgpu_emu_mode module parameter to control the emulation mode
Avoid vbios operation on emulation since there is no vbios post duirng 
emulation,
use the common hw_init to simulate the post

Change-Id: Iba32fa16e735490e7401e471219797b83c6c2a58
Signed-off-by: Shaoyun Liu 

Acked-by: Alex Deucher 


Acked-by: Christian König  as well.




---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 
  3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ab10295..4c9c320 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -129,6 +129,7 @@
  extern int amdgpu_lbpw;
  extern int amdgpu_compute_multipipe;
  extern int amdgpu_gpu_recovery;
+extern int amdgpu_emu_mode;

  #ifdef CONFIG_DRM_AMDGPU_SI
  extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6adb6e8..fe7a941 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1339,6 +1339,20 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
 return r;
 }
 adev->ip_blocks[i].status.sw = true;
+
+   if (amdgpu_emu_mode == 1) {
+   /* Need to do common hw init first on emulation  */
+   if (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_COMMON) {
+   r = 
adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
+   if (r) {
+   DRM_ERROR("hw_init of IP block <%s> failed 
%d\n",
+   
adev->ip_blocks[i].version->funcs->name, r);
+   return r;
+   }
+   adev->ip_blocks[i].status.hw = true;
+   }
+   }
+
 /* need to do gmc hw init early so we can allocate gpu mem */
 if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) 
{
 r = amdgpu_device_vram_scratch_init(adev);
@@ -1372,8 +1386,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
 for (i = 0; i < adev->num_ip_blocks; i++) {
 if (!adev->ip_blocks[i].status.sw)
 continue;
-   /* gmc hw init is done early */
-   if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC)
+   if (adev->ip_blocks[i].status.hw)
 continue;
 r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
 if (r) {
@@ -1914,6 +1927,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 if (runtime)
 vga_switcheroo_init_domain_pm_ops(adev->dev, 
&adev->vga_pm_domain);

+   if (amdgpu_emu_mode == 1)
+   goto fence_driver_init;
+
 /* Read BIOS */
 if (!amdgpu_get_bios(adev)) {
 r = -EINVAL;
@@ -1966,6 +1982,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 amdgpu_atombios_i2c_init(adev);
 }

+fence_driver_init:
 /* Fence driver */
 r = amdgpu_fence_driver_init(adev);
 if (r) {
@@ -2108,7 +2125,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 /* free i2c buses */
 if (!amdgpu_device_has_dc_support(adev))
 amdgpu_i2c_fini(adev);
-   amdgpu_atombios_fini(adev);
+
+   if (amdgpu_emu_mode != 1)
+   amdgpu_atombios_fini(adev);
+
 kfree(adev->bios);
 adev->bios = NULL;
 if (!pci_is_thunderbolt_attached(adev->pdev))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5a5ed47..fdd24d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -130,6 +130,7 @@
  int amdgpu_lbpw = -1;
  int amdgpu_compute_multipipe = -1;
  int amdgpu_gpu_recovery = -1; /* auto */
+int amdgpu_emu_mode = 0;

  MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
  module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
@@ -285,6 +286,9 @@
  MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (1 = enable, 0 = 
disable, -1 = auto");
  module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);

+MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable");
+module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
+
  #ifdef CONFIG_DRM_AMDGPU_SI

  #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
--
1.9.1

Re: [PATCH] drm/amdgpu: Basic emulation support

2018-02-05 Thread Alex Deucher 
On Thu, Feb 1, 2018 at 6:16 PM, Shaoyun Liu  wrote:
> Add amdgpu_emu_mode module parameter to control the emulation mode
> Avoid vbios operation on emulation since there is no vbios post duirng 
> emulation,
> use the common hw_init to simulate the post
>
> Change-Id: Iba32fa16e735490e7401e471219797b83c6c2a58
> Signed-off-by: Shaoyun Liu 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26 +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 
>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ab10295..4c9c320 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -129,6 +129,7 @@
>  extern int amdgpu_lbpw;
>  extern int amdgpu_compute_multipipe;
>  extern int amdgpu_gpu_recovery;
> +extern int amdgpu_emu_mode;
>
>  #ifdef CONFIG_DRM_AMDGPU_SI
>  extern int amdgpu_si_support;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 6adb6e8..fe7a941 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1339,6 +1339,20 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
> return r;
> }
> adev->ip_blocks[i].status.sw = true;
> +
> +   if (amdgpu_emu_mode == 1) {
> +   /* Need to do common hw init first on emulation  */
> +   if (adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_COMMON) {
> +   r = 
> adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> +   if (r) {
> +   DRM_ERROR("hw_init of IP block <%s> 
> failed %d\n",
> +   
> adev->ip_blocks[i].version->funcs->name, r);
> +   return r;
> +   }
> +   adev->ip_blocks[i].status.hw = true;
> +   }
> +   }
> +
> /* need to do gmc hw init early so we can allocate gpu mem */
> if (adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_GMC) {
> r = amdgpu_device_vram_scratch_init(adev);
> @@ -1372,8 +1386,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
> for (i = 0; i < adev->num_ip_blocks; i++) {
> if (!adev->ip_blocks[i].status.sw)
> continue;
> -   /* gmc hw init is done early */
> -   if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC)
> +   if (adev->ip_blocks[i].status.hw)
> continue;
> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> if (r) {
> @@ -1914,6 +1927,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> if (runtime)
> vga_switcheroo_init_domain_pm_ops(adev->dev, 
> &adev->vga_pm_domain);
>
> +   if (amdgpu_emu_mode == 1)
> +   goto fence_driver_init;
> +
> /* Read BIOS */
> if (!amdgpu_get_bios(adev)) {
> r = -EINVAL;
> @@ -1966,6 +1982,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> amdgpu_atombios_i2c_init(adev);
> }
>
> +fence_driver_init:
> /* Fence driver */
> r = amdgpu_fence_driver_init(adev);
> if (r) {
> @@ -2108,7 +2125,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> /* free i2c buses */
> if (!amdgpu_device_has_dc_support(adev))
> amdgpu_i2c_fini(adev);
> -   amdgpu_atombios_fini(adev);
> +
> +   if (amdgpu_emu_mode != 1)
> +   amdgpu_atombios_fini(adev);
> +
> kfree(adev->bios);
> adev->bios = NULL;
> if (!pci_is_thunderbolt_attached(adev->pdev))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 5a5ed47..fdd24d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -130,6 +130,7 @@
>  int amdgpu_lbpw = -1;
>  int amdgpu_compute_multipipe = -1;
>  int amdgpu_gpu_recovery = -1; /* auto */
> +int amdgpu_emu_mode = 0;
>
>  MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>  module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
> @@ -285,6 +286,9 @@
>  MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (1 = enable, 
> 0 = disable, -1 = auto");
>  module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);
>
> +MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable");
> +module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
> +
>  #ifde

Re: [PATCH] drm/amdgpu: sync the VM PD/PT before clearing it

2018-02-05 Thread Felix Kuehling 
Reviewed-by: Felix Kuehling 


On 2018-02-05 07:28 AM, Christian König wrote:
> Otherwise we might overwrite stuff which is still in use.
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 18ce47608bf1..0572d6072baa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -329,6 +329,11 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   amdgpu_ring_pad_ib(ring, &job->ibs[0]);
>  
>   WARN_ON(job->ibs[0].length_dw > 64);
> + r = amdgpu_sync_resv(adev, &job->sync, bo->tbo.resv,
> +  AMDGPU_FENCE_OWNER_UNDEFINED, false);
> + if (r)
> + goto error_free;
> +
>   r = amdgpu_job_submit(job, ring, &vm->entity,
> AMDGPU_FENCE_OWNER_UNDEFINED, &fence);
>   if (r)

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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Tom St Denis 
Attached is a patch for umr{master} which should in theory support both 
iova and iomem.


I can add the write method if you want since ya it should be fairly 
simple to copy/pasta that up.


Cheers,
Tom

On 05/02/18 07:07 AM, Christian König wrote:

Well adding write support is trivial.

What I'm more concerned about is if setting page->mapping during 
allocation of the page could have any negative effect?


I of hand don't see any since the page isn't reclaimable directly 
anyway, but I'm not 100% sure of that.


Christian.

Am 05.02.2018 um 12:49 schrieb Tom St Denis:
Another thing that occurred to me is this will break write access to 
GPU bound memory.  Previously we relied on iova to translate the 
address and then /dev/mem or /dev/fmem to read/write it.  But since 
this is literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
Behalf Of Christian K?nig

Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with 
optional IOMMU mapping.


Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 
-

  1 file changed, 35 insertions(+), 22 deletions(-)

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

index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

    #endif
  -static ssize_t amdgpu_iova_to_phys_read(struct file *f, char 
__user *buf,

-   size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+ size_t size, loff_t *pos)
  {
  struct amdgpu_device *adev = file_inode(f)->i_private;
-    int r;
-    uint64_t phys;
  struct iommu_domain *dom;
+    ssize_t result = 0;
+    int r;
  -    // always return 8 bytes
-    if (size != 8)
-    return -EINVAL;
+    dom = iommu_get_domain_for_dev(adev->dev);
  -    // only accept page addresses
-    if (*pos & 0xFFF)
-    return -EINVAL;
+    while (size) {
+    phys_addr_t addr = *pos & PAGE_MASK;
+    loff_t off = *pos & ~PAGE_MASK;
+    size_t bytes = PAGE_SIZE - off;
+    unsigned long pfn;
+    struct page *p;
+    void *ptr;
  -    dom = iommu_get_domain_for_dev(adev->dev);
-    if (dom)
-    phys = iommu_iova_to_phys(dom, *pos);
-    else
-    phys = *pos;
+    addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  -    r = copy_to_user(buf, &phys, 8);
-    if (r)
-    return -EFAULT;
+    pfn = addr >> PAGE_SHIFT;
+    if (!pfn_valid(pfn))
+    return -EPERM;
+
+    p = pfn_to_page(pfn);
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;
+
+    ptr = kmap(p);
+    r = copy_to_user(buf, ptr, bytes);
+    kunmap(p);
+    if (r)
+    return -EFAULT;
  -    return 8;
+    size -= bytes;
+    *pos += bytes;
+    result += bytes;
+    }
+
+    return result;
  }
  -static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
  .owner = THIS_MODULE,
-    .read = amdgpu_iova_to_phys_read,
+    .read = amdgpu_iomem_read,
  .llseek = default_llseek
  };
  @@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS

  { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, #endif
-    { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM },
+    { "amdgpu_iomem", &amdgpu_ttm_iomem_fops, TTM_PL_SYSTEM },
  };
    #endif
--
2.14.1

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







>From 67703a62763dfb2107bd503c5ae76414a50c50a4 Mon Sep 17 00:00:00 2001
From: Tom St Denis 
Date: Mon, 5 Feb 2018 08:53:40 -0500
Subject: [PATCH umr] add support for new iomem debugfs entry

Signed-off-by: Tom St Denis 
---
 src/lib/close_asic.c |  1 +
 src/lib/discover.c   |  3 +++
 src/lib/read_vram.c  | 29 +++--
 src/umr.h|  3 ++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/lib/close_asic.c b/src/lib/close_asic.c
index 782b1a0d029b..6b220cd578e9 100644
--- a/src/lib/close_asic.c
+++ b/src/lib/close_asic.c
@@ -57,6 +57,7 @@ void umr_close_asic(struct umr_asic *asic)
 		cond_close(asic->fd.gpr

Re: Deadlocks with multiple applications on AMD RX 460 and RX 550 - Update 2

2018-02-05 Thread Luís Mendes 
Hi everyone,

I have some updates. I left the system idle most of the time during
the weekend and from time to time I played a video on youtube and
turned off the screen. Yesterday night I did the same and today
morning I checked the system and it got hung up during the night. This
time it took a lot longer to hang, but I think it was related to a
Flash animation add that was only present on the youtube page the last
time I switched off the screen. The amdgpu always seem to hang when
that flash animation is present, from all the crash attempts I have
made.
There is a memory leak according to kmemleak which I attach along with
the crash dmesg log.

The kernel and patches are the same as on my previous email. I ended
up not changing either the mesa version, nor the kernel version and
patches.

Regards,
Luís


On Fri, Feb 2, 2018 at 6:46 PM, Luís Mendes  wrote:
> Hi Christian, Alexander,
>
> I have enabled kmemleak, but memleak didn't detect anything special,
> in fact this time, I don't know why, I didn't get any allocation
> failure at all, but the GPU did hang after around 4h 6m of uptime with
> Xorg.
> The log can be found in attachment. I will try again to see if the
> allocation failure reappears, or if it has become less apparent due to
> kmemleak scans.
>
> The kernel stack trace is similar to the GPU hangs I was getting on
> earlier kernel versions with Kodi, or Firefox when watching videos
> with either one, but if I left Xorg idle, it would remain up and
> available without hanging for more than one day.
> This stack trace also looks quite similar to what Daniel Andersson
> reported in "[BUG] Intermittent hang/deadlock when opening browser tab
> with Vega gpu", looks like another demonstration of the same bug on
> different architectures.
>
> Regards,
> Luís
>
> On Fri, Feb 2, 2018 at 7:48 AM, Christian König
>  wrote:
>> Hi Luis,
>>
>> please enable kmemleak in your build and watch out for any suspicious
>> messages in the system log.
>>
>> Regards,
>> Christian.
>>
>>
>> Am 02.02.2018 um 00:03 schrieb Luís Mendes:
>>>
>>> Hi Alexander,
>>>
>>> I didn't notice improvements on this issue with that particular patch
>>> applied. It still ends up failing to allocate kernel memory after a
>>> few hours of uptime with Xorg.
>>>
>>> I will try to upgrade to mesa 18.0.0-rc3 and to amd-staging-drm-next
>>> head, to see if the issue still occurs with those versions.
>>>
>>> If you have additional suggestions I'll be happy to try them.
>>>
>>> Regards,
>>> Luís Mendes
>>>
>>> On Thu, Feb 1, 2018 at 2:30 AM, Alex Deucher 
>>> wrote:

 On Wed, Jan 31, 2018 at 6:57 PM, Luís Mendes 
 wrote:
>
> Hi everyone,
>
> I am getting a new issue with amdgpu with RX460, that is, now I can
> play any videos with Kodi or play web videos with firefox and run
> OpenGL applications without running into any issues, however after
> some uptime with XOrg even when almost inactive I get a kmalloc
> allocation failure, normally followed by a GPU hang a while after the
> the allocation failure.
> I had a terminal window under Ubuntu Mate 17.10 and I was compiling
> code when I got the kernel messages that can be found in attachment.
>
> I am using the kernel as identified on my previous email, which can be
> found below.

 does this patch help?
 https://patchwork.freedesktop.org/patch/198258/

 Alex

> Regards,
> Luís Mendes
>
> On Wed, Jan 31, 2018 at 12:47 PM, Luís Mendes 
> wrote:
>>
>> Hi Alexander,
>>
>> I've cherry picked the patch you pointed out into kernel from
>> amd-drm-next-4.17-wip at commit
>> 9ab2894122275a6d636bb2654a157e88a0f7b9e2 ( drm/amdgpu: set
>> DRIVER_ATOMIC flag early) and tested it on ARMv7l and the problem has
>> gone indeed.
>>
>>
>> Working great on ARMv7l with AMD RX460.
>>
>> Thanks,
>> Luís Mendes
>>
>>
>> On Tue, Jan 30, 2018 at 6:44 PM, Deucher, Alexander
>>  wrote:
>>>
>>> Fixed with this patch:
>>>
>>>
>>> https://lists.freedesktop.org/archives/amd-gfx/2018-January/018472.html
>>>
>>>
>>> Alex
>
> <>
>>>
>>> __
>
> ___
> 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
>>
>>
ubuntu@linux:~$ sudo cat /sys/kernel/debug/kmemleak
[sudo] password for ubuntu:
unreferenced object 0xb0fac380 (size 128):
  comm "Xorg", pid 3750, jiffies 5608934 (age 178088.970s)
  hex dump (first 32 bytes):
00 4e 9f b9 00 f0 33 bb 80 1a 15 97 00 00 00 00  .N3.
fa 00 00 00 82 01 00 00 80 00 00 00 80 00 00 00  
  backtrace:
[<400a53a4>] kmem_cache_

[PATCH] drm/amdgpu: sync the VM PD/PT before clearing it

2018-02-05 Thread Christian König 
Otherwise we might overwrite stuff which is still in use.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 18ce47608bf1..0572d6072baa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -329,6 +329,11 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
amdgpu_ring_pad_ib(ring, &job->ibs[0]);
 
WARN_ON(job->ibs[0].length_dw > 64);
+   r = amdgpu_sync_resv(adev, &job->sync, bo->tbo.resv,
+AMDGPU_FENCE_OWNER_UNDEFINED, false);
+   if (r)
+   goto error_free;
+
r = amdgpu_job_submit(job, ring, &vm->entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, &fence);
if (r)
-- 
2.14.1

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

Re: [PATCH 1/2] [WIP]drm/ttm: add waiter list to prevent allocation not in order

2018-02-05 Thread Christian König 

Am 05.02.2018 um 09:22 schrieb Chunming Zhou:

On 2018年01月31日 18:54, Christian König wrote:

So I think preventing validation on same place is a simpler way:
process B bo's place is fpfn~lpfn, it will only try to evict LRU BOs 
in that range, while eviction, we just prevent those validation to 
this range(fpfn~lpfn), if out of this range, the 
allocation/validation still can be go on.


Any negative?
That won't work either. The most common use of fpfn~lpfn range is to 
limit a BO to visible VRAM, the other use cases are to fullfil 
hardware limitations.


So blocking this would result in blocking all normal GTT and VRAM 
allocations, adding a mutex to validate would have the same effect.
No, different effect, mutex will make the every allocation serialized. 
My approach only effects busy case, that is said, only when space is 
used up, the allocation is serialized in that range, otherwise still 
in parallel.


That would still not allow for concurrent evictions, not as worse as 
completely blocking concurrent validation but still not a good idea as 
far as I can see.


Going to give it a try today to use the ww_mutex owner to detect 
eviction of already reserved BOs.


Maybe that can be used instead,
Christian.



Regards,
David Zhou




Regards,
Christian.

Am 31.01.2018 um 11:30 schrieb Chunming Zhou:




On 2018年01月26日 22:35, Christian König wrote:
I just realized that a change I'm thinking about for a while would 
solve your problem as well, but keep concurrent allocation possible.


See ttm_mem_evict_first() unlocks the BO after evicting it:

    ttm_bo_del_from_lru(bo);
    spin_unlock(&glob->lru_lock);

    ret = ttm_bo_evict(bo, ctx);
    if (locked) {
    ttm_bo_unreserve(bo); < here
    } else {
    spin_lock(&glob->lru_lock);
    ttm_bo_add_to_lru(bo);
    spin_unlock(&glob->lru_lock);
    }

    kref_put(&bo->list_kref, ttm_bo_release_list);


The effect is that in your example process C can not only beat 
process B once, but many many times because we run into a ping/pong 
situation where B evicts resources while C moves them back in.
For ping/pong case, I want to disable busy placement for allocation 
period, only enable it for cs bo validation.




For a while now I'm thinking about dropping those reservations only 
after the original allocation succeeded.


The effect would be that process C can still beat process B 
initially, but sooner or process B would evict some resources from 
process C as well and then it can succeed with its allocation.
If it is from process C cs validation, process B still need evict 
the resource only after process C command submission completion.




The problem is for this approach to work we need to core change to 
the ww_mutexes to be able to handle this efficiently.
Yes, ww_mutex doesn't support this net lock, which easily deadlock 
without ticket and class.


So I think preventing validation on same place is a simpler way:
process B bo's place is fpfn~lpfn, it will only try to evict LRU BOs 
in that range, while eviction, we just prevent those validation to 
this range(fpfn~lpfn), if out of this range, the 
allocation/validation still can be go on.


Any negative?

Regards,
David Zhou


Regards,
Christian.

Am 26.01.2018 um 14:59 schrieb Christian König:
I know, but this has the same effect. You prevent concurrent 
allocation from happening.


What we could do is to pipeline reusing of deleted memory as well, 
this makes it less likely to cause the problem you are seeing 
because the evicting processes doesn't need to block for deleted 
BOs any more.


But that other processes can grab memory during eviction is 
intentional. Otherwise greedy processes would completely dominate 
command submission.


Regards,
Christian.

Am 26.01.2018 um 14:50 schrieb Zhou, David(ChunMing):
I don't want to prevent all, my new approach is to prevent the 
later allocation is trying and ahead of front to get the memory 
space that the front made from eviction.



发自坚果 Pro

Christian K鰊ig  于 2018年1月26日 
下午9:24写道：


Yes, exactly that's the problem.

See when you want to prevent a process B from allocating the 
memory process A has evicted, you need to prevent all concurrent 
allocation.


And we don't do that because it causes a major performance drop.

Regards,
Christian.

Am 26.01.2018 um 14:21 schrieb Zhou, David(ChunMing):
You patch will prevent concurrent allocation, and will result in 
allocation performance drop much.


发自坚果 Pro

Christian K鰊ig  于 2018年1月26日 
下午9:04写道：


Attached is what you actually want to do cleanly implemented. 
But as I said this is a NO-GO.


Regards,
Christian.

Am 26.01.2018 um 13:43 schrieb Christian König:
After my investigation, this issue should be detect of TTM 
design self, which breaks scheduling balance.

Yeah, but again. This is indented design we can't change easily.

Regards,
Christian.

Am 26.01.2018 um 13:36 schrieb Zhou, David(ChunMing):
I am off work

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Christian König 

Well adding write support is trivial.

What I'm more concerned about is if setting page->mapping during 
allocation of the page could have any negative effect?


I of hand don't see any since the page isn't reclaimable directly 
anyway, but I'm not 100% sure of that.


Christian.

Am 05.02.2018 um 12:49 schrieb Tom St Denis:
Another thing that occurred to me is this will break write access to 
GPU bound memory.  Previously we relied on iova to translate the 
address and then /dev/mem or /dev/fmem to read/write it.  But since 
this is literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
Behalf Of Christian K?nig

Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with 
optional IOMMU mapping.


Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 
-

  1 file changed, 35 insertions(+), 22 deletions(-)

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

index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

    #endif
  -static ssize_t amdgpu_iova_to_phys_read(struct file *f, char 
__user *buf,

-   size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+ size_t size, loff_t *pos)
  {
  struct amdgpu_device *adev = file_inode(f)->i_private;
-    int r;
-    uint64_t phys;
  struct iommu_domain *dom;
+    ssize_t result = 0;
+    int r;
  -    // always return 8 bytes
-    if (size != 8)
-    return -EINVAL;
+    dom = iommu_get_domain_for_dev(adev->dev);
  -    // only accept page addresses
-    if (*pos & 0xFFF)
-    return -EINVAL;
+    while (size) {
+    phys_addr_t addr = *pos & PAGE_MASK;
+    loff_t off = *pos & ~PAGE_MASK;
+    size_t bytes = PAGE_SIZE - off;
+    unsigned long pfn;
+    struct page *p;
+    void *ptr;
  -    dom = iommu_get_domain_for_dev(adev->dev);
-    if (dom)
-    phys = iommu_iova_to_phys(dom, *pos);
-    else
-    phys = *pos;
+    addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  -    r = copy_to_user(buf, &phys, 8);
-    if (r)
-    return -EFAULT;
+    pfn = addr >> PAGE_SHIFT;
+    if (!pfn_valid(pfn))
+    return -EPERM;
+
+    p = pfn_to_page(pfn);
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;
+
+    ptr = kmap(p);
+    r = copy_to_user(buf, ptr, bytes);
+    kunmap(p);
+    if (r)
+    return -EFAULT;
  -    return 8;
+    size -= bytes;
+    *pos += bytes;
+    result += bytes;
+    }
+
+    return result;
  }
  -static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
  .owner = THIS_MODULE,
-    .read = amdgpu_iova_to_phys_read,
+    .read = amdgpu_iomem_read,
  .llseek = default_llseek
  };
  @@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS

  { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT }, #endif
-    { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM },
+    { "amdgpu_iomem", &amdgpu_ttm_iomem_fops, TTM_PL_SYSTEM },
  };
    #endif
--
2.14.1

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
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 5/5] drm/amdgpu: replace iova debugfs file with iomem

2018-02-05 Thread Tom St Denis 
Another thing that occurred to me is this will break write access to GPU 
bound memory.  Previously we relied on iova to translate the address and 
then /dev/mem or /dev/fmem to read/write it.  But since this is 
literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Saturday, February 03, 2018 3:10 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional IOMMU 
mapping.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
  
-	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	r = copy_to_user(buf, &phys, 8);

-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
  
-	return 8;

+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
  }
  
-static const struct file_operations amdgpu_ttm_iova_fops = {

+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
  };
  
@@ -1973,7 +1986,7 @@ static const struct {  #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS

{ "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT },  #endif
-   { "amdgpu_iova", &amdgpu_ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", &amdgpu_ttm_iomem_fops, TTM_PL_SYSTEM },
  };
  
  #endif

--
2.14.1

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
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] [WIP]drm/ttm: add waiter list to prevent allocation not in order

2018-02-05 Thread Chunming Zhou 



On 2018年01月31日 18:54, Christian König wrote:

So I think preventing validation on same place is a simpler way:
process B bo's place is fpfn~lpfn, it will only try to evict LRU BOs 
in that range, while eviction, we just prevent those validation to 
this range(fpfn~lpfn), if out of this range, the 
allocation/validation still can be go on.


Any negative?
That won't work either. The most common use of fpfn~lpfn range is to 
limit a BO to visible VRAM, the other use cases are to fullfil 
hardware limitations.


So blocking this would result in blocking all normal GTT and VRAM 
allocations, adding a mutex to validate would have the same effect.
No, different effect, mutex will make the every allocation serialized. 
My approach only effects busy case, that is said, only when space is 
used up, the allocation is serialized in that range, otherwise still in 
parallel.


Regards,
David Zhou




Regards,
Christian.

Am 31.01.2018 um 11:30 schrieb Chunming Zhou:




On 2018年01月26日 22:35, Christian König wrote:
I just realized that a change I'm thinking about for a while would 
solve your problem as well, but keep concurrent allocation possible.


See ttm_mem_evict_first() unlocks the BO after evicting it:

    ttm_bo_del_from_lru(bo);
    spin_unlock(&glob->lru_lock);

    ret = ttm_bo_evict(bo, ctx);
    if (locked) {
    ttm_bo_unreserve(bo); < here
    } else {
    spin_lock(&glob->lru_lock);
    ttm_bo_add_to_lru(bo);
    spin_unlock(&glob->lru_lock);
    }

    kref_put(&bo->list_kref, ttm_bo_release_list);


The effect is that in your example process C can not only beat 
process B once, but many many times because we run into a ping/pong 
situation where B evicts resources while C moves them back in.
For ping/pong case, I want to disable busy placement for allocation 
period, only enable it for cs bo validation.




For a while now I'm thinking about dropping those reservations only 
after the original allocation succeeded.


The effect would be that process C can still beat process B 
initially, but sooner or process B would evict some resources from 
process C as well and then it can succeed with its allocation.
If it is from process C cs validation, process B still need evict the 
resource only after process C command submission completion.




The problem is for this approach to work we need to core change to 
the ww_mutexes to be able to handle this efficiently.
Yes, ww_mutex doesn't support this net lock, which easily deadlock 
without ticket and class.


So I think preventing validation on same place is a simpler way:
process B bo's place is fpfn~lpfn, it will only try to evict LRU BOs 
in that range, while eviction, we just prevent those validation to 
this range(fpfn~lpfn), if out of this range, the 
allocation/validation still can be go on.


Any negative?

Regards,
David Zhou


Regards,
Christian.

Am 26.01.2018 um 14:59 schrieb Christian König:
I know, but this has the same effect. You prevent concurrent 
allocation from happening.


What we could do is to pipeline reusing of deleted memory as well, 
this makes it less likely to cause the problem you are seeing 
because the evicting processes doesn't need to block for deleted 
BOs any more.


But that other processes can grab memory during eviction is 
intentional. Otherwise greedy processes would completely dominate 
command submission.


Regards,
Christian.

Am 26.01.2018 um 14:50 schrieb Zhou, David(ChunMing):
I don't want to prevent all, my new approach is to prevent the 
later allocation is trying and ahead of front to get the memory 
space that the front made from eviction.



发自坚果 Pro

Christian K鰊ig  于 2018年1月26日 
下午9:24写道：


Yes, exactly that's the problem.

See when you want to prevent a process B from allocating the 
memory process A has evicted, you need to prevent all concurrent 
allocation.


And we don't do that because it causes a major performance drop.

Regards,
Christian.

Am 26.01.2018 um 14:21 schrieb Zhou, David(ChunMing):
You patch will prevent concurrent allocation, and will result in 
allocation performance drop much.


发自坚果 Pro

Christian K鰊ig  于 2018年1月26日 
下午9:04写道：


Attached is what you actually want to do cleanly implemented. But 
as I said this is a NO-GO.


Regards,
Christian.

Am 26.01.2018 um 13:43 schrieb Christian König:
After my investigation, this issue should be detect of TTM 
design self, which breaks scheduling balance.

Yeah, but again. This is indented design we can't change easily.

Regards,
Christian.

Am 26.01.2018 um 13:36 schrieb Zhou, David(ChunMing):
I am off work, so reply mail by phone, the format could not be 
text.


back to topic itself:
the problem indeed happen on amdgpu driver, someone reports me 
that application runs with two instances, the performance are 
different.
I also reproduced the issue with unit test(bo_eviction_test). 
They always think our scheduler isn't working as expected.


After my inv