[Intel-gfx] [PATCH] drm/i915: Nuke debug messages from the pipe update critical section

[Ville Syrjälä]

On Wed, Mar 08, 2017 at 10:19:39AM +0200, Jani Nikula wrote:
> On Tue, 07 Mar 2017, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > printks are slow so we should not be doing them from the vblank evade
> > critical section. These could explain why we sometimes seem to
> > blow past our 100 usec deadline.
> >
> > The problem has been there ever since commit bfd16b2a23dc ("drm/i915:
> > Make updating pipe without modeset atomic.") but it may not have
> > been readily visible until commit e1edbd44e23b ("drm/i915: Complain
> > if we take too long under vblank evasion.") increased our chances
> > of noticing it.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Fixes: bfd16b2a23dc ("drm/i915: Make updating pipe without modeset atomic.")
> 
> Is this not worth it for cc: stable, because e1edbd44e23b is not in
> Linus' tree?

Hmm. We did have the other error message about exceeding the deadline
already before, so it's possible that this might have caused some noise
even before the e1edbd44e23b. Also removing printk()s can do harm 
(famous last words, right?) so we might as well cc:stable I suppose.

> 
> BR,
> Jani.
> 
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e77ca7dfa44d..726ae191076b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3650,10 +3650,6 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> >  	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> >  	crtc->base.mode = crtc->base.state->mode;
> >  
> > -	DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n",
> > -		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
> > -		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
> > -
> >  	/*
> >  	 * Update pipe size and adjust fitter if needed: the reason for this is
> >  	 * that in compute_mode_changes we check the native mode (not the pfit
> > @@ -4775,23 +4771,17 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)
> >  	struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc->config->scaler_state;
> >  
> > -	DRM_DEBUG_KMS("for crtc_state = %p\n", crtc->config);
> > -
> >  	if (crtc->config->pch_pfit.enabled) {
> >  		int id;
> >  
> > -		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) {
> > -			DRM_ERROR("Requesting pfit without getting a scaler first\n");
> > +		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
> >  			return;
> > -		}
> >  
> >  		id = scaler_state->scaler_id;
> >  		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> >  			PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> >  		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos);
> >  		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size);
> > -
> > -		DRM_DEBUG_KMS("for crtc_state = %p scaler_id = %d\n", crtc->config, id);
> >  	}
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC