Re: [PATCH 1/2] drm/amd/display: setup system context in dm_init

2020-10-19 Thread Matt Coffin
Hello all,

I just bisected an issue introduced recently for me where I get screen
corruption before even getting a TTY, and it came down to this commit.

I've got a Navi10 card, and after this commit all that is displayed is a
blank screen (with some obvious corruption artifacts), before I even get
to a tty.

If you'd rather a GitLab issue, just let me know in a reply and I can
file one, but I thought it would be easier to ping those involved since
I already bisected the issue.

I've attached a log of a boot/shutdown sequence from this commit on
amd-staging-drm-next to aid with debugging.

Let me know if you need help reproducing, or fixing. I haven't had my
hands in the display/dc code at all yet, so it would take me some time
to ramp up but I'd be more than willing to help if necessary.

Thanks in advance,
Matt

On 10/14/20 7:20 AM, Kazlauskas, Nicholas wrote:
> On 2020-10-14 3:04 a.m., Yifan Zhang wrote:
>> Change-Id: I831a5ade8b87c23d21a63d08cc4d338468769e2b
>> Signed-off-by: Yifan Zhang 
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 +++
>>   1 file changed, 61 insertions(+)
>>
>> 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 3cf4e08931bb..aaff8800c7a0 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -887,12 +887,67 @@ static void
>> amdgpu_check_debugfs_connector_property_change(struct amdgpu_device
>>   }
>>   }
>>   +static void mmhub_read_system_context(struct amdgpu_device *adev,
>> struct dc_phy_addr_space_config *pa_config)
>> +{
>> +    uint64_t pt_base;
>> +    uint32_t logical_addr_low;
>> +    uint32_t logical_addr_high;
>> +    uint32_t agp_base, agp_bot, agp_top;
>> +    PHYSICAL_ADDRESS_LOC page_table_start, page_table_end,
>> page_table_base;
>> +
>> +    logical_addr_low  = min(adev->gmc.fb_start, adev->gmc.agp_start)
>> >> 18;
>> +    pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
>> +
>> +    if (adev->apu_flags & AMD_APU_IS_RAVEN2)
>> +    /*
>> + * Raven2 has a HW issue that it is unable to use the vram which
>> + * is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the
>> + * workaround that increase system aperture high address (add 1)
>> + * to get rid of the VM fault and hardware hang.
>> + */
>> +    logical_addr_high = max((adev->gmc.fb_end >> 18) + 0x1,
>> adev->gmc.agp_end >> 18);
>> +    else
>> +    logical_addr_high = max(adev->gmc.fb_end, adev->gmc.agp_end)
>> >> 18;
>> +
>> +    agp_base = 0;
>> +    agp_bot = adev->gmc.agp_start >> 24;
>> +    agp_top = adev->gmc.agp_end >> 24;
>> +
>> +
>> +    page_table_start.high_part = (u32)(adev->gmc.gart_start >> 44) &
>> 0xF;
>> +    page_table_start.low_part = (u32)(adev->gmc.gart_start >> 12);
>> +    page_table_end.high_part = (u32)(adev->gmc.gart_end >> 44) & 0xF;
>> +    page_table_end.low_part = (u32)(adev->gmc.gart_end >> 12);
>> +    page_table_base.high_part = upper_32_bits(pt_base) & 0xF;
>> +    page_table_base.low_part = lower_32_bits(pt_base);
>> +
>> +    pa_config->system_aperture.start_addr =
>> (uint64_t)logical_addr_low << 18;
>> +    pa_config->system_aperture.end_addr = (uint64_t)logical_addr_high
>> << 18;
>> +
>> +    pa_config->system_aperture.agp_base = (uint64_t)agp_base << 24 ;
>> +    pa_config->system_aperture.agp_bot = (uint64_t)agp_bot << 24;
>> +    pa_config->system_aperture.agp_top = (uint64_t)agp_top << 24;
>> +
>> +    pa_config->system_aperture.fb_base = adev->gmc.fb_start;
>> +    pa_config->system_aperture.fb_offset = adev->gmc.aper_base;
>> +    pa_config->system_aperture.fb_top = adev->gmc.fb_end;
>> +
>> +    pa_config->gart_config.page_table_start_addr =
>> page_table_start.quad_part << 12;
>> +    pa_config->gart_config.page_table_end_addr =
>> page_table_end.quad_part << 12;
>> +    pa_config->gart_config.page_table_base_addr =
>> page_table_base.quad_part;
>> +
>> +    pa_config->is_hvm_enabled = 0;
>> +
>> +}
>> +
>> +
>>   static int amdgpu_dm_init(struct amdgpu_device *adev)
>>   {
>>   struct dc_init_data init_data;
>>   #ifdef CONFIG_DRM_AMD_DC_HDCP
>>   struct dc_callback_init init_params;
>>   #endif
>> +    struct dc_phy_addr_space_config pa_config;
>>   int r;
>>     adev->dm.ddev = adev_to_drm(adev);
>> @@ -1040,6 +1095,12 @@ static int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>   goto error;
>>   }
>>   +    mmhub_read_system_context(adev, _config);
>> +
>> +    // Call the DC init_memory func
>> +    dc_setup_system_context(adev->dm.dc, _config);
>> +
>> +
> 
> The dc_setup_system_context should come directly after dc_hardware_init().
> 
> With that fixed this series is
> 
> Reviewed-by: Nicholas Kazlauskas 
> 
> There's the vmid module as well that could be created after if needed
> but for s/g suport alone that's not necessary.
> 
> Regards,
> Nicholas Kazlauskas
> 
>>   

Re: [PATCH 1/2] drm/amd/display: setup system context in dm_init

2020-10-14 Thread Kazlauskas, Nicholas

On 2020-10-14 9:20 a.m., Kazlauskas, Nicholas wrote:

On 2020-10-14 3:04 a.m., Yifan Zhang wrote:

Change-Id: I831a5ade8b87c23d21a63d08cc4d338468769e2b
Signed-off-by: Yifan Zhang 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 +++
  1 file changed, 61 insertions(+)

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 3cf4e08931bb..aaff8800c7a0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -887,12 +887,67 @@ static void 
amdgpu_check_debugfs_connector_property_change(struct amdgpu_device

  }
  }
+static void mmhub_read_system_context(struct amdgpu_device *adev, 
struct dc_phy_addr_space_config *pa_config)

+{
+    uint64_t pt_base;
+    uint32_t logical_addr_low;
+    uint32_t logical_addr_high;
+    uint32_t agp_base, agp_bot, agp_top;
+    PHYSICAL_ADDRESS_LOC page_table_start, page_table_end, 
page_table_base;

+
+    logical_addr_low  = min(adev->gmc.fb_start, adev->gmc.agp_start) 
>> 18;

+    pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
+
+    if (adev->apu_flags & AMD_APU_IS_RAVEN2)
+    /*
+ * Raven2 has a HW issue that it is unable to use the vram which
+ * is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the
+ * workaround that increase system aperture high address (add 1)
+ * to get rid of the VM fault and hardware hang.
+ */
+    logical_addr_high = max((adev->gmc.fb_end >> 18) + 0x1, 
adev->gmc.agp_end >> 18);

+    else
+    logical_addr_high = max(adev->gmc.fb_end, adev->gmc.agp_end) 
>> 18;

+
+    agp_base = 0;
+    agp_bot = adev->gmc.agp_start >> 24;
+    agp_top = adev->gmc.agp_end >> 24;
+
+
+    page_table_start.high_part = (u32)(adev->gmc.gart_start >> 44) & 
0xF;

+    page_table_start.low_part = (u32)(adev->gmc.gart_start >> 12);
+    page_table_end.high_part = (u32)(adev->gmc.gart_end >> 44) & 0xF;
+    page_table_end.low_part = (u32)(adev->gmc.gart_end >> 12);
+    page_table_base.high_part = upper_32_bits(pt_base) & 0xF;
+    page_table_base.low_part = lower_32_bits(pt_base);
+
+    pa_config->system_aperture.start_addr = 
(uint64_t)logical_addr_low << 18;
+    pa_config->system_aperture.end_addr = (uint64_t)logical_addr_high 
<< 18;

+
+    pa_config->system_aperture.agp_base = (uint64_t)agp_base << 24 ;
+    pa_config->system_aperture.agp_bot = (uint64_t)agp_bot << 24;
+    pa_config->system_aperture.agp_top = (uint64_t)agp_top << 24;
+
+    pa_config->system_aperture.fb_base = adev->gmc.fb_start;
+    pa_config->system_aperture.fb_offset = adev->gmc.aper_base;
+    pa_config->system_aperture.fb_top = adev->gmc.fb_end;
+
+    pa_config->gart_config.page_table_start_addr = 
page_table_start.quad_part << 12;
+    pa_config->gart_config.page_table_end_addr = 
page_table_end.quad_part << 12;
+    pa_config->gart_config.page_table_base_addr = 
page_table_base.quad_part;

+
+    pa_config->is_hvm_enabled = 0;
+
+}
+
+
  static int amdgpu_dm_init(struct amdgpu_device *adev)
  {
  struct dc_init_data init_data;
  #ifdef CONFIG_DRM_AMD_DC_HDCP
  struct dc_callback_init init_params;
  #endif
+    struct dc_phy_addr_space_config pa_config;
  int r;
  adev->dm.ddev = adev_to_drm(adev);
@@ -1040,6 +1095,12 @@ static int amdgpu_dm_init(struct amdgpu_device 
*adev)

  goto error;
  }
