[PATCH] drm/amdgpu: replace ih ip block for vega20 and arcturus

2020-10-30 Thread Alex Sierra
[Why]
Vega20 and Arcturus asics use oss 5.0 version.

[How]
Replace ih ip block by navi10 for vega20 and arcturus.

Signed-off-by: Alex Sierra 
---
 drivers/gpu/drm/amd/amdgpu/soc15.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index f57c5f57efa8..fc5b11752931 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -62,6 +62,7 @@
 #include "nbio_v7_0.h"
 #include "nbio_v7_4.h"
 #include "vega10_ih.h"
+#include "navi10_ih.h"
 #include "sdma_v4_0.h"
 #include "uvd_v7_0.h"
 #include "vce_v4_0.h"
@@ -734,9 +735,15 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
else
amdgpu_device_ip_block_add(adev, 
_v3_1_ip_block);
}
-   amdgpu_device_ip_block_add(adev, _ih_ip_block);
+   if (adev->asic_type == CHIP_VEGA20)
+   amdgpu_device_ip_block_add(adev, 
_ih_ip_block);
+   else
+   amdgpu_device_ip_block_add(adev, 
_ih_ip_block);
} else {
-   amdgpu_device_ip_block_add(adev, _ih_ip_block);
+   if (adev->asic_type == CHIP_VEGA20)
+   amdgpu_device_ip_block_add(adev, 
_ih_ip_block);
+   else
+   amdgpu_device_ip_block_add(adev, 
_ih_ip_block);
if (likely(adev->firmware.load_type == 
AMDGPU_FW_LOAD_PSP)) {
if (adev->asic_type == CHIP_VEGA20)
amdgpu_device_ip_block_add(adev, 
_v11_0_ip_block);
@@ -787,9 +794,9 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
if (amdgpu_sriov_vf(adev)) {
if (likely(adev->firmware.load_type == 
AMDGPU_FW_LOAD_PSP))
amdgpu_device_ip_block_add(adev, 
_v11_0_ip_block);
-   amdgpu_device_ip_block_add(adev, _ih_ip_block);
+   amdgpu_device_ip_block_add(adev, _ih_ip_block);
} else {
-   amdgpu_device_ip_block_add(adev, _ih_ip_block);
+   amdgpu_device_ip_block_add(adev, _ih_ip_block);
if (likely(adev->firmware.load_type == 
AMDGPU_FW_LOAD_PSP))
amdgpu_device_ip_block_add(adev, 
_v11_0_ip_block);
}
-- 
2.17.1

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


Re: [PATCH v2] drm/amd/display: Tune min clk values for MPO for RV

2020-10-30 Thread Kazlauskas, Nicholas

On 2020-10-30 2:55 a.m., Pratik Vishwakarma wrote:

[Why]
Incorrect values were resulting in flash lines
when MPO was enabled and system was left idle.

[How]
Increase min clk values only when MPO is enabled
and display is active to not affect S3 power.

Signed-off-by: Pratik Vishwakarma 
Reviewed-by: Nicholas Kazlauskas 


Feel free to merge the patch. V2 still looks OK to me.

Regards,
Nicholas Kazlauskas


---
  .../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c| 30 +--
  1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
index e133edc587d3..75b8240ed059 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
@@ -187,6 +187,17 @@ static void ramp_up_dispclk_with_dpp(
clk_mgr->base.clks.max_supported_dppclk_khz = 
new_clocks->max_supported_dppclk_khz;
  }
  
+static bool is_mpo_enabled(struct dc_state *context)

+{
+   int i;
+
+   for (i = 0; i < context->stream_count; i++) {
+   if (context->stream_status[i].plane_count > 1)
+   return true;
+   }
+   return false;
+}
+
  static void rv1_update_clocks(struct clk_mgr *clk_mgr_base,
struct dc_state *context,
bool safe_to_lower)
@@ -284,9 +295,22 @@ static void rv1_update_clocks(struct clk_mgr *clk_mgr_base,
if (pp_smu->set_hard_min_fclk_by_freq &&
pp_smu->set_hard_min_dcfclk_by_freq &&
pp_smu->set_min_deep_sleep_dcfclk) {
-   pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu, 
new_clocks->fclk_khz / 1000);
-   pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu, 
new_clocks->dcfclk_khz / 1000);
-   pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu, 
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   // Only increase clocks when display is active and MPO 
is enabled
+   if (display_count && is_mpo_enabled(context)) {
+   
pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu,
+   ((new_clocks->fclk_khz / 1000) 
*  101) / 100);
+   
pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu,
+   ((new_clocks->dcfclk_khz / 
1000) * 101) / 100);
+   
pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu,
+   
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   } else {
+   
pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu,
+   new_clocks->fclk_khz / 1000);
+   
pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu,
+   new_clocks->dcfclk_khz / 1000);
+   
pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu,
+   
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   }
}
}
  }



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


Re: [PATCH 1/5] drm/radeon: Stop changing the drm_driver struct

2020-10-30 Thread Alex Deucher
On Fri, Oct 30, 2020 at 6:11 AM Daniel Vetter  wrote:
>
> With only the kms driver left, we can fold this in. This means
> we need to move the ioctl table, which means one additional ioctl
> must be defined in headers.
>
> Also there's a conflict between the radeon_init macro and the module
> init function, so rename the module functions to avoid that.
>
> Signed-off-by: Daniel Vetter 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Daniel Vetter 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon.h |  1 +
>  drivers/gpu/drm/radeon/radeon_drv.c | 85 -
>  drivers/gpu/drm/radeon/radeon_kms.c | 49 +
>  3 files changed, 62 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 5d54bccebd4d..f475785d6491 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2244,6 +2244,7 @@ int radeon_gem_set_tiling_ioctl(struct drm_device *dev, 
> void *data,
> struct drm_file *filp);
>  int radeon_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> +int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file 
> *filp);
>
>  /* VRAM scratch page for HDP bug, default vram page */
>  struct r600_vram_scratch {
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 65061c949aee..9c11e36ad33a 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -51,6 +51,7 @@
>  #include 
>
>  #include "radeon_drv.h"
> +#include "radeon.h"
>
>  /*
>   * KMS wrapper.
> @@ -129,8 +130,6 @@ extern int radeon_get_crtc_scanoutpos(struct drm_device 
> *dev, unsigned int crtc,
>   ktime_t *stime, ktime_t *etime,
>   const struct drm_display_mode *mode);
>  extern bool radeon_is_px(struct drm_device *dev);
> -extern const struct drm_ioctl_desc radeon_ioctls_kms[];
> -extern int radeon_max_kms_ioctl;
>  int radeon_mmap(struct file *filp, struct vm_area_struct *vma);
>  int radeon_mode_dumb_mmap(struct drm_file *filp,
>   struct drm_device *dev,
> @@ -584,9 +583,55 @@ static const struct file_operations 
> radeon_driver_kms_fops = {
>  #endif
>  };
>
> +static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_INIT, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_START, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_STOP, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_RESET, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_IDLE, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_CP_RESUME, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_RESET, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_FULLSCREEN, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_SWAP, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_CLEAR, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_VERTEX, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_INDICES, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_TEXTURE, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_STIPPLE, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_INDIRECT, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_VERTEX2, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_CMDBUF, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_GETPARAM, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_FLIP, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_ALLOC, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_FREE, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_INIT_HEAP, drm_invalid_op, 
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +   DRM_IOCTL_DEF_DRV(RADEON_IRQ_EMIT, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_IRQ_WAIT, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_SETPARAM, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_SURF_ALLOC, drm_invalid_op, DRM_AUTH),
> +   DRM_IOCTL_DEF_DRV(RADEON_SURF_FREE, drm_invalid_op, DRM_AUTH),
> +   /* KMS */
> +   DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, 
> DRM_AUTH|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, 
> DRM_AUTH|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, 
> DRM_AUTH|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, 

Re: [PATCH 4/5] drm/amdgpu: fix build_coefficients() argument

2020-10-30 Thread Alex Deucher
Applied.  Thanks!

Alex

On Fri, Oct 30, 2020 at 1:04 PM Harry Wentland  wrote:
>
>
>
> On 2020-10-29 11:53 p.m., Alex Deucher wrote:
> > On Mon, Oct 26, 2020 at 5:01 PM Arnd Bergmann  wrote:
> >>
> >> From: Arnd Bergmann 
> >>
> >> gcc -Wextra warns about a function taking an enum argument
> >> being called with a bool:
> >>
> >> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c: In 
> >> function 'apply_degamma_for_user_regamma':
> >> drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1617:29: 
> >> warning: implicit conversion from 'enum ' to 'enum 
> >> dc_transfer_func_predefined' [-Wenum-conversion]
> >>   1617 |  build_coefficients(, true);
> >>
> >> It appears that a patch was added using the old calling conventions
> >> after the type was changed, and the value should actually be 0
> >> (TRANSFER_FUNCTION_SRGB) here instead of 1 (true).
> >
> > This looks correct to me.  Harry, Leo?
> >
>
> Confirmed with Kruno, this is correct.
>
> Reviewed-by: Harry Wentland 
>
> Harry
>
> > Alex
> >
> >
> >>
> >> Fixes: 55a01d4023ce ("drm/amd/display: Add user_regamma to color module")
> >> Signed-off-by: Arnd Bergmann 
> >> ---
> >>   drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
> >> b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> >> index b8695660b480..09bc2c249e1a 100644
> >> --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> >> +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> >> @@ -1614,7 +1614,7 @@ static void apply_degamma_for_user_regamma(struct 
> >> pwl_float_data_ex *rgb_regamma
> >>  struct pwl_float_data_ex *rgb = rgb_regamma;
> >>  const struct hw_x_point *coord_x = coordinates_x;
> >>
> >> -   build_coefficients(, true);
> >> +   build_coefficients(, TRANSFER_FUNCTION_SRGB);
> >>
> >>  i = 0;
> >>  while (i != hw_points_num + 1) {
> >> --
> >> 2.27.0
> >>
> >> ___
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Charry.wentland%40amd.com%7C3b50cfb318a04e2708e308d87c875c07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396268091128887%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=QGijLrlFTXI3xx2sGx1iNczHBezfWdu%2FP2xkfoq%2FMB0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] amd/amdgpu: Disable VCN DPG mode for Picasso

2020-10-30 Thread Leo Liu

Reviewed-by: Leo Liu 

On 2020-10-30 1:10 p.m., veerabadhran.gopalakrish...@amd.com wrote:

From: Veerabadhran Gopalakrishnan 

Concurrent operation of VCN and JPEG decoder in DPG mode is
causing ring timeout due to power state.

Signed-off-by: Veerabadhran Gopalakrishnan 
---
  drivers/gpu/drm/amd/amdgpu/soc15.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index ed7342bbf801..f57c5f57efa8 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -1195,8 +1195,7 @@ static int soc15_common_early_init(void *handle)
  
  			adev->pg_flags = AMD_PG_SUPPORT_SDMA |

AMD_PG_SUPPORT_MMHUB |
-   AMD_PG_SUPPORT_VCN |
-   AMD_PG_SUPPORT_VCN_DPG;
+   AMD_PG_SUPPORT_VCN;
} else {
adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
AMD_CG_SUPPORT_GFX_MGLS |

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


[PATCH] amd/amdgpu: Disable VCN DPG mode for Picasso

2020-10-30 Thread veerabadhran.gopalakrishnan
From: Veerabadhran Gopalakrishnan 

Concurrent operation of VCN and JPEG decoder in DPG mode is
causing ring timeout due to power state.

Signed-off-by: Veerabadhran Gopalakrishnan 
---
 drivers/gpu/drm/amd/amdgpu/soc15.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index ed7342bbf801..f57c5f57efa8 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -1195,8 +1195,7 @@ static int soc15_common_early_init(void *handle)
 
adev->pg_flags = AMD_PG_SUPPORT_SDMA |
AMD_PG_SUPPORT_MMHUB |
-   AMD_PG_SUPPORT_VCN |
-   AMD_PG_SUPPORT_VCN_DPG;
+   AMD_PG_SUPPORT_VCN;
} else {
adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
AMD_CG_SUPPORT_GFX_MGLS |
-- 
2.25.1

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


Re: [PATCH 4/5] drm/amdgpu: fix build_coefficients() argument

2020-10-30 Thread Harry Wentland




On 2020-10-29 11:53 p.m., Alex Deucher wrote:

On Mon, Oct 26, 2020 at 5:01 PM Arnd Bergmann  wrote:


From: Arnd Bergmann 

gcc -Wextra warns about a function taking an enum argument
being called with a bool:

drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c: In function 
'apply_degamma_for_user_regamma':
drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1617:29: warning: 
implicit conversion from 'enum ' to 'enum 
dc_transfer_func_predefined' [-Wenum-conversion]
  1617 |  build_coefficients(, true);

It appears that a patch was added using the old calling conventions
after the type was changed, and the value should actually be 0
(TRANSFER_FUNCTION_SRGB) here instead of 1 (true).


This looks correct to me.  Harry, Leo?



Confirmed with Kruno, this is correct.

Reviewed-by: Harry Wentland 

Harry


Alex




Fixes: 55a01d4023ce ("drm/amd/display: Add user_regamma to color module")
Signed-off-by: Arnd Bergmann 
---
  drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c 
b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
index b8695660b480..09bc2c249e1a 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1614,7 +1614,7 @@ static void apply_degamma_for_user_regamma(struct 
pwl_float_data_ex *rgb_regamma
 struct pwl_float_data_ex *rgb = rgb_regamma;
 const struct hw_x_point *coord_x = coordinates_x;

-   build_coefficients(, true);
+   build_coefficients(, TRANSFER_FUNCTION_SRGB);

 i = 0;
 while (i != hw_points_num + 1) {
--
2.27.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Charry.wentland%40amd.com%7C3b50cfb318a04e2708e308d87c875c07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396268091128887%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=QGijLrlFTXI3xx2sGx1iNczHBezfWdu%2FP2xkfoq%2FMB0%3Dreserved=0

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


Re: [PATCH v4 00/11] amd/display: Add GFX9+ modifier support.

2020-10-30 Thread Alex Deucher
Applied.  Thanks for all of your hard work on this!

Alex

On Wed, Oct 28, 2020 at 7:52 PM Bas Nieuwenhuizen
 wrote:
>
> This adds modifier support to the amdgpu kernel drivers for GFX9 and
> later GPUs. It has been tested on
>
> - VEGA10, RAVEN, NAVI14
> - weston, sway, X with xf86-video-amdgpu (i.e. legacy path still works)
>
> and includes some basic testing of the layout code.
>
> The main goal is to keep it somewhat simple and regression free, so
> on the display side this series only exposes what the current GPU
> can render to. While we could expose more I think that is more
> suitable for follow-up work as the benefit would be minimal and
> there are some more design discussion there to discuss that are
> orthogonal from the initial implementation.
>
> Similarly this series only exposes 32-bpp displayable DCC in the cases
> that radeonsi would use it and any extra capabilities here should be
> future work.
>
> I believe these are by far the most complicated modifiers we've seen
> up till now, mostly related to
>
> - GPU identification for cases where it matters wrt tiling.
> - Every generation having tiling layout changes
> - Compression settings.
>
> I believe the complexity is necessary as every layout should be different
> and all layouts should be the best in some situation (though not all
> combinations of GPU parameters will actually have an existing GPU).
>
> That said, on the render side the number of modifiers actually listed for
> a given GPU is ~10, and in the current implementation that is the same
> for the display side. (we could expose more actually differing layouts
> on the display side for cross-GPU compatibility, but I consider that
> out of scope for this initial work).
>
> This series can be found on
> https://github.com/BNieuwenhuizen/linux/tree/modifiers
>
> An userspace implementation in radeonsi can be found on
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6176
>
> which has been reviewed and is ready for submission once these kernel
> patches land.
>
> v2:
>
> Per suggestion from Daniel Vetter I added logic to get the tiling_flags at
> addfb2 time and convert them into modifiers for GFX9+.  Furthermore, the DCC
> constant econding modifers only get exposed on RAVEN2 and newer.
>
> v3:
>
> Fixed some typos, rebased and CCing more people to actually get a review.
>
> v4:
>
> Changed an initialization from {0} to memset, fixed a missing switch case
> in the modifier enumeration and added review tags.
>
> Bas Nieuwenhuizen (11):
>   drm/amd/display: Do not silently accept DCC for multiplane formats.
>   drm/amd: Init modifier field of helper fb.
>   drm/amd/display: Honor the offset for plane 0.
>   drm/fourcc:  Add AMD DRM modifiers.
>   drm/amd/display: Store tiling_flags in the framebuffer.
>   drm/amd/display: Convert tiling_flags to modifiers.
>   drm/amd/display: Refactor surface tiling setup.
>   drm/amd/display: Set DC options from modifiers.
>   drm/amd/display: Add formats for DCC with 2/3 planes.
>   drm/amd/display: Expose modifiers.
>   drm/amd/display: Clean up GFX9 tiling_flags path.
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 169 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c|   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   3 +
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 754 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 -
>  include/uapi/drm/drm_fourcc.h | 115 +++
>  6 files changed, 880 insertions(+), 165 deletions(-)
>
> --
> 2.28.0
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for Polaris only

2020-10-30 Thread Christian König

Tested-by: Christian König 

That indeed fixes the issue.

Thanks,
Christian.

Am 30.10.20 um 12:11 schrieb Christian König:

Thanks, going to test it later today.

Christian.

Am 30.10.20 um 12:07 schrieb Quan, Evan:

[AMD Official Use Only - Internal Distribution Only]

Yes, it is intended for that.

BR
Evan
-Original Message-
From: Koenig, Christian 
Sent: Friday, October 30, 2020 7:06 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH] drm/amdgpu: apply 
dm_pp_notify_wm_clock_changes() for Polaris only


Am 30.10.20 um 12:04 schrieb Evan Quan:

Will expand it to other ASICs after verified.

Change-Id: I03e074ea0e921a984eb819b222e434e8e375
Signed-off-by: Evan Quan 

Acked-by: Christian König 

I assume this fixes my issue on Vega20?

Thanks,
Christian.


---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)

diff --git 
a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c

index fd39dd67bfa4..84065c12d4b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -462,7 +462,12 @@ bool dm_pp_notify_wm_clock_changes(
   void *pp_handle = adev->powerplay.pp_handle;
   const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;

-if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
+/*
+ * Limit this watermark setting for Polaris for now
+ * TODO: expand this to other ASICs
+ */
+if ((adev->asic_type >= CHIP_POLARIS10) && (adev->asic_type <= 
CHIP_VEGAM)

+ && pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
   if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
   (void *)wm_with_clock_ranges))
   return true;


___
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: Potential freesync Arithmetic Issue

2020-10-30 Thread Harry Wentland
Thanks, Bryan, for your report. It was indeed an issue with the merge. 
The fix will be in next week's set of DC patches.


Harry



On 2020-10-28 1:39 p.m., Bryan Angelo wrote:
While looking at the following commit, I noticed what might be an 
arithmetic issue potentially stemming from some merge/patch conflict 
resolution.


commit ad339f69114a6a145fc94d44376851c53dee3475
Author: Jaehyun Chung mailto:jaehyun.ch...@amd.com>>
Date:   Thu Jun 18 15:27:35 2020 -0400

     drm/amd/display: Fix incorrect rounding for 10Hz refresh range

     [Why]
     In cases where refresh range is slightly below 10, FreeSync is not
     active or supported. Need to round values before checking refresh range
     in order to have FreeSync supported in these cases.

     [How]
     Remove redundant values and round values before checking valid 
refresh range.


     Signed-off-by: Jaehyun Chung >

     Reviewed-by: Aric Cyr mailto:aric@amd.com>>
     Acked-by: Anthony Koo >

     Acked-by: Eryk Brol mailto:eryk.b...@amd.com>>
     Signed-off-by: Alex Deucher >


There appears to be an errant plus sign when calculating the 
refresh_range - 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/display/modules/freesync/freesync.c?h=v5.10-rc1#n948 



   refresh_range = div_u64(in_out_vrr->max_refresh_in_uhz + 50, 
100) -

+ div_u64(in_out_vrr->min_refresh_in_uhz + 50, 100);

I am unfamiliar with the freesync codebase so I opted to present the 
potential issue here as opposed to preparing a patch.


Thanks.

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CHARRY.WENTLAND%40amd.com%7Cd75a7f7d0f3c41a2079808d87b731dce%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637395082108267025%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000sdata=sgOQj%2BRfqpEvgb3SjwmyOBq5GzjztttYLimql3M%2FiSA%3Dreserved=0


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


Re: [PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Zhu, James
[AMD Official Use Only - Internal Distribution Only]

This patch is Reviewed-by: James Zhu 

From: Jiansong Chen 
Sent: Friday, October 30, 2020 11:05 AM
To: amd-gfx@lists.freedesktop.org 
Cc: Zhang, Hawking ; Zhu, James ; 
Liu, Leo ; Zhou1, Tao ; Chen, Jiansong 
(Simon) 
Subject: [PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

Toggle on/off gfxoff during video playback to fix gpu hang.

v2: change sequence to be more compatible with original code.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..62d4614d48eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
 }

 if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+   amdgpu_gfx_off_ctrl(adev, true);
 amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
 } else {
@@ -370,7 +371,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 struct amdgpu_device *adev = ring->adev;

 atomic_inc(>vcn.total_submission_cnt);
-   cancel_delayed_work_sync(>vcn.idle_work);
+
+   if (!cancel_delayed_work_sync(>vcn.idle_work))
+   amdgpu_gfx_off_ctrl(adev, false);

 mutex_lock(>vcn.vcn_pg_lock);
 amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
--
2.25.1

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


[PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Jiansong Chen
Toggle on/off gfxoff during video playback to fix gpu hang.

v2: change sequence to be more compatible with original code.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..62d4614d48eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
}
 
if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+   amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_GATE);
} else {
@@ -370,7 +371,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
struct amdgpu_device *adev = ring->adev;
 
atomic_inc(>vcn.total_submission_cnt);
-   cancel_delayed_work_sync(>vcn.idle_work);
+
+   if (!cancel_delayed_work_sync(>vcn.idle_work))
+   amdgpu_gfx_off_ctrl(adev, false);
 
mutex_lock(>vcn.vcn_pg_lock);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-- 
2.25.1

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


RE: [PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Chen, Jiansong (Simon)
[AMD Official Use Only - Internal Distribution Only]

Please ignore the change.

-Original Message-
From: Jiansong Chen 
Sent: Friday, October 30, 2020 10:57 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhu, James ; 
Liu, Leo ; Zhou1, Tao ; Chen, Jiansong 
(Simon) 
Subject: [PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

Toggle on/off gfxoff during video playback to fix gpu hang.

v2: change sequence to be more compatible with original code.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..ef0878e848de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
 }

 if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+amdgpu_gfx_off_ctrl(adev, true);
 amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
 } else {
@@ -370,7 +371,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 struct amdgpu_device *adev = ring->adev;

 atomic_inc(>vcn.total_submission_cnt);
-cancel_delayed_work_sync(>vcn.idle_work);
+
+if (!cancel_delayed_work_sync(>vcn.idle_work);)
+amdgpu_gfx_off_ctrl(adev, false);

 mutex_lock(>vcn.vcn_pg_lock);
 amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
--
2.25.1

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


RE: [PATCH] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Chen, Jiansong (Simon)
[AMD Official Use Only - Internal Distribution Only]

Hi James,
Thanks for your input, v2 patch is sent out.

Regards,
Jiansong
-Original Message-
From: Zhu, James 
Sent: Friday, October 30, 2020 9:06 PM
To: Chen, Jiansong (Simon) ; 
amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhu, James ; 
Liu, Leo ; Zhou1, Tao 
Subject: Re: [PATCH] drm/amdgpu: disable gfxoff if VCN is busy

Hi Jiansong

Pls check inline.

thanks!

James

On 2020-10-30 7:37 a.m., Jiansong Chen wrote:
> Toggle on/off gfxoff during video playback to fix gpu hang.
[JZ] It is a workaround, not a fix. Also Arcturus needn't this WA.
> Signed-off-by: Jiansong Chen 
> Change-Id: I5b938c446884268c2cda0801121a53da980e603a
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 10 +++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 277a8435dd06..444b89413232 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct 
> work_struct *work)
>   }
>
>   if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
> +amdgpu_gfx_off_ctrl(adev, true);
>   amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
>  AMD_PG_STATE_GATE);
>   } else {
> @@ -368,13 +369,16 @@ static void amdgpu_vcn_idle_work_handler(struct 
> work_struct *work)
>   void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>   {
>   struct amdgpu_device *adev = ring->adev;
> +bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
>
>   atomic_inc(>vcn.total_submission_cnt);
> -cancel_delayed_work_sync(>vcn.idle_work);
>
>   mutex_lock(>vcn.vcn_pg_lock);
> -amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> -   AMD_PG_STATE_UNGATE);
> +if (set_clocks) {
> +amdgpu_gfx_off_ctrl(adev, false);

[JZ] Move the above two lines before mutex_lock(>vcn.vcn_pg_lock);

Since it may cause S3 test failure.

> +amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +AMD_PG_STATE_UNGATE);
> +}
>
>   if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG){
>   struct dpg_pause_state new_state;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH v2] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Jiansong Chen
Toggle on/off gfxoff during video playback to fix gpu hang.

