Re: [PATCH v5] drm/i915: Enable scanline read based on frame timestamps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



HI, 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
> Ville Syrjälä
> Sent: perjantai 22. syyskuuta 2017 18.48
> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>
> Subject: Re:  [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@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 flag from uapi and
> > used a local private_flag instead.
> >
> > 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>
> > ---
> >  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@xxxxxxxxxxxxxxx>
> 
> 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@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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux