>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Ville Syrjälä >Sent: Monday, September 25, 2017 6:46 PM >To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx> >Subject: Re: [PATCH] drm/i915: Enable scanline read based on frame >timestamps > >On Mon, Sep 25, 2017 at 04:23:30PM +0530, Vidya Srinivas wrote: >> From: Uma Shankar <uma.shankar@xxxxxxxxx> >> >> For certain platforms on certain encoders, timings are driven from >> port instead of pipe. Thus, we can't rely on pipe scanline registers >> to get the timing information. Some cases scanline register read will >> not be functional. >> This is causing vblank evasion logic to fail since it relies on >> scanline, causing atomic update failure warnings. >> >> This patch uses pipe framestamp and current timestamp registers to >> calculate scanline. This is an indirect way to get the scanline. >> It helps resolve atomic update failure for gen9 dsi platforms. >> >> v2: Addressed Ville and Daniel's review comments. Updated the register >> MACROs, handled race condition for register reads, extracted timings >> from the hwmode. Removed the dependency on >> crtc->config to get the encoder type. >> >> v3: Made get scanline function generic >> >> v4: Addressed Ville's review comments. Added a flag to decide >> timestamp based scanline reporting. Changed 64bit variables to u32 >> >> v5: Adressed Ville's review comments. Put the scanline compute >> function at the place of caller. Removed hwmode flags from uapi and >> used a local >> i915 data structure instead. >> >> v6: Used vblank hwmode to get the timings. >> >> v7: Fixed sparse warnings, indentation and minor review comments. >> >> Credits-to: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> >> Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx> >> Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx> >> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 53 >++++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_dsi.c | 7 +++++ >> 4 files changed, 71 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c >> b/drivers/gpu/drm/i915/i915_irq.c index 91a2c5d..04aa099 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -772,6 +772,56 @@ static u32 g4x_get_vblank_counter(struct drm_device >*dev, unsigned int pipe) >> return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); >> } >> >> +/* >> + * On certain encoders on certain platforms, pipe >> + * scanline register will not work to get the scanline, >> + * since the timings are driven from the PORT or issues >> + * with scanline register updates. >> + * This function will use Framestamp and current >> + * timestamp registers to calculate the scanline. >> + */ >> +static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc >> +*crtc) { >> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> + struct drm_vblank_crtc *vblank = >> + &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)]; >> + const struct drm_display_mode *mode = &vblank->hwmode; >> + u32 vblank_start = mode->crtc_vblank_start; >> + u32 vtotal = mode->crtc_vtotal; >> + u32 htotal = mode->crtc_htotal; >> + u32 clock = mode->crtc_clock; >> + u32 scanline, scan_prev_time, scan_curr_time, scan_post_time; >> + >> + /* To avoid the race condition where we might cross into the >> + * next vblank just between the PIPE_FRMTMSTMP and TIMESTAMP_CTR >> + * reads. We make sure we read PIPE_FRMTMSTMP and >TIMESTAMP_CTR >> + * during the same frame. >> + */ >> + do { >> + /* >> + * This field provides read back of the display >> + * pipe frame time stamp. The time stamp value >> + * is sampled at every start of vertical blank. >> + */ >> + scan_prev_time = I915_READ_FW(PIPE_FRMTMSTMP(crtc- >>pipe)); >> + >> + /* >> + * The TIMESTAMP_CTR register has the current >> + * time stamp value. >> + */ >> + scan_curr_time = I915_READ_FW(IVB_TIMESTAMP_CTR); >> + >> + scan_post_time = I915_READ_FW(PIPE_FRMTMSTMP(crtc- >>pipe)); >> + } while (scan_post_time != scan_prev_time); >> + >> + scanline = div_u64(mul_u32_u32(scan_curr_time - scan_prev_time, >> + clock), 1000 * htotal); >> + scanline = min(scanline, vtotal - 1); >> + scanline = (scanline + vblank_start) % vtotal; >> + >> + return scanline; >> +} >> + >> /* I915_READ_FW, only for fast reads of display block, no need for >> forcewake etc. */ static int __intel_get_crtc_scanline(struct >> intel_crtc *crtc) { @@ -788,6 +838,9 @@ static int >> __intel_get_crtc_scanline(struct intel_crtc *crtc) >> vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)]; >> mode = &vblank->hwmode; >> >> + if (mode->private_flags & >I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP) >> + return __intel_get_crtc_scanline_from_timestamp(crtc); >> + >> vtotal = mode->crtc_vtotal; >> if (mode->flags & DRM_MODE_FLAG_INTERLACE) >> vtotal /= 2; >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 9f03cd0..fbd00cc 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -8804,6 +8804,15 @@ enum skl_power_gate { >> #define MIPIO_TXESC_CLK_DIV2 _MMIO(0x160008) >> #define GLK_TX_ESC_CLK_DIV2_MASK 0x3FF >> >> +/* Gen4+ Timestamp and Pipe Frame time stamp registers */ >> +#define GEN4_TIMESTAMP _MMIO(0x2358) >> +#define ILK_TIMESTAMP_HI _MMIO(0x70070) >> +#define IVB_TIMESTAMP_CTR _MMIO(0x44070) >> + >> +#define _PIPE_FRMTMSTMP_A 0x70048 >> +#define PIPE_FRMTMSTMP(pipe) \ >> + _MMIO_PIPE2(pipe, _PIPE_FRMTMSTMP_A) >> + >> /* BXT MIPI clock controls */ >> #define BXT_MAX_VAR_OUTPUT_KHZ 39500 >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h >> b/drivers/gpu/drm/i915/intel_drv.h >> index 3078076..4170490 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -494,6 +494,8 @@ struct intel_crtc_scaler_state { >> >> /* drm_mode->private_flags */ >> #define I915_MODE_FLAG_INHERITED 1 >> +/* Flag to get scanline using frame time stamps */ #define >> +I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1) >> >> struct intel_pipe_wm { >> struct intel_wm_level wm[5]; >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c >> b/drivers/gpu/drm/i915/intel_dsi.c >> index 578254a..c189726 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -328,6 +328,9 @@ static bool intel_dsi_compute_config(struct >> intel_encoder *encoder, >> >> /* DSI uses short packets for sync events, so clear mode flags for DSI */ >> adjusted_mode->flags = 0; >> + /* Enable Frame time stamo based scanline reporting */ >> + adjusted_mode->private_flags |= >> + I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP; > >I just realized that this is in the wrong place. We don't want it for VLV/CHV. > Yes indeed. Will fix and resend. Regards, Uma Shankar >> >> if (IS_GEN9_LP(dev_priv)) { >> /* Dual link goes to DSI transcoder A. */ @@ -1102,6 +1105,10 >@@ >> static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, >> pixel_format_from_register_bits(fmt)); >> bpp = pipe_config->pipe_bpp; >> >> + /* Enable Frame time stamo based scanline reporting */ >> + adjusted_mode->private_flags |= >> + I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP; >> + >> /* In terms of pixels */ >> adjusted_mode->crtc_hdisplay = >> I915_READ(BXT_MIPI_TRANS_HACTIVE(port)); >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel OTC >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