+    mmhub_read_system_context(adev, _config);
+
+    // Call the DC init_memory func
+    dc_setup_system_context(adev->dm.dc, _config);
+
+


The dc_setup_system_context should come directly after dc_hardware_init().

With that fixed this series is

Reviewed-by: Nicholas Kazlauskas 

There's the vmid module as well that could be created after if needed 
but for s/g suport alone that's not necessary.


Regards,
Nicholas Kazlauskas


Actually, the commit messages could use some work too - would be good to 
have at least a brief why/how description.


Don't forget to drop the Change-Id as well.

Regards,
Nicholas Kazlauskas




  DRM_DEBUG_DRIVER("KMS initialized.\n");
  return 0;



___
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/amd/display: setup system context in dm_init

2020-10-14 Thread Kazlauskas, Nicholas

On 2020-10-14 3:04 a.m., Yifan Zhang wrote:

Change-Id: I831a5ade8b87c23d21a63d08cc4d338468769e2b
Signed-off-by: Yifan Zhang 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 +++
  1 file changed, 61 insertions(+)

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 3cf4e08931bb..aaff8800c7a0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -887,12 +887,67 @@ static void 
amdgpu_check_debugfs_connector_property_change(struct amdgpu_device
}
  }
  
+static void mmhub_read_system_context(struct amdgpu_device *adev, struct dc_phy_addr_space_config *pa_config)

+{
+   uint64_t pt_base;
+   uint32_t logical_addr_low;
+   uint32_t logical_addr_high;
+   uint32_t agp_base, agp_bot, agp_top;
+   PHYSICAL_ADDRESS_LOC page_table_start, page_table_end, page_table_base;
+
+   logical_addr_low  = min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18;
+   pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
+
+   if (adev->apu_flags & AMD_APU_IS_RAVEN2)
+   /*
+* Raven2 has a HW issue that it is unable to use the vram which
+* is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the
+* workaround that increase system aperture high address (add 1)
+* to get rid of the VM fault and hardware hang.
+*/
+   logical_addr_high = max((adev->gmc.fb_end >> 18) + 0x1, 
adev->gmc.agp_end >> 18);
+   else
+   logical_addr_high = max(adev->gmc.fb_end, adev->gmc.agp_end) >> 
18;
+
+   agp_base = 0;
+   agp_bot = adev->gmc.agp_start >> 24;
+   agp_top = adev->gmc.agp_end >> 24;
+
+
+   page_table_start.high_part = (u32)(adev->gmc.gart_start >> 44) & 0xF;
+   page_table_start.low_part = (u32)(adev->gmc.gart_start >> 12);
+   page_table_end.high_part = (u32)(adev->gmc.gart_end >> 44) & 0xF;
+   page_table_end.low_part = (u32)(adev->gmc.gart_end >> 12);
+   page_table_base.high_part = upper_32_bits(pt_base) & 0xF;
+   page_table_base.low_part = lower_32_bits(pt_base);
+
+   pa_config->system_aperture.start_addr = (uint64_t)logical_addr_low << 
18;
+   pa_config->system_aperture.end_addr = (uint64_t)logical_addr_high << 18;
+
+   pa_config->system_aperture.agp_base = (uint64_t)agp_base << 24 ;
+   pa_config->system_aperture.agp_bot = (uint64_t)agp_bot << 24;
+   pa_config->system_aperture.agp_top = (uint64_t)agp_top << 24;
+
+   pa_config->system_aperture.fb_base = adev->gmc.fb_start;
+   pa_config->system_aperture.fb_offset = adev->gmc.aper_base;
+   pa_config->system_aperture.fb_top = adev->gmc.fb_end;
+
+   pa_config->gart_config.page_table_start_addr = page_table_start.quad_part 
<< 12;
+   pa_config->gart_config.page_table_end_addr = page_table_end.quad_part 
<< 12;
+   pa_config->gart_config.page_table_base_addr = page_table_base.quad_part;
+
+   pa_config->is_hvm_enabled = 0;
+
+}
+
+
  static int amdgpu_dm_init(struct amdgpu_device *adev)
  {
struct dc_init_data init_data;
  #ifdef CONFIG_DRM_AMD_DC_HDCP
struct dc_callback_init init_params;
  #endif
+   struct dc_phy_addr_space_config pa_config;
int r;
  
  	adev->dm.ddev = adev_to_drm(adev);

@@ -1040,6 +1095,12 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
goto error;
}
  
