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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Nuke debug messages from the pipe	update critical section
Date: Wed, 08 Mar 2017 10:19:39 +0200	[thread overview]
Message-ID: <87bmtcji3o.fsf@intel.com> (raw)
In-Reply-To: <20170307205419.19447-1-ville.syrjala@linux.intel.com>

On Tue, 07 Mar 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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@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?

BR,
Jani.

> Signed-off-by: Ville Syrjälä <ville.syrjala@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx