Re: [Intel-xe] [PATCH v3 1/2] drm/xe: Get GT clock to nanosecs

["Iddamsetty, Aravind" <aravind.iddamsetty@intel.com>]

On 09-08-2023 10:38, Dixit, Ashutosh wrote:
> On Tue, 08 Aug 2023 04:54:35 -0700, Aravind Iddamsetty wrote:
>>
>> Helpers to get GT clock to nanosecs
> 
> Helper to convert GT clock cycles to nanoseconds.

ok.
> 
>> v2: Use DIV_ROUND_CLOSEST_ULL helper(Ashutosh)
>>
>> Reviewed-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt_clock.c | 5 +++++
>>  drivers/gpu/drm/xe/xe_gt_clock.h | 4 +++-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_clock.c b/drivers/gpu/drm/xe/xe_gt_clock.c
>> index 2f77b8bbcf53..88c29ad84146 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_clock.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_clock.c
>> @@ -78,3 +78,8 @@ int xe_gt_clock_init(struct xe_gt *gt)
>> 	gt->info.clock_freq = freq;
>> 	return 0;
>>  }
>> +
>> +u64 xe_gt_clock_interval_to_ns(const struct xe_gt *gt, u64 count)
> 
> I know this name is used in i915 but my 2 cents on this is that a better
> name is 'xe_gt_clock_cycles_to_ns', since count is the number of clock
> cycles. So optional but you may consider this name.

hmmm.
> 
> 
>> +{
>> +	return DIV_ROUND_CLOSEST_ULL(count * NSEC_PER_SEC, gt->info.clock_freq);
>> +}
> 
> 
>> diff --git a/drivers/gpu/drm/xe/xe_gt_clock.h b/drivers/gpu/drm/xe/xe_gt_clock.h
>> index 511923afd224..91fc9b7e83f5 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_clock.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_clock.h
>> @@ -6,8 +6,10 @@
>>  #ifndef _XE_GT_CLOCK_H_
>>  #define _XE_GT_CLOCK_H_
>>
>> +#include <linux/types.h>
> 
> This is for HDRTEST I guess?

for u64 below.
> 
>> +
>>  struct xe_gt;
>>
>>  int xe_gt_clock_init(struct xe_gt *gt);
>> -
>> +u64 xe_gt_clock_interval_to_ns(const struct xe_gt *gt, u64 count);
>>  #endif
>> --
>> 2.25.1
>>
> 
> Also not sure if this merits a patch on its own so can be merged into the
> giant PMU patch if you wish.

ok will think about it.
> 
> In any case, with the optional tweaks above, this is:>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

Thanks for the review.

Aravind.