+	mmhub_read_system_context(adev, _config);

+
+   // Call the DC init_memory func
+   dc_setup_system_context(adev->dm.dc, _config);
+
+


The dc_setup_system_context should come directly after dc_hardware_init().

With that fixed this series is

Reviewed-by: Nicholas Kazlauskas 

There's the vmid module as well that could be created after if needed 
but for s/g suport alone that's not necessary.


Regards,
Nicholas Kazlauskas


DRM_DEBUG_DRIVER("KMS initialized.\n");
  
  	return 0;




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


[PATCH 1/2] drm/amd/display: setup system context in dm_init

2020-10-14 Thread Yifan Zhang
Change-Id: I831a5ade8b87c23d21a63d08cc4d338468769e2b
Signed-off-by: Yifan Zhang 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 +++
 1 file changed, 61 insertions(+)

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 3cf4e08931bb..aaff8800c7a0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -887,12 +887,67 @@ static void 
amdgpu_check_debugfs_connector_property_change(struct amdgpu_device
}
 }
 
+static void mmhub_read_system_context(struct amdgpu_device *adev, struct 
dc_phy_addr_space_config *pa_config)
+{
+   uint64_t pt_base;
+   uint32_t logical_addr_low;
+   uint32_t logical_addr_high;
+   uint32_t agp_base, agp_bot, agp_top;
+   PHYSICAL_ADDRESS_LOC page_table_start, page_table_end, page_table_base;
+
+   logical_addr_low  = min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18;
+   pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
+
+   if (adev->apu_flags & AMD_APU_IS_RAVEN2)
+   /*
+* Raven2 has a HW issue that it is unable to use the vram which
+* is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the
+* workaround that increase system aperture high address (add 1)
+* to get rid of the VM fault and hardware hang.
+*/
+   logical_addr_high = max((adev->gmc.fb_end >> 18) + 0x1, 
adev->gmc.agp_end >> 18);
+   else
+   logical_addr_high = max(adev->gmc.fb_end, adev->gmc.agp_end) >> 
18;
+
+   agp_base = 0;
+   agp_bot = adev->gmc.agp_start >> 24;
+   agp_top = adev->gmc.agp_end >> 24;
+
+
+   page_table_start.high_part = (u32)(adev->gmc.gart_start >> 44) & 0xF;
+   page_table_start.low_part = (u32)(adev->gmc.gart_start >> 12);
+   page_table_end.high_part = (u32)(adev->gmc.gart_end >> 44) & 0xF;
+   page_table_end.low_part = (u32)(adev->gmc.gart_end >> 12);
+   page_table_base.high_part = upper_32_bits(pt_base) & 0xF;
+   page_table_base.low_part = lower_32_bits(pt_base);
+
+   pa_config->system_aperture.start_addr = (uint64_t)logical_addr_low << 
18;
+   pa_config->system_aperture.end_addr = (uint64_t)logical_addr_high << 18;
+
+   pa_config->system_aperture.agp_base = (uint64_t)agp_base << 24 ;
+   pa_config->system_aperture.agp_bot = (uint64_t)agp_bot << 24;
+   pa_config->system_aperture.agp_top = (uint64_t)agp_top << 24;
+
+   pa_config->system_aperture.fb_base = adev->gmc.fb_start;
+   pa_config->system_aperture.fb_offset = adev->gmc.aper_base;
+   pa_config->system_aperture.fb_top = adev->gmc.fb_end;
+
+   pa_config->gart_config.page_table_start_addr = 
page_table_start.quad_part << 12;
+   pa_config->gart_config.page_table_end_addr = page_table_end.quad_part 
<< 12;
+   pa_config->gart_config.page_table_base_addr = page_table_base.quad_part;
+
+   pa_config->is_hvm_enabled = 0;
+
+}
+
+
 static int amdgpu_dm_init(struct amdgpu_device *adev)
 {
struct dc_init_data init_data;
 #ifdef CONFIG_DRM_AMD_DC_HDCP
struct dc_callback_init init_params;
 #endif
+   struct dc_phy_addr_space_config pa_config;
int r;
 
adev->dm.ddev = adev_to_drm(adev);
@@ -1040,6 +1095,12 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
goto error;
}
 
+   mmhub_read_system_context(adev, _config);
+
+   // Call the DC init_memory func
+   dc_setup_system_context(adev->dm.dc, _config);
+
+
DRM_DEBUG_DRIVER("KMS initialized.\n");
 
return 0;
-- 
2.17.1

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