diff mbox series

drm/i915/guc: Dump perf_limit_reasons for debug

Message ID 20230627021218.123999-1-vinay.belgaumkar@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/guc: Dump perf_limit_reasons for debug | expand

Commit Message

Vinay Belgaumkar June 27, 2023, 2:12 a.m. UTC
GuC load takes longer sometimes due to GT frequency not ramping up.
Add perf_limit_reasons to the existing warn print to see if frequency
is being throttled.

Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Dixit, Ashutosh June 27, 2023, 3:17 a.m. UTC | #1
On Mon, 26 Jun 2023 19:12:18 -0700, Vinay Belgaumkar wrote:
> 
> GuC load takes longer sometimes due to GT frequency not ramping up.
> Add perf_limit_reasons to the existing warn print to see if frequency
> is being throttled.
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 364d0d546ec8..73911536a8e7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -254,6 +254,8 @@ static int guc_wait_ucode(struct intel_guc *guc)
>  		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d]\n",
>  			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
>  			 before_freq, status, count, ret);
> +		guc_warn(guc, "perf limit reasons = 0x%08X\n",
> +			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));

Maybe just add at the end of the previous guc_warn?

>  	} else {
>  		guc_dbg(guc, "init took %lldms, freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d\n",
>  			delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
> -- 
> 2.38.1
>
Vinay Belgaumkar June 27, 2023, 4:02 a.m. UTC | #2
On 6/26/2023 8:17 PM, Dixit, Ashutosh wrote:
> On Mon, 26 Jun 2023 19:12:18 -0700, Vinay Belgaumkar wrote:
>> GuC load takes longer sometimes due to GT frequency not ramping up.
>> Add perf_limit_reasons to the existing warn print to see if frequency
>> is being throttled.
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> index 364d0d546ec8..73911536a8e7 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> @@ -254,6 +254,8 @@ static int guc_wait_ucode(struct intel_guc *guc)
>>   		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d]\n",
>>   			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
>>   			 before_freq, status, count, ret);
>> +		guc_warn(guc, "perf limit reasons = 0x%08X\n",
>> +			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
> Maybe just add at the end of the previous guc_warn?

Its already too long a line. If I try adding on the next line checkpatch 
complains about splitting double quotes.

Thanks,

Vinay.

>
>>   	} else {
>>   		guc_dbg(guc, "init took %lldms, freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d\n",
>>   			delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
>> -- 
>> 2.38.1
>>
Dixit, Ashutosh June 27, 2023, 6:43 a.m. UTC | #3
On Mon, 26 Jun 2023 21:02:14 -0700, Belgaumkar, Vinay wrote:
>
>
> On 6/26/2023 8:17 PM, Dixit, Ashutosh wrote:
> > On Mon, 26 Jun 2023 19:12:18 -0700, Vinay Belgaumkar wrote:
> >> GuC load takes longer sometimes due to GT frequency not ramping up.
> >> Add perf_limit_reasons to the existing warn print to see if frequency
> >> is being throttled.
> >>
> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> >> index 364d0d546ec8..73911536a8e7 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> >> @@ -254,6 +254,8 @@ static int guc_wait_ucode(struct intel_guc *guc)
> >>		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d]\n",
> >>			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
> >>			 before_freq, status, count, ret);
> >> +		guc_warn(guc, "perf limit reasons = 0x%08X\n",
> >> +			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
> > Maybe just add at the end of the previous guc_warn?
>
> Its already too long a line. If I try adding on the next line checkpatch
> complains about splitting double quotes.

In these cases of long quoted lines we generally ignore checkpatch. Because
perf limit reasons is part of the "excessive init time" message it should
be on the same line within the square brackets. So should not be
splitting double quotes.

Another idea would be something like this:

		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X]\n",
			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
			 before_freq, status);
		guc_warn(guc, "excessive init time: [count = %d, ret = %d, perf limit reasons = 0x%08X]\n",
			 count, ret, intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));

Thanks.
--
Ashutosh
Vinay Belgaumkar June 27, 2023, 7:03 p.m. UTC | #4
On 6/26/2023 11:43 PM, Dixit, Ashutosh wrote:
> On Mon, 26 Jun 2023 21:02:14 -0700, Belgaumkar, Vinay wrote:
>>
>> On 6/26/2023 8:17 PM, Dixit, Ashutosh wrote:
>>> On Mon, 26 Jun 2023 19:12:18 -0700, Vinay Belgaumkar wrote:
>>>> GuC load takes longer sometimes due to GT frequency not ramping up.
>>>> Add perf_limit_reasons to the existing warn print to see if frequency
>>>> is being throttled.
>>>>
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>>> index 364d0d546ec8..73911536a8e7 100644
>>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>>> @@ -254,6 +254,8 @@ static int guc_wait_ucode(struct intel_guc *guc)
>>>> 		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d]\n",
>>>> 			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
>>>> 			 before_freq, status, count, ret);
>>>> +		guc_warn(guc, "perf limit reasons = 0x%08X\n",
>>>> +			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
>>> Maybe just add at the end of the previous guc_warn?
>> Its already too long a line. If I try adding on the next line checkpatch
>> complains about splitting double quotes.
> In these cases of long quoted lines we generally ignore checkpatch. Because
> perf limit reasons is part of the "excessive init time" message it should
> be on the same line within the square brackets. So should not be
> splitting double quotes.
>
> Another idea would be something like this:
>
> 		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X]\n",
> 			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
> 			 before_freq, status);
> 		guc_warn(guc, "excessive init time: [count = %d, ret = %d, perf limit reasons = 0x%08X]\n",
> 			 count, ret, intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));

ok, I will split iut based on freq and non-freq based debug.

Thanks,

Vinay.

>
> Thanks.
> --
> Ashutosh
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 364d0d546ec8..73911536a8e7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -254,6 +254,8 @@  static int guc_wait_ucode(struct intel_guc *guc)
 		guc_warn(guc, "excessive init time: %lldms! [freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d]\n",
 			 delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),
 			 before_freq, status, count, ret);
+		guc_warn(guc, "perf limit reasons = 0x%08X\n",
+			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
 	} else {
 		guc_dbg(guc, "init took %lldms, freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d\n",
 			delta_ms, intel_rps_read_actual_frequency(&uncore->gt->rps),