>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Monday, September 11, 2017 11:51 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: Daniel Vetter <daniel@xxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Srinivas, >Vidya <vidya.srinivas@xxxxxxxxx> >Subject: Re: [PATCH] drm/i915: Enable scanline read for gen9 dsi > >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. > Thanks Ville for this info. Will update the Current Timestamp register for Gen4 and Gen7 (declare as separate macros). Also will define Frame Timestamp generically. >-- >Ville Syrjälä >Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
