Re: [PATCH 3/4] drm/xe: Drop usage of TIMESTAMP_OVERRIDE

["Vivekanandan, Balasubramani" <balasubramani.vivekanandan@intel.com>]

On 20.02.2025 16:38, Matt Roper wrote:
> On pre-Xe2 platforms, one of the approaches to initialize the GT command
> streamer frequency is to use the display reference clock.  That's no
> longer relevant from Xe2 onward (i.e., all of the platforms where Xe is
> officially supported).  Furthermore, use of TIMESTAMP_OVERRIDE to obtain
> the display reference clock is unnecessarily roundabout; the display
> driver already has a more reliable approach to obtain this value.  Let's
> use the display driver's existing logic to determine the proper display
> reference clock in the rare cases where the hardware indicates we should
> do this.
> 
> The one problem that arises here is if the Xe driver is run on an
> unsupported platform (i.e., pre-Xe2), CONFIG_DRM_XE_DISPLAY disabled
> (meaning we're not expecting to touch display hardware at all), and the
> platform has the rare CTC_MODE[1] setting indicating that display
> reference clock should be used as the GT CS refclk.  If all of these
> conditions are true, the hardcoded 38.4 MHz value will still be correct
> for DG2 and MTL platforms, but there's a chance that we might not have
> the right value on the older Xe_LP platforms if we're trying not to
> touch the display hardware at all.
> 
> Bspec: 62395
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Add my RB after addressing the warning from kernel test robot.

Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Regards,
Bala

> ---
>  drivers/gpu/drm/xe/display/xe_display.c |  6 ++++++
>  drivers/gpu/drm/xe/display/xe_display.h |  4 ++++
>  drivers/gpu/drm/xe/xe_gt_clock.c        | 28 ++++++++-----------------
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 02a413a07382..e35d58079f0d 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -17,6 +17,7 @@
>  #include "intel_acpi.h"
>  #include "intel_audio.h"
>  #include "intel_bw.h"
> +#include "intel_cdclk.h"
>  #include "intel_display.h"
>  #include "intel_display_driver.h"
>  #include "intel_display_irq.h"
> @@ -548,3 +549,8 @@ int xe_display_probe(struct xe_device *xe)
>  	unset_display_features(xe);
>  	return 0;
>  }
> +
> +u32 xe_display_get_refclk(struct xe_device *xe)
> +{
> +	return intel_display_get_refclk(&xe->display);
> +}
> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> index 685dc74402fb..b918f5d6b53a 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.h
> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> @@ -41,6 +41,8 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe);
>  void xe_display_pm_runtime_suspend_late(struct xe_device *xe);
>  void xe_display_pm_runtime_resume(struct xe_device *xe);
>  
> +u32 xe_display_get_refclk(struct xe_device *xe);
> +
>  #else
>  
>  static inline int xe_display_driver_probe_defer(struct pci_dev *pdev) { return 0; }
> @@ -72,5 +74,7 @@ static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>  static inline void xe_display_pm_runtime_suspend_late(struct xe_device *xe) {}
>  static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
>  
> +static u32 xe_display_get_refclk(struct xe_device *xe) { return 38400; }
> +
>  #endif /* CONFIG_DRM_XE_DISPLAY */
>  #endif /* _XE_DISPLAY_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_gt_clock.c b/drivers/gpu/drm/xe/xe_gt_clock.c
> index cc2ae159298e..b61f944a7b03 100644
> --- a/drivers/gpu/drm/xe/xe_gt_clock.c
> +++ b/drivers/gpu/drm/xe/xe_gt_clock.c
> @@ -7,6 +7,7 @@
>  
>  #include "xe_gt_clock.h"
>  
> +#include "display/xe_display.h"
>  #include "regs/xe_gt_regs.h"
>  #include "regs/xe_regs.h"
>  #include "xe_assert.h"
> @@ -15,22 +16,6 @@
>  #include "xe_macros.h"
>  #include "xe_mmio.h"
>  
> -static u32 read_reference_ts_freq(struct xe_gt *gt)
> -{
> -	u32 ts_override = xe_mmio_read32(&gt->mmio, TIMESTAMP_OVERRIDE);
> -	u32 base_freq, frac_freq;
> -
> -	base_freq = REG_FIELD_GET(TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK,
> -				  ts_override) + 1;
> -	base_freq *= 1000000;
> -
> -	frac_freq = REG_FIELD_GET(TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK,
> -				  ts_override);
> -	frac_freq = 1000000 / (frac_freq + 1);
> -
> -	return base_freq + frac_freq;
> -}
> -
>  static u32 get_crystal_clock_freq(u32 rpm_config_reg)
>  {
>  	const u32 f19_2_mhz = 19200000;
> @@ -57,14 +42,19 @@ static u32 get_crystal_clock_freq(u32 rpm_config_reg)
>  
>  int xe_gt_clock_init(struct xe_gt *gt)
>  {
> +	struct xe_device *xe = gt_to_xe(gt);
>  	u32 ctc_reg = xe_mmio_read32(&gt->mmio, CTC_MODE);
>  	u32 freq = 0;
>  
>  	/* Assuming gen11+ so assert this assumption is correct */
> -	xe_gt_assert(gt, GRAPHICS_VER(gt_to_xe(gt)) >= 11);
> +	xe_gt_assert(gt, GRAPHICS_VER(xe) >= 11);
>  
> -	if (ctc_reg & CTC_SOURCE_DIVIDE_LOGIC) {
> -		freq = read_reference_ts_freq(gt);
> +	/*
> +	 * Use of the display reference clock to determine GT CS frequency
> +	 * is only relevant to pre-Xe2 platforms.
> +	 */
> +	if (GRAPHICS_VER(xe) < 20 && ctc_reg & CTC_SOURCE_DIVIDE_LOGIC) {
> +		freq = xe_display_get_refclk(xe);
>  	} else {
>  		u32 c0 = xe_mmio_read32(&gt->mmio, RPM_CONFIG0);
>  
> -- 
> 2.48.1
>