v2: change sequence to be more compatible with original code.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..ef0878e848de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
}
 
if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+   amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_GATE);
} else {
@@ -370,7 +371,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
struct amdgpu_device *adev = ring->adev;
 
atomic_inc(>vcn.total_submission_cnt);
-   cancel_delayed_work_sync(>vcn.idle_work);
+
+   if (!cancel_delayed_work_sync(>vcn.idle_work);)
+   amdgpu_gfx_off_ctrl(adev, false);
 
mutex_lock(>vcn.vcn_pg_lock);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-- 
2.25.1

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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Greg KH
On Fri, Oct 30, 2020 at 09:00:04AM +0100, Christian König wrote:
> Am 30.10.20 um 08:57 schrieb Deepak R Varma:
> > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > > > function in place of the debugfs_create_file() function will make the
> > > > file operation struct "reset" aware of the file's lifetime. Additional
> > > > details here: 
> > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Cddd7a6ac8164415a639708d87ca97004%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396414464384011%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=o6GOHvMxNMuOPlC4nhDyURCHBLqfQZhYQq%2BiIMt3D3s%3Dreserved=0
> > > > 
> > > > Issue reported by Coccinelle script:
> > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > 
> > > > Signed-off-by: Deepak R Varma 
> > > > ---
> > > > Please Note: This is a Outreachy project task patch.
> > > > 
> > > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
> > > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, 
> > > > u64 val)
> > > > return 0;
> > > >   }
> > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > -   amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > +amdgpu_debugfs_ib_preempt, "%llu\n");
> > > Are you sure this is ok?  Do these devices need this additional
> > > "protection"?  Do they have the problem that these macros were written
> > > for?
> > > 
> > > Same for the other patches you just submitted here, I think you need to
> > > somehow "prove" that these changes are necessary, checkpatch isn't able
> > > to determine this all the time.
> > Hi Greg,
> > Based on my understanding, the current function debugfs_create_file()
> > adds an overhead of lifetime managing proxy for such fop structs. This
> > should be applicable to these set of drivers as well. Hence I think this
> > change will be useful.
> 
> Well since this is only created once per device instance I don't really care
> about this little overhead.
> 
> But what exactly is debugfs doing or not doing here?

It is trying to save drivers from having debugfs files open that point
to memory that can go away at any time.  For graphics devices, I doubt
that is the case.

thanks,

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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Greg KH
On Fri, Oct 30, 2020 at 01:47:05PM +0530, Sumera Priyadarsini wrote:
> On Fri, 30 Oct, 2020, 1:32 PM Greg KH,  wrote:
> 
> > On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with
> > debugfs_create_file_unsafe()
> > > > > function in place of the debugfs_create_file() function will make the
> > > > > file operation struct "reset" aware of the file's lifetime.
> > Additional
> > > > > details here:
> > https://lists.archive.carbon60.com/linux/kernel/2369498
> > > > >
> > > > > Issue reported by Coccinelle script:
> > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > >
> > > > > Signed-off-by: Deepak R Varma 
> > > > > ---
> > > > > Please Note: This is a Outreachy project task patch.
> > > > >
> > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20
> > ++--
> > > > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void
> > *data, u64 val)
> > > > >   return 0;
> > > > >  }
> > > > >
> > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > > >
> > > > Are you sure this is ok?  Do these devices need this additional
> > > > "protection"?  Do they have the problem that these macros were written
> > > > for?
> > > >
> > > > Same for the other patches you just submitted here, I think you need to
> > > > somehow "prove" that these changes are necessary, checkpatch isn't able
> > > > to determine this all the time.
> > >
> > > Hi Greg,
> > > Based on my understanding, the current function debugfs_create_file()
> > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > should be applicable to these set of drivers as well. Hence I think this
> > > change will be useful.
> >
> > Why do these drivers need these changes?  Are these files dynamically
> > removed from the system at random times?
> >
> > There is a reason we didn't just do a global search/replace for this in
> > the kernel when the new functions were added, so I don't know why
> > checkpatch is now saying it must be done.
> >
> 
> Hi,
> 
> Sorry to jump in on the thread this way, but what exactly does a 'lifetime
> managing proxy' for file operations mean? I am trying to understand how
> DEFINE_DEBUGFS_ATTRIBUTE changes things wrt debug_ fs file operations but
> can't find many resources. :(

It means that the debugfs core can handle debugfs files being removed
from the system while they are still open when they were created by a
driver/module that is now unloaded from memory.

This is only an issue for drivers that manage devices that have unknown
lifespans (i.e. they can be yanked out of the system at any time, and
the memory for those debugfs files can be freed).

For the entire DRM/GPU subsystem, I strongly doubt this is the case.

> Please let me know if I should be asking this in a different mailing
> list/irc instead.
> 
> The change seems to be suggested by a coccinelle script.

I know, and I don't think that script knows the nuances behind this, as,
again, we would have just done a global search/replace for this when the
debugfs fixes went into the kernel.

thanks,

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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Deepak R Varma
On Fri, Oct 30, 2020 at 10:24:57AM +0100, Daniel Vetter wrote:
> On Fri, Oct 30, 2020 at 10:15:21AM +0100, Daniel Vetter wrote:
> > On Fri, Oct 30, 2020 at 09:25:18AM +0100, Greg KH wrote:
> > > On Fri, Oct 30, 2020 at 09:00:04AM +0100, Christian König wrote:
> > > > Am 30.10.20 um 08:57 schrieb Deepak R Varma:
> > > > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with 
> > > > > > > debugfs_create_file_unsafe()
> > > > > > > function in place of the debugfs_create_file() function will make 
> > > > > > > the
> > > > > > > file operation struct "reset" aware of the file's lifetime. 
> > > > > > > Additional
> > > > > > > details here: 
> > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Cddd7a6ac8164415a639708d87ca97004%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396414464384011%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=o6GOHvMxNMuOPlC4nhDyURCHBLqfQZhYQq%2BiIMt3D3s%3Dreserved=0
> > > > > > > 
> > > > > > > Issue reported by Coccinelle script:
> > > > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > > > 
> > > > > > > Signed-off-by: Deepak R Varma 
> > > > > > > ---
> > > > > > > Please Note: This is a Outreachy project task patch.
> > > > > > > 
> > > > > > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 
> > > > > > > ++--
> > > > > > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void 
> > > > > > > *data, u64 val)
> > > > > > >   return 0;
> > > > > > >   }
> > > > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > > Are you sure this is ok?  Do these devices need this additional
> > > > > > "protection"?  Do they have the problem that these macros were 
> > > > > > written
> > > > > > for?
> > > > > > 
> > > > > > Same for the other patches you just submitted here, I think you 
> > > > > > need to
> > > > > > somehow "prove" that these changes are necessary, checkpatch isn't 
> > > > > > able
> > > > > > to determine this all the time.
> > > > > Hi Greg,
> > > > > Based on my understanding, the current function debugfs_create_file()
> > > > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > > > should be applicable to these set of drivers as well. Hence I think 
> > > > > this
> > > > > change will be useful.
> > > > 
> > > > Well since this is only created once per device instance I don't really 
> > > > care
> > > > about this little overhead.
> > > > 
> > > > But what exactly is debugfs doing or not doing here?
> > > 
> > > It is trying to save drivers from having debugfs files open that point
> > > to memory that can go away at any time.  For graphics devices, I doubt
> > > that is the case.
> > 
> > So for anything we add/remove we have two-stage cleanup
> > 
> > 1. drm_dev_unregister (or drm_connector_unregisters, those are the only
> > two hotunpluggable things we have) unregister all the uapi interfaces.
> > This deletes all the sysfs and debugfs files.
> > 
> > 2. Once all the references to the underlying object disappear from the
> > kernel, we free up the data structure.
> > 
> > Now for chardev and similar uapi, we hold full references for open files.
> > But for sysfs and debugfs we assume that those uapi layers will make sure
> > that after we deleted the files in step 1 all access through these
> > functions are guaranteed to have finished. If that's not the case, then we
> > need to rework our refcounting and also refcount the underlying drm
> > structure (drm_device or drm_connector) from sysfs/debugfs files.
> > 
> > Now I tried to look at the patch Deepak references, and I'm not really
> > clear what changes. Or whether we made a wrong assumption previously about
> > what debugfs/sysfs guarantee when we delete the files.
> 
> I read some more code and kerneldoc, and I still have no idea what this
> new _unsafe variant is used for. Only ones I've found seem to use
> debugfs_file_get/put like the normal variant, to protect against
> concurrently removed files due to hotunplug. Which is kinda what we've
> been expecting debugfs to do for us.
> 
> What's a use-case for 

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Julia Lawall



On Fri, 30 Oct 2020, Greg KH wrote:

> On Fri, Oct 30, 2020 at 01:47:05PM +0530, Sumera Priyadarsini wrote:
> > On Fri, 30 Oct, 2020, 1:32 PM Greg KH,  wrote:
> >
> > > On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with
> > > debugfs_create_file_unsafe()
> > > > > > function in place of the debugfs_create_file() function will make 
> > > > > > the
> > > > > > file operation struct "reset" aware of the file's lifetime.
> > > Additional
> > > > > > details here:
> > > https://lists.archive.carbon60.com/linux/kernel/2369498
> > > > > >
> > > > > > Issue reported by Coccinelle script:
> > > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > >
> > > > > > Signed-off-by: Deepak R Varma 
> > > > > > ---
> > > > > > Please Note: This is a Outreachy project task patch.
> > > > > >
> > > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20
> > > ++--
> > > > > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void
> > > *data, u64 val)
> > > > > >   return 0;
> > > > > >  }
> > > > > >
> > > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > >
> > > > > Are you sure this is ok?  Do these devices need this additional
> > > > > "protection"?  Do they have the problem that these macros were written
> > > > > for?
> > > > >
> > > > > Same for the other patches you just submitted here, I think you need 
> > > > > to
> > > > > somehow "prove" that these changes are necessary, checkpatch isn't 
> > > > > able
> > > > > to determine this all the time.
> > > >
> > > > Hi Greg,
> > > > Based on my understanding, the current function debugfs_create_file()
> > > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > > should be applicable to these set of drivers as well. Hence I think this
> > > > change will be useful.
> > >
> > > Why do these drivers need these changes?  Are these files dynamically
> > > removed from the system at random times?
> > >
> > > There is a reason we didn't just do a global search/replace for this in
> > > the kernel when the new functions were added, so I don't know why
> > > checkpatch is now saying it must be done.
> > >
> >
> > Hi,
> >
> > Sorry to jump in on the thread this way, but what exactly does a 'lifetime
> > managing proxy' for file operations mean? I am trying to understand how
> > DEFINE_DEBUGFS_ATTRIBUTE changes things wrt debug_ fs file operations but
> > can't find many resources. :(
>
> It means that the debugfs core can handle debugfs files being removed
> from the system while they are still open when they were created by a
> driver/module that is now unloaded from memory.
>
> This is only an issue for drivers that manage devices that have unknown
> lifespans (i.e. they can be yanked out of the system at any time, and
> the memory for those debugfs files can be freed).
>
> For the entire DRM/GPU subsystem, I strongly doubt this is the case.
>
> > Please let me know if I should be asking this in a different mailing
> > list/irc instead.
> >
> > The change seems to be suggested by a coccinelle script.
>
> I know, and I don't think that script knows the nuances behind this, as,
> again, we would have just done a global search/replace for this when the
> debugfs fixes went into the kernel.

The script comes from Nicolai Stange .

If there are some precise considerations that make the change likely to be
useful, then the script can be changed.

If the script is not helpful, then it can be removed.

julia


>
> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20201030082425.GA1619669%40kroah.com.
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Deucher, Alexander
[AMD Public Use]

Maybe it would be easier to check if the DCE IP exists? E.g.,
if (!amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_DCE) ||
(!amdgpu_device_has_dc_support(adev))
...

Also do we even need to skip DCN init for these cards, or will the display code 
handle it automatically since there will be no displays in the atombios object 
table.  We didn't do anything special for display with the polaris blockchain 
cards.

Alex


From: Zhang, Hawking 
Sent: Friday, October 30, 2020 9:00 AM
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org 

Cc: Tuikov, Luben ; Deucher, Alexander 
; Chen, Guchun ; Cui, Flora 
; Xu, Feifei ; Long, Gang 

Subject: RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU


[AMD Public Use]


[AMD Public Use]



I see, thanks for the clarifying. The patch is



Reviewed-by: Hawking Zhang 



I was thinking to have a new flag like AMD_IS_HEADLESS in amd_chip_flag, but 
after think it a bit more, that’s just complicated the case, I agree with your 
current approach.



Regards,
Hawking

From: Yin, Tianci (Rico) 
Sent: Friday, October 30, 2020 20:05
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Deucher, Alexander 
; Chen, Guchun ; Cui, Flora 
; Xu, Feifei ; Long, Gang 

Subject: Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU



[AMD Public Use]



Hi Hawking,



amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),

at that point, adev has not been allocated yet.



My change can make it to right code path.

int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
{

...

if (!amdgpu_device_has_dc_support(adev))

drm_helper_hpd_irq_event(dev);   //right path for headless SKU

else

drm_kms_helper_hotplug_event(dev);  //wrong path for headless SKU

...

}



Thanks!

Rico





From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Friday, October 30, 2020 19:48
To: Yin, Tianci (Rico) mailto:tianci@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Chen, 
Guchun mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU



[AMD Public Use]

I'm not sure I get your point on changing amdgpu_device_has_dc_support() 
interface by adding new parameter. but it seems to me change input parameter 
from pdev to adev for nv_is_headless_sku is more straightforward.

Regards,
Hawking
-Original Message-
From: Tianci Yin mailto:tianci@amd.com>>
Sent: Friday, October 30, 2020 19:32
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

From: "Tianci.Yin" mailto:tianci@amd.com>>

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting state to avoid 
calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin mailto:tianci@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --  
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
u32 pcie_index, u32 pcie_data,
u32 reg_addr, u64 reg_data);

-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);

 int 

Re: [PATCH] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread James Zhu

Hi Jiansong

Pls check inline.

thanks!

James

On 2020-10-30 7:37 a.m., Jiansong Chen wrote:

Toggle on/off gfxoff during video playback to fix
gpu hang.

[JZ] It is a workaround, not a fix. Also Arcturus needn't this WA.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..444b89413232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
}
  
  	if (!fences && !atomic_read(>vcn.total_submission_cnt)) {

+   amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_GATE);
} else {
@@ -368,13 +369,16 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)
  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
  {
struct amdgpu_device *adev = ring->adev;
+   bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
  
  	atomic_inc(>vcn.total_submission_cnt);

-   cancel_delayed_work_sync(>vcn.idle_work);
  
  	mutex_lock(>vcn.vcn_pg_lock);

-   amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-  AMD_PG_STATE_UNGATE);
+   if (set_clocks) {
+   amdgpu_gfx_off_ctrl(adev, false);


[JZ] Move the above two lines before mutex_lock(>vcn.vcn_pg_lock);

Since it may cause S3 test failure.


+   amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
+   AMD_PG_STATE_UNGATE);
+   }
  
  	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)	{

struct dpg_pause_state new_state;

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


RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Zhang, Hawking
[AMD Public Use]


[AMD Public Use]

I see, thanks for the clarifying. The patch is

Reviewed-by: Hawking Zhang 

I was thinking to have a new flag like AMD_IS_HEADLESS in amd_chip_flag, but 
after think it a bit more, that's just complicated the case, I agree with your 
current approach.

Regards,
Hawking
From: Yin, Tianci (Rico) 
Sent: Friday, October 30, 2020 20:05
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Deucher, Alexander 
; Chen, Guchun ; Cui, Flora 
; Xu, Feifei ; Long, Gang 

Subject: Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU


[AMD Public Use]

Hi Hawking,

amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),
at that point, adev has not been allocated yet.

My change can make it to right code path.
int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
{
...
if (!amdgpu_device_has_dc_support(adev))
drm_helper_hpd_irq_event(dev);   //right path for headless SKU
else
drm_kms_helper_hotplug_event(dev);  //wrong path for headless SKU
...
}

Thanks!
Rico


From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Friday, October 30, 2020 19:48
To: Yin, Tianci (Rico) mailto:tianci@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Chen, 
Guchun mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

[AMD Public Use]

I'm not sure I get your point on changing amdgpu_device_has_dc_support() 
interface by adding new parameter. but it seems to me change input parameter 
from pdev to adev for nv_is_headless_sku is more straightforward.

Regards,
Hawking
-Original Message-
From: Tianci Yin mailto:tianci@amd.com>>
Sent: Friday, October 30, 2020 19:32
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

From: "Tianci.Yin" mailto:tianci@amd.com>>

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting state to avoid 
calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin mailto:tianci@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --  
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
u32 pcie_index, u32 pcie_data,
u32 reg_addr, u64 reg_data);

-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);

 int emu_soc_asic_init(struct amdgpu_device *adev); diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1fe850e0a94d..323ed69032a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct 
amdgpu_device *adev)
  * amdgpu_device_asic_has_dc_support - determine if DC supports the asic
  *
  * @asic_type: AMD asic type
+ * @pdev: pointer to pci_dev instance
  *
  * Check if there is DC (new modesetting infrastructre) support for an asic.
  * returns true if DC has support, false if not.
  */
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,

RE: [PATCH] amdgpu: Add mmhub MGCG and MGLS for vangogh

2020-10-30 Thread Huang, Ray
Reviewed-by: Huang Rui 

-Original Message-
From: Su, Jinzhou (Joe)  
Sent: Friday, October 30, 2020 2:55 PM
To: amd-gfx@lists.freedesktop.org
Cc: Huang, Ray ; Su, Jinzhou (Joe) 
Subject: [PATCH] amdgpu: Add mmhub MGCG and MGLS for vangogh

Add AMD_CG_SUPPORT_MC_MGCG and AMD_CG_SUPPORT_MC_LS

Change-Id: If58cc127a5b5b2449253af6d0f7f2522628639a3
Signed-off-by: Jinzhou.Su 
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c 
index 026e0a8fd526..1f8659a1a4cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -951,6 +951,8 @@ static int nv_common_early_init(void *handle)
AMD_CG_SUPPORT_GFX_CGLS |
AMD_CG_SUPPORT_GFX_3D_CGCG |
AMD_CG_SUPPORT_GFX_3D_CGLS |
+   AMD_CG_SUPPORT_MC_MGCG |
+   AMD_CG_SUPPORT_MC_LS |
AMD_CG_SUPPORT_VCN_MGCG |
AMD_CG_SUPPORT_JPEG_MGCG;
adev->pg_flags = AMD_PG_SUPPORT_GFX_PG |
--
2.17.1

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


Recall: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Zhang, Hawking
Zhang, Hawking would like to recall the message, "[PATCH] drm/amdgpu: fix NULL 
pointer crash on navi10 headless SKU".
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Zhang, Hawking
[AMD Public Use]


[AMD Public Use]

I see. Thanks for the clarifying, Tianci.

In such case, how about we add a new flag AMD_IS_HEADLESS to amd_chip_flags, so 
we can identify headless asic at the beginning when we add a new item to 
pciidlist.

Regards,
Hawking
From: Yin, Tianci (Rico) 
Sent: Friday, October 30, 2020 20:05
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Deucher, Alexander 
; Chen, Guchun ; Cui, Flora 
; Xu, Feifei ; Long, Gang 

Subject: Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU


[AMD Public Use]

Hi Hawking,

amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),
at that point, adev has not been allocated yet.

My change can make it to right code path.
int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
{
...
if (!amdgpu_device_has_dc_support(adev))
drm_helper_hpd_irq_event(dev);   //right path for headless SKU
else
drm_kms_helper_hotplug_event(dev);  //wrong path for headless SKU
...
}

Thanks!
Rico


From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Friday, October 30, 2020 19:48
To: Yin, Tianci (Rico) mailto:tianci@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Chen, 
Guchun mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

[AMD Public Use]

I'm not sure I get your point on changing amdgpu_device_has_dc_support() 
interface by adding new parameter. but it seems to me change input parameter 
from pdev to adev for nv_is_headless_sku is more straightforward.

Regards,
Hawking
-Original Message-
From: Tianci Yin mailto:tianci@amd.com>>
Sent: Friday, October 30, 2020 19:32
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben mailto:luben.tui...@amd.com>>; Deucher, 
Alexander mailto:alexander.deuc...@amd.com>>; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>; Cui, Flora 
mailto:flora@amd.com>>; Xu, Feifei 
mailto:feifei...@amd.com>>; Long, Gang 
mailto:gang.l...@amd.com>>; Yin, Tianci (Rico) 
mailto:tianci@amd.com>>
Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

From: "Tianci.Yin" mailto:tianci@amd.com>>

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting state to avoid 
calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin mailto:tianci@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --  
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
u32 pcie_index, u32 pcie_data,
u32 reg_addr, u64 reg_data);

-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);

 int emu_soc_asic_init(struct amdgpu_device *adev); diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1fe850e0a94d..323ed69032a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct 
amdgpu_device *adev)
  * amdgpu_device_asic_has_dc_support - determine if DC supports the asic
  *
  * @asic_type: AMD asic type
+ * @pdev: pointer to pci_dev instance
  *
  * Check if there is DC (new modesetting infrastructre) support for an asic.
  * returns true if DC has support, false if not.
  */
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev)
 {
 switch 

Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Yin, Tianci (Rico)
[AMD Public Use]

Hi Hawking,

amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),
at that point, adev has not been allocated yet.

My change can make it to right code path.
int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
{
...
if (!amdgpu_device_has_dc_support(adev))
drm_helper_hpd_irq_event(dev);   //right path for headless SKU
else
drm_kms_helper_hotplug_event(dev);  //wrong path for headless SKU
...
}

Thanks!
Rico


From: Zhang, Hawking 
Sent: Friday, October 30, 2020 19:48
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org 

Cc: Tuikov, Luben ; Deucher, Alexander 
; Chen, Guchun ; Cui, Flora 
; Xu, Feifei ; Long, Gang 
; Yin, Tianci (Rico) 
Subject: RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

[AMD Public Use]

I'm not sure I get your point on changing amdgpu_device_has_dc_support() 
interface by adding new parameter. but it seems to me change input parameter 
from pdev to adev for nv_is_headless_sku is more straightforward.

Regards,
Hawking
-Original Message-
From: Tianci Yin 
Sent: Friday, October 30, 2020 19:32
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Deucher, Alexander 
; Zhang, Hawking ; Chen, 
Guchun ; Cui, Flora ; Xu, Feifei 
; Long, Gang ; Yin, Tianci (Rico) 

Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

From: "Tianci.Yin" 

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting state to avoid 
calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --  
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
u32 pcie_index, u32 pcie_data,
u32 reg_addr, u64 reg_data);

-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);

 int emu_soc_asic_init(struct amdgpu_device *adev); diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1fe850e0a94d..323ed69032a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct 
amdgpu_device *adev)
  * amdgpu_device_asic_has_dc_support - determine if DC supports the asic
  *
  * @asic_type: AMD asic type
+ * @pdev: pointer to pci_dev instance
  *
  * Check if there is DC (new modesetting infrastructre) support for an asic.
  * returns true if DC has support, false if not.
  */
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type,
+struct pci_dev *pdev)
 {
 switch (asic_type) {
 #if defined(CONFIG_DRM_AMD_DC)
@@ -3000,9 +3001,14 @@ bool amdgpu_device_asic_has_dc_support(enum 
amd_asic_type asic_type)
 case CHIP_VEGA20:
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 case CHIP_RAVEN:
+   return amdgpu_dc != 0;
 case CHIP_NAVI10:
 case CHIP_NAVI14:
 case CHIP_NAVI12:
+   if (nv_is_headless_sku(pdev))
+   return false;
+   else
+   return amdgpu_dc != 0;
 case CHIP_RENOIR:
 #endif
 #if defined(CONFIG_DRM_AMD_DC_DCN3_0)
@@ -3033,7 +3039,7 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
 if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
 return false;

-   return amdgpu_device_asic_has_dc_support(adev->asic_type);
+   return amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev);
 }


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 9e92d2a070ac..97014458d7de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -516,7 +516,7 @@ uint32_t amdgpu_display_supported_domains(struct 
amdgpu_device 

RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Zhang, Hawking
[AMD Public Use]

I'm not sure I get your point on changing amdgpu_device_has_dc_support() 
interface by adding new parameter. but it seems to me change input parameter 
from pdev to adev for nv_is_headless_sku is more straightforward. 

Regards,
Hawking
-Original Message-
From: Tianci Yin  
Sent: Friday, October 30, 2020 19:32
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Deucher, Alexander 
; Zhang, Hawking ; Chen, 
Guchun ; Cui, Flora ; Xu, Feifei 
; Long, Gang ; Yin, Tianci (Rico) 

Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

From: "Tianci.Yin" 

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting state to avoid 
calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --  
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
   u32 pcie_index, u32 pcie_data,
   u32 reg_addr, u64 reg_data);
 
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, 
+struct pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);
 
 int emu_soc_asic_init(struct amdgpu_device *adev); diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1fe850e0a94d..323ed69032a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct 
amdgpu_device *adev)
  * amdgpu_device_asic_has_dc_support - determine if DC supports the asic
  *
  * @asic_type: AMD asic type
+ * @pdev: pointer to pci_dev instance
  *
  * Check if there is DC (new modesetting infrastructre) support for an asic.
  * returns true if DC has support, false if not.
  */
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, 
+struct pci_dev *pdev)
 {
switch (asic_type) {
 #if defined(CONFIG_DRM_AMD_DC)
@@ -3000,9 +3001,14 @@ bool amdgpu_device_asic_has_dc_support(enum 
amd_asic_type asic_type)
case CHIP_VEGA20:
 #if defined(CONFIG_DRM_AMD_DC_DCN)
case CHIP_RAVEN:
+   return amdgpu_dc != 0;
case CHIP_NAVI10:
case CHIP_NAVI14:
case CHIP_NAVI12:
+   if (nv_is_headless_sku(pdev))
+   return false;
+   else
+   return amdgpu_dc != 0;
case CHIP_RENOIR:
 #endif
 #if defined(CONFIG_DRM_AMD_DC_DCN3_0)
@@ -3033,7 +3039,7 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
return false;
 
-   return amdgpu_device_asic_has_dc_support(adev->asic_type);
+   return amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev);
 }
 
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 9e92d2a070ac..97014458d7de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -516,7 +516,7 @@ uint32_t amdgpu_display_supported_domains(struct 
amdgpu_device *adev,
 */
if ((bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
amdgpu_bo_support_uswc(bo_flags) &&
-   amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+   amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev)) {
switch (adev->asic_type) {
case CHIP_CARRIZO:
case CHIP_STONEY:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4b78ecfd35f7..b23110241267 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1117,7 +1117,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
bool supports_atomic = false;
 
if (!amdgpu_virtual_display &&
-   amdgpu_device_asic_has_dc_support(flags & 

RE: [PATCH] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Zhang, Hawking
[AMD Public Use]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Jiansong Chen  
Sent: Friday, October 30, 2020 19:37
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhu, James ; 
Liu, Leo ; Zhou1, Tao ; Chen, Jiansong 
(Simon) 
Subject: [PATCH] drm/amdgpu: disable gfxoff if VCN is busy

Toggle on/off gfxoff during video playback to fix gpu hang.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..444b89413232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
}
 
if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+   amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_GATE);
} else {
@@ -368,13 +369,16 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)  {
struct amdgpu_device *adev = ring->adev;
+   bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
 
atomic_inc(>vcn.total_submission_cnt);
-   cancel_delayed_work_sync(>vcn.idle_work);
 
mutex_lock(>vcn.vcn_pg_lock);
-   amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-  AMD_PG_STATE_UNGATE);
+   if (set_clocks) {
+   amdgpu_gfx_off_ctrl(adev, false);
+   amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
+   AMD_PG_STATE_UNGATE);
+   }
 
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG){
struct dpg_pause_state new_state;
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: disable gfxoff if VCN is busy

2020-10-30 Thread Jiansong Chen
Toggle on/off gfxoff during video playback to fix
gpu hang.

Signed-off-by: Jiansong Chen 
Change-Id: I5b938c446884268c2cda0801121a53da980e603a
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 277a8435dd06..444b89413232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -358,6 +358,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
}
 
if (!fences && !atomic_read(>vcn.total_submission_cnt)) {
+   amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_GATE);
} else {
@@ -368,13 +369,16 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)
 void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 {
struct amdgpu_device *adev = ring->adev;
+   bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
 
atomic_inc(>vcn.total_submission_cnt);
-   cancel_delayed_work_sync(>vcn.idle_work);
 
mutex_lock(>vcn.vcn_pg_lock);
-   amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-  AMD_PG_STATE_UNGATE);
+   if (set_clocks) {
+   amdgpu_gfx_off_ctrl(adev, false);
+   amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
+   AMD_PG_STATE_UNGATE);
+   }
 
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG){
struct dpg_pause_state new_state;
-- 
2.25.1

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


[PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU

2020-10-30 Thread Tianci Yin
From: "Tianci.Yin" 

The crash caused by the NULL pointer of
adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),
but this function should not be called on headless SKU.

Fix the mismatch between the return value of
amdgpu_device_has_dc_support() and the real DCN supporting
state to avoid calling to drm_kms_helper_hotplug_event()
in amdgpu_device_resume().

Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d
Signed-off-by: Tianci.Yin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 10 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nv.h |  1 +
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ba65d4f2ab67..f0183271456f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device 
*adev,
   u32 pcie_index, u32 pcie_data,
   u32 reg_addr, u64 reg_data);
 
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, struct 
pci_dev *pdev);
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);
 
 int emu_soc_asic_init(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1fe850e0a94d..323ed69032a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct 
amdgpu_device *adev)
  * amdgpu_device_asic_has_dc_support - determine if DC supports the asic
  *
  * @asic_type: AMD asic type
+ * @pdev: pointer to pci_dev instance
  *
  * Check if there is DC (new modesetting infrastructre) support for an asic.
  * returns true if DC has support, false if not.
  */
-bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
+bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, struct 
pci_dev *pdev)
 {
switch (asic_type) {
 #if defined(CONFIG_DRM_AMD_DC)
@@ -3000,9 +3001,14 @@ bool amdgpu_device_asic_has_dc_support(enum 
amd_asic_type asic_type)
case CHIP_VEGA20:
 #if defined(CONFIG_DRM_AMD_DC_DCN)
case CHIP_RAVEN:
+   return amdgpu_dc != 0;
case CHIP_NAVI10:
case CHIP_NAVI14:
case CHIP_NAVI12:
+   if (nv_is_headless_sku(pdev))
+   return false;
+   else
+   return amdgpu_dc != 0;
case CHIP_RENOIR:
 #endif
 #if defined(CONFIG_DRM_AMD_DC_DCN3_0)
@@ -3033,7 +3039,7 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
return false;
 
-   return amdgpu_device_asic_has_dc_support(adev->asic_type);
+   return amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev);
 }
 
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 9e92d2a070ac..97014458d7de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -516,7 +516,7 @@ uint32_t amdgpu_display_supported_domains(struct 
amdgpu_device *adev,
 */
if ((bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
amdgpu_bo_support_uswc(bo_flags) &&
-   amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+   amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev)) {
switch (adev->asic_type) {
case CHIP_CARRIZO:
case CHIP_STONEY:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4b78ecfd35f7..b23110241267 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1117,7 +1117,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
bool supports_atomic = false;
 
if (!amdgpu_virtual_display &&
-   amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK))
+   amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK, pdev))
supports_atomic = true;
 
if ((flags & AMD_EXP_HW_SUPPORT) && !amdgpu_exp_hw_support) {
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 026e0a8fd526..97446ae75b0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -493,7 +493,7 @@ void nv_set_virt_ops(struct amdgpu_device *adev)
adev->virt.ops = _nv_virt_ops;
 }
 
-static bool nv_is_headless_sku(struct pci_dev *pdev)
+bool nv_is_headless_sku(struct 

Re: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for Polaris only

2020-10-30 Thread Christian König

Thanks, going to test it later today.

Christian.

Am 30.10.20 um 12:07 schrieb Quan, Evan:

[AMD Official Use Only - Internal Distribution Only]

Yes, it is intended for that.

BR
Evan
-Original Message-
From: Koenig, Christian 
Sent: Friday, October 30, 2020 7:06 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for 
Polaris only

Am 30.10.20 um 12:04 schrieb Evan Quan:

Will expand it to other ASICs after verified.

Change-Id: I03e074ea0e921a984eb819b222e434e8e375
Signed-off-by: Evan Quan 

Acked-by: Christian König 

I assume this fixes my issue on Vega20?

Thanks,
Christian.


---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index fd39dd67bfa4..84065c12d4b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -462,7 +462,12 @@ bool dm_pp_notify_wm_clock_changes(
   void *pp_handle = adev->powerplay.pp_handle;
   const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;

-if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
+/*
+ * Limit this watermark setting for Polaris for now
+ * TODO: expand this to other ASICs
+ */
+if ((adev->asic_type >= CHIP_POLARIS10) && (adev->asic_type <= CHIP_VEGAM)
+ && pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
   if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
   (void *)wm_with_clock_ranges))
   return true;


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


RE: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for Polaris only

2020-10-30 Thread Quan, Evan
[AMD Official Use Only - Internal Distribution Only]

Yes, it is intended for that.

BR
Evan
-Original Message-
From: Koenig, Christian 
Sent: Friday, October 30, 2020 7:06 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for 
Polaris only

Am 30.10.20 um 12:04 schrieb Evan Quan:
> Will expand it to other ASICs after verified.
>
> Change-Id: I03e074ea0e921a984eb819b222e434e8e375
> Signed-off-by: Evan Quan 

Acked-by: Christian König 

I assume this fixes my issue on Vega20?

Thanks,
Christian.

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 7 ++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> index fd39dd67bfa4..84065c12d4b8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> @@ -462,7 +462,12 @@ bool dm_pp_notify_wm_clock_changes(
>   void *pp_handle = adev->powerplay.pp_handle;
>   const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
>
> -if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
> +/*
> + * Limit this watermark setting for Polaris for now
> + * TODO: expand this to other ASICs
> + */
> +if ((adev->asic_type >= CHIP_POLARIS10) && (adev->asic_type <= CHIP_VEGAM)
> + && pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
>   if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
>   (void *)wm_with_clock_ranges))
>   return true;

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


RE: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting

2020-10-30 Thread Quan, Evan
[AMD Official Use Only - Internal Distribution Only]

Hi Christian,

Just sent out a patch which should be able to address this issue you saw.
https://lists.freedesktop.org/archives/amd-gfx/2020-October/055431.html

BR
Evan
-Original Message-
From: Christian König 
Sent: Thursday, October 29, 2020 5:46 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting

Hi guys,

this commit here completely breaks Vega20.

Any idea why a change for Polaris should have that effect? Can we please fix 
this ASAP?

Thanks,
Christian.

Am 16.10.20 um 05:26 schrieb Evan Quan:
> Enable watermark table setting for Polaris.
>
> Change-Id: I395b74f2ce5b74e6d1c7659816ee386ba556e14c
> Signed-off-by: Evan Quan 
> Acked-by: Alex Deucher 
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  | 11 +++-
>   .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c   | 50 +++
>   2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> index deb9164eb5fe..fd39dd67bfa4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> @@ -458,7 +458,16 @@ bool dm_pp_notify_wm_clock_changes(
>   const struct dc_context *ctx,
>   struct dm_pp_wm_sets_with_clock_ranges *wm_with_clock_ranges)
>   {
> -/* TODO: to be implemented */
> +struct amdgpu_device *adev = ctx->driver_context;
> +void *pp_handle = adev->powerplay.pp_handle;
> +const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> +
> +if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
> +if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
> +(void *)wm_with_clock_ranges))
> +return true;
> +}
> +
>   return false;
>   }
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> index 3700352286c5..ce8f368c0a8c 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> @@ -49,6 +49,8 @@
>   #include "processpptables.h"
>   #include "pp_thermal.h"
>   #include "smu7_baco.h"
> +#include "smu7_smumgr.h"
> +#include "polaris10_smumgr.h"
>
>   #include "ivsrcid/ivsrcid_vislands30.h"
>
> @@ -5107,6 +5109,53 @@ static int smu7_get_clock_by_type_with_latency(struct 
> pp_hwmgr *hwmgr,
>   return 0;
>   }
>
> +static int smu7_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
> + void *clock_range)
> +{
> +struct phm_ppt_v1_information *table_info =
> +(struct phm_ppt_v1_information *)hwmgr->pptable;
> +struct phm_ppt_v1_clock_voltage_dependency_table *dep_mclk_table =
> +table_info->vdd_dep_on_mclk;
> +struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table =
> +table_info->vdd_dep_on_sclk;
> +struct polaris10_smumgr *smu_data =
> +(struct polaris10_smumgr *)(hwmgr->smu_backend);
> +SMU74_Discrete_DpmTable  *table = &(smu_data->smc_state_table);
> +struct dm_pp_wm_sets_with_clock_ranges *watermarks =
> +(struct dm_pp_wm_sets_with_clock_ranges *)clock_range;
> +uint32_t i, j, k;
> +bool valid_entry;
> +
> +if (!(hwmgr->chip_id >= CHIP_POLARIS10 &&
> +  hwmgr->chip_id <= CHIP_VEGAM))
> +return -EINVAL;
> +
> +for (i = 0; i < dep_mclk_table->count; i++) {
> +for (j = 0; j < dep_sclk_table->count; j++) {
> +valid_entry = false;
> +for (k = 0; k < watermarks->num_wm_sets; k++) {
> +if (dep_sclk_table->entries[i].clk / 10 >= 
> watermarks->wm_clk_ranges[k].wm_min_eng_clk_in_khz &&
> +dep_sclk_table->entries[i].clk / 10 < 
> watermarks->wm_clk_ranges[k].wm_max_eng_clk_in_khz &&
> +dep_mclk_table->entries[i].clk / 10 >= 
> watermarks->wm_clk_ranges[k].wm_min_mem_clk_in_khz &&
> +dep_mclk_table->entries[i].clk / 10 < 
> watermarks->wm_clk_ranges[k].wm_max_mem_clk_in_khz) {
> +valid_entry = true;
> +table->DisplayWatermark[i][j] = watermarks->wm_clk_ranges[k].wm_set_id;
> +break;
> +}
> +}
> +PP_ASSERT_WITH_CODE(valid_entry,
> +"Clock is not in range of specified clock range for watermark from DAL!  
> Using highest water mark set.",
> +table->DisplayWatermark[i][j] = watermarks->wm_clk_ranges[k - 1].wm_set_id);
> +}
> +}
> +
> +return smu7_copy_bytes_to_smc(hwmgr,
> +  smu_data->smu7_data.dpm_table_start + 
> offsetof(SMU74_Discrete_DpmTable, DisplayWatermark),
> +  (uint8_t *)table->DisplayWatermark,
> +  sizeof(uint8_t) * SMU74_MAX_LEVELS_MEMORY * SMU74_MAX_LEVELS_GRAPHICS,
> +  SMC_RAM_END);
> +}
> +
>   static int smu7_notify_cac_buffer_info(struct pp_hwmgr *hwmgr,
>   uint32_t virtual_addr_low,
>   uint32_t virtual_addr_hi,
> @@ -5525,6 +5574,7 @@ static const struct pp_hwmgr_func smu7_hwmgr_funcs = {
>   .set_mclk_od = smu7_set_mclk_od,
>   .get_clock_by_type = smu7_get_clock_by_type,
>   .get_clock_by_type_with_latency =
> smu7_get_clock_by_type_with_latency,
> 

Re: [PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for Polaris only

2020-10-30 Thread Christian König

Am 30.10.20 um 12:04 schrieb Evan Quan:

Will expand it to other ASICs after verified.

Change-Id: I03e074ea0e921a984eb819b222e434e8e375
Signed-off-by: Evan Quan 


Acked-by: Christian König 

I assume this fixes my issue on Vega20?

Thanks,
Christian.


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index fd39dd67bfa4..84065c12d4b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -462,7 +462,12 @@ bool dm_pp_notify_wm_clock_changes(
void *pp_handle = adev->powerplay.pp_handle;
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
  
-	if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {

+   /*
+* Limit this watermark setting for Polaris for now
+* TODO: expand this to other ASICs
+*/
+   if ((adev->asic_type >= CHIP_POLARIS10) && (adev->asic_type <= 
CHIP_VEGAM)
+&& pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
(void *)wm_with_clock_ranges))
return true;


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


[PATCH] drm/amdgpu: apply dm_pp_notify_wm_clock_changes() for Polaris only

2020-10-30 Thread Evan Quan
Will expand it to other ASICs after verified.

Change-Id: I03e074ea0e921a984eb819b222e434e8e375
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index fd39dd67bfa4..84065c12d4b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -462,7 +462,12 @@ bool dm_pp_notify_wm_clock_changes(
void *pp_handle = adev->powerplay.pp_handle;
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
 
-   if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
+   /*
+* Limit this watermark setting for Polaris for now
+* TODO: expand this to other ASICs
+*/
+   if ((adev->asic_type >= CHIP_POLARIS10) && (adev->asic_type <= 
CHIP_VEGAM)
+&& pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
(void *)wm_with_clock_ranges))
return true;
-- 
2.29.0

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


[PATCH 1/5] drm/radeon: Stop changing the drm_driver struct

2020-10-30 Thread Daniel Vetter
With only the kms driver left, we can fold this in. This means
we need to move the ioctl table, which means one additional ioctl
must be defined in headers.

Also there's a conflict between the radeon_init macro and the module
init function, so rename the module functions to avoid that.

Signed-off-by: Daniel Vetter 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/radeon/radeon.h |  1 +
 drivers/gpu/drm/radeon/radeon_drv.c | 85 -
 drivers/gpu/drm/radeon/radeon_kms.c | 49 +
 3 files changed, 62 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 5d54bccebd4d..f475785d6491 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2244,6 +2244,7 @@ int radeon_gem_set_tiling_ioctl(struct drm_device *dev, 
void *data,
struct drm_file *filp);
 int radeon_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp);
+int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file 
*filp);
 
 /* VRAM scratch page for HDP bug, default vram page */
 struct r600_vram_scratch {
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 65061c949aee..9c11e36ad33a 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -51,6 +51,7 @@
 #include 
 
 #include "radeon_drv.h"
+#include "radeon.h"
 
 /*
  * KMS wrapper.
@@ -129,8 +130,6 @@ extern int radeon_get_crtc_scanoutpos(struct drm_device 
*dev, unsigned int crtc,
  ktime_t *stime, ktime_t *etime,
  const struct drm_display_mode *mode);
 extern bool radeon_is_px(struct drm_device *dev);
-extern const struct drm_ioctl_desc radeon_ioctls_kms[];
-extern int radeon_max_kms_ioctl;
 int radeon_mmap(struct file *filp, struct vm_area_struct *vma);
 int radeon_mode_dumb_mmap(struct drm_file *filp,
  struct drm_device *dev,
@@ -584,9 +583,55 @@ static const struct file_operations radeon_driver_kms_fops 
= {
 #endif
 };
 
+static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
+   DRM_IOCTL_DEF_DRV(RADEON_CP_INIT, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_CP_START, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_CP_STOP, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_CP_RESET, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_CP_IDLE, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_CP_RESUME, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_RESET, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_FULLSCREEN, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_SWAP, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_CLEAR, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_VERTEX, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_INDICES, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_TEXTURE, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_STIPPLE, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_INDIRECT, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_VERTEX2, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_CMDBUF, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_GETPARAM, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_FLIP, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_ALLOC, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_FREE, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_INIT_HEAP, drm_invalid_op, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+   DRM_IOCTL_DEF_DRV(RADEON_IRQ_EMIT, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_IRQ_WAIT, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_SETPARAM, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_SURF_ALLOC, drm_invalid_op, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_SURF_FREE, drm_invalid_op, DRM_AUTH),
+   /* KMS */
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, radeon_gem_set_domain_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_PREAD, radeon_gem_pread_ioctl, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_PWRITE, radeon_gem_pwrite_ioctl, DRM_AUTH),
+   DRM_IOCTL_DEF_DRV(RADEON_GEM_WAIT_IDLE, radeon_gem_wait_idle_ioctl, 

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Christian König

Am 30.10.20 um 09:25 schrieb Greg KH:

On Fri, Oct 30, 2020 at 09:00:04AM +0100, Christian König wrote:

Am 30.10.20 um 08:57 schrieb Deepak R Varma:

On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:

On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:

Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
function in place of the debugfs_create_file() function will make the
file operation struct "reset" aware of the file's lifetime. Additional
details here: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Ce3fb2f2236f44f8779bc08d87cad3a2d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396430734542672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=CuDnWSIacKfdcv6%2B00Q9mZ%2BAcXse5mlFpuSsBybZ%2Fww%3Dreserved=0

Issue reported by Coccinelle script:
scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci

Signed-off-by: Deepak R Varma 
---
Please Note: This is a Outreachy project task patch.

   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
   1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 2d125b8b15ee..f076b1ba7319 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 val)
return 0;
   }
-DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
-   amdgpu_debugfs_ib_preempt, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
+amdgpu_debugfs_ib_preempt, "%llu\n");

Are you sure this is ok?  Do these devices need this additional
"protection"?  Do they have the problem that these macros were written
for?

Same for the other patches you just submitted here, I think you need to
somehow "prove" that these changes are necessary, checkpatch isn't able
to determine this all the time.

Hi Greg,
Based on my understanding, the current function debugfs_create_file()
adds an overhead of lifetime managing proxy for such fop structs. This
should be applicable to these set of drivers as well. Hence I think this
change will be useful.

Well since this is only created once per device instance I don't really care
about this little overhead.

But what exactly is debugfs doing or not doing here?

It is trying to save drivers from having debugfs files open that point
to memory that can go away at any time.  For graphics devices, I doubt
that is the case.


Well we have somebody working on hot plug removal for dGPUs, but that is 
really not an issue here.


Even if our device are removed most of the debugfs files shouldn't be 
affected by that.


Thanks,
Christian.



thanks,

greg k-h


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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Daniel Vetter
On Fri, Oct 30, 2020 at 10:15:21AM +0100, Daniel Vetter wrote:
> On Fri, Oct 30, 2020 at 09:25:18AM +0100, Greg KH wrote:
> > On Fri, Oct 30, 2020 at 09:00:04AM +0100, Christian König wrote:
> > > Am 30.10.20 um 08:57 schrieb Deepak R Varma:
> > > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with 
> > > > > > debugfs_create_file_unsafe()
> > > > > > function in place of the debugfs_create_file() function will make 
> > > > > > the
> > > > > > file operation struct "reset" aware of the file's lifetime. 
> > > > > > Additional
> > > > > > details here: 
> > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Cddd7a6ac8164415a639708d87ca97004%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396414464384011%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=o6GOHvMxNMuOPlC4nhDyURCHBLqfQZhYQq%2BiIMt3D3s%3Dreserved=0
> > > > > > 
> > > > > > Issue reported by Coccinelle script:
> > > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > > 
> > > > > > Signed-off-by: Deepak R Varma 
> > > > > > ---
> > > > > > Please Note: This is a Outreachy project task patch.
> > > > > > 
> > > > > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 
> > > > > > ++--
> > > > > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void 
> > > > > > *data, u64 val)
> > > > > > return 0;
> > > > > >   }
> > > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > -   amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > > +amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > Are you sure this is ok?  Do these devices need this additional
> > > > > "protection"?  Do they have the problem that these macros were written
> > > > > for?
> > > > > 
> > > > > Same for the other patches you just submitted here, I think you need 
> > > > > to
> > > > > somehow "prove" that these changes are necessary, checkpatch isn't 
> > > > > able
> > > > > to determine this all the time.
> > > > Hi Greg,
> > > > Based on my understanding, the current function debugfs_create_file()
> > > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > > should be applicable to these set of drivers as well. Hence I think this
> > > > change will be useful.
> > > 
> > > Well since this is only created once per device instance I don't really 
> > > care
> > > about this little overhead.
> > > 
> > > But what exactly is debugfs doing or not doing here?
> > 
> > It is trying to save drivers from having debugfs files open that point
> > to memory that can go away at any time.  For graphics devices, I doubt
> > that is the case.
> 
> So for anything we add/remove we have two-stage cleanup
> 
> 1. drm_dev_unregister (or drm_connector_unregisters, those are the only
> two hotunpluggable things we have) unregister all the uapi interfaces.
> This deletes all the sysfs and debugfs files.
> 
> 2. Once all the references to the underlying object disappear from the
> kernel, we free up the data structure.
> 
> Now for chardev and similar uapi, we hold full references for open files.
> But for sysfs and debugfs we assume that those uapi layers will make sure
> that after we deleted the files in step 1 all access through these
> functions are guaranteed to have finished. If that's not the case, then we
> need to rework our refcounting and also refcount the underlying drm
> structure (drm_device or drm_connector) from sysfs/debugfs files.
> 
> Now I tried to look at the patch Deepak references, and I'm not really
> clear what changes. Or whether we made a wrong assumption previously about
> what debugfs/sysfs guarantee when we delete the files.

I read some more code and kerneldoc, and I still have no idea what this
new _unsafe variant is used for. Only ones I've found seem to use
debugfs_file_get/put like the normal variant, to protect against
concurrently removed files due to hotunplug. Which is kinda what we've
been expecting debugfs to do for us.

What's a use-case for _unsafe _without_ debugfs_file_get/put?

Decently confused me over here doesn't get this.
-Daniel

> -Daniel
> 
> > 
> > thanks,
> > 
> > greg k-h
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, 

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Daniel Vetter
On Fri, Oct 30, 2020 at 09:25:18AM +0100, Greg KH wrote:
> On Fri, Oct 30, 2020 at 09:00:04AM +0100, Christian König wrote:
> > Am 30.10.20 um 08:57 schrieb Deepak R Varma:
> > > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > > > > function in place of the debugfs_create_file() function will make the
> > > > > file operation struct "reset" aware of the file's lifetime. Additional
> > > > > details here: 
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Cddd7a6ac8164415a639708d87ca97004%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396414464384011%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=o6GOHvMxNMuOPlC4nhDyURCHBLqfQZhYQq%2BiIMt3D3s%3Dreserved=0
> > > > > 
> > > > > Issue reported by Coccinelle script:
> > > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > 
> > > > > Signed-off-by: Deepak R Varma 
> > > > > ---
> > > > > Please Note: This is a Outreachy project task patch.
> > > > > 
> > > > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 
> > > > > ++--
> > > > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void 
> > > > > *data, u64 val)
> > > > >   return 0;
> > > > >   }
> > > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > Are you sure this is ok?  Do these devices need this additional
> > > > "protection"?  Do they have the problem that these macros were written
> > > > for?
> > > > 
> > > > Same for the other patches you just submitted here, I think you need to
> > > > somehow "prove" that these changes are necessary, checkpatch isn't able
> > > > to determine this all the time.
> > > Hi Greg,
> > > Based on my understanding, the current function debugfs_create_file()
> > > adds an overhead of lifetime managing proxy for such fop structs. This
> > > should be applicable to these set of drivers as well. Hence I think this
> > > change will be useful.
> > 
> > Well since this is only created once per device instance I don't really care
> > about this little overhead.
> > 
> > But what exactly is debugfs doing or not doing here?
> 
> It is trying to save drivers from having debugfs files open that point
> to memory that can go away at any time.  For graphics devices, I doubt
> that is the case.

So for anything we add/remove we have two-stage cleanup

1. drm_dev_unregister (or drm_connector_unregisters, those are the only
two hotunpluggable things we have) unregister all the uapi interfaces.
This deletes all the sysfs and debugfs files.

2. Once all the references to the underlying object disappear from the
kernel, we free up the data structure.

Now for chardev and similar uapi, we hold full references for open files.
But for sysfs and debugfs we assume that those uapi layers will make sure
that after we deleted the files in step 1 all access through these
functions are guaranteed to have finished. If that's not the case, then we
need to rework our refcounting and also refcount the underlying drm
structure (drm_device or drm_connector) from sysfs/debugfs files.

