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.
> Another thought that just occurred to me: Maybe we could use these
> timestamps as a workaround for the DDI "scanline reads as 0 at the
> wrong time" problem. What we could do is check of the scanline counter
> reads as 0, and if it does we could switch over to checking the
> timestamps instead. Not sure if we should just do the full timestamp
> based scanline read like you do here, or we could just check that if the
> timestamps look like they're close to vblank_start we just return
> vblank_start-1. This could then remove the obnoxious retry loop from the
> scanline counter read.
Another concern I have on this is timeframe jitter. If the vblank
timestamp stuff isnt' perfectly accurately spaced, or we have a mismatch
in clocks, then we might think there's still plenty of time before vblank
while we're already racing.
Just a bit of testing won't catch that all that easily unfortunately, and
I'm not sure we have any good igts to stress-test this stuff. I'd advocate
we're playing it defensive and increase the vblank evasion window for this
trick quite a bit to stay on the safe side (maybe 1 full ms or so). Or
much, much better testing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
