On Mon, Sep 11, 2017 at 01:19:15PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > >Daniel Vetter > >Sent: Saturday, September 9, 2017 1:15 AM > >To: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Srinivas, Vidya <vidya.srinivas@xxxxxxxxx> > >Subject: Re: [PATCH] drm/i915: Enable scanline read for gen9 dsi > > > >On Fri, Sep 08, 2017 at 05:55:24PM +0300, Ville Syrjälä wrote: > >> On Fri, Sep 08, 2017 at 05:47:59PM +0300, Ville Syrjälä wrote: > >> > On Fri, Sep 08, 2017 at 07:18:55PM +0530, Vidya Srinivas wrote: > >> > > From: Uma Shankar <uma.shankar@xxxxxxxxx> > >> > > > >> > > For gen9 platforms, dsi timings are driven from port instead of > >> > > pipe (unlike ddi). Thus, we can't rely on pipe registers to get > >> > > the timing information. Even 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. > >> > > > >> > > 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_drv.h | 2 ++ > >> > > drivers/gpu/drm/i915/i915_irq.c | 5 +++++ > >> > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > >> > > drivers/gpu/drm/i915/intel_dsi.c | 46 > >> > > ++++++++++++++++++++++++++++++++++++++++ > >> > > 4 files changed, 56 insertions(+) > >> > > > >> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > >> > > b/drivers/gpu/drm/i915/i915_drv.h index d07d110..4213b54 100644 > >> > > --- a/drivers/gpu/drm/i915/i915_drv.h > >> > > +++ b/drivers/gpu/drm/i915/i915_drv.h > >> > > @@ -4077,6 +4077,8 @@ void intel_sbi_write(struct drm_i915_private > >> > > *dev_priv, u16 reg, u32 value, > >> > > u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); > >> > > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, > >> > > u32 val); > >> > > > >> > > +u32 bxt_dsi_get_scanline(struct intel_crtc *crtc); > >> > > + > >> > > /* intel_dpio_phy.c */ > >> > > void bxt_port_to_phy_channel(struct drm_i915_private *dev_priv, enum > >port port, > >> > > enum dpio_phy *phy, enum dpio_channel *ch); diff -- > >git > >> > > a/drivers/gpu/drm/i915/i915_irq.c > >> > > b/drivers/gpu/drm/i915/i915_irq.c index 5d391e6..31aa7f0 100644 > >> > > --- a/drivers/gpu/drm/i915/i915_irq.c > >> > > +++ b/drivers/gpu/drm/i915/i915_irq.c > >> > > @@ -781,6 +781,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc > >*crtc) > >> > > struct drm_vblank_crtc *vblank; > >> > > enum pipe pipe = crtc->pipe; > >> > > int position, vtotal; > >> > > + enum transcoder cpu_transcoder; > >> > > > >> > > if (!crtc->active) > >> > > return -1; > >> > > @@ -792,6 +793,10 @@ static int __intel_get_crtc_scanline(struct > >intel_crtc *crtc) > >> > > if (mode->flags & DRM_MODE_FLAG_INTERLACE) > >> > > vtotal /= 2; > >> > > > >> > > + cpu_transcoder = crtc->config->cpu_transcoder; > >> > > >> > Humm. Would be nice to be able to do this without adding more > >> > crtc->config uses. We're pretty much trying to get rid of that guy. > >> > > >> > > + if (IS_BROXTON(dev_priv) && transcoder_is_dsi(cpu_transcoder)) > >> > > + return bxt_dsi_get_scanline(crtc); > >> > > + > >> > > if (IS_GEN2(dev_priv)) > >> > > position = I915_READ_FW(PIPEDSL(pipe)) & > >DSL_LINEMASK_GEN2; > >> > > else > >> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > >> > > b/drivers/gpu/drm/i915/i915_reg.h index 9a73ea0..54582de 100644 > >> > > --- a/drivers/gpu/drm/i915/i915_reg.h > >> > > +++ b/drivers/gpu/drm/i915/i915_reg.h > >> > > @@ -8802,6 +8802,9 @@ enum skl_power_gate { > >> > > #define MIPIO_TXESC_CLK_DIV2 _MMIO(0x160008) > >> > > #define GLK_TX_ESC_CLK_DIV2_MASK 0x3FF > >> > > > >> > > +#define BXT_TIMESTAMP_CTR _MMIO(0x44070) > >> > > +#define BXT_PIPE_FRMTMSTMP_A _MMIO(0x70048) > >> > > >> > Please add proper parametrized define that works for all pipes. > >> > >> Oh, and these shouldn't be called BXT_something. I don't recall when > >> they got added to the hardware, but I'm pretty sure it was way before > >> BXT came out. > > > >gen5 or maybe gen4.5 iirc. > > > > As per spec, it says BDW+. Will rename them using BDW as prefix. You're not looking at the right spec then. Looks like ctg/elk is the right answer indeed. The gen4 spec is a bit confused in that it doesn't correctly state whether some of the regs apply to gen4 or ctg/elk. But testing on actual gen4 hardware gives me 0s from all the pipe timestamp registers, whereas elk gives sane looking value. The TIMESTAMP register did exist on gen4 already. There have been some changes in the TIMESTAMP register(s) throughout the years though. I'll try to summarize below: bw/cl: TIMESTAMP/0x2358. 64bit register that increments every 16 hclks ctg/elk: TIMESTAMP/0x2358. 64bit register where the upper 32 bits increment every 1.024us, the lower part has more 12 bits which means the whole value increments every 1/4 ns ilk/snb: TIMESTAMP/0x2358 "64bit" register where the upprt 32 bits increment every 1 us. Lower part is documented as MBZ. The upprt part is aliased as TIMESTAMP_HI high at offset 0x70070 ivb+: TIMESTAMP_HI got renamed to TIMESTAMP_CTR and moved to 0x44070 The pipe flip/frame timestamp registers seem to have remained unchanged ever since ctg/elk. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
