On Wed, Mar 07, 2018 at 03:23:12PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 07, 2018 at 01:10:26PM +0100, Maarten Lankhorst wrote:
> > This will get rid of the following error:
> > [   74.730271] WARNING: CPU: 4 PID: 0 at drivers/gpu/drm/drm_vblank.c:614 
> > drm_calc_vbltimestamp_from_scanoutpos+0x13e/0x2f0
> > [   74.730311] Modules linked in: vgem snd_hda_codec_hdmi 
> > snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal 
> > intel_powerclamp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec 
> > crc32_pclmul snd_hwdep broadcom ghash_clmulni_intel snd_hda_core 
> > bcm_phy_lib snd_pcm tg3 lpc_ich mei_me mei prime_numbers
> > [   74.730353] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G     U           
> > 4.16.0-rc2-CI-CI_DRM_3822+ #1
> > [   74.730355] Hardware name: Dell Inc. XPS 8300  /0Y2MRG, BIOS A06 
> > 10/17/2011
> > [   74.730359] RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x13e/0x2f0
> > [   74.730361] RSP: 0018:ffff88022fb03d10 EFLAGS: 00010086
> > [   74.730365] RAX: ffffffffa0291d20 RBX: ffff88021a180000 RCX: 
> > 0000000000000001
> > [   74.730367] RDX: ffffffff820e7db8 RSI: 0000000000000001 RDI: 
> > ffffffff82068cea
> > [   74.730369] RBP: ffff88022fb03d70 R08: 0000000000000000 R09: 
> > ffffffff815d26d0
> > [   74.730371] R10: 0000000000000000 R11: ffffffffa0161ca0 R12: 
> > 0000000000000001
> > [   74.730373] R13: ffff880212448008 R14: ffff880212448330 R15: 
> > 0000000000000000
> > [   74.730376] FS:  0000000000000000(0000) GS:ffff88022fb00000(0000) 
> > knlGS:0000000000000000
> > [   74.730378] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   74.730380] CR2: 000055edcbec9000 CR3: 0000000002210001 CR4: 
> > 00000000000606e0
> > [   74.730382] Call Trace:
> > [   74.730385]  <IRQ>
> > [   74.730397]  drm_get_last_vbltimestamp+0x36/0x50
> > [   74.730401]  drm_update_vblank_count+0x64/0x240
> > [   74.730409]  drm_crtc_accurate_vblank_count+0x41/0x90
> > [   74.730453]  display_pipe_crc_irq_handler+0x176/0x220 [i915]
> > [   74.730497]  i9xx_pipe_crc_irq_handler+0xfe/0x150 [i915]
> > [   74.730537]  ironlake_irq_handler+0x618/0xa30 [i915]
> > [   74.730548]  __handle_irq_event_percpu+0x3c/0x340
> > [   74.730556]  handle_irq_event_percpu+0x1b/0x50
> > [   74.730561]  handle_irq_event+0x2f/0x50
> > [   74.730566]  handle_edge_irq+0xe4/0x1b0
> > [   74.730572]  handle_irq+0x11/0x20
> > [   74.730576]  do_IRQ+0x5e/0x120
> > [   74.730584]  common_interrupt+0x84/0x84
> > [   74.730586]  </IRQ>
> > [   74.730591] RIP: 0010:cpuidle_enter_state+0xaa/0x350
> > [   74.730593] RSP: 0018:ffffc9000008beb8 EFLAGS: 00000212 ORIG_RAX: 
> > ffffffffffffffde
> > [   74.730597] RAX: ffff880226b80040 RBX: 000000000031fc3e RCX: 
> > 0000000000000001
> > [   74.730599] RDX: 0000000000000000 RSI: ffffffff8210fb59 RDI: 
> > ffffffff820c02e7
> > [   74.730601] RBP: 0000000000000004 R08: 00000000000040af R09: 
> > 0000000000000018
> > [   74.730603] R10: 0000000000000000 R11: 0000000000000000 R12: 
> > 0000000000000004
> > [   74.730606] R13: ffffe8ffffd00430 R14: 0000001166120bf4 R15: 
> > ffffffff82294460
> > [   74.730621]  ? cpuidle_enter_state+0xa6/0x350
> > [   74.730629]  do_idle+0x188/0x1d0
> > [   74.730636]  cpu_startup_entry+0x14/0x20
> > [   74.730641]  start_secondary+0x129/0x160
> > [   74.730646]  secondary_startup_64+0xa5/0xb0
> > [   74.730660] Code: e1 48 c7 c2 b8 7d 0e 82 be 01 00 00 00 48 c7 c7 ea 8c 
> > 06 82 e8 64 ec ff ff 48 8b 83 c8 07 00 00 48 83 78 28 00 0f 84 e2 fe ff ff 
> > <0f> 0b 45 31 ed e9 db fe ff ff 41 b8 d3 4d 62 10 89 c8 6a 03 41
> > [   74.730754] ---[ end trace 14b1345705b68565 ]---
> > 
> > Changes since v1:
> > - Don't try to apply CRC workaround when enabling pipe, it should already 
> > be enabled.
> > 
> > Cc: Marta Löfstedt <marta.lofst...@intel.com>
> > Reported-by: Marta Löfstedt <marta.lofst...@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105185
> > Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c  |  2 ++
> >  drivers/gpu/drm/i915/intel_drv.h      |  3 +++
> >  drivers/gpu/drm/i915/intel_pipe_crc.c | 44 
> > +++++++++++++++++++++++++++++------
> >  3 files changed, 42 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 331084082545..67d13fe4c24f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12147,6 +12147,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
> >     if (modeset) {
> >             update_scanline_offset(intel_crtc);
> >             dev_priv->display.crtc_enable(pipe_config, state);
> > +           intel_crtc_enable_pipe_crc(crtc);
> 
> I think I'd prefer this to live next to the vblank on/off, with a
> comment. Putting it here gives absolutely no clue why we have to
> do it.

...or at least we need a comment here. 

> 
> >     } else {
> >             intel_pre_plane_update(to_intel_crtc_state(old_crtc_state),
> >                                    pipe_config);
> > @@ -12327,6 +12328,7 @@ static void intel_atomic_commit_tail(struct 
> > drm_atomic_state *state)
> >  
> >             if (old_crtc_state->active) {
> >                     intel_crtc_disable_planes(crtc, 
> > old_crtc_state->plane_mask);
> > +                   intel_crtc_disable_pipe_crc(crtc);
> >                     
> > dev_priv->display.crtc_disable(to_intel_crtc_state(old_crtc_state), state);
> >                     intel_crtc->active = false;
> >                     intel_fbc_disable(intel_crtc);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index d4368589b355..002429957dd5 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -2138,8 +2138,11 @@ int intel_pipe_crc_create(struct drm_minor *minor);
> >  #ifdef CONFIG_DEBUG_FS
> >  int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char 
> > *source_name,
> >                           size_t *values_cnt);
> > +void intel_crtc_disable_pipe_crc(struct drm_crtc *crtc);
> > +void intel_crtc_enable_pipe_crc(struct drm_crtc *crtc);

Pass intel_crtc pls.

> >  #else
> >  #define intel_crtc_set_crc_source NULL
> > +#define intel_crtc_disable_pipe_crc(crtc) do {} while (0)

static inline ?

enable?

> >  #endif
> >  extern const struct file_operations i915_display_crc_ctl_fops;
> >  #endif /* __INTEL_DRV_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
> > b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > index 1f5cd572a7ff..9422ed98e987 100644
> > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > @@ -569,7 +569,8 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private 
> > *dev_priv,
> >  static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> >                             enum pipe pipe,
> >                             enum intel_pipe_crc_source *source,
> > -                           uint32_t *val)
> > +                           uint32_t *val,
> > +                           bool set_wa)
> >  {
> >     if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> >             *source = INTEL_PIPE_CRC_SOURCE_PF;
> > @@ -582,7 +583,7 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private 
> > *dev_priv,
> >             *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB;
> >             break;
> >     case INTEL_PIPE_CRC_SOURCE_PF:
> > -           if ((IS_HASWELL(dev_priv) ||
> > +           if (set_wa && (IS_HASWELL(dev_priv) ||
> >                  IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
> >                     hsw_pipe_A_crc_wa(dev_priv, true);
> >  
> > @@ -600,7 +601,8 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private 
> > *dev_priv,
> >  
> >  static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
> >                            enum pipe pipe,
> > -                          enum intel_pipe_crc_source *source, u32 *val)
> > +                          enum intel_pipe_crc_source *source, u32 *val,
> > +                          bool set_wa)
> >  {
> >     if (IS_GEN2(dev_priv))
> >             return i8xx_pipe_crc_ctl_reg(source, val);
> > @@ -611,7 +613,7 @@ static int get_new_crc_ctl_reg(struct drm_i915_private 
> > *dev_priv,
> >     else if (IS_GEN5(dev_priv) || IS_GEN6(dev_priv))
> >             return ilk_pipe_crc_ctl_reg(source, val);
> >     else
> > -           return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
> > +           return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val, 
> > set_wa);
> >  }
> >  
> >  static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> > @@ -636,7 +638,7 @@ static int pipe_crc_set_source(struct drm_i915_private 
> > *dev_priv,
> >             return -EIO;
> >     }
> >  
> > -   ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
> > +   ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val, true);
> >     if (ret != 0)
> >             goto out;
> >  
> > @@ -916,7 +918,7 @@ int intel_pipe_crc_create(struct drm_minor *minor)
> >  int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char 
> > *source_name,
> >                           size_t *values_cnt)
> >  {
> > -   struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> >     struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index];
> >     enum intel_display_power_domain power_domain;
> >     enum intel_pipe_crc_source source;
> > @@ -934,7 +936,7 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, 
> > const char *source_name,
> >             return -EIO;
> >     }
> >  
> > -   ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val);
> > +   ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val, true);
> >     if (ret != 0)
> >             goto out;
> >  
> > @@ -959,3 +961,31 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, 
> > const char *source_name,
> >  
> >     return ret;
> >  }
> > +
> > +void intel_crtc_enable_pipe_crc(struct drm_crtc *crtc)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> > +   enum intel_pipe_crc_source source;
> > +   u32 val = 0;
> > +
> > +   if (!crtc->crc.opened)
> > +           return;
> > +
> > +   if (display_crc_ctl_parse_source(crtc->crc.source, &source) < 0)
> > +           return;
> > +
> > +   if (get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val, false) < 
> > 0)
> > +           return;
> > +
> > +   I915_WRITE(PIPE_CRC_CTL(crtc->index), val);
> > +   POSTING_READ(PIPE_CRC_CTL(crtc->index));
> > +}
> > +
> > +void intel_crtc_disable_pipe_crc(struct drm_crtc *crtc)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> > +
> > +   I915_WRITE(PIPE_CRC_CTL(crtc->index), 0);
> > +   POSTING_READ(PIPE_CRC_CTL(crtc->index));
> > +   synchronize_irq(dev_priv->drm.irq);
> > +}
> > -- 
> > 2.16.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to