[Intel-gfx] [PATCH 12/16] drm/i915: Disable LP1+ watermarks while changing the number of active pipes
From: Ville Syrjälä
When we switch between one active pipe and multiple active pipes, the
display FIFO gets repartitioned. Disable the LP1+ waterwarks while that
is happening to make sure we don't get any glitches on other active
pipes while doing a modeset on another other pipe.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 45 ++
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 47
3 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bccf414..311c0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3985,6 +3985,27 @@ static struct intel_crtc *get_other_active_crtc(struct
intel_crtc *crtc)
return other_active_crtc;
}
+static void ilk_prepare_for_num_pipes_change(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc->base.dev;
+ struct intel_crtc *other_active_crtc = get_other_active_crtc(crtc);
+
+ /*
+* If we change between one pipe and multiple pipes,
+* make sure the other active pipe is prepared
+* for having its FIFO resized. We do that by making
+* sure the pipe isn't using LP1+ watermarks when
+* the second pipe gets enabled or disabled.
+*/
+ if (!other_active_crtc)
+ return;
+
+ ilk_wm_synchronize(other_active_crtc);
+
+ if (ilk_disable_lp_wm(dev))
+ intel_wait_for_vblank(dev, other_active_crtc->pipe);
+}
+
static void ironlake_crtc_enable(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -4023,6 +4044,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
intel_crtc->active = true;
+ /* Make sure other pipes are prepared for FIFO resizing */
+ ilk_prepare_for_num_pipes_change(intel_crtc);
+
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
@@ -4123,6 +4147,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_crtc->active = true;
+ /* Make sure other pipes are prepared for FIFO resizing */
+ ilk_prepare_for_num_pipes_change(intel_crtc);
+
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
if (intel_crtc->config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
@@ -4192,6 +4219,9 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
intel_crtc_disable_planes(crtc);
+ /* Make sure other pipes are prepared for FIFO resizing */
+ ilk_prepare_for_num_pipes_change(intel_crtc);
+
for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->disable(encoder);
@@ -4235,6 +4265,12 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
intel_crtc->active = false;
+ /*
+* Potentially let some other pipe use the
+* full FIFO, and re-enable LP1+ watermarks.
+*/
+ ilk_wm_pipe_post_disable(intel_crtc);
+
mutex_lock(&dev->struct_mutex);
intel_update_fbc(dev);
intel_edp_psr_update(dev);
@@ -4255,6 +4291,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
intel_crtc_disable_planes(crtc);
+ /* Make sure other pipes are prepared for FIFO resizing */
+ ilk_prepare_for_num_pipes_change(intel_crtc);
+
for_each_encoder_on_crtc(dev, crtc, encoder) {
intel_opregion_notify_encoder(encoder, false);
encoder->disable(encoder);
@@ -4282,6 +4321,12 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
intel_crtc->active = false;
+ /*
+* Potentially let some other pipe use the
+* full FIFO, and re-enable LP1+ watermarks.
+*/
+ ilk_wm_pipe_post_disable(intel_crtc);
+
mutex_lock(&dev->struct_mutex);
intel_update_fbc(dev);
intel_edp_psr_update(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ad7ad5..98f878f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1022,6 +1022,8 @@ void intel_program_watermarks_pre(struct intel_crtc *crtc,
void intel_program_watermarks_post(struct intel_crtc *crtc,
const struct intel_crtc_wm_config *config);
void ilk_wm_synchronize(struct intel_crtc *crtc);
+void ilk_wm_pipe_post_disable(struct intel_crtc *crtc);
+bool ilk_disable_lp_wm(struct drm_device *dev);
/* intel_sdvo.c */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1c072cd..18ea8b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2765,7 +2765,7 @@ static void ilk_write_wm_values(struct drm_i915_private
*dev_priv,
}
}
-static bool il
Re: [Intel-gfx] [PATCH 12/16] drm/i915: Disable LP1+ watermarks while changing the number of active pipes
2014-05-22 11:48 GMT-03:00 :
> From: Ville Syrjälä
>
> When we switch between one active pipe and multiple active pipes, the
> display FIFO gets repartitioned. Disable the LP1+ waterwarks while that
> is happening to make sure we don't get any glitches on other active
> pipes while doing a modeset on another other pipe.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 45 ++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 47
>
> 3 files changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index bccf414..311c0f0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3985,6 +3985,27 @@ static struct intel_crtc *get_other_active_crtc(struct
> intel_crtc *crtc)
> return other_active_crtc;
> }
>
> +static void ilk_prepare_for_num_pipes_change(struct intel_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct intel_crtc *other_active_crtc = get_other_active_crtc(crtc);
> +
> + /*
> +* If we change between one pipe and multiple pipes,
> +* make sure the other active pipe is prepared
> +* for having its FIFO resized. We do that by making
> +* sure the pipe isn't using LP1+ watermarks when
> +* the second pipe gets enabled or disabled.
> +*/
> + if (!other_active_crtc)
> + return;
But get_other_active_crtc() returns NULL in case 2 other CRTCs are
already active. If we're the third pipe being enabled, we may still
get an underrun.
> +
> + ilk_wm_synchronize(other_active_crtc);
> +
> + if (ilk_disable_lp_wm(dev))
> + intel_wait_for_vblank(dev, other_active_crtc->pipe);
> +}
> +
> static void ironlake_crtc_enable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -4023,6 +4044,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>
> intel_crtc->active = true;
>
> + /* Make sure other pipes are prepared for FIFO resizing */
> + ilk_prepare_for_num_pipes_change(intel_crtc);
> +
> intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
>
> @@ -4123,6 +4147,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_crtc->active = true;
>
> + /* Make sure other pipes are prepared for FIFO resizing */
> + ilk_prepare_for_num_pipes_change(intel_crtc);
> +
> intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> if (intel_crtc->config.has_pch_encoder)
> intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A,
> true);
> @@ -4192,6 +4219,9 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>
> intel_crtc_disable_planes(crtc);
>
> + /* Make sure other pipes are prepared for FIFO resizing */
> + ilk_prepare_for_num_pipes_change(intel_crtc);
> +
> for_each_encoder_on_crtc(dev, crtc, encoder)
> encoder->disable(encoder);
>
> @@ -4235,6 +4265,12 @@ static void ironlake_crtc_disable(struct drm_crtc
> *crtc)
>
> intel_crtc->active = false;
>
> + /*
> +* Potentially let some other pipe use the
> +* full FIFO, and re-enable LP1+ watermarks.
> +*/
> + ilk_wm_pipe_post_disable(intel_crtc);
> +
> mutex_lock(&dev->struct_mutex);
> intel_update_fbc(dev);
> intel_edp_psr_update(dev);
> @@ -4255,6 +4291,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
> intel_crtc_disable_planes(crtc);
>
> + /* Make sure other pipes are prepared for FIFO resizing */
> + ilk_prepare_for_num_pipes_change(intel_crtc);
> +
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> intel_opregion_notify_encoder(encoder, false);
> encoder->disable(encoder);
> @@ -4282,6 +4321,12 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
> intel_crtc->active = false;
>
> + /*
> +* Potentially let some other pipe use the
> +* full FIFO, and re-enable LP1+ watermarks.
> +*/
> + ilk_wm_pipe_post_disable(intel_crtc);
> +
> mutex_lock(&dev->struct_mutex);
> intel_update_fbc(dev);
> intel_edp_psr_update(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5ad7ad5..98f878f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1022,6 +1022,8 @@ void intel_program_watermarks_pre(struct intel_crtc
> *crtc,
> void intel_program_watermarks_post(struct intel_crtc *crtc,
>const struct intel_crtc_wm_config *config);
> void ilk_wm_synchronize(struct intel_crtc *crtc);
> +void ilk_wm_pipe_post_
Re: [Intel-gfx] [PATCH 12/16] drm/i915: Disable LP1+ watermarks while changing the number of active pipes
On Wed, Jun 04, 2014 at 03:24:13PM -0300, Paulo Zanoni wrote:
> 2014-05-22 11:48 GMT-03:00 :
> > From: Ville Syrjälä
> >
> > When we switch between one active pipe and multiple active pipes, the
> > display FIFO gets repartitioned. Disable the LP1+ waterwarks while that
> > is happening to make sure we don't get any glitches on other active
> > pipes while doing a modeset on another other pipe.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 45
> > ++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_pm.c | 47
> >
> > 3 files changed, 89 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index bccf414..311c0f0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3985,6 +3985,27 @@ static struct intel_crtc
> > *get_other_active_crtc(struct intel_crtc *crtc)
> > return other_active_crtc;
> > }
> >
> > +static void ilk_prepare_for_num_pipes_change(struct intel_crtc *crtc)
> > +{
> > + struct drm_device *dev = crtc->base.dev;
> > + struct intel_crtc *other_active_crtc = get_other_active_crtc(crtc);
> > +
> > + /*
> > +* If we change between one pipe and multiple pipes,
> > +* make sure the other active pipe is prepared
> > +* for having its FIFO resized. We do that by making
> > +* sure the pipe isn't using LP1+ watermarks when
> > +* the second pipe gets enabled or disabled.
> > +*/
> > + if (!other_active_crtc)
> > + return;
>
> But get_other_active_crtc() returns NULL in case 2 other CRTCs are
> already active. If we're the third pipe being enabled, we may still
> get an underrun.
The FIFO repartitioning happens only when going 1<->2 pipes, so the
2<->3 cases don't matter here.
>
> > +
> > + ilk_wm_synchronize(other_active_crtc);
> > +
> > + if (ilk_disable_lp_wm(dev))
> > + intel_wait_for_vblank(dev, other_active_crtc->pipe);
> > +}
> > +
> > static void ironlake_crtc_enable(struct drm_crtc *crtc)
> > {
> > struct drm_device *dev = crtc->dev;
> > @@ -4023,6 +4044,9 @@ static void ironlake_crtc_enable(struct drm_crtc
> > *crtc)
> >
> > intel_crtc->active = true;
> >
> > + /* Make sure other pipes are prepared for FIFO resizing */
> > + ilk_prepare_for_num_pipes_change(intel_crtc);
> > +
> > intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> > intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
> >
> > @@ -4123,6 +4147,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >
> > intel_crtc->active = true;
> >
> > + /* Make sure other pipes are prepared for FIFO resizing */
> > + ilk_prepare_for_num_pipes_change(intel_crtc);
> > +
> > intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> > if (intel_crtc->config.has_pch_encoder)
> > intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A,
> > true);
> > @@ -4192,6 +4219,9 @@ static void ironlake_crtc_disable(struct drm_crtc
> > *crtc)
> >
> > intel_crtc_disable_planes(crtc);
> >
> > + /* Make sure other pipes are prepared for FIFO resizing */
> > + ilk_prepare_for_num_pipes_change(intel_crtc);
> > +
> > for_each_encoder_on_crtc(dev, crtc, encoder)
> > encoder->disable(encoder);
> >
> > @@ -4235,6 +4265,12 @@ static void ironlake_crtc_disable(struct drm_crtc
> > *crtc)
> >
> > intel_crtc->active = false;
> >
> > + /*
> > +* Potentially let some other pipe use the
> > +* full FIFO, and re-enable LP1+ watermarks.
> > +*/
> > + ilk_wm_pipe_post_disable(intel_crtc);
> > +
> > mutex_lock(&dev->struct_mutex);
> > intel_update_fbc(dev);
> > intel_edp_psr_update(dev);
> > @@ -4255,6 +4291,9 @@ static void haswell_crtc_disable(struct drm_crtc
> > *crtc)
> >
> > intel_crtc_disable_planes(crtc);
> >
> > + /* Make sure other pipes are prepared for FIFO resizing */
> > + ilk_prepare_for_num_pipes_change(intel_crtc);
> > +
> > for_each_encoder_on_crtc(dev, crtc, encoder) {
> > intel_opregion_notify_encoder(encoder, false);
> > encoder->disable(encoder);
> > @@ -4282,6 +4321,12 @@ static void haswell_crtc_disable(struct drm_crtc
> > *crtc)
> >
> > intel_crtc->active = false;
> >
> > + /*
> > +* Potentially let some other pipe use the
> > +* full FIFO, and re-enable LP1+ watermarks.
> > +*/
> > + ilk_wm_pipe_post_disable(intel_crtc);
> > +
> > mutex_lock(&dev->struct_mutex);
> > intel_update_fbc(dev);
> > intel_edp_psr_update(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i
