[Intel-gfx] [PATCH v5] drm/i915: Enable scanline read based on frame timestamps
Saarinen, Jani
jani.saarinen at intel.com
Sat Sep 23 07:34:37 UTC 2017
HI,
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of
> Ville Syrjälä
> Sent: perjantai 22. syyskuuta 2017 18.48
> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Syrjala, Ville <ville.syrjala at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v5] drm/i915: Enable scanline read based on
> frame timestamps
>
> On Fri, Sep 22, 2017 at 09:11:30PM +0530, Vidya Srinivas wrote:
> > From: Uma Shankar <uma.shankar at intel.com>
> >
> > 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 flag from uapi and
> > used a local private_flag instead.
> >
> > Credits-to: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 50
> ++++++++++++++++++++++++++++++++++++++
> > 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, 68 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c index 91a2c5d..6d7cd20 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -772,6 +772,53 @@ 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.
> > + */
> > +u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
>
> static
>
> I think sparse would have caught that. Please compile w/ C=1 to make sure
> sparse stays happy. (+ run checkpatch ofc)
>
>
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + u32 vblank_start = crtc->base.hwmode.crtc_vblank_start;
> > + u32 vtotal = crtc->base.hwmode.crtc_vtotal;
> > + u32 htotal = crtc->base.hwmode.crtc_htotal;
> > + u32 clock = crtc->base.hwmode.crtc_clock;
> > + u32 scanline = 0, 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 +835,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 &
> DRM_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..ca458f5 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
> > +DRM_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
>
> s/DRM_/I915_/
>
> With that this is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Does it still work? :)
According to try-bot, yes:
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_1180/
>
> >
> > 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..f91ccc7 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 |=
> > +
> DRM_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
> >
> > 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 |=
> > +
> DRM_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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list