Now I tried to look at the patch Deepak references, and I'm not really
clear what changes. Or whether we made a wrong assumption previously about
what debugfs/sysfs guarantee when we delete the files.
-Daniel

> 
> thanks,
> 
> greg k-h

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Deepak R Varma
On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > function in place of the debugfs_create_file() function will make the
> > file operation struct "reset" aware of the file's lifetime. Additional
> > details here: https://lists.archive.carbon60.com/linux/kernel/2369498
> > 
> > Issue reported by Coccinelle script:
> > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > 
> > Signed-off-by: Deepak R Varma 
> > ---
> > Please Note: This is a Outreachy project task patch.
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index 2d125b8b15ee..f076b1ba7319 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 
> > val)
> > return 0;
> >  }
> >  
> > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > -   amdgpu_debugfs_ib_preempt, "%llu\n");
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > +amdgpu_debugfs_ib_preempt, "%llu\n");
> 
> Are you sure this is ok?  Do these devices need this additional
> "protection"?  Do they have the problem that these macros were written
> for?
> 
> Same for the other patches you just submitted here, I think you need to
> somehow "prove" that these changes are necessary, checkpatch isn't able
> to determine this all the time.

Hi Greg,
Based on my understanding, the current function debugfs_create_file()
adds an overhead of lifetime managing proxy for such fop structs. This
should be applicable to these set of drivers as well. Hence I think this
change will be useful.

I will wait for comments from other experts for driver specific
consideration / behavior.

Thanks,
drv


> 
> thanks,
> 
> greg k-h
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Greg KH
On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > > function in place of the debugfs_create_file() function will make the
> > > file operation struct "reset" aware of the file's lifetime. Additional
> > > details here: https://lists.archive.carbon60.com/linux/kernel/2369498
> > > 
> > > Issue reported by Coccinelle script:
> > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > 
> > > Signed-off-by: Deepak R Varma 
> > > ---
> > > Please Note: This is a Outreachy project task patch.
> > > 
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
> > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > index 2d125b8b15ee..f076b1ba7319 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, 
> > > u64 val)
> > >   return 0;
> > >  }
> > >  
> > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > 
> > Are you sure this is ok?  Do these devices need this additional
> > "protection"?  Do they have the problem that these macros were written
> > for?
> > 
> > Same for the other patches you just submitted here, I think you need to
> > somehow "prove" that these changes are necessary, checkpatch isn't able
> > to determine this all the time.
> 
> Hi Greg,
> Based on my understanding, the current function debugfs_create_file()
> adds an overhead of lifetime managing proxy for such fop structs. This
> should be applicable to these set of drivers as well. Hence I think this
> change will be useful.

Why do these drivers need these changes?  Are these files dynamically
removed from the system at random times?

There is a reason we didn't just do a global search/replace for this in
the kernel when the new functions were added, so I don't know why
checkpatch is now saying it must be done.

thanks,

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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Sumera Priyadarsini
On Fri, 30 Oct, 2020, 1:32 PM Greg KH,  wrote:

> On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with
> debugfs_create_file_unsafe()
> > > > function in place of the debugfs_create_file() function will make the
> > > > file operation struct "reset" aware of the file's lifetime.
> Additional
> > > > details here:
> https://lists.archive.carbon60.com/linux/kernel/2369498
> > > >
> > > > Issue reported by Coccinelle script:
> > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > >
> > > > Signed-off-by: Deepak R Varma 
> > > > ---
> > > > Please Note: This is a Outreachy project task patch.
> > > >
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20
> ++--
> > > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > index 2d125b8b15ee..f076b1ba7319 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > > @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void
> *data, u64 val)
> > > >   return 0;
> > > >  }
> > > >
> > > > -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > - amdgpu_debugfs_ib_preempt, "%llu\n");
> > > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> > > > +  amdgpu_debugfs_ib_preempt, "%llu\n");
> > >
> > > Are you sure this is ok?  Do these devices need this additional
> > > "protection"?  Do they have the problem that these macros were written
> > > for?
> > >
> > > Same for the other patches you just submitted here, I think you need to
> > > somehow "prove" that these changes are necessary, checkpatch isn't able
> > > to determine this all the time.
> >
> > Hi Greg,
> > Based on my understanding, the current function debugfs_create_file()
> > adds an overhead of lifetime managing proxy for such fop structs. This
> > should be applicable to these set of drivers as well. Hence I think this
> > change will be useful.
>
> Why do these drivers need these changes?  Are these files dynamically
> removed from the system at random times?
>
> There is a reason we didn't just do a global search/replace for this in
> the kernel when the new functions were added, so I don't know why
> checkpatch is now saying it must be done.
>

Hi,

Sorry to jump in on the thread this way, but what exactly does a 'lifetime
managing proxy' for file operations mean? I am trying to understand how
DEFINE_DEBUGFS_ATTRIBUTE changes things wrt debug_ fs file operations but
can't find many resources. :(

Please let me know if I should be asking this in a different mailing
list/irc instead.

The change seems to be suggested by a coccinelle script.

Regards,
Sumera


thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/20201030080316.GA1612206%40kroah.com
> .
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Greg KH
On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> function in place of the debugfs_create_file() function will make the
> file operation struct "reset" aware of the file's lifetime. Additional
> details here: https://lists.archive.carbon60.com/linux/kernel/2369498
> 
> Issue reported by Coccinelle script:
> scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> 
> Signed-off-by: Deepak R Varma 
> ---
> Please Note: This is a Outreachy project task patch.
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 2d125b8b15ee..f076b1ba7319 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 
> val)
>   return 0;
>  }
>  
> -DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> - amdgpu_debugfs_ib_preempt, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> +  amdgpu_debugfs_ib_preempt, "%llu\n");

Are you sure this is ok?  Do these devices need this additional
"protection"?  Do they have the problem that these macros were written
for?

Same for the other patches you just submitted here, I think you need to
somehow "prove" that these changes are necessary, checkpatch isn't able
to determine this all the time.

thanks,

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


Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

2020-10-30 Thread Christian König

Am 30.10.20 um 08:57 schrieb Deepak R Varma:

On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:

On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:

Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
function in place of the debugfs_create_file() function will make the
file operation struct "reset" aware of the file's lifetime. Additional
details here: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.archive.carbon60.com%2Flinux%2Fkernel%2F2369498data=04%7C01%7Cchristian.koenig%40amd.com%7Cddd7a6ac8164415a639708d87ca97004%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637396414464384011%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=o6GOHvMxNMuOPlC4nhDyURCHBLqfQZhYQq%2BiIMt3D3s%3Dreserved=0

Issue reported by Coccinelle script:
scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci

Signed-off-by: Deepak R Varma 
---
Please Note: This is a Outreachy project task patch.

  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 2d125b8b15ee..f076b1ba7319 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1551,29 +1551,29 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 val)
return 0;
  }
  
-DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,

-   amdgpu_debugfs_ib_preempt, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
+amdgpu_debugfs_ib_preempt, "%llu\n");

Are you sure this is ok?  Do these devices need this additional
"protection"?  Do they have the problem that these macros were written
for?

Same for the other patches you just submitted here, I think you need to
somehow "prove" that these changes are necessary, checkpatch isn't able
to determine this all the time.

Hi Greg,
Based on my understanding, the current function debugfs_create_file()
adds an overhead of lifetime managing proxy for such fop structs. This
should be applicable to these set of drivers as well. Hence I think this
change will be useful.


Well since this is only created once per device instance I don't really 
care about this little overhead.


But what exactly is debugfs doing or not doing here?

Regards,
Christian.



I will wait for comments from other experts for driver specific
consideration / behavior.

Thanks,
drv



thanks,

greg k-h


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


[PATCH v2] drm/amd/display: Tune min clk values for MPO for RV

2020-10-30 Thread Pratik Vishwakarma
[Why]
Incorrect values were resulting in flash lines
when MPO was enabled and system was left idle.

[How]
Increase min clk values only when MPO is enabled
and display is active to not affect S3 power.

Signed-off-by: Pratik Vishwakarma 
Reviewed-by: Nicholas Kazlauskas 
---
 .../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c| 30 +--
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
index e133edc587d3..75b8240ed059 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
@@ -187,6 +187,17 @@ static void ramp_up_dispclk_with_dpp(
clk_mgr->base.clks.max_supported_dppclk_khz = 
new_clocks->max_supported_dppclk_khz;
 }
 
+static bool is_mpo_enabled(struct dc_state *context)
+{
+   int i;
+
+   for (i = 0; i < context->stream_count; i++) {
+   if (context->stream_status[i].plane_count > 1)
+   return true;
+   }
+   return false;
+}
+
 static void rv1_update_clocks(struct clk_mgr *clk_mgr_base,
struct dc_state *context,
bool safe_to_lower)
@@ -284,9 +295,22 @@ static void rv1_update_clocks(struct clk_mgr *clk_mgr_base,
if (pp_smu->set_hard_min_fclk_by_freq &&
pp_smu->set_hard_min_dcfclk_by_freq &&
pp_smu->set_min_deep_sleep_dcfclk) {
-   pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu, 
new_clocks->fclk_khz / 1000);
-   pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu, 
new_clocks->dcfclk_khz / 1000);
-   pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu, 
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   // Only increase clocks when display is active and MPO 
is enabled
+   if (display_count && is_mpo_enabled(context)) {
+   
pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu,
+   ((new_clocks->fclk_khz / 1000) 
*  101) / 100);
+   
pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu,
+   ((new_clocks->dcfclk_khz / 
1000) * 101) / 100);
+   
pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu,
+   
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   } else {
+   
pp_smu->set_hard_min_fclk_by_freq(_smu->pp_smu,
+   new_clocks->fclk_khz / 1000);
+   
pp_smu->set_hard_min_dcfclk_by_freq(_smu->pp_smu,
+   new_clocks->dcfclk_khz / 1000);
+   
pp_smu->set_min_deep_sleep_dcfclk(_smu->pp_smu,
+   
(new_clocks->dcfclk_deep_sleep_khz + 999) / 1000);
+   }
}
}
 }
-- 
2.25.1

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


[PATCH] amdgpu: Add mmhub MGCG and MGLS for vangogh

2020-10-30 Thread Jinzhou.Su
Add AMD_CG_SUPPORT_MC_MGCG and AMD_CG_SUPPORT_MC_LS

Change-Id: If58cc127a5b5b2449253af6d0f7f2522628639a3
Signed-off-by: Jinzhou.Su 
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 026e0a8fd526..1f8659a1a4cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -951,6 +951,8 @@ static int nv_common_early_init(void *handle)
AMD_CG_SUPPORT_GFX_CGLS |
AMD_CG_SUPPORT_GFX_3D_CGCG |
AMD_CG_SUPPORT_GFX_3D_CGLS |
+   AMD_CG_SUPPORT_MC_MGCG |
+   AMD_CG_SUPPORT_MC_LS |
AMD_CG_SUPPORT_VCN_MGCG |
AMD_CG_SUPPORT_JPEG_MGCG;
adev->pg_flags = AMD_PG_SUPPORT_GFX_PG |
-- 
2.17.1

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