[PATCH 1/3] drm/i915: move lvds_border_bits to pipe_config

[PATCH 1/3] drm/i915: move lvds_border_bits to pipe_config

[Daniel Vetter]

pipe_config is the new dev_priv!

More seriously, this is actually better since a pipe_config can be
thrown away if the modeset compute config stage fails. Whereas any
state stored in dev_prive needs to be painstakingly restored, since
otherwise a dpms off/on will wreak massive havoc. Yes, that even
applies to state only used in ->mode_set callbacks, since we need to
call those even for dpms on when the Haswell power well cleared
everything out.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h    | 2 --
 drivers/gpu/drm/i915/intel_drv.h   | 1 +
 drivers/gpu/drm/i915/intel_lvds.c  | 2 +-
 drivers/gpu/drm/i915/intel_panel.c | 3 +--
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09282dd..d7fd002 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1026,8 +1026,6 @@ typedef struct drm_i915_private {
 	/* Kernel Modesetting */
 
 	struct sdvo_device_mapping sdvo_mappings[2];
-	/* indicate whether the LVDS_BORDER should be enabled or not */
-	unsigned int lvds_border_bits;
 
 	struct drm_crtc *plane_to_crtc_mapping[3];
 	struct drm_crtc *pipe_to_crtc_mapping[3];
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index be196ff..3f505bc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -252,6 +252,7 @@ struct intel_crtc_config {
 	struct {
 		u32 control;
 		u32 pgm_ratios;
+		u32 lvds_border_bits;
 	} gmch_pfit;
 
 	/* Panel fitter placement and size for Ironlake+ */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 8d65baf..47f47ea 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -116,7 +116,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
 	}
 
 	/* set the corresponsding LVDS_BORDER bit */
-	temp |= dev_priv->lvds_border_bits;
+	temp |= intel_crtc->config.gmch_pfit.lvds_border_bits;
 	/* Set the B0-B3 data pairs corresponding to whether we're going to
 	 * set the DPLLs for dual-channel mode or not.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 2526326..4bf1e18 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -183,7 +183,6 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      int fitting_mode)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	struct drm_display_mode *mode, *adjusted_mode;
 
@@ -312,7 +311,7 @@ out:
 		pipe_config->gmch_pfit.control = pfit_control;
 		pipe_config->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
 	}
-	dev_priv->lvds_border_bits = border;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
 }
 
 static int is_backlight_combination_mode(struct drm_device *dev)
-- 
1.7.11.7

[PATCH 2/3] drm/i915: rip out indirection for pfit pipe_config assignment

[Daniel Vetter]

This was still required a bit (on the cargo-cult side though) when the
state was stored in dev_priv, and when the enable/disable sequence was
botched a bit (to avoid too many updates).

But with pipeconfig we always get a clean slate, so this is pointless.
Rip it out.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_panel.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 4bf1e18..56f17b2 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -306,11 +306,8 @@ out:
 	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
 		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
 
-	if (pfit_control != pipe_config->gmch_pfit.control ||
-	    pfit_pgm_ratios != pipe_config->gmch_pfit.pgm_ratios) {
-		pipe_config->gmch_pfit.control = pfit_control;
-		pipe_config->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
-	}
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
 	pipe_config->gmch_pfit.lvds_border_bits = border;
 }
 
-- 
1.7.11.7

[PATCH 3/3] drm/i915: move border color writes to pfit_enable

[Daniel Vetter]

Writing hw registers from compute_config?
Just say no!

In this case not too horrible since we write a constant 0, and only
debugging would put something else in there. But while checking that
code I've noticed that this register disappeared on pch platforms, so
fix that up, too.

And adjust the comment a bit, it's outdated.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/intel_lvds.c    | 10 ----------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2259cdd..26ff7d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3633,6 +3633,10 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 
 	I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
 	I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
+
+	/* Border color in case we don't scale up to the full screen. Black by
+	 * default, change to something else for debugging. */
+	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 47f47ea..d256fe4 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -231,7 +231,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
 	unsigned int lvds_bpp;
-	int pipe;
 
 	/* Should never happen!! */
 	if (INTEL_INFO(dev)->gen < 4 && intel_crtc->pipe == 0) {
@@ -274,15 +273,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 					 intel_connector->panel.fitting_mode);
 	}
 
