All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
@ 2017-03-07 20:54 ville.syrjala
  2017-03-07 21:18 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: ville.syrjala @ 2017-03-07 20:54 UTC (permalink / raw)
  To: intel-gfx

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.")
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);
 	}
 }
 
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-07 20:54 [PATCH] drm/i915: Nuke debug messages from the pipe update critical section ville.syrjala
@ 2017-03-07 21:18 ` Patchwork
  2017-03-08  8:19 ` [PATCH] " Jani Nikula
  2017-03-08  9:36 ` Maarten Lankhorst
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-03-07 21:18 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Nuke debug messages from the pipe update critical section
URL   : https://patchwork.freedesktop.org/series/20853/
State : success

== Summary ==

Series 20853v1 drm/i915: Nuke debug messages from the pipe update critical section
https://patchwork.freedesktop.org/api/1.0/series/20853/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 471s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 613s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 534s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 620s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 501s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 432s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 441s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 506s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 475s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 505s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 603s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 493s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 552s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 549s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 415s

dcca4ca8923adc61254f23eb66ca18a4c9e1ffd3 drm-tip: 2017y-03m-07d-18h-17m-03s UTC integration manifest
92f5123 drm/i915: Nuke debug messages from the pipe update critical section

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4089/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-07 20:54 [PATCH] drm/i915: Nuke debug messages from the pipe update critical section ville.syrjala
  2017-03-07 21:18 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-08  8:19 ` Jani Nikula
  2017-03-08 10:20   ` Ville Syrjälä
  2017-03-08  9:36 ` Maarten Lankhorst
  2 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2017-03-08  8:19 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-07 20:54 [PATCH] drm/i915: Nuke debug messages from the pipe update critical section ville.syrjala
  2017-03-07 21:18 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-03-08  8:19 ` [PATCH] " Jani Nikula
@ 2017-03-08  9:36 ` Maarten Lankhorst
  2017-03-08 11:12   ` Ville Syrjälä
  2 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2017-03-08  9:36 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

Op 07-03-17 om 21:54 schreef ville.syrjala@linux.intel.com:
> 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.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-08  8:19 ` [PATCH] " Jani Nikula
@ 2017-03-08 10:20   ` Ville Syrjälä
  2017-03-08 10:31     ` Jani Nikula
  2017-03-08 10:38     ` Daniel Vetter
  0 siblings, 2 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-03-08 10:20 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Mar 08, 2017 at 10:19:39AM +0200, Jani Nikula wrote:
> 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?

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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-08 10:20   ` Ville Syrjälä
@ 2017-03-08 10:31     ` Jani Nikula
  2017-03-08 10:38     ` Daniel Vetter
  1 sibling, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2017-03-08 10:31 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Wed, 08 Mar 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Mar 08, 2017 at 10:19:39AM +0200, Jani Nikula wrote:
>> 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?
>
> 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.

Nice typo there! ;) But make it so.

BR,
Jani.


>
>> 
>> 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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-08 10:20   ` Ville Syrjälä
  2017-03-08 10:31     ` Jani Nikula
@ 2017-03-08 10:38     ` Daniel Vetter
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2017-03-08 10:38 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Wed, Mar 08, 2017 at 12:20:53PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2017 at 10:19:39AM +0200, Jani Nikula wrote:
> > 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?
> 
> 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.

+1 on cc: stable from me.

Also, is there any way we could try to prevent this? One idea that crossed
my mind is to create a set of forbid/allow_printk() functions, which
simple fake-take the printk buffer lock using lockdep (but not take the
lock for real, because that's where the stalls are from). Since we already
disable irqs there's no problem with recursion into irq handlers which
might call printk (and upset the lockdep rules), we just need to nest
forbid/allow_printk within the irq disabled section.

Can I volunteer to try this out pls?

Thanks, Daniel

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
  2017-03-08  9:36 ` Maarten Lankhorst
@ 2017-03-08 11:12   ` Ville Syrjälä
  0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-03-08 11:12 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Wed, Mar 08, 2017 at 10:36:15AM +0100, Maarten Lankhorst wrote:
> Op 07-03-17 om 21:54 schreef ville.syrjala@linux.intel.com:
> > 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.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Pushed to dinq with cc:stable. Thanls for the review.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-03-08 11:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07 20:54 [PATCH] drm/i915: Nuke debug messages from the pipe update critical section ville.syrjala
2017-03-07 21:18 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-03-08  8:19 ` [PATCH] " Jani Nikula
2017-03-08 10:20   ` Ville Syrjälä
2017-03-08 10:31     ` Jani Nikula
2017-03-08 10:38     ` Daniel Vetter
2017-03-08  9:36 ` Maarten Lankhorst
2017-03-08 11:12   ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.