RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
[AMD Official Use Only - Internal Distribution Only]
> -Original Message-
> From: Dai, Yuxian (David)
> Sent: Thursday, December 19, 2019 3:39 PM
> To: Huang, Ray ; Liang, Prike
> Cc: amd-gfx@lists.freedesktop.org; Quan, Evan
> Subject: RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile
> mode overrun message
>
> As Ray point out. We should set the SMU_MSG_SetWorkloadMask with
> specified value to indicate unsupported.
> But the current a value with system error value: "-EINVAL"
> The firmware maybe response with unexpected action to driver.
[Prike] If get nonsupport profile mode will exit and not issue any you
mentioned error value.
>
> -Original Message-
> From: Huang, Ray
> Sent: Thursday, December 19, 2019 3:17 PM
> To: Dai, Yuxian (David)
> Cc: Liang, Prike ; amd-gfx@lists.freedesktop.org;
> Quan, Evan
> Subject: Re: [PATCH] drm/amd/powerplay: suppress nonsupport profile
> mode overrun message
>
> [AMD Official Use Only - Internal Distribution Only]
>
> On Thu, Dec 19, 2019 at 03:04:12PM +0800, Dai, Yuxian (David) wrote:
> > For we don't support the mode, so shouldn't print the error message, or
> regard as a error.
> > For log message, the error is high level .maybe change from "error" to
> "warning" , it will be much better.
> >
> >
> > -Original Message-
> > From: Liang, Prike
> > Sent: Thursday, December 19, 2019 2:46 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Quan, Evan ; Huang, Ray
> ;
> > Dai, Yuxian (David) ; Liang, Prike
> >
> > Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
> > overrun message
> >
> > SMU12 not support WORKLOAD_DEFAULT_BIT and
> WORKLOAD_PPLIB_POWER_SAVING_BIT.
> >
>
> Probably smu firmware doesn't expose the feature mask to driver. Can you
> confirmware with smu firmware guy whehter this feature is really disabled or
> not in SMU12. If that, in my view, issue the message
> SMU_MSG_SetWorkloadMask with an unsupported state, it doesn't make
> sense.
>
> Just work around this with one time warnning is not a good solution.
>
> Thanks,
> Ray
>
> > Signed-off-by: Prike Liang
> > ---
> > drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > index 784903a3..f9a1817 100644
> > --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> > /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> > workload_type = smu_workload_get_type(smu, smu-
> >power_profile_mode);
> > if (workload_type < 0) {
> > - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> > + /*
> > +* TODO: If some case need switch to powersave/default
> power mode
> > +* then can consider enter
> WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for power saving.
> > +*/
> > + pr_err_once("Unsupported power profile mode %d on
> > +RENOIR\n",smu->power_profile_mode);
> > return -EINVAL;
> > }
> >
> > ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetWorkloadMask,
> > 1 << workload_type);
> > if (ret) {
> > - pr_err("Fail to set workload type %d\n", workload_type);
> > + pr_err_once("Fail to set workload type %d\n",
> workload_type);
> > return ret;
> > }
> >
> > --
> > 2.7.4
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
As Ray point out. We should set the SMU_MSG_SetWorkloadMask with specified
value to indicate unsupported.
But the current a value with system error value: "-EINVAL"
The firmware maybe response with unexpected action to driver.
-Original Message-
From: Huang, Ray
Sent: Thursday, December 19, 2019 3:17 PM
To: Dai, Yuxian (David)
Cc: Liang, Prike ; amd-gfx@lists.freedesktop.org; Quan,
Evan
Subject: Re: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
overrun message
[AMD Official Use Only - Internal Distribution Only]
On Thu, Dec 19, 2019 at 03:04:12PM +0800, Dai, Yuxian (David) wrote:
> For we don't support the mode, so shouldn't print the error message, or
> regard as a error.
> For log message, the error is high level .maybe change from "error" to
> "warning" , it will be much better.
>
>
> -Original Message-
> From: Liang, Prike
> Sent: Thursday, December 19, 2019 2:46 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan ; Huang, Ray ;
> Dai, Yuxian (David) ; Liang, Prike
>
> Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
> overrun message
>
> SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
>
Probably smu firmware doesn't expose the feature mask to driver. Can you
confirmware with smu firmware guy whehter this feature is really disabled or
not in SMU12. If that, in my view, issue the message SMU_MSG_SetWorkloadMask
with an unsupported state, it doesn't make sense.
Just work around this with one time warnning is not a good solution.
Thanks,
Ray
> Signed-off-by: Prike Liang
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 784903a3..f9a1817 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
> if (workload_type < 0) {
> - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> + /*
> + * TODO: If some case need switch to powersave/default power
> mode
> + * then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
> power saving.
> + */
> + pr_err_once("Unsupported power profile mode %d on
> +RENOIR\n",smu->power_profile_mode);
> return -EINVAL;
> }
>
> ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
> 1 << workload_type);
> if (ret) {
> - pr_err("Fail to set workload type %d\n", workload_type);
> + pr_err_once("Fail to set workload type %d\n", workload_type);
> return ret;
> }
>
> --
> 2.7.4
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
[AMD Official Use Only - Internal Distribution Only]
> -Original Message-
> From: Huang, Ray
> Sent: Thursday, December 19, 2019 3:17 PM
> To: Dai, Yuxian (David)
> Cc: Liang, Prike ; amd-gfx@lists.freedesktop.org;
> Quan, Evan
> Subject: Re: [PATCH] drm/amd/powerplay: suppress nonsupport profile
> mode overrun message
>
> [AMD Official Use Only - Internal Distribution Only]
>
> On Thu, Dec 19, 2019 at 03:04:12PM +0800, Dai, Yuxian (David) wrote:
> > For we don't support the mode, so shouldn't print the error message, or
> regard as a error.
> > For log message, the error is high level .maybe change from "error" to
> "warning" , it will be much better.
> >
> >
> > -Original Message-
> > From: Liang, Prike
> > Sent: Thursday, December 19, 2019 2:46 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Quan, Evan ; Huang, Ray
> ;
> > Dai, Yuxian (David) ; Liang, Prike
> >
> > Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
> > overrun message
> >
> > SMU12 not support WORKLOAD_DEFAULT_BIT and
> WORKLOAD_PPLIB_POWER_SAVING_BIT.
> >
>
> Probably smu firmware doesn't expose the feature mask to driver. Can you
> confirmware with smu firmware guy whehter this feature is really disabled or
> not in SMU12. If that, in my view, issue the message
> SMU_MSG_SetWorkloadMask with an unsupported state, it doesn't make
> sense.
>
> Just work around this with one time warnning is not a good solution.
[Prike] Yes, from SMU FW guy @Cai, Land SMU12 not support default and
power saving mode now. As the patch TODO item said we can consider switch to
compute/customer profile mode for power saving if needed.
>
> Thanks,
> Ray
>
> > Signed-off-by: Prike Liang
> > ---
> > drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > index 784903a3..f9a1817 100644
> > --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> > @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> > /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> > workload_type = smu_workload_get_type(smu, smu-
> >power_profile_mode);
> > if (workload_type < 0) {
> > - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> > + /*
> > +* TODO: If some case need switch to powersave/default
> power mode
> > +* then can consider enter
> WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for power saving.
> > +*/
> > + pr_err_once("Unsupported power profile mode %d on
> > +RENOIR\n",smu->power_profile_mode);
> > return -EINVAL;
> > }
> >
> > ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetWorkloadMask,
> > 1 << workload_type);
> > if (ret) {
> > - pr_err("Fail to set workload type %d\n", workload_type);
> > + pr_err_once("Fail to set workload type %d\n",
> workload_type);
> > return ret;
> > }
> >
> > --
> > 2.7.4
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
[AMD Official Use Only - Internal Distribution Only]
On Thu, Dec 19, 2019 at 03:04:12PM +0800, Dai, Yuxian (David) wrote:
> For we don't support the mode, so shouldn't print the error message, or
> regard as a error.
> For log message, the error is high level .maybe change from "error" to
> "warning" , it will be much better.
>
>
> -Original Message-
> From: Liang, Prike
> Sent: Thursday, December 19, 2019 2:46 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan ; Huang, Ray ; Dai,
> Yuxian (David) ; Liang, Prike
> Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun
> message
>
> SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
>
Probably smu firmware doesn't expose the feature mask to driver. Can you
confirmware with smu firmware guy whehter this feature is really disabled
or not in SMU12. If that, in my view, issue the message
SMU_MSG_SetWorkloadMask with an unsupported state, it doesn't make sense.
Just work around this with one time warnning is not a good solution.
Thanks,
Ray
> Signed-off-by: Prike Liang
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 784903a3..f9a1817 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
> if (workload_type < 0) {
> - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> + /*
> + * TODO: If some case need switch to powersave/default power
> mode
> + * then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
> power saving.
> + */
> + pr_err_once("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> return -EINVAL;
> }
>
> ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
> 1 << workload_type);
> if (ret) {
> - pr_err("Fail to set workload type %d\n", workload_type);
> + pr_err_once("Fail to set workload type %d\n", workload_type);
> return ret;
> }
>
> --
> 2.7.4
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
For we don't support the mode, so shouldn't print the error message, or regard
as a error.
For log message, the error is high level .maybe change from "error" to
"warning" , it will be much better.
-Original Message-
From: Liang, Prike
Sent: Thursday, December 19, 2019 2:46 PM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan ; Huang, Ray ; Dai, Yuxian
(David) ; Liang, Prike
Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun
message
SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
Signed-off-by: Prike Liang
---
drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 784903a3..f9a1817 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
smu_context *smu, long *input, u
/* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
if (workload_type < 0) {
- pr_err("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
+ /*
+* TODO: If some case need switch to powersave/default power
mode
+* then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
power saving.
+*/
+ pr_err_once("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
return -EINVAL;
}
ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
1 << workload_type);
if (ret) {
- pr_err("Fail to set workload type %d\n", workload_type);
+ pr_err_once("Fail to set workload type %d\n", workload_type);
return ret;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
Reviewed-by: Evan Quan
> -Original Message-
> From: Liang, Prike
> Sent: Thursday, December 19, 2019 2:46 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan ; Huang, Ray ;
> Dai, Yuxian (David) ; Liang, Prike
>
> Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
> overrun message
>
> SMU12 not support WORKLOAD_DEFAULT_BIT and
> WORKLOAD_PPLIB_POWER_SAVING_BIT.
>
> Signed-off-by: Prike Liang
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 784903a3..f9a1817 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> workload_type = smu_workload_get_type(smu, smu-
> >power_profile_mode);
> if (workload_type < 0) {
> - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> + /*
> + * TODO: If some case need switch to powersave/default
> power mode
> + * then can consider enter
> WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for power saving.
> + */
> + pr_err_once("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> return -EINVAL;
> }
>
> ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetWorkloadMask,
> 1 << workload_type);
> if (ret) {
> - pr_err("Fail to set workload type %d\n", workload_type);
> + pr_err_once("Fail to set workload type %d\n", workload_type);
> return ret;
> }
>
> --
> 2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
Signed-off-by: Prike Liang
---
drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 784903a3..f9a1817 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
smu_context *smu, long *input, u
/* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
if (workload_type < 0) {
- pr_err("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
+ /*
+* TODO: If some case need switch to powersave/default power
mode
+* then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
power saving.
+*/
+ pr_err_once("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
return -EINVAL;
}
ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
1 << workload_type);
if (ret) {
- pr_err("Fail to set workload type %d\n", workload_type);
+ pr_err_once("Fail to set workload type %d\n", workload_type);
return ret;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
>>> I would like to check why we need a special sequences for sriov on this
>>> pre_reset. If possible, make it the same as bare metal mode sounds better
>>> solution.
Because before VF FLR calling function would lead to register access through
KIQ, which will not complete because KIQ/GFX already hang by that time
>>> I don't remember any register access by amdkfd_pre_reset call, please let
>>> me know if this assumption is wrong .
Please check "void pm_uninit(struct packet_manager *pm)" which is invoked
inside of amdkfd_pre_reset() :
It will call uninitialized() in kfd_kernel_queue.c file
And then go to the path of "kq->mqd_mgr->destroy_mqd(...)"
And finally it calls "static int kgd_hqd_destroy(...)" in
amdgpu_amdkfd_gfx_v10.c
539 {
540 struct amdgpu_device *adev = get_amdgpu_device(kgd);
541 enum hqd_dequeue_request_type type;
542 unsigned long end_jiffies;
543 uint32_t temp;
544 struct v10_compute_mqd *m = get_mqd(mqd);
545
546 #if 0
547 unsigned long flags;
548 int retry;
549 #endif
550
551 acquire_queue(kgd, pipe_id, queue_id); //this introduce register access
via KIQ
552
553 if (m->cp_hqd_vmid == 0)
554 WREG32_FIELD15(GC, 0, RLC_CP_SCHEDULERS, scheduler1, 0); //this
introduce register access via KIQ
555
556 switch (reset_type) {
557 case KFD_PREEMPT_TYPE_WAVEFRONT_DRAIN:
558 type = DRAIN_PIPE;
559 break;
560 case KFD_PREEMPT_TYPE_WAVEFRONT_RESET:
561 type = RESET_WAVES;
562 break;
563 default:
564 type = DRAIN_PIPE;
565 break;
566 }
624 WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_DEQUEUE_REQUEST), type); //this
introduce register access via KIQ
625
626 end_jiffies = (utimeout * HZ / 1000) + jiffies;
627 while (true) {
628 temp = RREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_ACTIVE)); //this
introduce register access via KIQ
629 if (!(temp & CP_HQD_ACTIVE__ACTIVE_MASK))
630 break;
631 if (time_after(jiffies, end_jiffies)) {
632 pr_err("cp queue preemption time out.\n");
633 release_queue(kgd);
634 return -ETIME;
635 }
636 usleep_range(500, 1000);
637 }
638
639 release_queue(kgd);
640 return 0;
If we use the sequence from bare-metal, all above highlighted register access
will not work because KIQ/GFX already died by that time which means the
amdkfd_pre_reset() is actually not working as expected.
_
Monk Liu|GPU Virtualization Team |AMD
[sig-cloud-gpu]
From: Liu, Shaoyun
Sent: Thursday, December 19, 2019 12:30 PM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
I don't remember any register access by amdkfd_pre_reset call, please let me
know if this assumption is wrong .
This function will use hiq to access CP, in case CP already hang, we might not
able to get the response from hw and will got a timeout. I think kfd internal
should handle this. Felix already have some comments on that.
I would like to check why we need a special sequences for sriov on this
pre_reset. If possible, make it the same as bare metal mode sounds better
solution.
Regards
Shaoyun.liu
From: Liu, Monk mailto:monk@amd.com>>
Sent: December 18, 2019 10:52:47 PM
To: Liu, Shaoyun mailto:shaoyun@amd.com>>;
amd-gfx@lists.freedesktop.org
mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
Oh, by the way
>>> Do we know the root cause why this function would ruin MEC ?
Only we call this function right after VF FLR will ruin MEC and lead to
following KIQ ring test fail , and on bare-metal it is called before gpu rest ,
so that's why on bare-metal we don't have this issue
But the reason we cannot call it before VF FLR on SRIOV case was already stated
in this thread
Thanks
_
Monk Liu|GPU Virtualization Team |AMD
-Original Message-
From: Liu, Monk
Sent: Thursday, December 19, 2019 11:49 AM
To: shaoyunl mailto:shaoyun@amd.com>>;
amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
Hi Shaoyun
>>> Do we know the root cause why this function would ruin MEC ? From the
>>> logic, I think this function should be called before FLR since we need to
>>> disable the user queue submission first.
Right now I don't know which detail step lead to KIQ ring test fail, I totally
agree with you that this func should be called before VF FLR, but we cannot do
it and the reason is described in The comment:
> if we do pre_reset() before VF FLR, it would go KIQ way to do register
> access and stuck there, because KIQ probably won't work by that time
> (e.g. you already made GFX hang)
>>> I remembered the function shou
[PATCH] drm/amdgpu/vcn2.5: Silence a compiler warning
Set r to 0 as a default value.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 3271de38cb08..4ea8e20ed15d 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -250,7 +250,7 @@ static int vcn_v2_5_hw_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct amdgpu_ring *ring;
- int i, j, r;
+ int i, j, r = 0;
if (amdgpu_sriov_vf(adev))
r = vcn_v2_5_sriov_start(adev);
--
2.24.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for HDCP
On Wed, Dec 18, 2019 at 6:07 PM Dave Airlie wrote:
>
> Hey,
>
> I've pulled in these two patches to drm-next directly because my arm
> test build was broken.
Sounds good.
Alex
>
> Dave.
>
> On Wed, 18 Dec 2019 at 06:47, Alex Deucher wrote:
> >
> > ARM has a 2000us limit for udelay. Switch to msleep. This code
> > executes in a worker thread so shouldn't be an atomic context.
> >
> > Signed-off-by: Alex Deucher
> > ---
> > drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> > b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> > index bcbc0b8a9aa0..f730b94ac3c0 100644
> > --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> > +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> > @@ -153,7 +153,7 @@ static enum mod_hdcp_status
> > poll_l_prime_available(struct mod_hdcp *hdcp)
> > {
> > enum mod_hdcp_status status;
> > uint8_t size;
> > - uint16_t max_wait = 2; // units of us
> > + uint16_t max_wait = 20; // units of ms
> > uint16_t num_polls = 5;
> > uint16_t wait_time = max_wait / num_polls;
> >
> > @@ -161,7 +161,7 @@ static enum mod_hdcp_status
> > poll_l_prime_available(struct mod_hdcp *hdcp)
> > status = MOD_HDCP_STATUS_INVALID_OPERATION;
> > else
> > for (; num_polls; num_polls--) {
> > - udelay(wait_time);
> > + msleep(wait_time);
> >
> > status = mod_hdcp_read_rxstatus(hdcp);
> > if (status != MOD_HDCP_STATUS_SUCCESS)
> > @@ -474,7 +474,7 @@ static enum mod_hdcp_status locality_check(struct
> > mod_hdcp *hdcp,
> > hdcp, "lc_init_write"))
> > goto out;
> > if (is_dp_hdcp(hdcp))
> > - udelay(16000);
> > + msleep(16);
> > else
> > if (!mod_hdcp_execute_and_set(poll_l_prime_available,
> > &input->l_prime_available_poll, &status,
> > --
> > 2.23.0
> >
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Re: [PATCH 2/3] gpu: drm: dead code elimination
On 2019-12-18 5:52 p.m., Michel Dänzer wrote: >> this set adds support for removal of gpu drm dead code. >> >> patch2: >> `num_entries` is a data of unsigned type(compilers always treat as >> unsigned int) and SIZE_MAX == ~0, >> >> so there is a impossible condition: >> '(num_entries > ((~0) - 56) / 72) => (max(0-u32) > 256204778801521549)'. >While this looks correct for 64-bit, where size_t is unsigned long, on 32-bit >it's unsigned int, in which case this isn't dead code. and On 2019-12-18 5:52 p.m., Christian König wrote: >NAK, that calculation is not correct on 32-bit systems. >Christian. Yes, you are right. I just figured this out (because our Server usually uses 64 bits). thanks. >>Signed-off-by: Pan Zhang >>--- >> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 4 >> 1 file changed, 4 deletions(-) >>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c >>b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c >>index 85b0515..10a7f30 100644 >>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c >>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c >>@@ -71,10 +71,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, >>struct drm_file *filp, >> unsigned i; >> int r; >> >>- if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) >>- / sizeof(struct amdgpu_bo_list_entry)) >>- return -EINVAL; >>- >> size = sizeof(struct amdgpu_bo_list); >> size += num_entries * sizeof(struct amdgpu_bo_list_entry); >> list = kvmalloc(size, GFP_KERNEL); >> ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
I don't remember any register access by amdkfd_pre_reset call, please let me know if this assumption is wrong . This function will use hiq to access CP, in case CP already hang, we might not able to get the response from hw and will got a timeout. I think kfd internal should handle this. Felix already have some comments on that. I would like to check why we need a special sequences for sriov on this pre_reset. If possible, make it the same as bare metal mode sounds better solution. Regards Shaoyun.liu From: Liu, Monk Sent: December 18, 2019 10:52:47 PM To: Liu, Shaoyun ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV Oh, by the way >>> Do we know the root cause why this function would ruin MEC ? Only we call this function right after VF FLR will ruin MEC and lead to following KIQ ring test fail , and on bare-metal it is called before gpu rest , so that's why on bare-metal we don't have this issue But the reason we cannot call it before VF FLR on SRIOV case was already stated in this thread Thanks _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: Liu, Monk Sent: Thursday, December 19, 2019 11:49 AM To: shaoyunl ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV Hi Shaoyun >>> Do we know the root cause why this function would ruin MEC ? From the >>> logic, I think this function should be called before FLR since we need to >>> disable the user queue submission first. Right now I don't know which detail step lead to KIQ ring test fail, I totally agree with you that this func should be called before VF FLR, but we cannot do it and the reason is described in The comment: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) >>> I remembered the function should use hiq to communicate with HW , shouldn't >>> use kiq to access HW registerm, has this been changed ? Tis function use WREG32/RREG32 to do register access, like all other functions in KMD, and WREG32/RREG32 will let KIQ to do the register access If we are under dynamic SRIOV mode (means we are SRIOV VF and isn't under full exclusive mode) You see that if you call this func before EVENT_5 (event 5 triggers VF FLR) then it will run under dynamic mode and KIQ will handle the register access, which is not an option Since ME/MEC probably already hang ( if we are testing quark on gfx/compute rings) Do you have a good suggestion ? thanks _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: amd-gfx On Behalf Of shaoyunl Sent: Tuesday, December 17, 2019 11:38 PM To: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV I think amdkfd side depends on this call to stop the user queue, without this call, the user queue can submit to HW during the reset which could cause hang again ... Do we know the root cause why this function would ruin MEC ? From the logic, I think this function should be called before FLR since we need to disable the user queue submission first. I remembered the function should use hiq to communicate with HW , shouldn't use kiq to access HW registerm, has this been changed ? Regards shaoyun.liu On 2019-12-17 5:19 a.m., Monk Liu wrote: > issues: > MEC is ruined by the amdkfd_pre_reset after VF FLR done > > fix: > amdkfd_pre_reset() would ruin MEC after hypervisor finished the VF > FLR, the correct sequence is do amdkfd_pre_reset before VF FLR but > there is a limitation to block this sequence: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) > > so the best way right now is to simply remove it. > > Signed-off-by: Monk Liu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 605cef6..ae962b9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3672,8 +3672,6 @@ static int amdgpu_device_reset_sriov(struct > amdgpu_device *adev, >if (r) >return r; > > - amdgpu_amdkfd_pre_reset(adev); > - >/* Resume IP prior to SMC */ >r = amdgpu_device_ip_reinit_early_sriov(adev); >if (r) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmonk.liu%40amd.com%7Cee9c811452634fc2739808d7830718f6%7C3dd8961fe48
RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
Oh, by the way >>> Do we know the root cause why this function would ruin MEC ? Only we call this function right after VF FLR will ruin MEC and lead to following KIQ ring test fail , and on bare-metal it is called before gpu rest , so that's why on bare-metal we don't have this issue But the reason we cannot call it before VF FLR on SRIOV case was already stated in this thread Thanks _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: Liu, Monk Sent: Thursday, December 19, 2019 11:49 AM To: shaoyunl ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV Hi Shaoyun >>> Do we know the root cause why this function would ruin MEC ? From the >>> logic, I think this function should be called before FLR since we need to >>> disable the user queue submission first. Right now I don't know which detail step lead to KIQ ring test fail, I totally agree with you that this func should be called before VF FLR, but we cannot do it and the reason is described in The comment: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) >>> I remembered the function should use hiq to communicate with HW , shouldn't >>> use kiq to access HW registerm, has this been changed ? Tis function use WREG32/RREG32 to do register access, like all other functions in KMD, and WREG32/RREG32 will let KIQ to do the register access If we are under dynamic SRIOV mode (means we are SRIOV VF and isn't under full exclusive mode) You see that if you call this func before EVENT_5 (event 5 triggers VF FLR) then it will run under dynamic mode and KIQ will handle the register access, which is not an option Since ME/MEC probably already hang ( if we are testing quark on gfx/compute rings) Do you have a good suggestion ? thanks _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: amd-gfx On Behalf Of shaoyunl Sent: Tuesday, December 17, 2019 11:38 PM To: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV I think amdkfd side depends on this call to stop the user queue, without this call, the user queue can submit to HW during the reset which could cause hang again ... Do we know the root cause why this function would ruin MEC ? From the logic, I think this function should be called before FLR since we need to disable the user queue submission first. I remembered the function should use hiq to communicate with HW , shouldn't use kiq to access HW registerm, has this been changed ? Regards shaoyun.liu On 2019-12-17 5:19 a.m., Monk Liu wrote: > issues: > MEC is ruined by the amdkfd_pre_reset after VF FLR done > > fix: > amdkfd_pre_reset() would ruin MEC after hypervisor finished the VF > FLR, the correct sequence is do amdkfd_pre_reset before VF FLR but > there is a limitation to block this sequence: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) > > so the best way right now is to simply remove it. > > Signed-off-by: Monk Liu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 605cef6..ae962b9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3672,8 +3672,6 @@ static int amdgpu_device_reset_sriov(struct > amdgpu_device *adev, > if (r) > return r; > > - amdgpu_amdkfd_pre_reset(adev); > - > /* Resume IP prior to SMC */ > r = amdgpu_device_ip_reinit_early_sriov(adev); > if (r) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmonk.liu%40amd.com%7Cee9c811452634fc2739808d7830718f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637121938885721447&sdata=FiqkgiUX8k5rD%2F%2FiJQU2cF1MGExO8yXEzYOoBtpdfYU%3D&reserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV
Hi Shaoyun >>> Do we know the root cause why this function would ruin MEC ? From the >>> logic, I think this function should be called before FLR since we need to >>> disable the user queue submission first. Right now I don't know which detail step lead to KIQ ring test fail, I totally agree with you that this func should be called before VF FLR, but we cannot do it and the reason is described in The comment: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) >>> I remembered the function should use hiq to communicate with HW , shouldn't >>> use kiq to access HW registerm, has this been changed ? Tis function use WREG32/RREG32 to do register access, like all other functions in KMD, and WREG32/RREG32 will let KIQ to do the register access If we are under dynamic SRIOV mode (means we are SRIOV VF and isn't under full exclusive mode) You see that if you call this func before EVENT_5 (event 5 triggers VF FLR) then it will run under dynamic mode and KIQ will handle the register access, which is not an option Since ME/MEC probably already hang ( if we are testing quark on gfx/compute rings) Do you have a good suggestion ? thanks _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: amd-gfx On Behalf Of shaoyunl Sent: Tuesday, December 17, 2019 11:38 PM To: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: fix KIQ ring test fail in TDR of SRIOV I think amdkfd side depends on this call to stop the user queue, without this call, the user queue can submit to HW during the reset which could cause hang again ... Do we know the root cause why this function would ruin MEC ? From the logic, I think this function should be called before FLR since we need to disable the user queue submission first. I remembered the function should use hiq to communicate with HW , shouldn't use kiq to access HW registerm, has this been changed ? Regards shaoyun.liu On 2019-12-17 5:19 a.m., Monk Liu wrote: > issues: > MEC is ruined by the amdkfd_pre_reset after VF FLR done > > fix: > amdkfd_pre_reset() would ruin MEC after hypervisor finished the VF > FLR, the correct sequence is do amdkfd_pre_reset before VF FLR but > there is a limitation to block this sequence: > if we do pre_reset() before VF FLR, it would go KIQ way to do register > access and stuck there, because KIQ probably won't work by that time > (e.g. you already made GFX hang) > > so the best way right now is to simply remove it. > > Signed-off-by: Monk Liu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 605cef6..ae962b9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3672,8 +3672,6 @@ static int amdgpu_device_reset_sriov(struct > amdgpu_device *adev, > if (r) > return r; > > - amdgpu_amdkfd_pre_reset(adev); > - > /* Resume IP prior to SMC */ > r = amdgpu_device_ip_reinit_early_sriov(adev); > if (r) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmonk.liu%40amd.com%7Cee9c811452634fc2739808d7830718f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637121938885721447&sdata=FiqkgiUX8k5rD%2F%2FiJQU2cF1MGExO8yXEzYOoBtpdfYU%3D&reserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
On Wed, Dec 18, 2019 at 9:12 PM Quan, Evan wrote:
>
> Hi Alex,
>
> "Power saving" means the regular suspend/resume case, right? That was
> considered.
I mean BACO for runtime power management support. I landed the code
to enable BACO for saving power at runtime when the GPU is not in use.
It's still disabled by default until we properly handle KFD support,
but you can enable it with amdgpu.runpm=1.
> With current amdgpu code, the MP1 state was not correctly set for the regular
> suspend case.
> More straightforwardly I believe PrepareMP1_for_unload should be issued to
> MP1 on regular suspend path(excluding gpu reset case).
>
> And with the MP1 state correctly set for all case, we can remove the
> "adev->in_gpu_reset".
> But for now, I do not want to involve too many changes and limit this to the
> gpu reset case.
>
> P.S. the mp1 state was correctly handled for mode1 reset. So, it's safe to
> enforce this for all gpu reset case instead of baco reset only.
Ah, good to hear.
Alex
>
> Regards,
> Evan
> > -Original Message-
> > From: Alex Deucher
> > Sent: Wednesday, December 18, 2019 10:56 PM
> > To: Quan, Evan
> > Cc: amd-gfx list
> > Subject: Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS
> > baco reset
> >
> > On Tue, Dec 17, 2019 at 10:25 PM Evan Quan wrote:
> > >
> > > For non-RAS baco reset, there is no need to reset the SMC. Thus the
> > > firmware reloading should be avoided.
> > >
> > > Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> > > Signed-off-by: Evan Quan
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> > > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++
> > > 2 files changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > index c14f2ccd0677..9bf7e92394f5 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> > > continue;
> > >
> > > if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> > > - (psp_smu_reload_quirk(psp) ||
> > > psp->autoload_supported))
> > > + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> > > + || psp->autoload_supported))
> >
> > Will this cover the power saving case as well? Do we need to check
> > adev->in_gpu_reset as well or can we drop that part?
> >
> > Alex
> >
> > > continue;
> > >
> > > if (amdgpu_sriov_vf(adev) && diff --git
> > > a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > index c66ca8cc2ebd..ba761e9366e3 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct
> > psp_context *psp,
> > > return true;
> > > }
> > >
> > > +/*
> > > + * Check whether SMU is still alive. If that's true
> > > + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> > > + */
> > > +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp) {
> > > + struct amdgpu_device *adev = psp->adev;
> > > + uint32_t reg;
> > > +
> > > + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b0);
> > > + return (reg &
> > MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ?
> > > +true : false; }
> > > +
> > > static int psp_v11_0_mode1_reset(struct psp_context *psp) {
> > > int ret;
> > > @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> > > .ring_stop = psp_v11_0_ring_stop,
> > > .ring_destroy = psp_v11_0_ring_destroy,
> > > .compare_sram_data = psp_v11_0_compare_sram_data,
> > > + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> > > .mode1_reset = psp_v11_0_mode1_reset,
> > > .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> > > .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> > > --
> > > 2.24.0
> > >
> > > ___
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> > gfx&data=02%7C01%7Cev
> > >
> > an.quan%40amd.com%7C8781ad2ef92d4a188c3008d783ca6846%7C3dd8961fe
> > 4884e6
> > >
> > 08e11a82d994e183d%7C0%7C0%7C637122777663939524&sdata=DMLV%
> > 2Bz%2FsG
> > > nXhpsiOdv9EZrsBcn6HGJ3L7lKdKL2PaPQ%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: simplify padding calculations (v3)
On 2019-12-18 9:59 p.m., Deucher, Alexander wrote:
>> -Original Message-
>> From: Tuikov, Luben
>> Sent: Wednesday, December 18, 2019 6:17 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander ; Koenig, Christian
>>
>> Subject: Re: [PATCH] drm/amdgpu: simplify padding calculations (v3)
>>
>> This patch seems to have been dropped after posting and reposting. I'm not
>> sure why it keeps being dropped.
>
> Dropped from what? Once it's been reviewed, add the RBs and go ahead and
> push it to amd-staging-drm-next.
Oh, thanks!
I seem to forget about this push to refs/for/amd-staging-drm-next.
I'll write it on a post-it note so I can remember.
Okay, so then that's what I need to do.
Regards,
Luben
>
> Alex
>
>>
>> In v3, I added an explanation in the commit text below the original commit
>> text of the single sentence of "Simplify padding calculations." Identical
>> diffstat as v2.
>>
>> Regards,
>> Luben
>>
>> On 2019-12-18 6:12 p.m., Luben Tuikov wrote:
>>> Simplify padding calculations.
>>>
>>> 1. Taking the remainder of a binary value when the divisor is a power
>>> of two, such as, a % 2^n is identical to, a & (2^n - 1) in base-2
>>> arithmetic, and so expressions like this:
>>>
>>> (12 - (lower_32_bits(ring->wptr) & 7)) % 8
>>>
>>> are superflous. And can be reduced to a single remainder-taking
>>> operation.
>>>
>>> 2. Subtracting the remainder modulo 8 out of 12 and then again taking
>>> the remainder modulo 8, yields the same result as simply subtracting
>>> the value out of 4 and then taking the remainder.
>>> This is true because 4 belongs to the congruence
>>> (residue) class {4 + 8*k}, k in Z. (So using, {..., -12, -4, 4, 12,
>>> 20, ...}, would yield the same final result.
>>>
>>> So the above expression, becomes,
>>>
>>> (4 - lower_32_bits(ring->wptr)) & 7
>>>
>>> 3. Now since 8 is part of the conguence class
>>> {0 + 8*k}, k in Z, and from 1) and 2) above, then,
>>>
>>> (8 - (ib->length_dw & 0x7)) % 8
>>>
>>> becomes, simply,
>>>
>>> (-ib->length_dw) & 7.
>>>
>>> This patch implements all of the above in this code.
>>>
>>> v2: Comment update and spacing.
>>> v3: Add 1, 2, 3, above, in this commit message.
>>>
>>> Signed-off-by: Luben Tuikov
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 ++--
>>> drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 4 ++--
>>> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 4 ++--
>>> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
>>> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 17 -
>>> 5 files changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> index 1f22a8d0f7f3..4274ccf765de 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> @@ -228,7 +228,7 @@ static void cik_sdma_ring_emit_ib(struct
>> amdgpu_ring *ring,
>>> u32 extra_bits = vmid & 0xf;
>>>
>>> /* IB packet must end on a 8 DW boundary */
>>> - cik_sdma_ring_insert_nop(ring, (12 - (lower_32_bits(ring->wptr) &
>> 7)) % 8);
>>> + cik_sdma_ring_insert_nop(ring, (4 - lower_32_bits(ring->wptr)) & 7);
>>>
>>> amdgpu_ring_write(ring,
>> SDMA_PACKET(SDMA_OPCODE_INDIRECT_BUFFER, 0, extra_bits));
>>> amdgpu_ring_write(ring, ib->gpu_addr & 0xffe0); /* base must
>> be
>>> 32 byte aligned */ @@ -811,7 +811,7 @@ static void
>> cik_sdma_ring_pad_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib)
>>> u32 pad_count;
>>> int i;
>>>
>>> - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
>>> + pad_count = (-ib->length_dw) & 7;
>>> for (i = 0; i < pad_count; i++)
>>> if (sdma && sdma->burst_nop && (i == 0))
>>> ib->ptr[ib->length_dw++] =
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> index 606b621145a1..fd7fa6082563 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> @@ -255,7 +255,7 @@ static void sdma_v2_4_ring_emit_ib(struct
>> amdgpu_ring *ring,
>>> unsigned vmid = AMDGPU_JOB_GET_VMID(job);
>>>
>>> /* IB packet must end on a 8 DW boundary */
>>> - sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) &
>> 7)) % 8);
>>> + sdma_v2_4_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) &
>>> +7);
>>>
>>> amdgpu_ring_write(ring,
>> SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
>>> SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
>> @@ -750,7 +750,7 @@
>>> static void sdma_v2_4_ring_pad_ib(struct amdgpu_ring *ring, struct
>> amdgpu_ib *ib
>>> u32 pad_count;
>>> int i;
>>>
>>> - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
>>> + pad_count = (-ib->length_dw) & 7;
>>> for (i = 0; i < pad_count; i++)
>>> if (sdma && sdma->burst_nop && (i == 0))
>>> ib->ptr[ib->length_dw++] =
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>>>
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V3)
On 2019-12-18 9:44 p.m., Tianci Yin wrote:
> From: "Tianci.Yin"
>
> The method of getting fb_loc changed from parsing VBIOS to
> taking certain offset from top of VRAM
>
> Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
> Signed-off-by: Tianci.Yin
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
> .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 38 ++-
> .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 13 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
> drivers/gpu/drm/amd/include/atomfirmware.h| 14 ---
> 7 files changed, 26 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a78a363b1d71..fa2cf8e7bc07 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
> struct amdgpu_bo *reserved_bo;
> void *va;
>
> - /* Offset on the top of VRAM, used as c2p write buffer.
> + /* GDDR6 training support flag.
> */
> - u64 mem_train_fb_loc;
> bool mem_train_support;
> };
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index 9ba80d828876..fdd52d86a4d7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
> if (adev->is_atom_fw) {
> amdgpu_atomfirmware_scratch_regs_init(adev);
> amdgpu_atomfirmware_allocate_fb_scratch(adev);
> - ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);
> + ret = amdgpu_atomfirmware_get_mem_train_info(adev);
> if (ret) {
> DRM_ERROR("Failed to get mem train fb location.\n");
> return ret;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> index ff4eb96bdfb5..58f9d8c3a17a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> @@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device
> *adev)
> return ret;
> }
>
> -int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
> +int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)
> {
> struct atom_context *ctx = adev->mode_info.atom_context;
> - unsigned char *bios = ctx->bios;
> - struct vram_reserve_block *reserved_block;
> - int index, block_number;
> + int index;
> uint8_t frev, crev;
> uint16_t data_offset, size;
> - uint32_t start_address_in_kb;
> - uint64_t offset;
> int ret;
>
> adev->fw_vram_usage.mem_train_support = false;
> @@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
> amdgpu_device *adev)
> return -EINVAL;
> }
>
> - reserved_block = (struct vram_reserve_block *)
> - (bios + data_offset + sizeof(struct atom_common_table_header));
> - block_number = ((unsigned int)size - sizeof(struct
> atom_common_table_header))
> - / sizeof(struct vram_reserve_block);
> - reserved_block += (block_number > 0) ? block_number-1 : 0;
> - DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
> drv.\n",
> - block_number,
> - le32_to_cpu(reserved_block->start_address_in_kb),
> - le16_to_cpu(reserved_block->used_by_firmware_in_kb),
> - le16_to_cpu(reserved_block->used_by_driver_in_kb));
> - if (reserved_block->used_by_firmware_in_kb > 0) {
> - start_address_in_kb =
> le32_to_cpu(reserved_block->start_address_in_kb);
> - offset = (uint64_t)start_address_in_kb * ONE_KiB;
> - if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
> - offset -= ONE_MiB;
> - }
> -
> - offset &= ~(ONE_MiB - 1);
> - adev->fw_vram_usage.mem_train_fb_loc = offset;
> - adev->fw_vram_usage.mem_train_support = true;
> - DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);
> - ret = 0;
> - } else {
> - DRM_ERROR("used_by_firmware_in_kb is 0!\n");
> - ret = -EINVAL;
> - }
> -
> - return ret;
> + adev->fw_vram_usage.mem_train_support = true;
> + return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
> index f871af5ea6f3..434fe2fa0089 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
> @@ -31,7 +31,7 @@ void amdgpu_atomfirmware_scratch_regs_init(struct
> amdg
Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
Thanks Xiaojie!
From: Yuan, Xiaojie
Sent: Thursday, December 19, 2019 11:02
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Long,
Gang ; Wang, Kevin(Yang)
Subject: Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Xiaojie Yuan
BR,
Xiaojie
From: Tianci Yin
Sent: Thursday, December 19, 2019 10:44 AM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben; Koenig, Christian; Deucher, Alexander; Zhang, Hawking; Xu,
Feifei; Yuan, Xiaojie; Long, Gang; Wang, Kevin(Yang); Yin, Tianci (Rico)
Subject: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Reviewed-by: Kevin Wang
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ec84acdd43a2..60f17e989014 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1725,17 +1722,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1744,15 +1730,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Xiaojie Yuan
BR,
Xiaojie
From: Tianci Yin
Sent: Thursday, December 19, 2019 10:44 AM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben; Koenig, Christian; Deucher, Alexander; Zhang, Hawking; Xu,
Feifei; Yuan, Xiaojie; Long, Gang; Wang, Kevin(Yang); Yin, Tianci (Rico)
Subject: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Reviewed-by: Kevin Wang
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ec84acdd43a2..60f17e989014 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1725,17 +1722,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1744,15 +1730,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Reviewed-by: Kevin Wang
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ec84acdd43a2..60f17e989014 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1725,17 +1722,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1744,15 +1730,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: simplify padding calculations (v3)
> -Original Message-
> From: Tuikov, Luben
> Sent: Wednesday, December 18, 2019 6:17 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
>
> Subject: Re: [PATCH] drm/amdgpu: simplify padding calculations (v3)
>
> This patch seems to have been dropped after posting and reposting. I'm not
> sure why it keeps being dropped.
Dropped from what? Once it's been reviewed, add the RBs and go ahead and push
it to amd-staging-drm-next.
Alex
>
> In v3, I added an explanation in the commit text below the original commit
> text of the single sentence of "Simplify padding calculations." Identical
> diffstat as v2.
>
> Regards,
> Luben
>
> On 2019-12-18 6:12 p.m., Luben Tuikov wrote:
> > Simplify padding calculations.
> >
> > 1. Taking the remainder of a binary value when the divisor is a power
> > of two, such as, a % 2^n is identical to, a & (2^n - 1) in base-2
> > arithmetic, and so expressions like this:
> >
> > (12 - (lower_32_bits(ring->wptr) & 7)) % 8
> >
> > are superflous. And can be reduced to a single remainder-taking
> > operation.
> >
> > 2. Subtracting the remainder modulo 8 out of 12 and then again taking
> > the remainder modulo 8, yields the same result as simply subtracting
> > the value out of 4 and then taking the remainder.
> > This is true because 4 belongs to the congruence
> > (residue) class {4 + 8*k}, k in Z. (So using, {..., -12, -4, 4, 12,
> > 20, ...}, would yield the same final result.
> >
> > So the above expression, becomes,
> >
> > (4 - lower_32_bits(ring->wptr)) & 7
> >
> > 3. Now since 8 is part of the conguence class
> > {0 + 8*k}, k in Z, and from 1) and 2) above, then,
> >
> > (8 - (ib->length_dw & 0x7)) % 8
> >
> > becomes, simply,
> >
> > (-ib->length_dw) & 7.
> >
> > This patch implements all of the above in this code.
> >
> > v2: Comment update and spacing.
> > v3: Add 1, 2, 3, above, in this commit message.
> >
> > Signed-off-by: Luben Tuikov
> > ---
> > drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 ++--
> > drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 4 ++--
> > drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 4 ++--
> > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
> > drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 17 -
> > 5 files changed, 20 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> > b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> > index 1f22a8d0f7f3..4274ccf765de 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> > @@ -228,7 +228,7 @@ static void cik_sdma_ring_emit_ib(struct
> amdgpu_ring *ring,
> > u32 extra_bits = vmid & 0xf;
> >
> > /* IB packet must end on a 8 DW boundary */
> > - cik_sdma_ring_insert_nop(ring, (12 - (lower_32_bits(ring->wptr) &
> 7)) % 8);
> > + cik_sdma_ring_insert_nop(ring, (4 - lower_32_bits(ring->wptr)) & 7);
> >
> > amdgpu_ring_write(ring,
> SDMA_PACKET(SDMA_OPCODE_INDIRECT_BUFFER, 0, extra_bits));
> > amdgpu_ring_write(ring, ib->gpu_addr & 0xffe0); /* base must
> be
> > 32 byte aligned */ @@ -811,7 +811,7 @@ static void
> cik_sdma_ring_pad_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib)
> > u32 pad_count;
> > int i;
> >
> > - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
> > + pad_count = (-ib->length_dw) & 7;
> > for (i = 0; i < pad_count; i++)
> > if (sdma && sdma->burst_nop && (i == 0))
> > ib->ptr[ib->length_dw++] =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> > index 606b621145a1..fd7fa6082563 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> > @@ -255,7 +255,7 @@ static void sdma_v2_4_ring_emit_ib(struct
> amdgpu_ring *ring,
> > unsigned vmid = AMDGPU_JOB_GET_VMID(job);
> >
> > /* IB packet must end on a 8 DW boundary */
> > - sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) &
> 7)) % 8);
> > + sdma_v2_4_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) &
> > +7);
> >
> > amdgpu_ring_write(ring,
> SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
> > SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
> @@ -750,7 +750,7 @@
> > static void sdma_v2_4_ring_pad_ib(struct amdgpu_ring *ring, struct
> amdgpu_ib *ib
> > u32 pad_count;
> > int i;
> >
> > - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
> > + pad_count = (-ib->length_dw) & 7;
> > for (i = 0; i < pad_count; i++)
> > if (sdma && sdma->burst_nop && (i == 0))
> > ib->ptr[ib->length_dw++] =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > index a559573ec8fd..4a8a7f0f3a9c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > @@ -429,7 +429,7 @@ static void sdma_v3_0_ring_emit_ib(struct
> amdgpu_ring *ring,
> > unsigned vmid = AMDGPU_JOB_GET_VMID(job);
> >
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V2)
[AMD Official Use Only - Internal Distribution Only]
Hi Luben,
Thank you very much for your suggestion and detailed explanation!
Patch has been refined, please help review.
Rico
From: Tuikov, Luben
Sent: Thursday, December 19, 2019 4:14
To: Yin, Tianci (Rico) ; Wang, Kevin(Yang)
; amd-gfx@lists.freedesktop.org
Cc: Koenig, Christian ; Deucher, Alexander
; Zhang, Hawking ; Xu, Feifei
; Yuan, Xiaojie ; Long, Gang
Subject: Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training(V2)
On 2019-12-18 4:14 a.m., Yin, Tianci (Rico) wrote:
> Hi Kevin,
>
> You mean like this? It's a bit lengthy.
> - ctx->c2p_train_data_offset &= ~(ONE_MiB - 1);
> + ctx->c2p_train_data_offset = ALIGN(ctx->c2p_train_data_offset, ONE_MiB);
>
> - ctx->c2p_train_data_offset = adev->fw_vram_usage.mem_train_fb_loc;
> + ctx->c2p_train_data_offset = adev->gmc.mc_vram_size;
> + if ((ctx->c2p_train_data_offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1)
> ) {
> + ctx->c2p_train_data_offset -= ONE_MiB;
> + }
> + ctx->c2p_train_data_offset &= ~(ONE_MiB - 1);
Using the macro ALIGN() is a good practice.
Usually when calculating a quantity, such as the one above, you'd
use a working variable, say 'a', and after you're done with
the calculation, you'd then assign it to the variable which
needs it. Something like this:
a = adev->gmc.mc_vram_size;
if ((a & (ONE_MiB - 1)) < (4 * ONE_KiB + 1))
a -= ONE_MiB;
ctx->c2p_train_data_offset = ALIGN(a, ONE_MiB);
The easiest way to see this is, imagine, if all this calculation
was offloaded to a dedicated function, f(), to do:
ctx->c2p_train_data_offset = f(adev->gmc.mc_vram_size);
Now, by using the working variable 'a', you've shown this
abstraction just the same. (By using the working variable 'a',
you've shown to the reader,that this calculation is abstracted,
and could be relocated to a function.)
Regards,
Luben
P.S. The compiler is probably already doing this, and not working
directly on the ctx->c2p_train_data_offs, but assigns a final
result, as explicitly shown above. The above is to make it easy
for humans to read and understand the code. Hope this helps.
>
> *[kevin]:*
> *i'd like to use the marco ALIGN() to simple above code.*
> *anyway, the patch Reviewed-by: Kevin Wang *
>
> ctx->p2c_train_data_offset = (adev->gmc.mc_vram_size -
> GDDR6_MEM_TRAINING_OFFSET);
> ctx->train_data_size = GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index f1ebd424510c..19eb3e8456c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -66,6 +66,13 @@ struct amdgpu_copy_mem {
> unsigned long offset;
> };
>
> +/* Definitions for constance */
> +enum amdgpu_internal_constants
> +{
> + ONE_KiB = 0x400,
> + ONE_MiB = 0x10,
> +};
> +
> extern const struct ttm_mem_type_manager_func amdgpu_gtt_mgr_func;
> extern const struct ttm_mem_type_manager_func amdgpu_vram_mgr_func;
>
> diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h
> b/drivers/gpu/drm/amd/include/atomfirmware.h
> index dd7cbc00a0aa..70146518174c 100644
> --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> @@ -672,20 +672,6 @@ struct vram_usagebyfirmware_v2_1
>uint16_t used_by_driver_in_kb;
> };
>
> -/* This is part of vram_usagebyfirmware_v2_1 */
> -struct vram_reserve_block
> -{
> - uint32_t start_address_in_kb;
> - uint16_t used_by_firmware_in_kb;
> - uint16_t used_by_driver_in_kb;
> -};
> -
> -/* Definitions for constance */
> -enum atomfirmware_internal_constants
> -{
> - ONE_KiB = 0x400,
> - ONE_MiB = 0x10,
> -};
>
> /*
>***
> --
> 2.17.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V3)
From: "Tianci.Yin"
The method of getting fb_loc changed from parsing VBIOS to
taking certain offset from top of VRAM
Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 38 ++-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 13 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
drivers/gpu/drm/amd/include/atomfirmware.h| 14 ---
7 files changed, 26 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a78a363b1d71..fa2cf8e7bc07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
struct amdgpu_bo *reserved_bo;
void *va;
- /* Offset on the top of VRAM, used as c2p write buffer.
+ /* GDDR6 training support flag.
*/
- u64 mem_train_fb_loc;
bool mem_train_support;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 9ba80d828876..fdd52d86a4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
if (adev->is_atom_fw) {
amdgpu_atomfirmware_scratch_regs_init(adev);
amdgpu_atomfirmware_allocate_fb_scratch(adev);
- ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);
+ ret = amdgpu_atomfirmware_get_mem_train_info(adev);
if (ret) {
DRM_ERROR("Failed to get mem train fb location.\n");
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index ff4eb96bdfb5..58f9d8c3a17a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device
*adev)
return ret;
}
-int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
+int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
- unsigned char *bios = ctx->bios;
- struct vram_reserve_block *reserved_block;
- int index, block_number;
+ int index;
uint8_t frev, crev;
uint16_t data_offset, size;
- uint32_t start_address_in_kb;
- uint64_t offset;
int ret;
adev->fw_vram_usage.mem_train_support = false;
@@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
amdgpu_device *adev)
return -EINVAL;
}
- reserved_block = (struct vram_reserve_block *)
- (bios + data_offset + sizeof(struct atom_common_table_header));
- block_number = ((unsigned int)size - sizeof(struct
atom_common_table_header))
- / sizeof(struct vram_reserve_block);
- reserved_block += (block_number > 0) ? block_number-1 : 0;
- DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
drv.\n",
- block_number,
- le32_to_cpu(reserved_block->start_address_in_kb),
- le16_to_cpu(reserved_block->used_by_firmware_in_kb),
- le16_to_cpu(reserved_block->used_by_driver_in_kb));
- if (reserved_block->used_by_firmware_in_kb > 0) {
- start_address_in_kb =
le32_to_cpu(reserved_block->start_address_in_kb);
- offset = (uint64_t)start_address_in_kb * ONE_KiB;
- if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
- offset -= ONE_MiB;
- }
-
- offset &= ~(ONE_MiB - 1);
- adev->fw_vram_usage.mem_train_fb_loc = offset;
- adev->fw_vram_usage.mem_train_support = true;
- DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);
- ret = 0;
- } else {
- DRM_ERROR("used_by_firmware_in_kb is 0!\n");
- ret = -EINVAL;
- }
-
- return ret;
+ adev->fw_vram_usage.mem_train_support = true;
+ return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
index f871af5ea6f3..434fe2fa0089 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
@@ -31,7 +31,7 @@ void amdgpu_atomfirmware_scratch_regs_init(struct
amdgpu_device *adev);
int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev);
int amdgpu_atomfirmware_get_vram_info(struct amdgpu_device *adev,
RE: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
Hi Alex,
"Power saving" means the regular suspend/resume case, right? That was
considered.
With current amdgpu code, the MP1 state was not correctly set for the regular
suspend case.
More straightforwardly I believe PrepareMP1_for_unload should be issued to MP1
on regular suspend path(excluding gpu reset case).
And with the MP1 state correctly set for all case, we can remove the
"adev->in_gpu_reset".
But for now, I do not want to involve too many changes and limit this to the
gpu reset case.
P.S. the mp1 state was correctly handled for mode1 reset. So, it's safe to
enforce this for all gpu reset case instead of baco reset only.
Regards,
Evan
> -Original Message-
> From: Alex Deucher
> Sent: Wednesday, December 18, 2019 10:56 PM
> To: Quan, Evan
> Cc: amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS
> baco reset
>
> On Tue, Dec 17, 2019 at 10:25 PM Evan Quan wrote:
> >
> > For non-RAS baco reset, there is no need to reset the SMC. Thus the
> > firmware reloading should be avoided.
> >
> > Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> > Signed-off-by: Evan Quan
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index c14f2ccd0677..9bf7e92394f5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> > continue;
> >
> > if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> > - (psp_smu_reload_quirk(psp) || psp->autoload_supported))
> > + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> > + || psp->autoload_supported))
>
> Will this cover the power saving case as well? Do we need to check
> adev->in_gpu_reset as well or can we drop that part?
>
> Alex
>
> > continue;
> >
> > if (amdgpu_sriov_vf(adev) && diff --git
> > a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > index c66ca8cc2ebd..ba761e9366e3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct
> psp_context *psp,
> > return true;
> > }
> >
> > +/*
> > + * Check whether SMU is still alive. If that's true
> > + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> > + */
> > +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp) {
> > + struct amdgpu_device *adev = psp->adev;
> > + uint32_t reg;
> > +
> > + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b0);
> > + return (reg &
> MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ?
> > +true : false; }
> > +
> > static int psp_v11_0_mode1_reset(struct psp_context *psp) {
> > int ret;
> > @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> > .ring_stop = psp_v11_0_ring_stop,
> > .ring_destroy = psp_v11_0_ring_destroy,
> > .compare_sram_data = psp_v11_0_compare_sram_data,
> > + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> > .mode1_reset = psp_v11_0_mode1_reset,
> > .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> > .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> > --
> > 2.24.0
> >
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&data=02%7C01%7Cev
> >
> an.quan%40amd.com%7C8781ad2ef92d4a188c3008d783ca6846%7C3dd8961fe
> 4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637122777663939524&sdata=DMLV%
> 2Bz%2FsG
> > nXhpsiOdv9EZrsBcn6HGJ3L7lKdKL2PaPQ%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: correctly report gpu recover status
Hi Christian,
Here is some background for this change:
I'm debugging a random failure issue on baco reset.
I used a while loop to run the continuous baco reset tests and hope it can exit
immediately on failure occurred.
However, due to wrong return value, it did not. And as you can image, the
failure scene was ruined.
I can add this "seq_printf(m, "gpu recover %d\n", r);".
But still what I care more(which is also the easiest way to me) is the correct
return value of the API.
Regards,
Evan
> -Original Message-
> From: Christian König
> Sent: Wednesday, December 18, 2019 5:57 PM
> To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: correctly report gpu recover status
>
> Am 18.12.19 um 04:25 schrieb Evan Quan:
> > Knowing whether gpu recovery was performed successfully or not is
> > important for our BACO development.
> >
> > Change-Id: I0e3ca4dcb65a053eb26bc55ad7431e4a42e160de
> > Signed-off-by: Evan Quan
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > index e9efee04ca23..5dff5c0dd882 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > @@ -743,9 +743,7 @@ static int amdgpu_debugfs_gpu_recover(struct
> seq_file *m, void *data)
> > struct amdgpu_device *adev = dev->dev_private;
> >
> > seq_printf(m, "gpu recover\n");
> > - amdgpu_device_gpu_recover(adev, NULL);
> > -
> > - return 0;
> > + return amdgpu_device_gpu_recover(adev, NULL);
>
> NAK, what we could do here is the following:
>
> r = amdgpu_device_gpu_recover();
> seq_printf(m, "gpu recover %d\n", r);
>
> But returning the error code from the GPU recovery to userspace doesn't make
> to much sense.
>
> Christian.
>
> > }
> >
> > static const struct drm_info_list amdgpu_debugfs_fence_list[] = {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: simplify padding calculations (v3)
This patch seems to have been dropped after posting
and reposting. I'm not sure why it keeps being dropped.
In v3, I added an explanation in the commit text
below the original commit text of the single sentence
of "Simplify padding calculations." Identical diffstat
as v2.
Regards,
Luben
On 2019-12-18 6:12 p.m., Luben Tuikov wrote:
> Simplify padding calculations.
>
> 1. Taking the remainder of a binary value when
> the divisor is a power of two, such as,
> a % 2^n is identical to, a & (2^n - 1) in base-2
> arithmetic, and so expressions like this:
>
> (12 - (lower_32_bits(ring->wptr) & 7)) % 8
>
> are superflous. And can be reduced to a single
> remainder-taking operation.
>
> 2. Subtracting the remainder modulo 8 out of 12
> and then again taking the remainder modulo 8,
> yields the same result as simply subtracting
> the value out of 4 and then taking the remainder.
> This is true because 4 belongs to the congruence
> (residue) class {4 + 8*k}, k in Z. (So using,
> {..., -12, -4, 4, 12, 20, ...}, would yield
> the same final result.
>
> So the above expression, becomes,
>
> (4 - lower_32_bits(ring->wptr)) & 7
>
> 3. Now since 8 is part of the conguence class
> {0 + 8*k}, k in Z, and from 1) and 2) above, then,
>
> (8 - (ib->length_dw & 0x7)) % 8
>
> becomes, simply,
>
> (-ib->length_dw) & 7.
>
> This patch implements all of the above in this code.
>
> v2: Comment update and spacing.
> v3: Add 1, 2, 3, above, in this commit message.
>
> Signed-off-by: Luben Tuikov
> ---
> drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 ++--
> drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 4 ++--
> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 4 ++--
> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 17 -
> 5 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index 1f22a8d0f7f3..4274ccf765de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -228,7 +228,7 @@ static void cik_sdma_ring_emit_ib(struct amdgpu_ring
> *ring,
> u32 extra_bits = vmid & 0xf;
>
> /* IB packet must end on a 8 DW boundary */
> - cik_sdma_ring_insert_nop(ring, (12 - (lower_32_bits(ring->wptr) & 7)) %
> 8);
> + cik_sdma_ring_insert_nop(ring, (4 - lower_32_bits(ring->wptr)) & 7);
>
> amdgpu_ring_write(ring, SDMA_PACKET(SDMA_OPCODE_INDIRECT_BUFFER, 0,
> extra_bits));
> amdgpu_ring_write(ring, ib->gpu_addr & 0xffe0); /* base must be 32
> byte aligned */
> @@ -811,7 +811,7 @@ static void cik_sdma_ring_pad_ib(struct amdgpu_ring
> *ring, struct amdgpu_ib *ib)
> u32 pad_count;
> int i;
>
> - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
> + pad_count = (-ib->length_dw) & 7;
> for (i = 0; i < pad_count; i++)
> if (sdma && sdma->burst_nop && (i == 0))
> ib->ptr[ib->length_dw++] =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index 606b621145a1..fd7fa6082563 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -255,7 +255,7 @@ static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring
> *ring,
> unsigned vmid = AMDGPU_JOB_GET_VMID(job);
>
> /* IB packet must end on a 8 DW boundary */
> - sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7))
> % 8);
> + sdma_v2_4_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) & 7);
>
> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
> SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
> @@ -750,7 +750,7 @@ static void sdma_v2_4_ring_pad_ib(struct amdgpu_ring
> *ring, struct amdgpu_ib *ib
> u32 pad_count;
> int i;
>
> - pad_count = (8 - (ib->length_dw & 0x7)) % 8;
> + pad_count = (-ib->length_dw) & 7;
> for (i = 0; i < pad_count; i++)
> if (sdma && sdma->burst_nop && (i == 0))
> ib->ptr[ib->length_dw++] =
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index a559573ec8fd..4a8a7f0f3a9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -429,7 +429,7 @@ static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring
> *ring,
> unsigned vmid = AMDGPU_JOB_GET_VMID(job);
>
> /* IB packet must end on a 8 DW boundary */
> - sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7))
> % 8);
> + sdma_v3_0_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) & 7);
>
> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
> SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
> @@ -1021,7 +1021,7 @@ static void sdma_v3_0_ring_pad_ib(struct amdgpu_ring
> *ring, struct amdgpu_ib *ib
> u32 pad_count;
[PATCH] drm/amdgpu: simplify padding calculations (v3)
Simplify padding calculations.
1. Taking the remainder of a binary value when
the divisor is a power of two, such as,
a % 2^n is identical to, a & (2^n - 1) in base-2
arithmetic, and so expressions like this:
(12 - (lower_32_bits(ring->wptr) & 7)) % 8
are superflous. And can be reduced to a single
remainder-taking operation.
2. Subtracting the remainder modulo 8 out of 12
and then again taking the remainder modulo 8,
yields the same result as simply subtracting
the value out of 4 and then taking the remainder.
This is true because 4 belongs to the congruence
(residue) class {4 + 8*k}, k in Z. (So using,
{..., -12, -4, 4, 12, 20, ...}, would yield
the same final result.
So the above expression, becomes,
(4 - lower_32_bits(ring->wptr)) & 7
3. Now since 8 is part of the conguence class
{0 + 8*k}, k in Z, and from 1) and 2) above, then,
(8 - (ib->length_dw & 0x7)) % 8
becomes, simply,
(-ib->length_dw) & 7.
This patch implements all of the above in this code.
v2: Comment update and spacing.
v3: Add 1, 2, 3, above, in this commit message.
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 17 -
5 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index 1f22a8d0f7f3..4274ccf765de 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -228,7 +228,7 @@ static void cik_sdma_ring_emit_ib(struct amdgpu_ring *ring,
u32 extra_bits = vmid & 0xf;
/* IB packet must end on a 8 DW boundary */
- cik_sdma_ring_insert_nop(ring, (12 - (lower_32_bits(ring->wptr) & 7)) %
8);
+ cik_sdma_ring_insert_nop(ring, (4 - lower_32_bits(ring->wptr)) & 7);
amdgpu_ring_write(ring, SDMA_PACKET(SDMA_OPCODE_INDIRECT_BUFFER, 0,
extra_bits));
amdgpu_ring_write(ring, ib->gpu_addr & 0xffe0); /* base must be 32
byte aligned */
@@ -811,7 +811,7 @@ static void cik_sdma_ring_pad_ib(struct amdgpu_ring *ring,
struct amdgpu_ib *ib)
u32 pad_count;
int i;
- pad_count = (8 - (ib->length_dw & 0x7)) % 8;
+ pad_count = (-ib->length_dw) & 7;
for (i = 0; i < pad_count; i++)
if (sdma && sdma->burst_nop && (i == 0))
ib->ptr[ib->length_dw++] =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index 606b621145a1..fd7fa6082563 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -255,7 +255,7 @@ static void sdma_v2_4_ring_emit_ib(struct amdgpu_ring *ring,
unsigned vmid = AMDGPU_JOB_GET_VMID(job);
/* IB packet must end on a 8 DW boundary */
- sdma_v2_4_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7))
% 8);
+ sdma_v2_4_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) & 7);
amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
@@ -750,7 +750,7 @@ static void sdma_v2_4_ring_pad_ib(struct amdgpu_ring *ring,
struct amdgpu_ib *ib
u32 pad_count;
int i;
- pad_count = (8 - (ib->length_dw & 0x7)) % 8;
+ pad_count = (-ib->length_dw) & 7;
for (i = 0; i < pad_count; i++)
if (sdma && sdma->burst_nop && (i == 0))
ib->ptr[ib->length_dw++] =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index a559573ec8fd..4a8a7f0f3a9c 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -429,7 +429,7 @@ static void sdma_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
unsigned vmid = AMDGPU_JOB_GET_VMID(job);
/* IB packet must end on a 8 DW boundary */
- sdma_v3_0_ring_insert_nop(ring, (10 - (lower_32_bits(ring->wptr) & 7))
% 8);
+ sdma_v3_0_ring_insert_nop(ring, (2 - lower_32_bits(ring->wptr)) & 7);
amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_INDIRECT) |
SDMA_PKT_INDIRECT_HEADER_VMID(vmid & 0xf));
@@ -1021,7 +1021,7 @@ static void sdma_v3_0_ring_pad_ib(struct amdgpu_ring
*ring, struct amdgpu_ib *ib
u32 pad_count;
int i;
- pad_count = (8 - (ib->length_dw & 0x7)) % 8;
+ pad_count = (-ib->length_dw) & 7;
for (i = 0; i < pad_count; i++)
if (sdma && sdma->burst_nop && (i == 0))
ib->ptr[ib->length_dw++] =
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index bd9ed33bab43..c69df0cb21ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -698,7 +698,7 @@ static void
Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for HDCP
Hey,
I've pulled in these two patches to drm-next directly because my arm
test build was broken.
Dave.
On Wed, 18 Dec 2019 at 06:47, Alex Deucher wrote:
>
> ARM has a 2000us limit for udelay. Switch to msleep. This code
> executes in a worker thread so shouldn't be an atomic context.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> index bcbc0b8a9aa0..f730b94ac3c0 100644
> --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
> @@ -153,7 +153,7 @@ static enum mod_hdcp_status poll_l_prime_available(struct
> mod_hdcp *hdcp)
> {
> enum mod_hdcp_status status;
> uint8_t size;
> - uint16_t max_wait = 2; // units of us
> + uint16_t max_wait = 20; // units of ms
> uint16_t num_polls = 5;
> uint16_t wait_time = max_wait / num_polls;
>
> @@ -161,7 +161,7 @@ static enum mod_hdcp_status poll_l_prime_available(struct
> mod_hdcp *hdcp)
> status = MOD_HDCP_STATUS_INVALID_OPERATION;
> else
> for (; num_polls; num_polls--) {
> - udelay(wait_time);
> + msleep(wait_time);
>
> status = mod_hdcp_read_rxstatus(hdcp);
> if (status != MOD_HDCP_STATUS_SUCCESS)
> @@ -474,7 +474,7 @@ static enum mod_hdcp_status locality_check(struct
> mod_hdcp *hdcp,
> hdcp, "lc_init_write"))
> goto out;
> if (is_dp_hdcp(hdcp))
> - udelay(16000);
> + msleep(16);
> else
> if (!mod_hdcp_execute_and_set(poll_l_prime_available,
> &input->l_prime_available_poll, &status,
> --
> 2.23.0
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: attempt xgmi perfmon re-arm on failed arm
Some nit-picks inline. Looks good otherwise.
On 2019-12-18 2:04 p.m., Jonathan Kim wrote:
The DF routines to arm xGMI performance will attempt to re-arm both on
performance monitoring start and read on initial failure to arm.
v2: Roll back reset_perfmon_cntr to void return since new perfmon
counters are now safe to write to during DF C-States. Do single perfmon
controller re-arm when counter is deferred in pmc_count versus multiple
perfmon controller re-arms that could last 1 millisecond. Avoid holding
spin lock during sleep in perfmon_arm_with_retry. Rename counter arm
defer function to be less confusing.
Signed-off-by: Jonathan Kim
---
drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 149 +++
1 file changed, 127 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 4043ebcea5de..56dead3a9718 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -183,6 +183,61 @@ static void df_v3_6_perfmon_wreg(struct amdgpu_device
*adev, uint32_t lo_addr,
spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
+/* same as perfmon arm but return status on write value check */
+static int df_v3_6_perfmon_arm_with_status(struct amdgpu_device *adev,
+ uint32_t lo_addr, uint32_t lo_val,
+ uint32_t hi_addr, uint32_t hi_val)
+{
+ unsigned long flags, address, data;
+ uint32_t lo_val_rb, hi_val_rb;
+
+ address = adev->nbio.funcs->get_pcie_index_offset(adev);
+ data = adev->nbio.funcs->get_pcie_data_offset(adev);
+
+ spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ WREG32(address, lo_addr);
+ WREG32(data, lo_val);
+ WREG32(address, hi_addr);
+ WREG32(data, hi_val);
+
+ WREG32(address, lo_addr);
+ lo_val_rb = RREG32(data);
+ WREG32(address, hi_addr);
+ hi_val_rb = RREG32(data);
+ spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+
+ if (!(lo_val == lo_val_rb && hi_val == hi_val_rb))
+ return -EBUSY;
+
+ return 0;
+}
+
+
+/*
+ * retry arming counters every 100 usecs within 1 millisecond interval.
+ * if retry fails after time out, return error.
+ */
+#define ARM_RETRY_USEC_TIMEOUT 1000
+#define ARM_RETRY_USEC_INTERVAL100
+static int df_v3_6_perfmon_arm_with_retry(struct amdgpu_device *adev,
+ uint32_t lo_addr, uint32_t lo_val,
+ uint32_t hi_addr, uint32_t hi_val)
+{
+ int countdown = ARM_RETRY_USEC_TIMEOUT;
+
+ while (countdown) {
+
+ if (!df_v3_6_perfmon_arm_with_status(adev, lo_addr, lo_val,
+hi_addr, hi_val))
+ break;
+
+ countdown -= ARM_RETRY_USEC_INTERVAL;
+ udelay(ARM_RETRY_USEC_INTERVAL);
+ }
+
+ return countdown > 0 ? 0 : -ETIME;
+}
+
/* get the number of df counters available */
static ssize_t df_v3_6_get_df_cntr_avail(struct device *dev,
struct device_attribute *attr,
@@ -334,20 +389,20 @@ static void df_v3_6_pmc_get_addr(struct amdgpu_device
*adev,
switch (target_cntr) {
case 0:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo0 : smnPerfMonCtrLo0;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi0 : smnPerfMonCtrHi0;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo4 : smnPerfMonCtrLo4;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi4 : smnPerfMonCtrHi4;
break;
case 1:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo1 : smnPerfMonCtrLo1;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi1 : smnPerfMonCtrHi1;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo5 : smnPerfMonCtrLo5;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi5 : smnPerfMonCtrHi5;
break;
case 2:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo2 : smnPerfMonCtrLo2;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi2 : smnPerfMonCtrHi2;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo6 : smnPerfMonCtrLo6;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi6 : smnPerfMonCtrHi6;
break;
case 3:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo3 : smnPerfMonCtrLo3;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi3 : smnPerfMonCtrHi3;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo7 : smnPerfMonCtrLo7;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi7 : smnPerfMonCtrHi7;
break;
}
@@ -422,6 +477,42 @@ static int df_v3_6_pmc_add_cntr(struct amdgpu_device *adev,
return -ENOSPC;
}
+#define DEFERRED_ARM_MASK (1 << 31)
+static int df_v3_6_pmc_set_deferred(struct amdgpu_device *adev,
+ uint6
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V2)
On 2019-12-18 4:14 a.m., Yin, Tianci (Rico) wrote:
> Hi Kevin,
>
> You mean like this? It's a bit lengthy.
> - ctx->c2p_train_data_offset &= ~(ONE_MiB - 1);
> + ctx->c2p_train_data_offset = ALIGN(ctx->c2p_train_data_offset, ONE_MiB);
>
> - ctx->c2p_train_data_offset = adev->fw_vram_usage.mem_train_fb_loc;
> + ctx->c2p_train_data_offset = adev->gmc.mc_vram_size;
> + if ((ctx->c2p_train_data_offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1)
> ) {
> + ctx->c2p_train_data_offset -= ONE_MiB;
> + }
> + ctx->c2p_train_data_offset &= ~(ONE_MiB - 1);
Using the macro ALIGN() is a good practice.
Usually when calculating a quantity, such as the one above, you'd
use a working variable, say 'a', and after you're done with
the calculation, you'd then assign it to the variable which
needs it. Something like this:
a = adev->gmc.mc_vram_size;
if ((a & (ONE_MiB - 1)) < (4 * ONE_KiB + 1))
a -= ONE_MiB;
ctx->c2p_train_data_offset = ALIGN(a, ONE_MiB);
The easiest way to see this is, imagine, if all this calculation
was offloaded to a dedicated function, f(), to do:
ctx->c2p_train_data_offset = f(adev->gmc.mc_vram_size);
Now, by using the working variable 'a', you've shown this
abstraction just the same. (By using the working variable 'a',
you've shown to the reader,that this calculation is abstracted,
and could be relocated to a function.)
Regards,
Luben
P.S. The compiler is probably already doing this, and not working
directly on the ctx->c2p_train_data_offs, but assigns a final
result, as explicitly shown above. The above is to make it easy
for humans to read and understand the code. Hope this helps.
>
> *[kevin]:*
> *i'd like to use the marco ALIGN() to simple above code.*
> *anyway, the patch Reviewed-by: Kevin Wang *
>
> ctx->p2c_train_data_offset = (adev->gmc.mc_vram_size -
> GDDR6_MEM_TRAINING_OFFSET);
> ctx->train_data_size = GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index f1ebd424510c..19eb3e8456c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -66,6 +66,13 @@ struct amdgpu_copy_mem {
> unsigned long offset;
> };
>
> +/* Definitions for constance */
> +enum amdgpu_internal_constants
> +{
> + ONE_KiB = 0x400,
> + ONE_MiB = 0x10,
> +};
> +
> extern const struct ttm_mem_type_manager_func amdgpu_gtt_mgr_func;
> extern const struct ttm_mem_type_manager_func amdgpu_vram_mgr_func;
>
> diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h
> b/drivers/gpu/drm/amd/include/atomfirmware.h
> index dd7cbc00a0aa..70146518174c 100644
> --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> @@ -672,20 +672,6 @@ struct vram_usagebyfirmware_v2_1
> uint16_t used_by_driver_in_kb;
> };
>
> -/* This is part of vram_usagebyfirmware_v2_1 */
> -struct vram_reserve_block
> -{
> - uint32_t start_address_in_kb;
> - uint16_t used_by_firmware_in_kb;
> - uint16_t used_by_driver_in_kb;
> -};
> -
> -/* Definitions for constance */
> -enum atomfirmware_internal_constants
> -{
> - ONE_KiB = 0x400,
> - ONE_MiB = 0x10,
> -};
>
> /*
> ***
> --
> 2.17.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/3] gpu: drm: dead code elimination
On Wed, Dec 18, 2019 at 3:14 AM Pan Zhang wrote:
>
> this set adds support for removal of gpu drm dead code.
>
> patch3 is similar with patch 1:
> `num` is a data of u8 type and ATOM_MAX_HW_I2C_READ == 255,
>
> so there is a impossible condition '(num > 255) => (0-255 > 255)'.
>
> Signed-off-by: Pan Zhang
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/radeon/atombios_i2c.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c
> b/drivers/gpu/drm/radeon/atombios_i2c.c
> index a570ce4..ab4d210 100644
> --- a/drivers/gpu/drm/radeon/atombios_i2c.c
> +++ b/drivers/gpu/drm/radeon/atombios_i2c.c
> @@ -68,11 +68,6 @@ static int radeon_process_i2c_ch(struct radeon_i2c_chan
> *chan,
> memcpy(&out, &buf[1], num);
> args.lpI2CDataOut = cpu_to_le16(out);
> } else {
> - if (num > ATOM_MAX_HW_I2C_READ) {
> - DRM_ERROR("hw i2c: tried to read too many bytes (%d
> vs 255)\n", num);
> - r = -EINVAL;
> - goto done;
> - }
> args.ucRegIndex = 0;
> args.lpI2CDataOut = 0;
> }
> --
> 2.7.4
>
> ___
> 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 1/3] gpu: drm: dead code elimination
On Wed, Dec 18, 2019 at 3:13 AM Pan Zhang wrote:
>
> this set adds support for removal of gpu drm dead code.
>
> patch1:
> `num` is a data of u8 type and ATOM_MAX_HW_I2C_READ == 255,
>
> so there is a impossible condition '(num > 255) => (0-255 > 255)'.
>
> Signed-off-by: Pan Zhang
This change was already made by someone else.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/atombios_i2c.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
> index 980c363..b4cc7c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
> @@ -76,11 +76,6 @@ static int amdgpu_atombios_i2c_process_i2c_ch(struct
> amdgpu_i2c_chan *chan,
> }
> args.lpI2CDataOut = cpu_to_le16(out);
> } else {
> - if (num > ATOM_MAX_HW_I2C_READ) {
> - DRM_ERROR("hw i2c: tried to read too many bytes (%d
> vs 255)\n", num);
> - r = -EINVAL;
> - goto done;
> - }
> args.ucRegIndex = 0;
> args.lpI2CDataOut = 0;
> }
> --
> 2.7.4
>
> ___
> 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 2/2] drm/amdgpu: enlarge agp_start address into 48bit
On Wed, Dec 18, 2019 at 7:14 AM Frank.Min wrote:
>
> enlarge agp_start address into 48bit with all bits set
Might want to add a note that the max range of the agp aperture is 48
bits. With that fixed, the series is:
Reviewed-by: Alex Deucher
>
> Change-Id: I36eb757310fa71555c8355f99f89c89fed306638
> Signed-off-by: Frank.Min
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index a12f33c0f5df..bbcd11ac5bbb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -223,7 +223,7 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
> struct amdgpu_gmc *mc)
> u64 size_af, size_bf;
>
> if (amdgpu_sriov_vf(adev)) {
> - mc->agp_start = 0x;
> + mc->agp_start = 0x;
> mc->agp_end = 0x0;
> mc->agp_size = 0;
>
> --
> 2.17.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
Re: [GIT PULL] Please pull hmm changes
On Wed, Dec 18, 2019 at 10:37 AM Jason Gunthorpe wrote: > > I think this is what you are looking for? I think that with these names, I would have had an easier time reading the original patch that made me go "Eww", yes. Of course, now that it's just a rename patch, it's not like the patch is all that legible, but yeah, I think the naming is saner. Linus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [GIT PULL] Please pull hmm changes
On Wed, Dec 18, 2019 at 08:53:05AM -0800, Linus Torvalds wrote:
> On Wed, Dec 18, 2019 at 6:59 AM Jason Gunthorpe wrote:
> Yes, global function names need to be unique, and if they aren't
> really core, they want some prefix that explains the context, because
> global functions are called from _outside_ the context that explains
> them.
Yes, in this thread I have mostly talked about changing the global
struct names, and for that mmn_ is the context explaining prefix.
> But if it's a "struct mmu_interval_notifier" pointer, and it's inside
> a file that is all about these pointers, it shouldn't be called
> "mmn_xyz". That's not a name. That's line noise.
>
> So call it a "notifier". Maybe even an "interval_notifier" if you
> don't mind the typing. Name it by something _descriptive_. And if you
> want.
>
> And "subscriptions" is a lovely name. What does the "mmn" buy you?
To be clear, I was proposing this as the struct name:
'struct mmu_notifier_mm' becomes 'struct mmn_subscriptions'
(and similar for other mmu_notifier_x* and mmu_x_notifier patterns)
>From there we now have a natural and readable local variable name like
'subscriptions' within mm/mmu_notifier.c
I've just started looking at this in detail, but it seems sticking
with 'mmu_notifier_' as the global prefix will avoid a fair amount of
churn. So lets not try to shorten it to mmn_ as the global prefix.
> Just to clarify: the names I really hated were the local variable
> names (and the argument names) that were all entirely within the
> context of mm/mmu_notifier.c. Calling something "mmn_mm" is a random
> jumble of letters that looks more like you're humming than you're
> speaking.
Yes, I understood - I've approached trying to have good names for the
variables via having good names for their struct's.
Below is what I'm suggesting for the first patch. I am intending a
patch series to make the naming better across mmu_notifier.h, this
would be the first.
Next patches would probably replace 'mn' with 'list_sub' (struct
mmu_notifier_subscription) and 'mni' with 'range_sub' (struct
mmu_notifier_range_subscription).
Thus we have code working on a 'list_sub/range_sub' and storing it in
a 'subscriptions'.
I think this is what you are looking for?
>From c656d0862dedcc2f5f4beda129a2ac51c892be7e Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe
Date: Wed, 18 Dec 2019 13:40:35 -0400
Subject: [PATCH] mm/mmu_notifier: Rename struct mmu_notifier_mm to
mmu_notifier_subscriptions
The name mmu_notifier_mm implies that the thing is a mm_struct pointer,
and is difficult to abbreviate. The struct is actually holding the
interval tree and hlist containing the notifiers subscribed to a mm.
Use 'subscriptions' as the variable name for this struct instead of the
really terrible and misleading 'mmn_mm'.
Signed-off-by: Jason Gunthorpe
---
include/linux/mm_types.h | 2 +-
include/linux/mmu_notifier.h | 18 +-
kernel/fork.c| 4 +-
mm/debug.c | 4 +-
mm/mmu_notifier.c| 322 ++-
5 files changed, 182 insertions(+), 168 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 270aa8fd2800b4..e87bb864bdb29a 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -490,7 +490,7 @@ struct mm_struct {
/* store ref to file /proc//exe symlink points to */
struct file __rcu *exe_file;
#ifdef CONFIG_MMU_NOTIFIER
- struct mmu_notifier_mm *mmu_notifier_mm;
+ struct mmu_notifier_subscriptions *notifier_subscriptions;
#endif
#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS
pgtable_t pmd_huge_pte; /* protected by page_table_lock */
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 9e6caa8ecd1938..a302925fbc6177 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -8,7 +8,7 @@
#include
#include
-struct mmu_notifier_mm;
+struct mmu_notifier_subscriptions;
struct mmu_notifier;
struct mmu_notifier_range;
struct mmu_interval_notifier;
@@ -265,7 +265,7 @@ struct mmu_notifier_range {
static inline int mm_has_notifiers(struct mm_struct *mm)
{
- return unlikely(mm->mmu_notifier_mm);
+ return unlikely(mm->notifier_subscriptions);
}
struct mmu_notifier *mmu_notifier_get_locked(const struct mmu_notifier_ops
*ops,
@@ -364,7 +364,7 @@ static inline bool mmu_interval_check_retry(struct
mmu_interval_notifier *mni,
return READ_ONCE(mni->invalidate_seq) != seq;
}
-extern void __mmu_notifier_mm_destroy(struct mm_struct *mm);
+extern void __mmu_notifier_subscriptions_destroy(struct mm_struct *mm);
extern void __mmu_notifier_release(struct mm_struct *mm);
extern int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
unsigned long start,
@@ -480,15 +480,15 @@ static inline void mmu_notifier_invalidate_range(struct
mm_stru
Re: [PATCH v3] drm/amd/display: Fix AppleDongle can't be detected
On Tue, Dec 17, 2019 at 10:57:11AM -0500, Harry Wentland wrote:
> On 2019-12-11 2:33 a.m., Louis Li wrote:
> > [Why]
> > External monitor cannot be displayed consistently, if connecting
> > via this Apple dongle (A1621, USB Type-C to HDMI).
> > Experiments prove that the dongle needs 200ms at least to be ready
> > for communication, after it drives HPDsignal high, and DPCD cannot
> > be read correctly during the period, even reading it repeatedly.
> > In such a case, driver does not perform link training bcz of no DPCD.
> >
> > [How]
> > When driver is run to the modified point, EDID is read correctly
> > and dpcd_sink_count of link is not zero. Therefore, link training
> > should be successfully performed. Which implies parameters should
> > be updated, e.g. lane count, link rate, etc. Checking parameters,
> > if values of those parameters are zero, link training is not
> > performed. So, do link-training to have detection completed.
> >
> > With this patch applied, the problem cannot be reproduced.
> > Testing other dongles, results are PASS.
> > Patch(v3) is verified PASS by both AMD internal lab and customer.
> >
> >
> > Signed-off-by: Louis Li
> > ---
> > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 7372dedd2f48..6188edc92d0f 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -725,7 +725,9 @@ bool dc_link_detect(struct dc_link *link, enum
> > dc_detect_reason reason)
> >
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
> > sink_caps.transaction_type ==
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX &&
> > - reason != DETECT_REASON_HPDRX) {
>
> Do we need to drop this line? This looks like it'll break the previous
> fix here.
>
> It looks like Abdoulaye added this here to fix the 400.1.1 DP compliance
> test. If you can check with him that your solution is fine and make sure
> to test that you can get a consistent pass of 400.1.1 over 30 runs I'm
> okay to take the change.
>
> Harry
>
Yes, need drop this line for this fix. Good to know it may impact 400.1.1.
I will verify it with this patch. And update test result.
Louis
> > + link->verified_link_cap.lane_count == 0 &&
> > + link->verified_link_cap.link_rate == 0 &&
> > + link->verified_link_cap.link_spread == 0) {
> > /*
> > * TODO debug why Dell 2413 doesn't like
> > * two link trainings
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: attempt xgmi perfmon re-arm on failed arm
The DF routines to arm xGMI performance will attempt to re-arm both on
performance monitoring start and read on initial failure to arm.
v2: Roll back reset_perfmon_cntr to void return since new perfmon
counters are now safe to write to during DF C-States. Do single perfmon
controller re-arm when counter is deferred in pmc_count versus multiple
perfmon controller re-arms that could last 1 millisecond. Avoid holding
spin lock during sleep in perfmon_arm_with_retry. Rename counter arm
defer function to be less confusing.
Signed-off-by: Jonathan Kim
---
drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 149 +++
1 file changed, 127 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 4043ebcea5de..56dead3a9718 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -183,6 +183,61 @@ static void df_v3_6_perfmon_wreg(struct amdgpu_device
*adev, uint32_t lo_addr,
spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
}
+/* same as perfmon arm but return status on write value check */
+static int df_v3_6_perfmon_arm_with_status(struct amdgpu_device *adev,
+ uint32_t lo_addr, uint32_t lo_val,
+ uint32_t hi_addr, uint32_t hi_val)
+{
+ unsigned long flags, address, data;
+ uint32_t lo_val_rb, hi_val_rb;
+
+ address = adev->nbio.funcs->get_pcie_index_offset(adev);
+ data = adev->nbio.funcs->get_pcie_data_offset(adev);
+
+ spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+ WREG32(address, lo_addr);
+ WREG32(data, lo_val);
+ WREG32(address, hi_addr);
+ WREG32(data, hi_val);
+
+ WREG32(address, lo_addr);
+ lo_val_rb = RREG32(data);
+ WREG32(address, hi_addr);
+ hi_val_rb = RREG32(data);
+ spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+
+ if (!(lo_val == lo_val_rb && hi_val == hi_val_rb))
+ return -EBUSY;
+
+ return 0;
+}
+
+
+/*
+ * retry arming counters every 100 usecs within 1 millisecond interval.
+ * if retry fails after time out, return error.
+ */
+#define ARM_RETRY_USEC_TIMEOUT 1000
+#define ARM_RETRY_USEC_INTERVAL100
+static int df_v3_6_perfmon_arm_with_retry(struct amdgpu_device *adev,
+ uint32_t lo_addr, uint32_t lo_val,
+ uint32_t hi_addr, uint32_t hi_val)
+{
+ int countdown = ARM_RETRY_USEC_TIMEOUT;
+
+ while (countdown) {
+
+ if (!df_v3_6_perfmon_arm_with_status(adev, lo_addr, lo_val,
+hi_addr, hi_val))
+ break;
+
+ countdown -= ARM_RETRY_USEC_INTERVAL;
+ udelay(ARM_RETRY_USEC_INTERVAL);
+ }
+
+ return countdown > 0 ? 0 : -ETIME;
+}
+
/* get the number of df counters available */
static ssize_t df_v3_6_get_df_cntr_avail(struct device *dev,
struct device_attribute *attr,
@@ -334,20 +389,20 @@ static void df_v3_6_pmc_get_addr(struct amdgpu_device
*adev,
switch (target_cntr) {
case 0:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo0 : smnPerfMonCtrLo0;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi0 : smnPerfMonCtrHi0;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo4 : smnPerfMonCtrLo4;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi4 : smnPerfMonCtrHi4;
break;
case 1:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo1 : smnPerfMonCtrLo1;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi1 : smnPerfMonCtrHi1;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo5 : smnPerfMonCtrLo5;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi5 : smnPerfMonCtrHi5;
break;
case 2:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo2 : smnPerfMonCtrLo2;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi2 : smnPerfMonCtrHi2;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo6 : smnPerfMonCtrLo6;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi6 : smnPerfMonCtrHi6;
break;
case 3:
- *lo_base_addr = is_ctrl ? smnPerfMonCtlLo3 : smnPerfMonCtrLo3;
- *hi_base_addr = is_ctrl ? smnPerfMonCtlHi3 : smnPerfMonCtrHi3;
+ *lo_base_addr = is_ctrl ? smnPerfMonCtlLo7 : smnPerfMonCtrLo7;
+ *hi_base_addr = is_ctrl ? smnPerfMonCtlHi7 : smnPerfMonCtrHi7;
break;
}
@@ -422,6 +477,42 @@ static int df_v3_6_pmc_add_cntr(struct amdgpu_device *adev,
return -ENOSPC;
}
+#define DEFERRED_ARM_MASK (1 << 31)
+static int df_v3_6_pmc_set_deferred(struct amdgpu_device *adev,
+ uint64_t config, bool is_deferred)
+{
+ int target_cntr;
+
+ target_cntr = df_v3_6_pm
Re: [GIT PULL] Please pull hmm changes
On Wed, Dec 18, 2019 at 6:59 AM Jason Gunthorpe wrote: > > Do you think calling it 'mmn_subscriptions' is clear? Why do you want that "mmn"? Guys, the "mmn" part is clear from the _context_. The function name is When the function name is something like "mmu_interval_read_begin()", and the filename is "mm/mmu_notifier.c", you do NOT NEED silly prefixes like "mmn" for local variables. They add NOTHING. And they make the code an illegible mess. Yes, global function names need to be unique, and if they aren't really core, they want some prefix that explains the context, because global functions are called from _outside_ the context that explains them. But if it's a "struct mmu_interval_notifier" pointer, and it's inside a file that is all about these pointers, it shouldn't be called "mmn_xyz". That's not a name. That's line noise. So call it a "notifier". Maybe even an "interval_notifier" if you don't mind the typing. Name it by something _descriptive_. And if you want. And "subscriptions" is a lovely name. What does the "mmn" buy you? Just to clarify: the names I really hated were the local variable names (and the argument names) that were all entirely within the context of mm/mmu_notifier.c. Calling something "mmn_mm" is a random jumble of letters that looks more like you're humming than you're speaking. Don't mumble. Speak _clearly_. The other side of "short names" is that some non-local conventions exist because they are _so_ global. So if it's just a mm pointer, call it "mm". We do have some very core concepts in the kernel that permeate _everything_, and those core things we tend to have very short names for. So whenever you're working with VM code, you'll see lots of small names like "mm", "vma", "pte" etc. They aren't exactly clear, but they are _globally_ something you read and learn when you work on the Linux VM code. That's very diofferent from "mmn" - the "mmn" thing isn't some global shorthand, it is just a local abomination. So "notifier_mm" makes sense - it's the mm for a notifier. But "mmn_notifier" does not, because "mmn" only makes sense in a local context, and in that local context it's not any new information at all. See the difference? Two shorthands, but one makes sense and adds information, while the other is just unnecessary and pointless and doesn't add anything at all. Linus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 2/2] drm/amdkfd: expose num_cp_queues data field to topology node (v2)
On 2019-12-18 3:45 a.m., Huang Rui wrote:
Thunk driver would like to know the num_cp_queues data, however this data relied
on different asic specific. So it's better to get it from kfd driver.
v2: don't update name size.
Signed-off-by: Huang Rui
The series is
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index cc01ccd..203c823 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -488,6 +488,8 @@ static ssize_t node_show(struct kobject *kobj, struct
attribute *attr,
dev->node_props.num_sdma_xgmi_engines);
sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
dev->node_props.num_sdma_queues_per_engine);
+ sysfs_show_32bit_prop(buffer, "num_cp_queues",
+ dev->node_props.num_cp_queues);
if (dev->gpu) {
log_max_watch_addr =
@@ -1316,6 +1318,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
dev->node_props.num_gws = (hws_gws_support &&
dev->gpu->dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS) ?
amdgpu_amdkfd_get_num_gws(dev->gpu->kgd) : 0;
+ dev->node_props.num_cp_queues = get_queues_num(dev->gpu->dqm);
kfd_fill_mem_clk_max_info(dev);
kfd_fill_iolink_non_crat_info(dev);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
index e1c9719..74e9b16 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
@@ -82,6 +82,7 @@ struct kfd_node_properties {
uint32_t num_sdma_engines;
uint32_t num_sdma_xgmi_engines;
uint32_t num_sdma_queues_per_engine;
+ uint32_t num_cp_queues;
char name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE];
};
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: replace BUG_ON with WARN_ON
In skip_modeset label within dm_update_crtc_state(), the dc stream cannot be NULL. Using BUG_ON as an assertion is not required and can be removed. The patch replaces the check with a WARN_ON in case dm_new_crtc_state->stream is NULL. Signed-off-by: Aditya Pakki --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7aac9568d3be..03cb30913c20 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7012,7 +7012,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, * 3. Is currently active and enabled. * => The dc stream state currently exists. */ - BUG_ON(dm_new_crtc_state->stream == NULL); + WARN_ON(!dm_new_crtc_state->stream); /* Scaling or underscan settings */ if (is_scaling_state_different(dm_old_conn_state, dm_new_conn_state)) -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [GIT PULL] Please pull hmm changes
On Fri, Dec 13, 2019 at 11:19:16AM +0100, Daniel Vetter wrote: > On Wed, Dec 11, 2019 at 10:57:13PM +, Jason Gunthorpe wrote: > > On Thu, Dec 05, 2019 at 11:03:24AM -0500, Jerome Glisse wrote: > > > > > > struct mmu_notifier_mm (ie the mm->mmu_notifier_mm) > > > >-> mmn_mm > > > > struct mm_struct > > > >-> mm > > > > struct mmu_notifier (ie the user subscription to the mm_struct) > > > >-> mn > > > > struct mmu_interval_notifier (the other kind of user subscription) > > > >-> mni > > > > > > What about "interval" the context should already tell people > > > it is related to mmu notifier and thus a notifier. I would > > > just remove the notifier suffix, this would match the below > > > range. > > > > Interval could be a good replacement for mni in the mm/mmu_notififer > > file if we don't do the wholesale rename > > > > > > I think it would be overall nicer with better names for the original > > > > structs. Perhaps: > > > > > > > > mmn_* - MMU notifier prefix > > > > mmn_state <- struct mmu_notifier_mm > > > > mmn_subscription (mmn_sub) <- struct mmu_notifier > > > > mmn_range_subscription (mmn_range_sub) <- struct mmu_interval_notifier > > > > mmn_invalidate_desc <- struct mmu_notifier_range > > > > > > This looks good. > > > > Well, lets just bite the bullet then and switch it. Do you like > > 'state'? I thought that was the weakest one > > Since you're asking, here's my bikeshed. I kinda agree _state looks a bit > strange for this, what about a _link suffix in the spirit of Do you think calling it 'mmn_subscriptions' is clear? Ie a struct mmn_subscriptions holds the lists of struct mmn_subscription and struct mmn_range_subscription? Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
On Tue, Dec 17, 2019 at 10:25 PM Evan Quan wrote:
>
> For non-RAS baco reset, there is no need to reset the SMC. Thus
> the firmware reloading should be avoided.
>
> Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> Signed-off-by: Evan Quan
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index c14f2ccd0677..9bf7e92394f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> continue;
>
> if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> - (psp_smu_reload_quirk(psp) || psp->autoload_supported))
> + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> + || psp->autoload_supported))
Will this cover the power saving case as well? Do we need to check
adev->in_gpu_reset as well or can we drop that part?
Alex
> continue;
>
> if (amdgpu_sriov_vf(adev) &&
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index c66ca8cc2ebd..ba761e9366e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct
> psp_context *psp,
> return true;
> }
>
> +/*
> + * Check whether SMU is still alive. If that's true
> + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> + */
> +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp)
> +{
> + struct amdgpu_device *adev = psp->adev;
> + uint32_t reg;
> +
> + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b0);
> + return (reg & MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ? true :
> false;
> +}
> +
> static int psp_v11_0_mode1_reset(struct psp_context *psp)
> {
> int ret;
> @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> .ring_stop = psp_v11_0_ring_stop,
> .ring_destroy = psp_v11_0_ring_destroy,
> .compare_sram_data = psp_v11_0_compare_sram_data,
> + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> .mode1_reset = psp_v11_0_mode1_reset,
> .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> --
> 2.24.0
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu: enable xgmi init for sriov use case
1. enable xgmi ta initialization for sriov
2. enable xgmi initialization for sriov
Change-Id: I0b333ede6933381debba6b6d61d986c897c32a2b
Signed-off-by: Frank.Min
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 26 +++--
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 3e293a3c2fbf..8469834d90ff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -466,8 +466,6 @@ static int psp_xgmi_load(struct psp_context *psp)
/*
* TODO: bypass the loading in sriov for now
*/
- if (amdgpu_sriov_vf(psp->adev))
- return 0;
cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
if (!cmd)
@@ -508,8 +506,6 @@ static int psp_xgmi_unload(struct psp_context *psp)
/*
* TODO: bypass the unloading in sriov for now
*/
- if (amdgpu_sriov_vf(psp->adev))
- return 0;
cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
if (!cmd)
@@ -540,11 +536,6 @@ int psp_xgmi_invoke(struct psp_context *psp, uint32_t
ta_cmd_id)
int ret;
struct psp_gfx_cmd_resp *cmd;
- /*
-* TODO: bypass the loading in sriov for now
- */
- if (amdgpu_sriov_vf(psp->adev))
- return 0;
cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
if (!cmd)
@@ -1506,16 +1497,13 @@ static int psp_load_fw(struct amdgpu_device *adev)
if (!psp->cmd)
return -ENOMEM;
- /* this fw pri bo is not used under SRIOV */
- if (!amdgpu_sriov_vf(psp->adev)) {
- ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
- AMDGPU_GEM_DOMAIN_GTT,
- &psp->fw_pri_bo,
- &psp->fw_pri_mc_addr,
- &psp->fw_pri_buf);
- if (ret)
- goto failed;
- }
+ ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
+ AMDGPU_GEM_DOMAIN_GTT,
+ &psp->fw_pri_bo,
+ &psp->fw_pri_mc_addr,
+ &psp->fw_pri_buf);
+ if (ret)
+ goto failed;
ret = amdgpu_bo_create_kernel(adev, PSP_FENCE_BUFFER_SIZE, PAGE_SIZE,
AMDGPU_GEM_DOMAIN_VRAM,
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: enlarge agp_start address into 48bit
enlarge agp_start address into 48bit with all bits set
Change-Id: I36eb757310fa71555c8355f99f89c89fed306638
Signed-off-by: Frank.Min
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index a12f33c0f5df..bbcd11ac5bbb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -223,7 +223,7 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
struct amdgpu_gmc *mc)
u64 size_af, size_bf;
if (amdgpu_sriov_vf(adev)) {
- mc->agp_start = 0x;
+ mc->agp_start = 0x;
mc->agp_end = 0x0;
mc->agp_size = 0;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: skip disable dynamic state management
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Kenneth Feng
-Original Message-
From: Yintian Tao
Sent: Wednesday, December 18, 2019 6:15 PM
To: Feng, Kenneth
Cc: amd-gfx@lists.freedesktop.org; Tao, Yintian
Subject: [PATCH] drm/amd/powerplay: skip disable dynamic state management
Under sriov, the disable operation is no allowed.
Signed-off-by: Yintian Tao
---
drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
index 253860d30b20..9454ab50f9a1 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
@@ -99,6 +99,9 @@ int phm_disable_dynamic_state_management(struct pp_hwmgr
*hwmgr)
PHM_FUNC_CHECK(hwmgr);
+ if (!hwmgr->not_vf)
+ return 0;
+
if (!smum_is_dpm_running(hwmgr)) {
pr_info("dpm has been disabled\n");
return 0;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: skip disable dynamic state management
Under sriov, the disable operation is no allowed.
Signed-off-by: Yintian Tao
---
drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
index 253860d30b20..9454ab50f9a1 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
@@ -99,6 +99,9 @@ int phm_disable_dynamic_state_management(struct pp_hwmgr
*hwmgr)
PHM_FUNC_CHECK(hwmgr);
+ if (!hwmgr->not_vf)
+ return 0;
+
if (!smum_is_dpm_running(hwmgr)) {
pr_info("dpm has been disabled\n");
return 0;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] gpu: drm: dead code elimination
On 2019-12-18 4:50 a.m., Pan Zhang wrote: > this set adds support for removal of gpu drm dead code. > > patch2: > `num_entries` is a data of unsigned type(compilers always treat as unsigned > int) and SIZE_MAX == ~0, > > so there is a impossible condition: > '(num_entries > ((~0) - 56) / 72) => (max(0-u32) > 256204778801521549)'. While this looks correct for 64-bit, where size_t is unsigned long, on 32-bit it's unsigned int, in which case this isn't dead code. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > index 85b0515..10a7f30 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > @@ -71,10 +71,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, > struct drm_file *filp, > unsigned i; > int r; > > - if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) > - / sizeof(struct amdgpu_bo_list_entry)) > - return -EINVAL; > - > size = sizeof(struct amdgpu_bo_list); > size += num_entries * sizeof(struct amdgpu_bo_list_entry); > list = kvmalloc(size, GFP_KERNEL); > -- Earthling Michel Dänzer | https://redhat.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/amdgpu: correctly report gpu recover status
Am 18.12.19 um 04:25 schrieb Evan Quan:
Knowing whether gpu recovery was performed successfully or not
is important for our BACO development.
Change-Id: I0e3ca4dcb65a053eb26bc55ad7431e4a42e160de
Signed-off-by: Evan Quan
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index e9efee04ca23..5dff5c0dd882 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -743,9 +743,7 @@ static int amdgpu_debugfs_gpu_recover(struct seq_file *m,
void *data)
struct amdgpu_device *adev = dev->dev_private;
seq_printf(m, "gpu recover\n");
- amdgpu_device_gpu_recover(adev, NULL);
-
- return 0;
+ return amdgpu_device_gpu_recover(adev, NULL);
NAK, what we could do here is the following:
r = amdgpu_device_gpu_recover();
seq_printf(m, "gpu recover %d\n", r);
But returning the error code from the GPU recovery to userspace doesn't
make to much sense.
Christian.
}
static const struct drm_info_list amdgpu_debugfs_fence_list[] = {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] gpu: drm: dead code elimination
Am 18.12.19 um 04:50 schrieb Pan Zhang: this set adds support for removal of gpu drm dead code. patch2: `num_entries` is a data of unsigned type(compilers always treat as unsigned int) and SIZE_MAX == ~0, so there is a impossible condition: '(num_entries > ((~0) - 56) / 72) => (max(0-u32) > 256204778801521549)'. NAK, that calculation is not correct on 32-bit systems. Christian. Signed-off-by: Pan Zhang --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 85b0515..10a7f30 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -71,10 +71,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, unsigned i; int r; - if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) - / sizeof(struct amdgpu_bo_list_entry)) - return -EINVAL; - size = sizeof(struct amdgpu_bo_list); size += num_entries * sizeof(struct amdgpu_bo_list_entry); list = kvmalloc(size, GFP_KERNEL); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V2)
Hi Kevin,
You mean like this? It's a bit lengthy.
- ctx->c2p_train_data_offset &= ~(ONE_MiB - 1);
+ ctx->c2p_train_data_offset = ALIGN(ctx->c2p_train_data_offset, ONE_MiB);
Rico
From: Wang, Kevin(Yang)
Sent: Wednesday, December 18, 2019 16:56
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang
Subject: Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training(V2)
[AMD Official Use Only - Internal Distribution Only]
comment inline.
From: Tianci Yin
Sent: Wednesday, December 18, 2019 4:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang ; Wang,
Kevin(Yang) ; Yin, Tianci (Rico)
Subject: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training(V2)
From: "Tianci.Yin"
The method of getting fb_loc changed from parsing VBIOS to
taking certain offset from top of VRAM
Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 38 ++-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
drivers/gpu/drm/amd/include/atomfirmware.h| 14 ---
7 files changed, 19 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a78a363b1d71..fa2cf8e7bc07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
struct amdgpu_bo *reserved_bo;
void *va;
- /* Offset on the top of VRAM, used as c2p write buffer.
+ /* GDDR6 training support flag.
*/
- u64 mem_train_fb_loc;
bool mem_train_support;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 9ba80d828876..fdd52d86a4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
if (adev->is_atom_fw) {
amdgpu_atomfirmware_scratch_regs_init(adev);
amdgpu_atomfirmware_allocate_fb_scratch(adev);
- ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);
+ ret = amdgpu_atomfirmware_get_mem_train_info(adev);
if (ret) {
DRM_ERROR("Failed to get mem train fb location.\n");
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index ff4eb96bdfb5..58f9d8c3a17a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device
*adev)
return ret;
}
-int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
+int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
- unsigned char *bios = ctx->bios;
- struct vram_reserve_block *reserved_block;
- int index, block_number;
+ int index;
uint8_t frev, crev;
uint16_t data_offset, size;
- uint32_t start_address_in_kb;
- uint64_t offset;
int ret;
adev->fw_vram_usage.mem_train_support = false;
@@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
amdgpu_device *adev)
return -EINVAL;
}
- reserved_block = (struct vram_reserve_block *)
- (bios + data_offset + sizeof(struct atom_common_table_header));
- block_number = ((unsigned int)size - sizeof(struct
atom_common_table_header))
- / sizeof(struct vram_reserve_block);
- reserved_block += (block_number > 0) ? block_number-1 : 0;
- DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
drv.\n",
- block_number,
- le32_to_cpu(reserved_block->start_address_in_kb),
- le16_to_cpu(reserved_block->used_by_firmware_in_kb),
- le16_to_cpu(reserved_block->used_by_driver_in_kb));
- if (reserved_block->used_by_firmware_in_kb > 0) {
- start_address_in_kb =
le32_to_cpu(reserved_block->start_address_in_kb);
- offset = (uint64_t)start_address_in_kb * ONE_KiB;
- if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
- offset -= ONE_MiB;
-
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V2)
[AMD Official Use Only - Internal Distribution Only]
comment inline.
From: Tianci Yin
Sent: Wednesday, December 18, 2019 4:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang ; Wang,
Kevin(Yang) ; Yin, Tianci (Rico)
Subject: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training(V2)
From: "Tianci.Yin"
The method of getting fb_loc changed from parsing VBIOS to
taking certain offset from top of VRAM
Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 38 ++-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
drivers/gpu/drm/amd/include/atomfirmware.h| 14 ---
7 files changed, 19 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a78a363b1d71..fa2cf8e7bc07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
struct amdgpu_bo *reserved_bo;
void *va;
- /* Offset on the top of VRAM, used as c2p write buffer.
+ /* GDDR6 training support flag.
*/
- u64 mem_train_fb_loc;
bool mem_train_support;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 9ba80d828876..fdd52d86a4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
if (adev->is_atom_fw) {
amdgpu_atomfirmware_scratch_regs_init(adev);
amdgpu_atomfirmware_allocate_fb_scratch(adev);
- ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);
+ ret = amdgpu_atomfirmware_get_mem_train_info(adev);
if (ret) {
DRM_ERROR("Failed to get mem train fb location.\n");
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index ff4eb96bdfb5..58f9d8c3a17a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device
*adev)
return ret;
}
-int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
+int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
- unsigned char *bios = ctx->bios;
- struct vram_reserve_block *reserved_block;
- int index, block_number;
+ int index;
uint8_t frev, crev;
uint16_t data_offset, size;
- uint32_t start_address_in_kb;
- uint64_t offset;
int ret;
adev->fw_vram_usage.mem_train_support = false;
@@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
amdgpu_device *adev)
return -EINVAL;
}
- reserved_block = (struct vram_reserve_block *)
- (bios + data_offset + sizeof(struct atom_common_table_header));
- block_number = ((unsigned int)size - sizeof(struct
atom_common_table_header))
- / sizeof(struct vram_reserve_block);
- reserved_block += (block_number > 0) ? block_number-1 : 0;
- DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
drv.\n",
- block_number,
- le32_to_cpu(reserved_block->start_address_in_kb),
- le16_to_cpu(reserved_block->used_by_firmware_in_kb),
- le16_to_cpu(reserved_block->used_by_driver_in_kb));
- if (reserved_block->used_by_firmware_in_kb > 0) {
- start_address_in_kb =
le32_to_cpu(reserved_block->start_address_in_kb);
- offset = (uint64_t)start_address_in_kb * ONE_KiB;
- if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
- offset -= ONE_MiB;
- }
-
- offset &= ~(ONE_MiB - 1);
- adev->fw_vram_usage.mem_train_fb_loc = offset;
- adev->fw_vram_usage.mem_train_support = true;
- DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);
- ret = 0;
- } else {
- DRM_ERROR("used_by_firmware_in_kb is 0!\n");
- ret = -EINVAL;
- }
-
- return ret;
+ adev->fw_vram_usage.mem_train_support = true;
+ return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
b/dr
Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
Thanks Kevin!
From: Wang, Kevin(Yang)
Sent: Wednesday, December 18, 2019 16:53
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang
Subject: Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Kevin Wang
Best Regards,
Kevin
From: Tianci Yin
Sent: Wednesday, December 18, 2019 4:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang ; Wang,
Kevin(Yang) ; Yin, Tianci (Rico)
Subject: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ce5cb854bdb9..476ea4a4dc03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1718,17 +1715,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1737,15 +1723,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Kevin Wang
Best Regards,
Kevin
From: Tianci Yin
Sent: Wednesday, December 18, 2019 4:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang ; Wang,
Kevin(Yang) ; Yin, Tianci (Rico)
Subject: [PATCH 2/2] drm/amdgpu: remove memory training p2c buffer
reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ce5cb854bdb9..476ea4a4dc03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1718,17 +1715,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1737,15 +1723,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training
Hi Kevin,
Thanks very much!
I have rename the amdgpu_atomfirmware_get_mem_train_fb_loc function to
amdgpu_atomfirmware_get_mem_train_info, please review again.
Rico
From: Wang, Kevin(Yang)
Sent: Wednesday, December 18, 2019 13:32
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang
Subject: Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training
[AMD Official Use Only - Internal Distribution Only]
From: Tianci Yin
Sent: Wednesday, December 18, 2019 10:21 AM
To: amd-gfx@lists.freedesktop.org
Cc: Tuikov, Luben ; Koenig, Christian
; Deucher, Alexander ;
Zhang, Hawking ; Xu, Feifei ; Yuan,
Xiaojie ; Long, Gang ; Wang,
Kevin(Yang) ; Yin, Tianci (Rico)
Subject: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory
training
From: "Tianci.Yin"
The method of getting fb_loc changed from parsing VBIOS to
taking certain offset from top of VRAM
Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 36 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
drivers/gpu/drm/amd/include/atomfirmware.h| 14
5 files changed, 16 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a78a363b1d71..fa2cf8e7bc07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
struct amdgpu_bo *reserved_bo;
void *va;
- /* Offset on the top of VRAM, used as c2p write buffer.
+ /* GDDR6 training support flag.
*/
- u64 mem_train_fb_loc;
bool mem_train_support;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index ff4eb96bdfb5..009cb0b03d13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -528,13 +528,9 @@ static int gddr6_mem_train_support(struct amdgpu_device
*adev)
int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
- unsigned char *bios = ctx->bios;
- struct vram_reserve_block *reserved_block;
- int index, block_number;
+ int index;
uint8_t frev, crev;
uint16_t data_offset, size;
- uint32_t start_address_in_kb;
- uint64_t offset;
int ret;
adev->fw_vram_usage.mem_train_support = false;
@@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
amdgpu_device *adev)
[kevin]:
this function is not return any address after change,
i think we'd better to rename this function to another is well.
the code can be merge to function gddr6_mem_train_support().
return -EINVAL;
}
- reserved_block = (struct vram_reserve_block *)
- (bios + data_offset + sizeof(struct atom_common_table_header));
- block_number = ((unsigned int)size - sizeof(struct
atom_common_table_header))
- / sizeof(struct vram_reserve_block);
- reserved_block += (block_number > 0) ? block_number-1 : 0;
- DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
drv.\n",
- block_number,
- le32_to_cpu(reserved_block->start_address_in_kb),
- le16_to_cpu(reserved_block->used_by_firmware_in_kb),
- le16_to_cpu(reserved_block->used_by_driver_in_kb));
- if (reserved_block->used_by_firmware_in_kb > 0) {
- start_address_in_kb =
le32_to_cpu(reserved_block->start_address_in_kb);
- offset = (uint64_t)start_address_in_kb * ONE_KiB;
- if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
- offset -= ONE_MiB;
- }
-
- offset &= ~(ONE_MiB - 1);
- adev->fw_vram_usage.mem_train_fb_loc = offset;
- adev->fw_vram_usage.mem_train_support = true;
- DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);
- ret = 0;
- } else {
- DRM_ERROR("used_by_firmware_in_kb is 0!\n");
- ret = -EINVAL;
- }
-
- return ret;
+ adev->fw_vram_usage.mem_train_support = true;
+ return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2ff63d0414c9..ce5cb854bdb9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1705,7 +1705,11 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev
[PATCH 2/2] drm/amdgpu: remove memory training p2c buffer reservation(V2)
From: "Tianci.Yin"
IP discovery TMR(occupied the top VRAM with size DISCOVERY_TMR_SIZE)
has been reserved, and the p2c buffer is in the range of this TMR, so
the p2c buffer reservation is unnecessary.
Change-Id: Ib1f2f2b4a1f3869c03ffe22e2836cdbee17ba99f
Signed-off-by: Tianci.Yin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 ++---
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 5f8fd3e3535b..3265487b859f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -202,7 +202,6 @@ struct psp_memory_training_context {
/*vram offset of the p2c training data*/
u64 p2c_train_data_offset;
- struct amdgpu_bo *p2c_bo;
/*vram offset of the c2p training data*/
u64 c2p_train_data_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ce5cb854bdb9..476ea4a4dc03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1681,9 +1681,6 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct
amdgpu_device *adev)
amdgpu_bo_free_kernel(&ctx->c2p_bo, NULL, NULL);
ctx->c2p_bo = NULL;
- amdgpu_bo_free_kernel(&ctx->p2c_bo, NULL, NULL);
- ctx->p2c_bo = NULL;
-
return 0;
}
@@ -1718,17 +1715,6 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
ctx->p2c_train_data_offset,
ctx->c2p_train_data_offset);
- ret = amdgpu_bo_create_kernel_at(adev,
-ctx->p2c_train_data_offset,
-ctx->train_data_size,
-AMDGPU_GEM_DOMAIN_VRAM,
-&ctx->p2c_bo,
-NULL);
- if (ret) {
- DRM_ERROR("alloc p2c_bo failed(%d)!\n", ret);
- goto Err_out;
- }
-
ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
@@ -1737,15 +1723,12 @@ static int amdgpu_ttm_training_reserve_vram_init(struct
amdgpu_device *adev)
NULL);
if (ret) {
DRM_ERROR("alloc c2p_bo failed(%d)!\n", ret);
- goto Err_out;
+ amdgpu_ttm_training_reserve_vram_fini(adev);
+ return ret;
}
ctx->init = PSP_MEM_TRAIN_RESERVE_SUCCESS;
return 0;
-
-Err_out:
- amdgpu_ttm_training_reserve_vram_fini(adev);
- return ret;
}
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V2)
From: "Tianci.Yin"
The method of getting fb_loc changed from parsing VBIOS to
taking certain offset from top of VRAM
Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 38 ++-
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7
drivers/gpu/drm/amd/include/atomfirmware.h| 14 ---
7 files changed, 19 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a78a363b1d71..fa2cf8e7bc07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {
struct amdgpu_bo *reserved_bo;
void *va;
- /* Offset on the top of VRAM, used as c2p write buffer.
+ /* GDDR6 training support flag.
*/
- u64 mem_train_fb_loc;
bool mem_train_support;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index 9ba80d828876..fdd52d86a4d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
if (adev->is_atom_fw) {
amdgpu_atomfirmware_scratch_regs_init(adev);
amdgpu_atomfirmware_allocate_fb_scratch(adev);
- ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);
+ ret = amdgpu_atomfirmware_get_mem_train_info(adev);
if (ret) {
DRM_ERROR("Failed to get mem train fb location.\n");
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index ff4eb96bdfb5..58f9d8c3a17a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device
*adev)
return ret;
}
-int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)
+int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
- unsigned char *bios = ctx->bios;
- struct vram_reserve_block *reserved_block;
- int index, block_number;
+ int index;
uint8_t frev, crev;
uint16_t data_offset, size;
- uint32_t start_address_in_kb;
- uint64_t offset;
int ret;
adev->fw_vram_usage.mem_train_support = false;
@@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct
amdgpu_device *adev)
return -EINVAL;
}
- reserved_block = (struct vram_reserve_block *)
- (bios + data_offset + sizeof(struct atom_common_table_header));
- block_number = ((unsigned int)size - sizeof(struct
atom_common_table_header))
- / sizeof(struct vram_reserve_block);
- reserved_block += (block_number > 0) ? block_number-1 : 0;
- DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb
drv.\n",
- block_number,
- le32_to_cpu(reserved_block->start_address_in_kb),
- le16_to_cpu(reserved_block->used_by_firmware_in_kb),
- le16_to_cpu(reserved_block->used_by_driver_in_kb));
- if (reserved_block->used_by_firmware_in_kb > 0) {
- start_address_in_kb =
le32_to_cpu(reserved_block->start_address_in_kb);
- offset = (uint64_t)start_address_in_kb * ONE_KiB;
- if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {
- offset -= ONE_MiB;
- }
-
- offset &= ~(ONE_MiB - 1);
- adev->fw_vram_usage.mem_train_fb_loc = offset;
- adev->fw_vram_usage.mem_train_support = true;
- DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);
- ret = 0;
- } else {
- DRM_ERROR("used_by_firmware_in_kb is 0!\n");
- ret = -EINVAL;
- }
-
- return ret;
+ adev->fw_vram_usage.mem_train_support = true;
+ return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
index f871af5ea6f3..434fe2fa0089 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h
@@ -31,7 +31,7 @@ void amdgpu_atomfirmware_scratch_regs_init(struct
amdgpu_device *adev);
int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev);
int amdgpu_atomfirmware_get_vram_info(struct amdgpu_device *adev,
int *vram_width, int *vram_ty
[PATCH v2 1/2] drm/amdkfd: expose num_sdma_queues_per_engine data field to topology node (v2)
Thunk driver would like to know the num_sdma_queues_per_engine data, however
this data relied on different asic specific. So it's better to get it from kfd
driver.
v2: don't update the name size.
Signed-off-by: Huang Rui
---
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 4
drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 69bd062..cc01ccd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -486,6 +486,8 @@ static ssize_t node_show(struct kobject *kobj, struct
attribute *attr,
dev->node_props.num_sdma_engines);
sysfs_show_32bit_prop(buffer, "num_sdma_xgmi_engines",
dev->node_props.num_sdma_xgmi_engines);
+ sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
+ dev->node_props.num_sdma_queues_per_engine);
if (dev->gpu) {
log_max_watch_addr =
@@ -1309,6 +1311,8 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
dev->node_props.num_sdma_engines = gpu->device_info->num_sdma_engines;
dev->node_props.num_sdma_xgmi_engines =
gpu->device_info->num_xgmi_sdma_engines;
+ dev->node_props.num_sdma_queues_per_engine =
+ gpu->device_info->num_sdma_queues_per_engine;
dev->node_props.num_gws = (hws_gws_support &&
dev->gpu->dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS) ?
amdgpu_amdkfd_get_num_gws(dev->gpu->kgd) : 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
index 15843e0..e1c9719 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
@@ -81,6 +81,7 @@ struct kfd_node_properties {
int32_t drm_render_minor;
uint32_t num_sdma_engines;
uint32_t num_sdma_xgmi_engines;
+ uint32_t num_sdma_queues_per_engine;
char name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE];
};
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 2/2] drm/amdkfd: expose num_cp_queues data field to topology node (v2)
Thunk driver would like to know the num_cp_queues data, however this data relied
on different asic specific. So it's better to get it from kfd driver.
v2: don't update name size.
Signed-off-by: Huang Rui
---
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index cc01ccd..203c823 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -488,6 +488,8 @@ static ssize_t node_show(struct kobject *kobj, struct
attribute *attr,
dev->node_props.num_sdma_xgmi_engines);
sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
dev->node_props.num_sdma_queues_per_engine);
+ sysfs_show_32bit_prop(buffer, "num_cp_queues",
+ dev->node_props.num_cp_queues);
if (dev->gpu) {
log_max_watch_addr =
@@ -1316,6 +1318,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
dev->node_props.num_gws = (hws_gws_support &&
dev->gpu->dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS) ?
amdgpu_amdkfd_get_num_gws(dev->gpu->kgd) : 0;
+ dev->node_props.num_cp_queues = get_queues_num(dev->gpu->dqm);
kfd_fill_mem_clk_max_info(dev);
kfd_fill_iolink_non_crat_info(dev);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
index e1c9719..74e9b16 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
@@ -82,6 +82,7 @@ struct kfd_node_properties {
uint32_t num_sdma_engines;
uint32_t num_sdma_xgmi_engines;
uint32_t num_sdma_queues_per_engine;
+ uint32_t num_cp_queues;
char name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE];
};
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdkfd: expose num_cp_queues data field to topology node
[AMD Official Use Only - Internal Distribution Only]
On Wed, Dec 18, 2019 at 07:40:26AM +0800, Kuehling, Felix wrote:
> See comment inline. Other than that, the series looks good to me.
>
> On 2019-12-16 2:02, Huang Rui wrote:
> > Thunk driver would like to know the num_cp_queues data, however this data
> > relied
> > on different asic specific. So it's better to get it from kfd driver.
> >
> > Signed-off-by: Huang Rui
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 3 ++-
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > index cc01ccd..203c823 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > @@ -488,6 +488,8 @@ static ssize_t node_show(struct kobject *kobj, struct
> > attribute *attr,
> > dev->node_props.num_sdma_xgmi_engines);
> > sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
> > dev->node_props.num_sdma_queues_per_engine);
> > + sysfs_show_32bit_prop(buffer, "num_cp_queues",
> > + dev->node_props.num_cp_queues);
> >
> > if (dev->gpu) {
> > log_max_watch_addr =
> > @@ -1316,6 +1318,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
> > dev->node_props.num_gws = (hws_gws_support &&
> > dev->gpu->dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS) ?
> > amdgpu_amdkfd_get_num_gws(dev->gpu->kgd) : 0;
> > + dev->node_props.num_cp_queues = get_queues_num(dev->gpu->dqm);
> >
> > kfd_fill_mem_clk_max_info(dev);
> > kfd_fill_iolink_non_crat_info(dev);
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > index 9346cc1..e447901 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > @@ -27,7 +27,7 @@
> > #include
> > #include "kfd_crat.h"
> >
> > -#define KFD_TOPOLOGY_PUBLIC_NAME_SIZE 28
> > +#define KFD_TOPOLOGY_PUBLIC_NAME_SIZE 24
>
> I don't see why you need to change the name size here. I'm not aware of
> any requirement that the structure size cannot change. This comment
> applies to patch 1 as well.
>
Oh, sorry, I miss read this field as "reserved", so I kept the struct size.
Will updated in V2.
Thanks,
Ray
> Regards,
> Felix
>
> >
> > #define HSA_CAP_HOT_PLUGGABLE 0x0001
> > #define HSA_CAP_ATS_PRESENT 0x0002
> > @@ -82,6 +82,7 @@ struct kfd_node_properties {
> > uint32_t num_sdma_engines;
> > uint32_t num_sdma_xgmi_engines;
> > uint32_t num_sdma_queues_per_engine;
> > + uint32_t num_cp_queues;
> > char name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE];
> > };
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/3] gpu: drm: dead code elimination
this set adds support for removal of gpu drm dead code.
patch3 is similar with patch 1:
`num` is a data of u8 type and ATOM_MAX_HW_I2C_READ == 255,
so there is a impossible condition '(num > 255) => (0-255 > 255)'.
Signed-off-by: Pan Zhang
---
drivers/gpu/drm/radeon/atombios_i2c.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c
b/drivers/gpu/drm/radeon/atombios_i2c.c
index a570ce4..ab4d210 100644
--- a/drivers/gpu/drm/radeon/atombios_i2c.c
+++ b/drivers/gpu/drm/radeon/atombios_i2c.c
@@ -68,11 +68,6 @@ static int radeon_process_i2c_ch(struct radeon_i2c_chan
*chan,
memcpy(&out, &buf[1], num);
args.lpI2CDataOut = cpu_to_le16(out);
} else {
- if (num > ATOM_MAX_HW_I2C_READ) {
- DRM_ERROR("hw i2c: tried to read too many bytes (%d vs
255)\n", num);
- r = -EINVAL;
- goto done;
- }
args.ucRegIndex = 0;
args.lpI2CDataOut = 0;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/3] gpu: drm: dead code elimination
this set adds support for removal of gpu drm dead code.
patch1:
`num` is a data of u8 type and ATOM_MAX_HW_I2C_READ == 255,
so there is a impossible condition '(num > 255) => (0-255 > 255)'.
Signed-off-by: Pan Zhang
---
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
index 980c363..b4cc7c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c
@@ -76,11 +76,6 @@ static int amdgpu_atombios_i2c_process_i2c_ch(struct
amdgpu_i2c_chan *chan,
}
args.lpI2CDataOut = cpu_to_le16(out);
} else {
- if (num > ATOM_MAX_HW_I2C_READ) {
- DRM_ERROR("hw i2c: tried to read too many bytes (%d vs
255)\n", num);
- r = -EINVAL;
- goto done;
- }
args.ucRegIndex = 0;
args.lpI2CDataOut = 0;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] gpu: drm: dead code elimination
this set adds support for removal of gpu drm dead code. patch2: `num_entries` is a data of unsigned type(compilers always treat as unsigned int) and SIZE_MAX == ~0, so there is a impossible condition: '(num_entries > ((~0) - 56) / 72) => (max(0-u32) > 256204778801521549)'. Signed-off-by: Pan Zhang --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 85b0515..10a7f30 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -71,10 +71,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, unsigned i; int r; - if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) - / sizeof(struct amdgpu_bo_list_entry)) - return -EINVAL; - size = sizeof(struct amdgpu_bo_list); size += num_entries * sizeof(struct amdgpu_bo_list_entry); list = kvmalloc(size, GFP_KERNEL); -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