-	/*
-	 * Enable automatic panel scaling for non-native modes so that they fill
-	 * the screen.  Should be enabled before the pipe is enabled, according
-	 * to register description and PRM.
-	 * Change the value here to see the borders for debugging
-	 */
-	for_each_pipe(pipe)
-		I915_WRITE(BCLRPAT(pipe), 0);
-
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 	pipe_config->timings_set = true;
 
-- 
1.7.11.7

Re: [PATCH 3/3] drm/i915: move border color writes to pfit_enable

[Mika Kuoppala]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

Patches 1,2 and 3:

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Writing hw registers from compute_config?
> Just say no!
>
> In this case not too horrible since we write a constant 0, and only
> debugging would put something else in there. But while checking that
> code I've noticed that this register disappeared on pch platforms, so
> fix that up, too.
>
> And adjust the comment a bit, it's outdated.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  4 ++++
>  drivers/gpu/drm/i915/intel_lvds.c    | 10 ----------
>  2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2259cdd..26ff7d6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3633,6 +3633,10 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
>  
>  	I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
>  	I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> +
> +	/* Border color in case we don't scale up to the full screen. Black by
> +	 * default, change to something else for debugging. */
> +	I915_WRITE(BCLRPAT(crtc->pipe), 0);
>  }
>  
>  static void valleyview_crtc_enable(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 47f47ea..d256fe4 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -231,7 +231,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>  	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
>  	unsigned int lvds_bpp;
> -	int pipe;
>  
>  	/* Should never happen!! */
>  	if (INTEL_INFO(dev)->gen < 4 && intel_crtc->pipe == 0) {
> @@ -274,15 +273,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  					 intel_connector->panel.fitting_mode);
>  	}
>  
> -	/*
> -	 * Enable automatic panel scaling for non-native modes so that they fill
> -	 * the screen.  Should be enabled before the pipe is enabled, according
> -	 * to register description and PRM.
> -	 * Change the value here to see the borders for debugging
> -	 */
> -	for_each_pipe(pipe)
> -		I915_WRITE(BCLRPAT(pipe), 0);
> -
>  	drm_mode_set_crtcinfo(adjusted_mode, 0);
>  	pipe_config->timings_set = true;
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: move border color writes to pfit_enable

[Daniel Vetter]

On Tue, Apr 30, 2013 at 04:53:41PM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> 
> Patches 1,2 and 3:
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

All merged to dinq, thanks for the review.
-Daniel

> 
> > Writing hw registers from compute_config?
> > Just say no!
> >
> > In this case not too horrible since we write a constant 0, and only
> > debugging would put something else in there. But while checking that
> > code I've noticed that this register disappeared on pch platforms, so
> > fix that up, too.
> >
> > And adjust the comment a bit, it's outdated.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  4 ++++
> >  drivers/gpu/drm/i915/intel_lvds.c    | 10 ----------
> >  2 files changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2259cdd..26ff7d6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3633,6 +3633,10 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> >  
> >  	I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
> >  	I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> > +
> > +	/* Border color in case we don't scale up to the full screen. Black by
> > +	 * default, change to something else for debugging. */
> > +	I915_WRITE(BCLRPAT(crtc->pipe), 0);
> >  }
> >  
> >  static void valleyview_crtc_enable(struct drm_crtc *crtc)
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 47f47ea..d256fe4 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -231,7 +231,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> >  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> >  	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
> >  	unsigned int lvds_bpp;
> > -	int pipe;
> >  
> >  	/* Should never happen!! */
> >  	if (INTEL_INFO(dev)->gen < 4 && intel_crtc->pipe == 0) {
> > @@ -274,15 +273,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> >  					 intel_connector->panel.fitting_mode);
> >  	}
> >  
> > -	/*
> > -	 * Enable automatic panel scaling for non-native modes so that they fill
> > -	 * the screen.  Should be enabled before the pipe is enabled, according
> > -	 * to register description and PRM.
> > -	 * Change the value here to see the borders for debugging
> > -	 */
> > -	for_each_pipe(pipe)
> > -		I915_WRITE(BCLRPAT(pipe), 0);
> > -
> >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> >  	pipe_config->timings_set = true;
> >  
> > -- 
> > 1.7.11.7
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch