[PATCH v2 1/3] drm/xe: Move display reference timestamp readout to display/
Matt Roper
matthew.d.roper at intel.com
Mon Sep 16 20:39:12 UTC 2024
On Mon, Sep 16, 2024 at 02:09:45PM -0400, Rodrigo Vivi wrote:
> On Fri, Sep 13, 2024 at 09:29:11AM -0700, Matt Roper wrote:
> > It's quite unusual to read display registers as part of GT
> > initialization, but use of the display reference timestamp is one
> > approach to calculating the GT clock frequency on older platforms.
> > Rename the function that does this readout and move it to display/ to
> > make it more clear what's actually happening when this route is taken.
> > Also add an assert that we've probed display before calling this
> > function since we never expect this to be the route taken on platforms
> > that lack display.
>
> oh! that's ugly.
> How much of this calculations and clocks are actually a coincidence then?
>
> Shouldn't we be using a GT based timestamp then?
> Or perhaps a MCHBAR one? But why display timestamp would give anything
> GT related?
We choose whether or not to use this register based on CTC_MODE[0]. The
two options are either "Crystal Clock Pin Source (default)" or
"Broadwell Divide Logic." The description of the latter doesn't really
mean anything to me and doesn't seem to be explained further anywhere in
the bspec that I can find, but presumably it means the clock is tied to
some soc-level clock that also happens to be the same one used as the
display's reference clock. In fact I suspect that DSSM[31:29] would
have worked just as well for this purpose as TIMESTAMP_OVERRIDE[9:0]
does, but that's still just another display register. That also likely
means that if we didn't still care about gen12lp platforms in Xe, we
could probably avoid reading the register at all. I.e., everything
Xe_HP and later has only had a single display reference clock (38.4 MHz)
and we could probably just hardcode that value if we see the "Broadwell
Divide Logic" setting.
The use of this register was introduced (in i915) in commit dab91783338b
("drm/i915: expose command stream timestamp frequency to userspace").
Adding Lionel to the Cc list just in case he remembers any more details
about where the "use display register when CTC_MODE[0] says so"
information was documented at the time.
Matt
>
> >
> > In the future we may want to move to an intel_display implementation
> > that can be shared with i915, but we'll leave that for later.
> >
> > Suggested-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> > drivers/gpu/drm/xe/display/xe_display.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/xe/display/xe_display.h | 4 ++++
> > drivers/gpu/drm/xe/xe_gt_clock.c | 24 ++++++------------------
> > 3 files changed, 28 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> > index a3131a67e5b1..ac6d08a5cc73 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -29,6 +29,7 @@
> > #include "intel_hdcp.h"
> > #include "intel_hotplug.h"
> > #include "intel_opregion.h"
> > +#include "xe_mmio.h"
> > #include "xe_module.h"
> >
> > /* Xe device functions */
> > @@ -510,3 +511,20 @@ int xe_display_probe(struct xe_device *xe)
> > unset_display_features(xe);
> > return 0;
> > }
> > +
> > +u32 xe_display_read_ref_ts_freq(struct xe_device *xe)
> > +{
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + u32 ts_override = xe_mmio_read32(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;
> > +}
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> > index 17afa537aee5..40030cac7fe9 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > @@ -43,6 +43,8 @@ void xe_display_pm_resume(struct xe_device *xe);
> > void xe_display_pm_runtime_suspend(struct xe_device *xe);
> > void xe_display_pm_runtime_resume(struct xe_device *xe);
> >
> > +u32 xe_display_read_ref_ts_freq(struct xe_device *xe);
> > +
> > #else
> >
> > static inline int xe_display_driver_probe_defer(struct pci_dev *pdev) { return 0; }
> > @@ -76,5 +78,7 @@ static inline void xe_display_pm_resume(struct xe_device *xe) {}
> > static inline void xe_display_pm_runtime_suspend(struct xe_device *xe) {}
> > static inline void xe_display_pm_runtime_resume(struct xe_device *xe) {}
> >
> > +static u32 xe_display_read_ref_ts_freq(struct xe_device *xe) { return 0; }
> > +
> > #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..886c071c10f5 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(>->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,17 @@ 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(>->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);
> > + xe_gt_assert(gt, xe->info.probe_display);
> > +
> > + freq = xe_display_read_ref_ts_freq(xe);
> > } else {
> > u32 c0 = xe_mmio_read32(>->mmio, RPM_CONFIG0);
> >
> > --
> > 2.45.2
> >
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list