[PATCH] drm/amdgpu: Replace kzalloc with kcalloc

2019-01-14 Thread Gustavo A. R. Silva
Replace kzalloc() function with its 2-factor argument form, kcalloc().

This patch replaces cases of:

kzalloc(a * b, gfp)

with:
kcalloc(a, b, gfp)

Also, improve the coding style and the use of sizeof during
allocation by changing sizeof(struct dc_surface_update) and
sizeof(struct dc_plane_state) to sizeof(*updates) and
sizeof(*surfaces), correspondingly.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a3e65e457348..4c201de38329 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5782,11 +5782,14 @@ dm_determine_update_type_for_commit(struct dc *dc,
struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state;
struct dc_stream_status *status = NULL;
 
-   struct dc_surface_update *updates = kzalloc(MAX_SURFACES * 
sizeof(struct dc_surface_update), GFP_KERNEL);
-   struct dc_plane_state *surface = kzalloc(MAX_SURFACES * sizeof(struct 
dc_plane_state), GFP_KERNEL);
+   struct dc_surface_update *updates;
+   struct dc_plane_state *surface;
struct dc_stream_update stream_update;
enum surface_update_type update_type = UPDATE_TYPE_FAST;
 
+   updates = kcalloc(MAX_SURFACES, sizeof(*updates), GFP_KERNEL);
+   surface = kcalloc(MAX_SURFACES, sizeof(*surface), GFP_KERNEL);
+
if (!updates || !surface) {
DRM_ERROR("Plane or surface update failed to allocate");
/* Set type to FULL to avoid crashing in DC*/
-- 
2.20.1

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


RE: [PATCH 4/4] drm/amd/powerplay: support retrieving and adjusting dcefclock power levels

2019-01-14 Thread Quan, Evan
For ppfeatures, it will be seen only if (adev->asic_type >= CHIP_VEGA10 && 
!APU).

Regards,
Evan
> -Original Message-
> From: Quan, Evan
> Sent: Tuesday, January 15, 2019 2:44 PM
> To: Alex Deucher 
> Cc: amd-gfx list ; Deucher, Alexander
> 
> Subject: RE: [PATCH 4/4] drm/amd/powerplay: support retrieving and
> adjusting dcefclock power levels
> 
> I think we can use asic_type to determine whether to expose these new
> interfaces.
> If (adev->asic_type >= CHIP_VEGA10), socclk and dcefclk are OK to expose If
> (adev->asic_type >= CHIP_VEGA20), fclk is OK to expose
> 
> Regards,
> Evan
> > -Original Message-
> > From: Alex Deucher 
> > Sent: Tuesday, January 15, 2019 1:00 AM
> > To: Quan, Evan 
> > Cc: amd-gfx list ; Deucher, Alexander
> > 
> > Subject: Re: [PATCH 4/4] drm/amd/powerplay: support retrieving and
> > adjusting dcefclock power levels
> >
> > On Mon, Jan 14, 2019 at 5:02 AM Evan Quan  wrote:
> > >
> > > User can use "pp_dpm_dcefclk" to retrieve and adjust dcefclock power
> > > levels.
> > >
> > > Change-Id: Ia3f61558ca96104c88d129ba5194103b2fe702ec
> > > Signed-off-by: Evan Quan 
> >
> > We should probably find a way to hide these new files on asics which
> > don't support them.  Other than that, the series looks good to me.
> >
> > Alex
> >
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 53
> > ++-
> > >  .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
> > >  .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 52
> > +-
> > >  3 files changed, 103 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > index f6646a522c06..b7b70f590236 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > @@ -731,11 +731,13 @@ static ssize_t
> > > amdgpu_get_ppfeature_status(struct device *dev,  }
> > >
> > >  /**
> > > - * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk
> > pp_dpm_pcie
> > > + * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk
> > > + pp_dpm_dcefclk
> > > + * pp_dpm_pcie
> > >   *
> > >   * The amdgpu driver provides a sysfs API for adjusting what power
> levels
> > >   * are enabled for a given power state.  The files pp_dpm_sclk,
> > > pp_dpm_mclk,
> > > - * pp_dpm_socclk, pp_dpm_fclk and pp_dpm_pcie are used for this.
> > > + * pp_dpm_socclk, pp_dpm_fclk, pp_dpm_dcefclk and pp_dpm_pcie
> are
> > > + used for
> > > + * this.
> > >   *
> > >   * Reading back the files will show you the available power levels within
> > >   * the power state and the clock information for those levels.
> > > @@ -745,6 +747,8 @@ static ssize_t
> > > amdgpu_get_ppfeature_status(struct
> > device *dev,
> > >   * Secondly,Enter a new value for each level by inputing a string that
> > >   * contains " echo xx xx xx > pp_dpm_sclk/mclk/pcie"
> > >   * E.g., echo 4 5 6 to > pp_dpm_sclk will enable sclk levels 4, 5, and 6.
> > > + *
> > > + * NOTE: change to the dcefclk max dpm level is not supported now
> > >   */
> > >
> > >  static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev, @@ -927,6
> > > +931,42 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
> > > return count;
> > >  }
> > >
> > > +static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
> > > +   struct device_attribute *attr,
> > > +   char *buf)
> > > +{
> > > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > > +   struct amdgpu_device *adev = ddev->dev_private;
> > > +
> > > +   if (adev->powerplay.pp_funcs->print_clock_levels)
> > > +   return amdgpu_dpm_print_clock_levels(adev, PP_DCEFCLK,
> buf);
> > > +   else
> > > +   return snprintf(buf, PAGE_SIZE, "\n"); }
> > > +
> > > +static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
> > > +   struct device_attribute *attr,
> > > +   const char *buf,
> > > +   size_t count)
> > > +{
> > > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > > +   struct amdgpu_device *adev = ddev->dev_private;
> > > +   int ret;
> > > +   uint32_t mask = 0;
> > > +
> > > +   ret = amdgpu_read_mask(buf, count, );
> > > +   if (ret)
> > > +   return ret;
> > > +
> > > +   if (adev->powerplay.pp_funcs->force_clock_level)
> > > +   ret = amdgpu_dpm_force_clock_level(adev, PP_DCEFCLK,
> > > + mask);
> > > +
> > > +   if (ret)
> > > +   return -EINVAL;
> > > +
> > > +   return count;
> > > +}
> > > +
> > >  static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
> > > struct device_attribute *attr,
> > > char *buf)
> > > @@ -1216,6 +1256,9 @@ static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO |
> > > S_IWUSR,  static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR,
> > > amdgpu_get_pp_dpm_fclk,
> > > amdgpu_set_pp_dpm_fclk);
> > > +static 

RE: [PATCH 4/4] drm/amd/powerplay: support retrieving and adjusting dcefclock power levels

2019-01-14 Thread Quan, Evan
I think we can use asic_type to determine whether to expose these new 
interfaces.
If (adev->asic_type >= CHIP_VEGA10), socclk and dcefclk are OK to expose
If (adev->asic_type >= CHIP_VEGA20), fclk is OK to expose

Regards,
Evan
> -Original Message-
> From: Alex Deucher 
> Sent: Tuesday, January 15, 2019 1:00 AM
> To: Quan, Evan 
> Cc: amd-gfx list ; Deucher, Alexander
> 
> Subject: Re: [PATCH 4/4] drm/amd/powerplay: support retrieving and
> adjusting dcefclock power levels
> 
> On Mon, Jan 14, 2019 at 5:02 AM Evan Quan  wrote:
> >
> > User can use "pp_dpm_dcefclk" to retrieve and adjust dcefclock power
> > levels.
> >
> > Change-Id: Ia3f61558ca96104c88d129ba5194103b2fe702ec
> > Signed-off-by: Evan Quan 
> 
> We should probably find a way to hide these new files on asics which don't
> support them.  Other than that, the series looks good to me.
> 
> Alex
> 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 53
> ++-
> >  .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
> >  .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 52
> +-
> >  3 files changed, 103 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > index f6646a522c06..b7b70f590236 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > @@ -731,11 +731,13 @@ static ssize_t
> > amdgpu_get_ppfeature_status(struct device *dev,  }
> >
> >  /**
> > - * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk
> pp_dpm_pcie
> > + * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk
> > + pp_dpm_dcefclk
> > + * pp_dpm_pcie
> >   *
> >   * The amdgpu driver provides a sysfs API for adjusting what power levels
> >   * are enabled for a given power state.  The files pp_dpm_sclk,
> > pp_dpm_mclk,
> > - * pp_dpm_socclk, pp_dpm_fclk and pp_dpm_pcie are used for this.
> > + * pp_dpm_socclk, pp_dpm_fclk, pp_dpm_dcefclk and pp_dpm_pcie are
> > + used for
> > + * this.
> >   *
> >   * Reading back the files will show you the available power levels within
> >   * the power state and the clock information for those levels.
> > @@ -745,6 +747,8 @@ static ssize_t amdgpu_get_ppfeature_status(struct
> device *dev,
> >   * Secondly,Enter a new value for each level by inputing a string that
> >   * contains " echo xx xx xx > pp_dpm_sclk/mclk/pcie"
> >   * E.g., echo 4 5 6 to > pp_dpm_sclk will enable sclk levels 4, 5, and 6.
> > + *
> > + * NOTE: change to the dcefclk max dpm level is not supported now
> >   */
> >
> >  static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev, @@ -927,6
> > +931,42 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
> > return count;
> >  }
> >
> > +static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
> > +   struct device_attribute *attr,
> > +   char *buf)
> > +{
> > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > +   struct amdgpu_device *adev = ddev->dev_private;
> > +
> > +   if (adev->powerplay.pp_funcs->print_clock_levels)
> > +   return amdgpu_dpm_print_clock_levels(adev, PP_DCEFCLK, buf);
> > +   else
> > +   return snprintf(buf, PAGE_SIZE, "\n"); }
> > +
> > +static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
> > +   struct device_attribute *attr,
> > +   const char *buf,
> > +   size_t count)
> > +{
> > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > +   struct amdgpu_device *adev = ddev->dev_private;
> > +   int ret;
> > +   uint32_t mask = 0;
> > +
> > +   ret = amdgpu_read_mask(buf, count, );
> > +   if (ret)
> > +   return ret;
> > +
> > +   if (adev->powerplay.pp_funcs->force_clock_level)
> > +   ret = amdgpu_dpm_force_clock_level(adev, PP_DCEFCLK,
> > + mask);
> > +
> > +   if (ret)
> > +   return -EINVAL;
> > +
> > +   return count;
> > +}
> > +
> >  static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
> > struct device_attribute *attr,
> > char *buf)
> > @@ -1216,6 +1256,9 @@ static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO |
> > S_IWUSR,  static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR,
> > amdgpu_get_pp_dpm_fclk,
> > amdgpu_set_pp_dpm_fclk);
> > +static DEVICE_ATTR(pp_dpm_dcefclk, S_IRUGO | S_IWUSR,
> > +   amdgpu_get_pp_dpm_dcefclk,
> > +   amdgpu_set_pp_dpm_dcefclk);
> >  static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
> > amdgpu_get_pp_dpm_pcie,
> > amdgpu_set_pp_dpm_pcie); @@ -2387,6 +2430,11 @@ int
> > amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> > DRM_ERROR("failed to create device file pp_dpm_fclk\n");
> > return ret;
> > }
> > +   ret = device_create_file(adev->dev, _attr_pp_dpm_dcefclk);
> > +   if (ret) {
> > +   

RE: [[PATCH v2]drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Liu, Aaron
Hi Paul,

The psp code only support ucode loading at the beginning, but forgot to remove 
the checking when more and more PSP command get into PSP.

So some old PSP FW maybe have problem but PSP driver didn’t find it out.
If reverting the patch, we also can’t find out wrong PSP FW and have big 
problem when support new PSP command.

The temporary solution is: do not return error if response is invalid but add 
warning message to notify, it will not block old FW and get attention if new 
PSP FW have some issue.

BR,
Aaron Liu

> -Original Message-
> From: Paul Menzel 
> Sent: Monday, January 14, 2019 9:24 PM
> To: Liu, Aaron 
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [[PATCH v2]drm/amdgpu/psp: ignore psp reponse status]
> drm/amdgpu/psp: ignore psp reponse status
> 
> Dear Aaron,
> 
> 
> Am 14.01.19 um 09:47 schrieb Aaron Liu:
> > In some cases, psp response status is not 0 even there is no problem
> > while the command is submitted. Some version of PSP FW doesn't write 0
> > to that field.
> > So here we would like to only print a warning instead of an error
> > during psp initialization to avoid breaking hw_init and it doesn't
> > return -EINVAL.
> >
> > Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
> > Signed-off-by: Aaron Liu 
> > Reviewed-by: Huang Rui 
> > Reviewed-by: Xiangliang Yu
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 13 +
> >   1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index 53c2d60..f26d8fa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -140,14 +140,19 @@ psp_cmd_submit_buf(struct psp_context *psp,
> > while (*((unsigned int *)psp->fence_buf) != index)
> > msleep(1);
> >
> > -   /* the status field must be 0 after psp command completion */
> > +   /* In some cases, psp response status is not 0 even there is no
> > +* problem while the command is submitted. Some version of PSP
> FW
> > +* doesn't write 0 to that field.
> > +* So here we would like to only print a warning instead of an error
> > +* during psp initialization to avoid breaking hw_init and it doesn't
> > +* return -EINVAL.
> > +*/
> > if (psp->cmd_buf_mem->resp.status) {
> > if (ucode)
> > -   DRM_ERROR("failed to load ucode id (%d) ",
> > +   DRM_WARN("failed to load ucode id (%d) ",
> >   ucode->ucode_id);
> > -   DRM_ERROR("psp command failed and response status is
> (%d)\n",
> > +   DRM_WARN("psp command failed and response status is
> (%d)\n",
> >   psp->cmd_buf_mem->resp.status);
> > -   return -EINVAL;
> > }
> >
> > /* get xGMI session id from response buffer */
> 
> Please describe, why this error can be ignored, and the rest of the function
> be executed. Won’t that introduce other problems?
> 
> How can real error situations be determined now?
> 
> Also, please extend the commit messages, that this only affects microcode
> update loading (if I understand this correctly).
> 
> 
> Kind regards,
> 
> Paul
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: csa_vaddr should not larger than AMDGPU_GMC_HOLE_START

2019-01-14 Thread Liu, Monk
The max_pfn is now 1'''' (bytes) which is above 48 bit now, and 
it with AMDGPU_GMC_HOLE_MASK make it to zero 

And in code "amdgpu_driver_open_kms()" I saw @Zhu, Rex write the code as :

"csa_addr = amdgpu_csa_vadr(adev) & AMDGPU_GMC_HOLE_MASK", I think this is 
wrong since you intentionally place the csa above GMC hole, right ?

Looks like  we should modify this place 

/Monk

-Original Message-
From: amd-gfx  On Behalf Of Christian 
K?nig
Sent: Monday, January 14, 2019 9:05 PM
To: Lou, Wentao ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: csa_vaddr should not larger than 
AMDGPU_GMC_HOLE_START

Am 14.01.19 um 09:40 schrieb wentalou:
> After removing unnecessary VM size calculations, vm_manager.max_pfn 
> would reach 0x10,, max_pfn << AMDGPU_GPU_PAGE_SHIFT exceeding 
> AMDGPU_GMC_HOLE_START would caused GPU reset.
>
> Change-Id: I47ad0be2b0bd9fb7490c4e1d7bb7bdacf71132cb
> Signed-off-by: wentalou 

NAK, that is incorrect. We intentionally place the csa above the GMC hole.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> index 7e22be7..dd3bd01 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> @@ -26,7 +26,8 @@
>   
>   uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
>   {
> - uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
> + uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
> + AMDGPU_GMC_HOLE_START);
>   
>   addr -= AMDGPU_VA_RESERVED_SIZE;
>   addr = amdgpu_gmc_sign_extend(addr);

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


答复: [PATCH] drm/amdgpu/soc15: return proper error codes in baco reset

2019-01-14 Thread Qu, Jim
Reviewed-by: JimQu 

Thanks
JimQu


发件人: amd-gfx  代表 Alex Deucher 

发送时间: 2019年1月15日 3:57:56
收件人: amd-gfx@lists.freedesktop.org
抄送: Deucher, Alexander
主题: [PATCH] drm/amdgpu/soc15: return proper error codes in baco reset

Rather than just -1.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 5248b03df5fa..7130a4c8dd5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -427,7 +427,7 @@ static int soc15_asic_get_baco_capability(struct 
amdgpu_device *adev, bool *cap)

if (!pp_funcs || !pp_funcs->get_asic_baco_capability) {
*cap = false;
-   return -1;
+   return -ENOENT;
}

return pp_funcs->get_asic_baco_capability(pp_handle, cap);
@@ -439,15 +439,15 @@ static int soc15_asic_baco_reset(struct amdgpu_device 
*adev)
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;

if (!pp_funcs ||!pp_funcs->get_asic_baco_state 
||!pp_funcs->set_asic_baco_state)
-   return -1;
+   return -ENOENT;

/* enter BACO state */
if (pp_funcs->set_asic_baco_state(pp_handle, 1))
-   return -1;
+   return -EIO;

/* exit BACO state */
if (pp_funcs->set_asic_baco_state(pp_handle, 0))
-   return -1;
+   return -EIO;

dev_info(adev->dev, "GPU BACO reset\n");

--
2.20.1

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


[PATCH] drm/amdgpu: Setting doorbell range registers earlier

2019-01-14 Thread Zeng, Oak
HW doorbell writing routing policy: writing to doorbell
not in SDMA/IH/MM/ACV doorbell range will be routed to CP.
So CP doorbell routing depends on doorbell range setting
of above blocks. Setting doorbell range of above blocks
earlier (soc15_common_hw_init) to make sure CP doorbell
writing be routed to CP block.

Change-Id: I3f8edd582fb7cc20a83f48f7a1ff789036b3550e
Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c |  2 --
 drivers/gpu/drm/amd/amdgpu/soc15.c | 21 +
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  2 --
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 59638b8..48a166b 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -834,8 +834,6 @@ static void sdma_v4_0_gfx_resume(struct amdgpu_device 
*adev, unsigned int i)
OFFSET, ring->doorbell_index);
WREG32_SDMA(i, mmSDMA0_GFX_DOORBELL, doorbell);
WREG32_SDMA(i, mmSDMA0_GFX_DOORBELL_OFFSET, doorbell_offset);
-   adev->nbio_funcs->sdma_doorbell_range(adev, i, ring->use_doorbell,
- ring->doorbell_index);
 
sdma_v4_0_ring_set_wptr(ring);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 5248b03..4cae547 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -966,6 +966,21 @@ static int soc15_common_sw_fini(void *handle)
return 0;
 }
 
+static void soc15_doorbell_range_init(struct amdgpu_device *adev)
+{
+   int i;
+   struct amdgpu_ring *ring;
+
+   for (i = 0; i < adev->sdma.num_instances; i++) {
+   ring = >sdma.instance[i].ring;
+   adev->nbio_funcs->sdma_doorbell_range(adev, i,
+   ring->use_doorbell, ring->doorbell_index);
+   }
+
+   adev->nbio_funcs->ih_doorbell_range(adev, adev->irq.ih.use_doorbell,
+   adev->irq.ih.doorbell_index);
+}
+
 static int soc15_common_hw_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -978,6 +993,12 @@ static int soc15_common_hw_init(void *handle)
adev->nbio_funcs->init_registers(adev);
/* enable the doorbell aperture */
soc15_enable_doorbell_aperture(adev, true);
+   /* HW doorbell routing policy: doorbell writing not
+* in SDMA/IH/MM/ACV range will be routed to CP. So
+* we need to init SDMA/IH/MM/ACV doorbell range prior
+* to CP ip block init and ring test.
+*/
+   soc15_doorbell_range_init(adev);
 
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 5627019..877b4a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -140,8 +140,6 @@ static int vega10_ih_irq_init(struct amdgpu_device *adev)
 ENABLE, 0);
}
WREG32_SOC15(OSSSYS, 0, mmIH_DOORBELL_RPTR, ih_doorbell_rtpr);
-   adev->nbio_funcs->ih_doorbell_range(adev, adev->irq.ih.use_doorbell,
-   adev->irq.ih.doorbell_index);
 
tmp = RREG32_SOC15(OSSSYS, 0, mmIH_STORM_CLIENT_LIST_CNTL);
tmp = REG_SET_FIELD(tmp, IH_STORM_CLIENT_LIST_CNTL,
-- 
2.7.4

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


[PATCH] drm/amd/display: eDP fast bootup does not work for pre-raven asic

2019-01-14 Thread sunpeng.li
From: hersen wu 

[Why] bios will light up eDP before sw driver loaded. sw driver will
check if eDP lighted up by bios by reading BIOS_SCRATCH_3. If yes,
sw driver will not power down eDP power, phy to save time.
definition of BIOS_SCRATCH_3 are missed for pre-raven asic. this
cuase eDP fast boot up not work. for some eDP panel, even AMD dp tx
send NoVideoStream_flag =1 and dpcd 0x600=2, eDP rx may not handle
properly. this may cause short period flash on screen.

[How] add definition of BIOS_SCRATCH_3 for all asic

CC: Harry Wentland 
Signed-off-by: hersen wu 
Reviewed-by: Charlene Liu 
Acked-by: Yongqiang Sun 
Acked-by: Leo Li 
---
 drivers/gpu/drm/amd/display/dc/bios/bios_parser_helper.c | 3 +--
 drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c  | 2 ++
 drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c  | 2 ++
 drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c  | 2 ++
 drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c  | 1 +
 drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c| 2 ++
 6 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser_helper.c 
b/drivers/gpu/drm/amd/display/dc/bios/bios_parser_helper.c
index fdda8aa..d8275ceb 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser_helper.c
@@ -83,8 +83,7 @@ uint32_t bios_get_vga_enabled_displays(
 {
uint32_t active_disp = 1;
 
-   if (bios->regs->BIOS_SCRATCH_3) /*follow up with other asic, todo*/
-   active_disp = REG_READ(BIOS_SCRATCH_3) & 0X;
+   active_disp = REG_READ(BIOS_SCRATCH_3) & 0X;
return active_disp;
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
index c3f616a..23044e6 100644
--- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
@@ -76,6 +76,7 @@
 
 #ifndef mmBIOS_SCRATCH_2
#define mmBIOS_SCRATCH_2 0x05CB
+   #define mmBIOS_SCRATCH_3 0x05CC
#define mmBIOS_SCRATCH_6 0x05CF
 #endif
 
@@ -365,6 +366,7 @@ static const struct dce_abm_mask abm_mask = {
 #define DCFE_MEM_PWR_CTRL_REG_BASE 0x1b03
 
 static const struct bios_registers bios_regs = {
+   .BIOS_SCRATCH_3 = mmBIOS_SCRATCH_3,
.BIOS_SCRATCH_6 = mmBIOS_SCRATCH_6
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index 7d46eb7..7549ada 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -84,6 +84,7 @@
 
 #ifndef mmBIOS_SCRATCH_2
#define mmBIOS_SCRATCH_2 0x05CB
+   #define mmBIOS_SCRATCH_3 0x05CC
#define mmBIOS_SCRATCH_6 0x05CF
 #endif
 
@@ -369,6 +370,7 @@ static const struct dce110_clk_src_mask cs_mask = {
 };
 
 static const struct bios_registers bios_regs = {
+   .BIOS_SCRATCH_3 = mmBIOS_SCRATCH_3,
.BIOS_SCRATCH_6 = mmBIOS_SCRATCH_6
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
index d930e09..ea3065d6 100644
--- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
@@ -76,6 +76,7 @@
 
 #ifndef mmBIOS_SCRATCH_2
#define mmBIOS_SCRATCH_2 0x05CB
+   #define mmBIOS_SCRATCH_3 0x05CC
#define mmBIOS_SCRATCH_6 0x05CF
 #endif
 
@@ -376,6 +377,7 @@ static const struct dce110_clk_src_mask cs_mask = {
 };
 
 static const struct bios_registers bios_regs = {
+   .BIOS_SCRATCH_3 = mmBIOS_SCRATCH_3,
.BIOS_SCRATCH_6 = mmBIOS_SCRATCH_6
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
index 23d7d4d..312a0ae 100644
--- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
@@ -442,6 +442,7 @@ struct dce_i2c_hw *dce120_i2c_hw_create(
return dce_i2c_hw;
 }
 static const struct bios_registers bios_regs = {
+   .BIOS_SCRATCH_3 = mmBIOS_SCRATCH_3 + 
NBIO_BASE(mmBIOS_SCRATCH_3_BASE_IDX),
.BIOS_SCRATCH_6 = mmBIOS_SCRATCH_6 + 
NBIO_BASE(mmBIOS_SCRATCH_6_BASE_IDX)
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
index e0bba0bc..2eca81b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
@@ -77,6 +77,7 @@
 
 #ifndef mmBIOS_SCRATCH_2
#define mmBIOS_SCRATCH_2 0x05CB
+   #define mmBIOS_SCRATCH_3 0x05CC
#define mmBIOS_SCRATCH_6 0x05CF
 #endif
 
@@ -358,6 +359,7 @@ static const struct dce110_clk_src_mask cs_mask = {
 };
 
 static const struct bios_registers bios_regs = {
+   .BIOS_SCRATCH_3 = mmBIOS_SCRATCH_3,

[PATCH] drm: radeon: annotate implicit fall through

2019-01-14 Thread Mathieu Malaterre
There is a plan to build the kernel with -Wimplicit-fallthrough and
this place in the code produced a warning (W=1).

This commit remove the following warning:

  drivers/gpu/drm/radeon/evergreen_cs.c:1301:11: warning: this statement may 
fall through [-Wimplicit-fallthrough=]

Signed-off-by: Mathieu Malaterre 
---
 drivers/gpu/drm/radeon/evergreen_cs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c 
b/drivers/gpu/drm/radeon/evergreen_cs.c
index f471537c852f..fbf346185790 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -1299,6 +1299,7 @@ static int evergreen_cs_handle_reg(struct 
radeon_cs_parser *p, u32 reg, u32 idx)
return -EINVAL;
}
ib[idx] += (u32)((reloc->gpu_offset >> 8) & 0x);
+   /* fall through */
case CB_TARGET_MASK:
track->cb_target_mask = radeon_get_ib_value(p, idx);
track->cb_dirty = true;
-- 
2.19.2

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


[PATCH] drm/amdgpu/soc15: return proper error codes in baco reset

2019-01-14 Thread Alex Deucher
Rather than just -1.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 5248b03df5fa..7130a4c8dd5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -427,7 +427,7 @@ static int soc15_asic_get_baco_capability(struct 
amdgpu_device *adev, bool *cap)
 
if (!pp_funcs || !pp_funcs->get_asic_baco_capability) {
*cap = false;
-   return -1;
+   return -ENOENT;
}
 
return pp_funcs->get_asic_baco_capability(pp_handle, cap);
@@ -439,15 +439,15 @@ static int soc15_asic_baco_reset(struct amdgpu_device 
*adev)
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
 
if (!pp_funcs ||!pp_funcs->get_asic_baco_state 
||!pp_funcs->set_asic_baco_state)
-   return -1;
+   return -ENOENT;
 
/* enter BACO state */
if (pp_funcs->set_asic_baco_state(pp_handle, 1))
-   return -1;
+   return -EIO;
 
/* exit BACO state */
if (pp_funcs->set_asic_baco_state(pp_handle, 0))
-   return -1;
+   return -EIO;
 
dev_info(adev->dev, "GPU BACO reset\n");
 
-- 
2.20.1

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


Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

2019-01-14 Thread Sibren Vasse
On Mon, 14 Jan 2019 at 19:13, Christoph Hellwig  wrote:
>
> Hmm, I wonder if we are not actually using swiotlb in the end,
> can you check if your dmesg contains this line or not?
>
> PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
This line does not appear in my dmesg.

>
> If not I guess we found a bug in swiotlb exit vs is_swiotlb_buffer,
> and you can try this patch:
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index d6361776dc5c..1fb6fd68b9c7 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -378,6 +378,8 @@ void __init swiotlb_exit(void)
> memblock_free_late(io_tlb_start,
>PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
> }
> +   io_tlb_start = 0;
> +   io_tlb_end = 0;
> io_tlb_nslabs = 0;
> max_segment = 0;
>  }
With the patch applied to v5.0-rc2 I can no longer reproduce the issue.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

2019-01-14 Thread Sibren Vasse
On Mon, 14 Jan 2019 at 19:10, Christoph Hellwig  wrote:
>
> On Thu, Jan 10, 2019 at 06:52:26PM +0100, Sibren Vasse wrote:
> > On Thu, 10 Jan 2019 at 15:48, Christoph Hellwig  wrote:
> > >
> > > On Thu, Jan 10, 2019 at 03:00:31PM +0100, Christian König wrote:
> > > >>  From the trace it looks like we git the case where swiotlb tries
> > > >> to copy back data from a bounce buffer, but hits a dangling or NULL
> > > >> pointer.  So a couple questions for the submitter:
> > > >>
> > > >>   - does the system have more than 4GB memory and thus use swiotlb?
> > > >> (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
> > > >>   - does the device this happens on have a DMA mask smaller than
> > > >> the available memory, that is should swiotlb be used here to start
> > > >> with?
> > > >
> > > > Rather unlikely. The device is an AMD GPU, so we can address memory up 
> > > > to
> > > > 1TB.
> > >
> > > So we probably somehow got a false positive.
> > >
> > > For now I'like the reported to confirm that the dma_direct_unmap_page+0x92
> > > backtrace really is in the swiotlb code (I can't think of anything else,
> > > but I'd rather be sure).
> > I'm not sure what you want me to confirm. Could you elaborate?
>
> Please open the vmlinux file for which this happend in gdb,
> then send the output from this command
>
> l *(dma_direct_unmap_page+0x92)
>
> to this thread.
My call trace contained:
Jan 10 16:34:51  kernel:  dma_direct_unmap_page+0x7a/0x80

(gdb) list *(dma_direct_unmap_page+0x7a)
0x810fa28a is in dma_direct_unmap_page (kernel/dma/direct.c:291).
286 size_t size, enum dma_data_direction dir,
unsigned long attrs)
287 {
288 phys_addr_t phys = dma_to_phys(dev, addr);
289
290 if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
291 dma_direct_sync_single_for_cpu(dev, addr, size, dir);
292
293 if (unlikely(is_swiotlb_buffer(phys)))
294 swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
295 }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

2019-01-14 Thread Christoph Hellwig
On Thu, Jan 10, 2019 at 06:52:26PM +0100, Sibren Vasse wrote:
> On Thu, 10 Jan 2019 at 15:48, Christoph Hellwig  wrote:
> >
> > On Thu, Jan 10, 2019 at 03:00:31PM +0100, Christian König wrote:
> > >>  From the trace it looks like we git the case where swiotlb tries
> > >> to copy back data from a bounce buffer, but hits a dangling or NULL
> > >> pointer.  So a couple questions for the submitter:
> > >>
> > >>   - does the system have more than 4GB memory and thus use swiotlb?
> > >> (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
> > >>   - does the device this happens on have a DMA mask smaller than
> > >> the available memory, that is should swiotlb be used here to start
> > >> with?
> > >
> > > Rather unlikely. The device is an AMD GPU, so we can address memory up to
> > > 1TB.
> >
> > So we probably somehow got a false positive.
> >
> > For now I'like the reported to confirm that the dma_direct_unmap_page+0x92
> > backtrace really is in the swiotlb code (I can't think of anything else,
> > but I'd rather be sure).
> I'm not sure what you want me to confirm. Could you elaborate?

Please open the vmlinux file for which this happend in gdb,
then send the output from this command

l *(dma_direct_unmap_page+0x92)

to this thread.

> > Second it would be great to print what the contents of io_tlb_start
> > and io_tlb_end are, e.g. by doing a printk_once in is_swiotlb_buffer,
> > maybe that gives a clue why we are hitting the swiotlb code here.
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 7c007ed7505f..042246dbae00 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -69,6 +69,7 @@ extern phys_addr_t io_tlb_start, io_tlb_end;
> 
>  static inline bool is_swiotlb_buffer(phys_addr_t paddr)
>  {
> +printk_once(KERN_INFO "io_tlb_start: %llu, io_tlb_end: %llu",
> io_tlb_start, io_tlb_end);
>  return paddr >= io_tlb_start && paddr < io_tlb_end;
>  }
> 
> Result on boot:
> [   11.405558] io_tlb_start: 3782983680, io_tlb_end: 3850092544

So this is a normal swiotlb location, and it does defintively exist.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

2019-01-14 Thread Christoph Hellwig
Hmm, I wonder if we are not actually using swiotlb in the end,
can you check if your dmesg contains this line or not?

PCI-DMA: Using software bounce buffering for IO (SWIOTLB)

If not I guess we found a bug in swiotlb exit vs is_swiotlb_buffer,
and you can try this patch:

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d6361776dc5c..1fb6fd68b9c7 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -378,6 +378,8 @@ void __init swiotlb_exit(void)
memblock_free_late(io_tlb_start,
   PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
}
+   io_tlb_start = 0;
+   io_tlb_end = 0;
io_tlb_nslabs = 0;
max_segment = 0;
 }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amd/display: Skip create new stream if crtc state doesn't change

2019-01-14 Thread Kazlauskas, Nicholas
On 1/14/19 1:41 PM, Jerry (Fangzhi) Zuo wrote:
> Need to check if crtc state is changed so that mode set is
> required before trying to create new stream.
> 
> It deals with the MST hotplug use case when plug back to the
> same connector where the failure to create new stream for the
> inactive crtc state. Simply skip the check for the old connector
> and go through the new one.
> 
> Signed-off-by: Jerry (Fangzhi) Zuo 

Reviewed-by: Nicholas Kazlauskas 

Maybe in the future we should look into cleaning up the disable/enable 
codepath into a single function that does both.

This looks fine to me for now at least.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 987095ccd2c5..31ac44b56eaf 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5611,6 +5611,9 @@ static int dm_update_crtc_state(struct 
> amdgpu_display_manager *dm,
>   dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
>   dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
>   
> + if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> + goto skip_modeset;
> +
>   new_stream = create_stream_for_sink(aconnector,
>_crtc_state->mode,
>   dm_new_conn_state,
> @@ -5640,6 +5643,7 @@ static int dm_update_crtc_state(struct 
> amdgpu_display_manager *dm,
>   }
>   }
>   
> + /* mode_changed flag may get updated above, need to check again */
>   if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
>   goto skip_modeset;
>   
> 

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


[PATCH] drm/amd/display: Skip create new stream if crtc state doesn't change

2019-01-14 Thread Jerry (Fangzhi) Zuo
Need to check if crtc state is changed so that mode set is
required before trying to create new stream.

It deals with the MST hotplug use case when plug back to the
same connector where the failure to create new stream for the
inactive crtc state. Simply skip the check for the old connector
and go through the new one.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 987095ccd2c5..31ac44b56eaf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5611,6 +5611,9 @@ static int dm_update_crtc_state(struct 
amdgpu_display_manager *dm,
dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
 
+   if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+   goto skip_modeset;
+
new_stream = create_stream_for_sink(aconnector,
 _crtc_state->mode,
dm_new_conn_state,
@@ -5640,6 +5643,7 @@ static int dm_update_crtc_state(struct 
amdgpu_display_manager *dm,
}
}
 
+   /* mode_changed flag may get updated above, need to check again */
if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
goto skip_modeset;
 
-- 
2.14.1

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


Re: [PATCH] drm/amd/display: Include names of all PP clock types

2019-01-14 Thread Alex Deucher
On Sun, Jan 13, 2019 at 8:24 AM Rafał Miłecki  wrote:
>
> From: Rafał Miłecki 
>
> This fixes printing clock names in cases like:
> [5.352311] [drm] DM_PPLIB: values for Invalid clock
> [5.352313] [drm] DM_PPLIB:   40 in kHz
> [5.352313] [drm] DM_PPLIB:   933000 in kHz
> [5.352314] [drm] DM_PPLIB:   1067000 in kHz
> [5.352315] [drm] DM_PPLIB:   120 in kHz
> [5.352317] [drm] DM_PPLIB: values for Invalid clock
> [5.352318] [drm] DM_PPLIB:   30 in kHz
> [5.352318] [drm] DM_PPLIB:   60 in kHz
> [5.352319] [drm] DM_PPLIB:   626000 in kHz
> [5.352320] [drm] DM_PPLIB:   654000 in kHz
> (source: HP EliteBook 745 G5 w. RAVEN 0x1002:0x15DD 0x103C:0x83D5 0xD1)
>
> On my system above "Invalid" names got replaced by "F" and "DCF".
>
> The same problem was occurring on Huawei Matebook D with just 667000 kHz
> instead of 40 kHz.
>
> Signed-off-by: Rafał Miłecki 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/display/dc/dm_services_types.h | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dm_services_types.h 
> b/drivers/gpu/drm/amd/display/dc/dm_services_types.h
> index 1af8c777b3ac..9afd36a031a9 100644
> --- a/drivers/gpu/drm/amd/display/dc/dm_services_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/dm_services_types.h
> @@ -82,7 +82,15 @@ enum dm_pp_clock_type {
>  #define DC_DECODE_PP_CLOCK_TYPE(clk_type) \
> (clk_type) == DM_PP_CLOCK_TYPE_DISPLAY_CLK ? "Display" : \
> (clk_type) == DM_PP_CLOCK_TYPE_ENGINE_CLK ? "Engine" : \
> -   (clk_type) == DM_PP_CLOCK_TYPE_MEMORY_CLK ? "Memory" : "Invalid"
> +   (clk_type) == DM_PP_CLOCK_TYPE_MEMORY_CLK ? "Memory" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_DCFCLK ? "DCF" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_DCEFCLK ? "DCEF" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_SOCCLK ? "SoC" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_PIXELCLK ? "Pixel" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_DISPLAYPHYCLK ? "Display PHY" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_DPPCLK ? "DPP" : \
> +   (clk_type) == DM_PP_CLOCK_TYPE_FCLK ? "F" : \
> +   "Invalid"
>
>  #define DM_PP_MAX_CLOCK_LEVELS 8
>
> --
> 2.20.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: pitch alignment questions

2019-01-14 Thread Michel Dänzer
On 2019-01-11 10:37 p.m., Yu Zhao wrote:
> On Fri, Jan 11, 2019 at 04:27:44PM +0100, Michel Dänzer wrote:
>> On 2019-01-10 6:56 p.m., Przemek Socha wrote:
>>>
>>> [  147.846148] [drm:amdgpu_display_user_framebuffer_create [amdgpu]] 
>>> Invalid 
>>> pitch: expecting 10752 but got 10624
>>> [  147.846155] [drm:drm_internal_framebuffer_create] could not create 
>>> framebuffer"
>>
>> Thanks, this confirms that the check is too strict. I've sent a patch
>> reverting this as well.
>>
>>
>> Yu, I like the idea behind your changes, but unfortunately it's more
>> complicated than that. If you want to work on similar checks which
>> accurately reflect the hardware constraints, people on the amd-gfx list
>> should be able to help with that.
> 
> Hi Michel, sorry for the troubles.

No worries, I missed these issues as well in my review.


> Background: after we turned on iommu with amd_iommu=force_isolation,
> we saw io page faults from amd gpu (stoney ridge). We tracked it
> down to userspace using 32-pixel pitch alignment, which seems smaller
> than the minimum alignment supported by the hw.

If that was the case, the corresponding surface would be displayed badly
distorted, because the hardware would use a different pitch.


> Instead of rejecting the alignment, we suspect, it uses 64-pixel
> alignment to do dma.

Actually, it's more likely that it would use the next smaller
well-aligned pitch, as it would probably simply ignore the least
significant bits smaller than the minimum alignment.


> The larger alignment sometimes causes out of bound memory accesses, thus
> the io page faults.

Per the above, this is more likely due to insufficient alignment of the
vertical size of the surface, resulting in the allocated memory being
too small.


> 1) how do properly validate pitch alignment passed to kernel space?

It's pretty complicated, I'm afraid. A case-insensitive search for
"display" in
https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/amd/addrlib
might serve to give an idea of the complexity.


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


Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

2019-01-14 Thread Michel Dänzer
On 2019-01-10 3:48 p.m., Christoph Hellwig wrote:
> On Thu, Jan 10, 2019 at 03:00:31PM +0100, Christian König wrote:
>>>  From the trace it looks like we git the case where swiotlb tries
>>> to copy back data from a bounce buffer, but hits a dangling or NULL
>>> pointer.  So a couple questions for the submitter:
>>>
>>>   - does the system have more than 4GB memory and thus use swiotlb?
>>> (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
>>>   - does the device this happens on have a DMA mask smaller than
>>> the available memory, that is should swiotlb be used here to start
>>> with?
>>
>> Rather unlikely. The device is an AMD GPU, so we can address memory up to 
>> 1TB.
> 
> So we probably somehow got a false positive.
> 
> For now I'like the reported to confirm that the dma_direct_unmap_page+0x92
> backtrace really is in the swiotlb code (I can't think of anything else,
> but I'd rather be sure).
> 
> Second it would be great to print what the contents of io_tlb_start
> and io_tlb_end are, e.g. by doing a printk_once in is_swiotlb_buffer,
> maybe that gives a clue why we are hitting the swiotlb code here.

Any progress? https://bugzilla.kernel.org/show_bug.cgi?id=202261 was
also filed about this.

I hope everyone's clear that this needs to be resolved one way or
another by 5.0 final (though the sooner, the better :).


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


Re: [PATCH 4/4] drm/amd/powerplay: support retrieving and adjusting dcefclock power levels

2019-01-14 Thread Alex Deucher
On Mon, Jan 14, 2019 at 5:02 AM Evan Quan  wrote:
>
> User can use "pp_dpm_dcefclk" to retrieve and adjust dcefclock power
> levels.
>
> Change-Id: Ia3f61558ca96104c88d129ba5194103b2fe702ec
> Signed-off-by: Evan Quan 

We should probably find a way to hide these new files on asics which
don't support them.  Other than that, the series looks good to me.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 53 ++-
>  .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
>  .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 52 +-
>  3 files changed, 103 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index f6646a522c06..b7b70f590236 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -731,11 +731,13 @@ static ssize_t amdgpu_get_ppfeature_status(struct 
> device *dev,
>  }
>
>  /**
> - * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk pp_dpm_pcie
> + * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk pp_dpm_dcefclk
> + * pp_dpm_pcie
>   *
>   * The amdgpu driver provides a sysfs API for adjusting what power levels
>   * are enabled for a given power state.  The files pp_dpm_sclk, pp_dpm_mclk,
> - * pp_dpm_socclk, pp_dpm_fclk and pp_dpm_pcie are used for this.
> + * pp_dpm_socclk, pp_dpm_fclk, pp_dpm_dcefclk and pp_dpm_pcie are used for
> + * this.
>   *
>   * Reading back the files will show you the available power levels within
>   * the power state and the clock information for those levels.
> @@ -745,6 +747,8 @@ static ssize_t amdgpu_get_ppfeature_status(struct device 
> *dev,
>   * Secondly,Enter a new value for each level by inputing a string that
>   * contains " echo xx xx xx > pp_dpm_sclk/mclk/pcie"
>   * E.g., echo 4 5 6 to > pp_dpm_sclk will enable sclk levels 4, 5, and 6.
> + *
> + * NOTE: change to the dcefclk max dpm level is not supported now
>   */
>
>  static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
> @@ -927,6 +931,42 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
> return count;
>  }
>
> +static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> +   struct drm_device *ddev = dev_get_drvdata(dev);
> +   struct amdgpu_device *adev = ddev->dev_private;
> +
> +   if (adev->powerplay.pp_funcs->print_clock_levels)
> +   return amdgpu_dpm_print_clock_levels(adev, PP_DCEFCLK, buf);
> +   else
> +   return snprintf(buf, PAGE_SIZE, "\n");
> +}
> +
> +static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
> +   struct device_attribute *attr,
> +   const char *buf,
> +   size_t count)
> +{
> +   struct drm_device *ddev = dev_get_drvdata(dev);
> +   struct amdgpu_device *adev = ddev->dev_private;
> +   int ret;
> +   uint32_t mask = 0;
> +
> +   ret = amdgpu_read_mask(buf, count, );
> +   if (ret)
> +   return ret;
> +
> +   if (adev->powerplay.pp_funcs->force_clock_level)
> +   ret = amdgpu_dpm_force_clock_level(adev, PP_DCEFCLK, mask);
> +
> +   if (ret)
> +   return -EINVAL;
> +
> +   return count;
> +}
> +
>  static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -1216,6 +1256,9 @@ static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO | S_IWUSR,
>  static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR,
> amdgpu_get_pp_dpm_fclk,
> amdgpu_set_pp_dpm_fclk);
> +static DEVICE_ATTR(pp_dpm_dcefclk, S_IRUGO | S_IWUSR,
> +   amdgpu_get_pp_dpm_dcefclk,
> +   amdgpu_set_pp_dpm_dcefclk);
>  static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
> amdgpu_get_pp_dpm_pcie,
> amdgpu_set_pp_dpm_pcie);
> @@ -2387,6 +2430,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> DRM_ERROR("failed to create device file pp_dpm_fclk\n");
> return ret;
> }
> +   ret = device_create_file(adev->dev, _attr_pp_dpm_dcefclk);
> +   if (ret) {
> +   DRM_ERROR("failed to create device file pp_dpm_dcefclk\n");
> +   return ret;
> +   }
> ret = device_create_file(adev->dev, _attr_pp_dpm_pcie);
> if (ret) {
> DRM_ERROR("failed to create device file pp_dpm_pcie\n");
> @@ -2474,6 +2522,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
> device_remove_file(adev->dev, _attr_pp_dpm_socclk);
> device_remove_file(adev->dev, _attr_pp_dpm_pcie);
> device_remove_file(adev->dev, _attr_pp_dpm_fclk);
> +   device_remove_file(adev->dev, _attr_pp_dpm_dcefclk);
> device_remove_file(adev->dev, _attr_pp_sclk_od);
> device_remove_file(adev->dev, _attr_pp_mclk_od);
> 

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

2019-01-14 Thread Michel Dänzer
On 2019-01-11 8:12 p.m., Christian König wrote:
> Am 11.01.19 um 15:17 schrieb Michel Dänzer:
>> On 2019-01-11 2:15 p.m., Christian König wrote:
>>> Move the BO on the LRU only when it is actually moved by a DMA
>>> operation.
>>>
>>> [...]
>>>
>>> @@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct
>>> vm_fault *vmf)
>>>   ret = VM_FAULT_SIGBUS;
>>>   goto out_unlock;
>>>   }
>>> +
>>> +    if (bo->moving != moving) {
>> Hmm, could a driver just update the existing fence instead of attaching
>> a new one?
> 
> Mhm, not as far as I know. That would violate similar checks elsewhere.

Makes sense, my R-b stands.


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


Re: [PATCH] drm/amd/display: Skip create new stream if crtc state doesn't change

2019-01-14 Thread Kazlauskas, Nicholas
On 1/14/19 11:05 AM, Jerry (Fangzhi) Zuo wrote:
> Need to check if crtc state is changed so that mode set is
> required before trying to create new stream.
> 
> It deals with the MST hotplug use case when plug back to the
> same connector where the failure to create new stream for the
> inactive crtc state. Simply skip the check for the old connector
> and go through the new one.
> 
> Signed-off-by: Jerry (Fangzhi) Zuo 
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 987095ccd2c5..68739d0c600c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5593,6 +5593,9 @@ static int dm_update_crtc_state(struct 
> amdgpu_display_manager *dm,
>   goto fail;
>   }
>   
> + if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> + return 0;

I had thought this was okay at first, but this will skip stream updates 
when needs_modeset = false.

Changing this line to just goto skip_modeset isn't sufficient either, we 
need drm_new_conn_state and drm_old_conn_state I believe below.

This check should probably be placed within the

if (aconnector && enable) block directly before the

new_stream = create_stream_for_sink(aconnector,

line. This should give you the behavior you're looking for.

Nicholas Kazlauskas

> +
>   aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
>   
>   /* TODO This hack should go away */
> @@ -5640,6 +5643,7 @@ static int dm_update_crtc_state(struct 
> amdgpu_display_manager *dm,
>   }
>   }
>   
> + /* mode_changed flag may get updated above, need to check again */
>   if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
>   goto skip_modeset;
>   
> 

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


[PATCH] drm/amd/display: Skip create new stream if crtc state doesn't change

2019-01-14 Thread Jerry (Fangzhi) Zuo
Need to check if crtc state is changed so that mode set is
required before trying to create new stream.

It deals with the MST hotplug use case when plug back to the
same connector where the failure to create new stream for the
inactive crtc state. Simply skip the check for the old connector
and go through the new one.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 987095ccd2c5..68739d0c600c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5593,6 +5593,9 @@ static int dm_update_crtc_state(struct 
amdgpu_display_manager *dm,
goto fail;
}
 
+   if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+   return 0;
+
aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
 
/* TODO This hack should go away */
@@ -5640,6 +5643,7 @@ static int dm_update_crtc_state(struct 
amdgpu_display_manager *dm,
}
}
 
+   /* mode_changed flag may get updated above, need to check again */
if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
goto skip_modeset;
 
-- 
2.14.1

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


Re: [PATCH] drm/amd: fix typo

2019-01-14 Thread Alex Deucher
Applied.  thanks.

Alex

On Mon, Jan 7, 2019 at 8:06 AM Matteo Croce  wrote:
>
> Fix spelling mistake: "lenght" -> "length"
>
> Signed-off-by: Matteo Croce 
> ---
>  drivers/gpu/drm/amd/include/atombios.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/include/atombios.h 
> b/drivers/gpu/drm/amd/include/atombios.h
> index 7931502fa54f..8ba21747b40a 100644
> --- a/drivers/gpu/drm/amd/include/atombios.h
> +++ b/drivers/gpu/drm/amd/include/atombios.h
> @@ -4106,7 +4106,7 @@ typedef struct  _ATOM_LCD_MODE_CONTROL_CAP
>  typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
>  {
>UCHAR ucRecordType;
> -  UCHAR ucFakeEDIDLength;   // = 128 means EDID lenght is 128 bytes, 
> otherwise the EDID length = ucFakeEDIDLength*128
> +  UCHAR ucFakeEDIDLength;   // = 128 means EDID length is 128 bytes, 
> otherwise the EDID length = ucFakeEDIDLength*128
>UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength 
> elements.
>  } ATOM_FAKE_EDID_PATCH_RECORD;
>
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2019-01-14 Thread Yang, Philip
Ping Christian, any comments for the GEM and CS part changes?

Thanks. Philip

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

Re: [[PATCH v2]drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Paul Menzel

Dear Aaron,


Am 14.01.19 um 09:47 schrieb Aaron Liu:

In some cases, psp response status is not 0 even there is no
problem while the command is submitted. Some version of PSP FW
doesn't write 0 to that field.
So here we would like to only print a warning instead of an error
during psp initialization to avoid breaking hw_init and it doesn't
return -EINVAL.

Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
Signed-off-by: Aaron Liu 
Reviewed-by: Huang Rui 
Reviewed-by: Xiangliang Yu
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 53c2d60..f26d8fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -140,14 +140,19 @@ psp_cmd_submit_buf(struct psp_context *psp,
while (*((unsigned int *)psp->fence_buf) != index)
msleep(1);
  
-	/* the status field must be 0 after psp command completion */

+   /* In some cases, psp response status is not 0 even there is no
+* problem while the command is submitted. Some version of PSP FW
+* doesn't write 0 to that field.
+* So here we would like to only print a warning instead of an error
+* during psp initialization to avoid breaking hw_init and it doesn't
+* return -EINVAL.
+*/
if (psp->cmd_buf_mem->resp.status) {
if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
+   DRM_WARN("failed to load ucode id (%d) ",
  ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
+   DRM_WARN("psp command failed and response status is (%d)\n",
  psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
}
  
  	/* get xGMI session id from response buffer */


Please describe, why this error can be ignored, and the rest of the 
function be executed. Won’t that introduce other problems?


How can real error situations be determined now?

Also, please extend the commit messages, that this only affects 
microcode update loading (if I understand this correctly).



Kind regards,

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


Re: [PATCH] drm/amdgpu: csa_vaddr should not larger than AMDGPU_GMC_HOLE_START

2019-01-14 Thread Christian König

Am 14.01.19 um 09:40 schrieb wentalou:

After removing unnecessary VM size calculations,
vm_manager.max_pfn would reach 0x10,,
max_pfn << AMDGPU_GPU_PAGE_SHIFT exceeding AMDGPU_GMC_HOLE_START
would caused GPU reset.

Change-Id: I47ad0be2b0bd9fb7490c4e1d7bb7bdacf71132cb
Signed-off-by: wentalou 


NAK, that is incorrect. We intentionally place the csa above the GMC hole.

Regards,
Christian.


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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 7e22be7..dd3bd01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -26,7 +26,8 @@
  
  uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)

  {
-   uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
+   uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
+   AMDGPU_GMC_HOLE_START);
  
  	addr -= AMDGPU_VA_RESERVED_SIZE;

addr = amdgpu_gmc_sign_extend(addr);


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


Re: [PATCH 2/2] drm/amdgpu: set bulk_moveable to false when lru changed v2

2019-01-14 Thread Koenig, Christian
Am 14.01.19 um 08:02 schrieb Chunming Zhou:
> if lru is changed, we cannot do bulk moving.
> v2:
> root bo isn't bulk moving, skip its change.
>
> Change-Id: Ide0fe920295cc25f7cabcf41a6400519e5783f2a
> Signed-off-by: Chunming Zhou 

We could now remove all other cases where we set bulk_movable to false, 
but that can also be a follow up patch.

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 22 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
>   3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c91ec3101d00..b852abb9db0f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1546,7 +1546,8 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
>   .io_mem_pfn = amdgpu_ttm_io_mem_pfn,
> - .access_memory = _ttm_access_memory
> + .access_memory = _ttm_access_memory,
> + .del_from_lru_notify = _vm_del_from_lru_notify
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e73d152659a2..ef012d47cbeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -623,6 +623,28 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
>   list_add(>tv.head, validated);
>   }
>   
> +void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
> +{
> + struct amdgpu_bo *abo;
> + struct amdgpu_vm_bo_base *bo_base;
> +
> + if (!amdgpu_bo_is_amdgpu_bo(bo))
> + return;
> +
> + if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
> + return;
> +
> + abo = ttm_to_amdgpu_bo(bo);
> + if (!abo->parent)
> + return;
> + for (bo_base = abo->vm_bo; bo_base; bo_base = bo_base->next) {
> + struct amdgpu_vm *vm = bo_base->vm;
> +
> + if (abo->tbo.resv == vm->root.base.bo->tbo.resv)
> + vm->bulk_moveable = false;
> + }
> +
> +}
>   /**
>* amdgpu_vm_move_to_lru_tail - move all BOs to the end of LRU
>*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index e8dcfd59fc93..81ff8177f092 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -363,4 +363,6 @@ int amdgpu_vm_add_fault(struct 
> amdgpu_retryfault_hashtable *fault_hash, u64 key)
>   
>   void amdgpu_vm_clear_fault(struct amdgpu_retryfault_hashtable *fault_hash, 
> u64 key);
>   
> +void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo);
> +
>   #endif

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


Re: [PATCH 1/2] drm/ttm: add lru notify to bo driver v2

2019-01-14 Thread Koenig, Christian
Am 14.01.19 um 08:02 schrieb Chunming Zhou:
> allow driver do somethings when lru changed.
> v2:
> address Michel's comments.
>
> Change-Id: Ie82053da49272881d4b52b1c406f7c221a3fcadf
> Signed-off-by: Chunming Zhou 

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c| 11 +++
>   include/drm/ttm/ttm_bo_driver.h |  9 +
>   2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 0ec08394e17a..de088c8070fb 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -198,19 +198,22 @@ static void ttm_bo_ref_bug(struct kref *list_kref)
>   
>   void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>   {
> + struct ttm_bo_device *bdev = bo->bdev;
> + bool notify = false;
> +
>   if (!list_empty(>swap)) {
>   list_del_init(>swap);
>   kref_put(>list_kref, ttm_bo_ref_bug);
> + notify = true;
>   }
>   if (!list_empty(>lru)) {
>   list_del_init(>lru);
>   kref_put(>list_kref, ttm_bo_ref_bug);
> + notify = true;
>   }
>   
> - /*
> -  * TODO: Add a driver hook to delete from
> -  * driver-specific LRU's here.
> -  */
> + if (notify && bdev->driver->del_from_lru_notify)
> + bdev->driver->del_from_lru_notify(bo);
>   }
>   
>   void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 1021106438b2..15829b24277c 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -381,6 +381,15 @@ struct ttm_bo_driver {
>*/
>   int (*access_memory)(struct ttm_buffer_object *bo, unsigned long offset,
>void *buf, int len, int write);
> +
> + /**
> +  * struct ttm_bo_driver member del_from_lru_notify
> +  *
> +  * @bo: the buffer object deleted from lru
> +  *
> +  * notify driver that a BO was deleted from LRU.
> +  */
> + void (*del_from_lru_notify)(struct ttm_buffer_object *bo);
>   };
>   
>   /**

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


Re: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Christian König

Am 14.01.19 um 09:16 schrieb Aaron Liu:

Don't return err if psp reponse status isn't zero

Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
Signed-off-by: Aaron Liu 


Thanks for taking care of that. Fix is Acked-by: Christian König 




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 53c2d60..48778b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -140,14 +140,15 @@ psp_cmd_submit_buf(struct psp_context *psp,
while (*((unsigned int *)psp->fence_buf) != index)
msleep(1);
  
-	/* the status field must be 0 after psp command completion */

+   /* the status field should be 0 after psp command completion
+* if not, print WARN msg
+*/
if (psp->cmd_buf_mem->resp.status) {
if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
+   DRM_WARN("failed to load ucode id (%d) ",
  ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
+   DRM_WARN("psp command failed and response status is (%d)\n",
  psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
}
  
  	/* get xGMI session id from response buffer */


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


[PATCH 4/4] drm/amd/powerplay: support retrieving and adjusting dcefclock power levels

2019-01-14 Thread Evan Quan
User can use "pp_dpm_dcefclk" to retrieve and adjust dcefclock power
levels.

Change-Id: Ia3f61558ca96104c88d129ba5194103b2fe702ec
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 53 ++-
 .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 52 +-
 3 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index f6646a522c06..b7b70f590236 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -731,11 +731,13 @@ static ssize_t amdgpu_get_ppfeature_status(struct device 
*dev,
 }
 
 /**
- * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk pp_dpm_pcie
+ * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk pp_dpm_dcefclk
+ * pp_dpm_pcie
  *
  * The amdgpu driver provides a sysfs API for adjusting what power levels
  * are enabled for a given power state.  The files pp_dpm_sclk, pp_dpm_mclk,
- * pp_dpm_socclk, pp_dpm_fclk and pp_dpm_pcie are used for this.
+ * pp_dpm_socclk, pp_dpm_fclk, pp_dpm_dcefclk and pp_dpm_pcie are used for
+ * this.
  *
  * Reading back the files will show you the available power levels within
  * the power state and the clock information for those levels.
@@ -745,6 +747,8 @@ static ssize_t amdgpu_get_ppfeature_status(struct device 
*dev,
  * Secondly,Enter a new value for each level by inputing a string that
  * contains " echo xx xx xx > pp_dpm_sclk/mclk/pcie"
  * E.g., echo 4 5 6 to > pp_dpm_sclk will enable sclk levels 4, 5, and 6.
+ *
+ * NOTE: change to the dcefclk max dpm level is not supported now
  */
 
 static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
@@ -927,6 +931,42 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
return count;
 }
 
+static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+
+   if (adev->powerplay.pp_funcs->print_clock_levels)
+   return amdgpu_dpm_print_clock_levels(adev, PP_DCEFCLK, buf);
+   else
+   return snprintf(buf, PAGE_SIZE, "\n");
+}
+
+static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf,
+   size_t count)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   int ret;
+   uint32_t mask = 0;
+
+   ret = amdgpu_read_mask(buf, count, );
+   if (ret)
+   return ret;
+
+   if (adev->powerplay.pp_funcs->force_clock_level)
+   ret = amdgpu_dpm_force_clock_level(adev, PP_DCEFCLK, mask);
+
+   if (ret)
+   return -EINVAL;
+
+   return count;
+}
+
 static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -1216,6 +1256,9 @@ static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_fclk,
amdgpu_set_pp_dpm_fclk);
+static DEVICE_ATTR(pp_dpm_dcefclk, S_IRUGO | S_IWUSR,
+   amdgpu_get_pp_dpm_dcefclk,
+   amdgpu_set_pp_dpm_dcefclk);
 static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_pcie,
amdgpu_set_pp_dpm_pcie);
@@ -2387,6 +2430,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
DRM_ERROR("failed to create device file pp_dpm_fclk\n");
return ret;
}
+   ret = device_create_file(adev->dev, _attr_pp_dpm_dcefclk);
+   if (ret) {
+   DRM_ERROR("failed to create device file pp_dpm_dcefclk\n");
+   return ret;
+   }
ret = device_create_file(adev->dev, _attr_pp_dpm_pcie);
if (ret) {
DRM_ERROR("failed to create device file pp_dpm_pcie\n");
@@ -2474,6 +2522,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev, _attr_pp_dpm_socclk);
device_remove_file(adev->dev, _attr_pp_dpm_pcie);
device_remove_file(adev->dev, _attr_pp_dpm_fclk);
+   device_remove_file(adev->dev, _attr_pp_dpm_dcefclk);
device_remove_file(adev->dev, _attr_pp_sclk_od);
device_remove_file(adev->dev, _attr_pp_mclk_od);
device_remove_file(adev->dev,
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index f82de14f6560..2b579ba9b685 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -94,6 +94,7 @@ enum pp_clock_type {
PP_PCIE,
PP_SOCCLK,
PP_FCLK,
+   PP_DCEFCLK,
OD_SCLK,
OD_MCLK,
 

[PATCH 2/4] drm/amd/powerplay: support retrieving and adjusting SOC clock power levels

2019-01-14 Thread Evan Quan
User can use "pp_dpm_socclk" to retrieve and adjust SOC clock power
levels.

Change-Id: I24d2f1df2dcf6b8dfa188f82767b36624610406f
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 49 ++-
 .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 45 +
 3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index db490587464c..f735118a6070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -731,11 +731,11 @@ static ssize_t amdgpu_get_ppfeature_status(struct device 
*dev,
 }
 
 /**
- * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_pcie
+ * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_pcie
  *
  * The amdgpu driver provides a sysfs API for adjusting what power levels
  * are enabled for a given power state.  The files pp_dpm_sclk, pp_dpm_mclk,
- * and pp_dpm_pcie are used for this.
+ * pp_dpm_socclk and pp_dpm_pcie are used for this.
  *
  * Reading back the files will show you the available power levels within
  * the power state and the clock information for those levels.
@@ -855,6 +855,42 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
return count;
 }
 
+static ssize_t amdgpu_get_pp_dpm_socclk(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+
+   if (adev->powerplay.pp_funcs->print_clock_levels)
+   return amdgpu_dpm_print_clock_levels(adev, PP_SOCCLK, buf);
+   else
+   return snprintf(buf, PAGE_SIZE, "\n");
+}
+
+static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf,
+   size_t count)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   int ret;
+   uint32_t mask = 0;
+
+   ret = amdgpu_read_mask(buf, count, );
+   if (ret)
+   return ret;
+
+   if (adev->powerplay.pp_funcs->force_clock_level)
+   ret = amdgpu_dpm_force_clock_level(adev, PP_SOCCLK, mask);
+
+   if (ret)
+   return -EINVAL;
+
+   return count;
+}
+
 static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -1138,6 +1174,9 @@ static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_mclk,
amdgpu_set_pp_dpm_mclk);
+static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO | S_IWUSR,
+   amdgpu_get_pp_dpm_socclk,
+   amdgpu_set_pp_dpm_socclk);
 static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_pcie,
amdgpu_set_pp_dpm_pcie);
@@ -2299,6 +2338,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
DRM_ERROR("failed to create device file pp_dpm_mclk\n");
return ret;
}
+   ret = device_create_file(adev->dev, _attr_pp_dpm_socclk);
+   if (ret) {
+   DRM_ERROR("failed to create device file pp_dpm_socclk\n");
+   return ret;
+   }
ret = device_create_file(adev->dev, _attr_pp_dpm_pcie);
if (ret) {
DRM_ERROR("failed to create device file pp_dpm_pcie\n");
@@ -2383,6 +2427,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
 
device_remove_file(adev->dev, _attr_pp_dpm_sclk);
device_remove_file(adev->dev, _attr_pp_dpm_mclk);
+   device_remove_file(adev->dev, _attr_pp_dpm_socclk);
device_remove_file(adev->dev, _attr_pp_dpm_pcie);
device_remove_file(adev->dev, _attr_pp_sclk_od);
device_remove_file(adev->dev, _attr_pp_mclk_od);
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 1130f293c4ee..f5ec25a6ab54 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -92,6 +92,7 @@ enum pp_clock_type {
PP_SCLK,
PP_MCLK,
PP_PCIE,
+   PP_SOCCLK,
OD_SCLK,
OD_MCLK,
OD_VDDC_CURVE,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 58b63f864227..61583d6fc20e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -2295,6 +2295,34 @@ static int vega20_force_clock_level(struct pp_hwmgr 
*hwmgr,
 
break;
 
+   case PP_SOCCLK:
+   soft_min_level = mask ? (ffs(mask) - 1) : 0;
+   soft_max_level = mask ? (fls(mask) - 1) : 0;
+
+   if (soft_max_level >= 

[PATCH 3/4] drm/amd/powerplay: support retrieving and adjusting fclock power levels

2019-01-14 Thread Evan Quan
User can use "pp_dpm_fclk" to retrieve and adjust fclock power
levels.

Change-Id: Ib5325a9533b5fe8c125738a2f26e7b976d3ec991
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 49 +-
 .../gpu/drm/amd/include/kgd_pp_interface.h|  1 +
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 64 +++
 3 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index f735118a6070..f6646a522c06 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -731,11 +731,11 @@ static ssize_t amdgpu_get_ppfeature_status(struct device 
*dev,
 }
 
 /**
- * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_pcie
+ * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_socclk pp_dpm_fclk pp_dpm_pcie
  *
  * The amdgpu driver provides a sysfs API for adjusting what power levels
  * are enabled for a given power state.  The files pp_dpm_sclk, pp_dpm_mclk,
- * pp_dpm_socclk and pp_dpm_pcie are used for this.
+ * pp_dpm_socclk, pp_dpm_fclk and pp_dpm_pcie are used for this.
  *
  * Reading back the files will show you the available power levels within
  * the power state and the clock information for those levels.
@@ -891,6 +891,42 @@ static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev,
return count;
 }
 
+static ssize_t amdgpu_get_pp_dpm_fclk(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+
+   if (adev->powerplay.pp_funcs->print_clock_levels)
+   return amdgpu_dpm_print_clock_levels(adev, PP_FCLK, buf);
+   else
+   return snprintf(buf, PAGE_SIZE, "\n");
+}
+
+static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf,
+   size_t count)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   int ret;
+   uint32_t mask = 0;
+
+   ret = amdgpu_read_mask(buf, count, );
+   if (ret)
+   return ret;
+
+   if (adev->powerplay.pp_funcs->force_clock_level)
+   ret = amdgpu_dpm_force_clock_level(adev, PP_FCLK, mask);
+
+   if (ret)
+   return -EINVAL;
+
+   return count;
+}
+
 static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -1177,6 +1213,9 @@ static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_socclk,
amdgpu_set_pp_dpm_socclk);
+static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR,
+   amdgpu_get_pp_dpm_fclk,
+   amdgpu_set_pp_dpm_fclk);
 static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
amdgpu_get_pp_dpm_pcie,
amdgpu_set_pp_dpm_pcie);
@@ -2343,6 +2382,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
DRM_ERROR("failed to create device file pp_dpm_socclk\n");
return ret;
}
+   ret = device_create_file(adev->dev, _attr_pp_dpm_fclk);
+   if (ret) {
+   DRM_ERROR("failed to create device file pp_dpm_fclk\n");
+   return ret;
+   }
ret = device_create_file(adev->dev, _attr_pp_dpm_pcie);
if (ret) {
DRM_ERROR("failed to create device file pp_dpm_pcie\n");
@@ -2429,6 +2473,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev, _attr_pp_dpm_mclk);
device_remove_file(adev->dev, _attr_pp_dpm_socclk);
device_remove_file(adev->dev, _attr_pp_dpm_pcie);
+   device_remove_file(adev->dev, _attr_pp_dpm_fclk);
device_remove_file(adev->dev, _attr_pp_sclk_od);
device_remove_file(adev->dev, _attr_pp_mclk_od);
device_remove_file(adev->dev,
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index f5ec25a6ab54..f82de14f6560 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -93,6 +93,7 @@ enum pp_clock_type {
PP_MCLK,
PP_PCIE,
PP_SOCCLK,
+   PP_FCLK,
OD_SCLK,
OD_MCLK,
OD_VDDC_CURVE,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 61583d6fc20e..1832dcb965b1 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -1735,6 +1735,17 @@ static int vega20_upload_dpm_min_level(struct pp_hwmgr 
*hwmgr, uint32_t feature_
return ret);
}
 
+   if (data->smu_features[GNLD_DPM_FCLK].enabled &&

[PATCH 1/4] drm/amd/powerplay: support enabled ppfeatures retrieving and setting

2019-01-14 Thread Evan Quan
User can use "ppfeatures" sysfs interface to retrieve and set enabled
powerplay features.

Change-Id: I00d5b4931ab426b506e1c187f81a279b00afbb66
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h   |   8 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c|  66 +++
 .../gpu/drm/amd/include/kgd_pp_interface.h|   2 +
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c |  42 +++
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 104 ++
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   2 +
 6 files changed, 224 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index f972cd156795..2f61e9edb1c1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -364,6 +364,14 @@ enum amdgpu_pcie_gen {
((adev)->powerplay.pp_funcs->enable_mgpu_fan_boost(\
(adev)->powerplay.pp_handle))
 
+#define amdgpu_dpm_get_ppfeature_status(adev, buf) \
+   ((adev)->powerplay.pp_funcs->get_ppfeature_status(\
+   (adev)->powerplay.pp_handle, (buf)))
+
+#define amdgpu_dpm_set_ppfeature_status(adev, ppfeatures) \
+   ((adev)->powerplay.pp_funcs->set_ppfeature_status(\
+   (adev)->powerplay.pp_handle, (ppfeatures)))
+
 struct amdgpu_dpm {
struct amdgpu_ps*ps;
/* number of valid power states */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 30ab304eb8b6..db490587464c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -676,6 +676,60 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device 
*dev,
 
 }
 
+/**
+ * DOC: ppfeatures
+ *
+ * The amdgpu driver provides a sysfs API for adjusting what powerplay
+ * features to be enabled. The file ppfeatures is used for this.
+ *
+ * Reading back the file will show you the followings:
+ * - Current ppfeature masks
+ * - List of the all supported powerplay features with their naming,
+ *   bitmasks and enablement status('Y'/'N' means "enabled"/"disabled").
+ *
+ * To manually enable or disable a specific feature, just set or clear
+ * the corresponding bit from original ppfeature masks and input the
+ * new ppfeature masks.
+ */
+static ssize_t amdgpu_set_ppfeature_status(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf,
+   size_t count)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   uint64_t featuremask;
+   int ret;
+
+   ret = kstrtou64(buf, 0, );
+   if (ret)
+   return -EINVAL;
+
+   pr_debug("featuremask = 0x%llx\n", featuremask);
+
+   if (adev->powerplay.pp_funcs->set_ppfeature_status) {
+   ret = amdgpu_dpm_set_ppfeature_status(adev, featuremask);
+   if (ret)
+   return -EINVAL;
+   }
+
+   return count;
+}
+
+static ssize_t amdgpu_get_ppfeature_status(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   uint32_t size = 0;
+
+   if (adev->powerplay.pp_funcs->get_ppfeature_status)
+   return amdgpu_dpm_get_ppfeature_status(adev, buf);
+
+   return snprintf(buf, PAGE_SIZE, "\n");
+}
+
 /**
  * DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_pcie
  *
@@ -1102,6 +1156,9 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
amdgpu_get_busy_percent, NULL);
 static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
+static DEVICE_ATTR(ppfeatures, S_IRUGO | S_IWUSR,
+   amdgpu_get_ppfeature_status,
+   amdgpu_set_ppfeature_status);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
  struct device_attribute *attr,
@@ -2294,6 +2351,14 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
return ret;
}
 
+   ret = device_create_file(adev->dev,
+   _attr_ppfeatures);
+   if (ret) {
+   DRM_ERROR("failed to create device file "
+   "ppfeatures\n");
+   return ret;
+   }
+
adev->pm.sysfs_initialized = true;
 
return 0;
@@ -2329,6 +2394,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev, _attr_gpu_busy_percent);
if (adev->flags & !AMD_IS_APU)
device_remove_file(adev->dev, _attr_pcie_bw);
+   device_remove_file(adev->dev, _attr_ppfeatures);
 }
 
 void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 

RE: [PATCH] drm/amdgpu: csa_vaddr should not larger than AMDGPU_GMC_HOLE_START

2019-01-14 Thread Liu, Monk
Good catch, patch is

Reviewed-by: Monk Liu 

-Original Message-
From: amd-gfx  On Behalf Of wentalou
Sent: Monday, January 14, 2019 4:41 PM
To: amd-gfx@lists.freedesktop.org
Cc: Lou, Wentao 
Subject: [PATCH] drm/amdgpu: csa_vaddr should not larger than 
AMDGPU_GMC_HOLE_START

After removing unnecessary VM size calculations, vm_manager.max_pfn would reach 
0x10,, max_pfn << AMDGPU_GPU_PAGE_SHIFT exceeding AMDGPU_GMC_HOLE_START 
would caused GPU reset.

Change-Id: I47ad0be2b0bd9fb7490c4e1d7bb7bdacf71132cb
Signed-off-by: wentalou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 7e22be7..dd3bd01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -26,7 +26,8 @@
 
 uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)  {
-   uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
+   uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
+   AMDGPU_GMC_HOLE_START);
 
addr -= AMDGPU_VA_RESERVED_SIZE;
addr = amdgpu_gmc_sign_extend(addr);
--
2.7.4

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


[[PATCH v2]drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Aaron Liu
In some cases, psp response status is not 0 even there is no
problem while the command is submitted. Some version of PSP FW
doesn't write 0 to that field.
So here we would like to only print a warning instead of an error
during psp initialization to avoid breaking hw_init and it doesn't
return -EINVAL.

Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
Signed-off-by: Aaron Liu 
Reviewed-by: Huang Rui 
Reviewed-by: Xiangliang Yu
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 53c2d60..f26d8fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -140,14 +140,19 @@ psp_cmd_submit_buf(struct psp_context *psp,
while (*((unsigned int *)psp->fence_buf) != index)
msleep(1);
 
-   /* the status field must be 0 after psp command completion */
+   /* In some cases, psp response status is not 0 even there is no
+* problem while the command is submitted. Some version of PSP FW
+* doesn't write 0 to that field.
+* So here we would like to only print a warning instead of an error
+* during psp initialization to avoid breaking hw_init and it doesn't
+* return -EINVAL.
+*/
if (psp->cmd_buf_mem->resp.status) {
if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
+   DRM_WARN("failed to load ucode id (%d) ",
  ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
+   DRM_WARN("psp command failed and response status is (%d)\n",
  psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
}
 
/* get xGMI session id from response buffer */
-- 
2.7.4

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


[PATCH] drm/amdgpu: csa_vaddr should not larger than AMDGPU_GMC_HOLE_START

2019-01-14 Thread wentalou
After removing unnecessary VM size calculations,
vm_manager.max_pfn would reach 0x10,,
max_pfn << AMDGPU_GPU_PAGE_SHIFT exceeding AMDGPU_GMC_HOLE_START
would caused GPU reset.

Change-Id: I47ad0be2b0bd9fb7490c4e1d7bb7bdacf71132cb
Signed-off-by: wentalou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 7e22be7..dd3bd01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -26,7 +26,8 @@
 
 uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
 {
-   uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
+   uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
+   AMDGPU_GMC_HOLE_START);
 
addr -= AMDGPU_VA_RESERVED_SIZE;
addr = amdgpu_gmc_sign_extend(addr);
-- 
2.7.4

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


RE: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Xu, Feifei
Reviewed-by: Feifei Xu 

-Original Message-
From: amd-gfx  On Behalf Of Huang, Ray
Sent: Monday, January 14, 2019 4:28 PM
To: Liu, Aaron ; amd-gfx@lists.freedesktop.org; Yu, 
Xiangliang ; Deucher, Alexander 
; Zhang, Hawking 
Cc: Liu, Aaron ; Koenig, Christian 
Subject: RE: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: 
ignore psp reponse status

Is some cases, response status is not 0 even there is no problem while the 
command is submitted.
Some version of PSP FW doesn't write 0 to that field. So here we would like to 
only print a warning instead of an error during psp initialization to avoid 
breaking hw_init.  

Reviewed-by: Huang Rui   


> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf 
> Of Aaron Liu
> Sent: Monday, January 14, 2019 4:17 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Aaron 
> Subject: [PATCH drm/amdgpu/psp: ignore psp reponse status]
> drm/amdgpu/psp: ignore psp reponse status
> 
> Don't return err if psp reponse status isn't zero
> 
> Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
> Signed-off-by: Aaron Liu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 53c2d60..48778b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -140,14 +140,15 @@ psp_cmd_submit_buf(struct psp_context *psp,
>   while (*((unsigned int *)psp->fence_buf) != index)
>   msleep(1);
> 
> - /* the status field must be 0 after psp command completion */
> + /* the status field should be 0 after psp command completion
> +  * if not, print WARN msg
> +  */
>   if (psp->cmd_buf_mem->resp.status) {
>   if (ucode)
> - DRM_ERROR("failed to load ucode id (%d) ",
> + DRM_WARN("failed to load ucode id (%d) ",
> ucode->ucode_id);
> - DRM_ERROR("psp command failed and response status is
> (%d)\n",
> + DRM_WARN("psp command failed and response status is
> (%d)\n",
> psp->cmd_buf_mem->resp.status);
> - return -EINVAL;
>   }
> 
>   /* get xGMI session id from response buffer */
> --
> 2.7.4
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Yu, Xiangliang
Please add more comments to explain why don't return error.
And add my RB with that fix.

Reviewed-by: Xiangliang Yu

-Original Message-
From: amd-gfx  On Behalf Of Aaron Liu
Sent: Monday, January 14, 2019 4:17 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Aaron 
Subject: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: 
ignore psp reponse status

Don't return err if psp reponse status isn't zero

Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
Signed-off-by: Aaron Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 53c2d60..48778b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -140,14 +140,15 @@ psp_cmd_submit_buf(struct psp_context *psp,
while (*((unsigned int *)psp->fence_buf) != index)
msleep(1);
 
-   /* the status field must be 0 after psp command completion */
+   /* the status field should be 0 after psp command completion
+* if not, print WARN msg
+*/
if (psp->cmd_buf_mem->resp.status) {
if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
+   DRM_WARN("failed to load ucode id (%d) ",
  ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
+   DRM_WARN("psp command failed and response status is (%d)\n",
  psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
}
 
/* get xGMI session id from response buffer */
-- 
2.7.4

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


RE: [PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Huang, Ray
Is some cases, response status is not 0 even there is no problem while the 
command is submitted.
Some version of PSP FW doesn't write 0 to that field. So here we would like to 
only print a warning instead of an error during psp initialization to avoid 
breaking hw_init.  

Reviewed-by: Huang Rui   


> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Aaron Liu
> Sent: Monday, January 14, 2019 4:17 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Aaron 
> Subject: [PATCH drm/amdgpu/psp: ignore psp reponse status]
> drm/amdgpu/psp: ignore psp reponse status
> 
> Don't return err if psp reponse status isn't zero
> 
> Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
> Signed-off-by: Aaron Liu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 53c2d60..48778b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -140,14 +140,15 @@ psp_cmd_submit_buf(struct psp_context *psp,
>   while (*((unsigned int *)psp->fence_buf) != index)
>   msleep(1);
> 
> - /* the status field must be 0 after psp command completion */
> + /* the status field should be 0 after psp command completion
> +  * if not, print WARN msg
> +  */
>   if (psp->cmd_buf_mem->resp.status) {
>   if (ucode)
> - DRM_ERROR("failed to load ucode id (%d) ",
> + DRM_WARN("failed to load ucode id (%d) ",
> ucode->ucode_id);
> - DRM_ERROR("psp command failed and response status is
> (%d)\n",
> + DRM_WARN("psp command failed and response status is
> (%d)\n",
> psp->cmd_buf_mem->resp.status);
> - return -EINVAL;
>   }
> 
>   /* get xGMI session id from response buffer */
> --
> 2.7.4
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH drm/amdgpu/psp: ignore psp reponse status] drm/amdgpu/psp: ignore psp reponse status

2019-01-14 Thread Aaron Liu
Don't return err if psp reponse status isn't zero

Change-Id: I680679983f972b6969f4949f1faafaf17fe996a6
Signed-off-by: Aaron Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 53c2d60..48778b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -140,14 +140,15 @@ psp_cmd_submit_buf(struct psp_context *psp,
while (*((unsigned int *)psp->fence_buf) != index)
msleep(1);
 
-   /* the status field must be 0 after psp command completion */
+   /* the status field should be 0 after psp command completion
+* if not, print WARN msg
+*/
if (psp->cmd_buf_mem->resp.status) {
if (ucode)
-   DRM_ERROR("failed to load ucode id (%d) ",
+   DRM_WARN("failed to load ucode id (%d) ",
  ucode->ucode_id);
-   DRM_ERROR("psp command failed and response status is (%d)\n",
+   DRM_WARN("psp command failed and response status is (%d)\n",
  psp->cmd_buf_mem->resp.status);
-   return -EINVAL;
}
 
/* get xGMI session id from response buffer */
-- 
2.7.4

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