diff mbox

[1/5] drm/i915: preserve SSC if previously set v2

Message ID 1401992671-2548-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes June 5, 2014, 6:24 p.m. UTC
Some machines may have a broken VBT or no VBT at all, but we still want
to use SSC there.  So check for it and keep it enabled if we see it
already on.  Based on an earlier fix from Kristian.

v2: honor modparam if set too (Daniel)
    read out at init time and store for panel_use_ssc() use (Jesse)

Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Jani Nikula June 6, 2014, 11:04 a.m. UTC | #1
On Thu, 05 Jun 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Some machines may have a broken VBT or no VBT at all, but we still want
> to use SSC there.  So check for it and keep it enabled if we see it
> already on.  Based on an earlier fix from Kristian.
>
> v2: honor modparam if set too (Daniel)
>     read out at init time and store for panel_use_ssc() use (Jesse)
>
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8631fb3..f57b752 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1404,6 +1404,8 @@ struct drm_i915_private {
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> +	bool bios_ssc; /* BIOS had SSC enabled at boot? */
> +
>  	/* overlay */
>  	struct intel_overlay *overlay;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b5cbb28..0e8c9bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5355,7 +5355,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  {
>  	if (i915.panel_use_ssc >= 0)
>  		return i915.panel_use_ssc != 0;
> -	return dev_priv->vbt.lvds_use_ssc
> +	return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> @@ -12478,6 +12478,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  
>  void intel_modeset_gem_init(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *c;
>  	struct intel_framebuffer *fb;
>  
> @@ -12485,6 +12486,14 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  	intel_init_gt_powersave(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> +	/*
> +	 * There may be no VBT; and if the BIOS enabled SSC we can
> +	 * just keep using it to avoid unnecessary flicker.
> +	 */
> +	if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) &&
> +	    (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE))
> +			dev_priv->bios_ssc = true;

Do we have any need to differentiate between this and what VBT has? We
could just treat dev_priv->vbt as "VBT or what BIOS did", and do
dev_priv->vbt.lvds_use_ssc = true; here, possibly with a
DRM_DEBUG_KMS. It would simplify code elsewhere and be less error prone.
We already do a similar thing for the eDP bpp (see intel_dp.c, look for
"This is a big fat ugly hack").

BR,
Jani.


> +
>  	intel_modeset_init_hw(dev);
>  
>  	intel_setup_overlay(dev);
> -- 
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Chris Wilson June 6, 2014, 11:06 a.m. UTC | #2
On Fri, Jun 06, 2014 at 02:04:21PM +0300, Jani Nikula wrote:
> Do we have any need to differentiate between this and what VBT has? We
> could just treat dev_priv->vbt as "VBT or what BIOS did", and do
> dev_priv->vbt.lvds_use_ssc = true; here, possibly with a
> DRM_DEBUG_KMS. It would simplify code elsewhere and be less error prone.
> We already do a similar thing for the eDP bpp (see intel_dp.c, look for
> "This is a big fat ugly hack").

Indeed, that is a nice simplification. Conceptually, at least.
-Chris
Daniel Vetter June 10, 2014, 2 p.m. UTC | #3
On Thu, Jun 05, 2014 at 11:24:27AM -0700, Jesse Barnes wrote:
> Some machines may have a broken VBT or no VBT at all, but we still want
> to use SSC there.  So check for it and keep it enabled if we see it
> already on.  Based on an earlier fix from Kristian.
> 
> v2: honor modparam if set too (Daniel)
>     read out at init time and store for panel_use_ssc() use (Jesse)
> 
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Ugh.

This means the BIOS idea of when we need SSC and ours are diverging, which
isn't good. And I think we should further lock this down by tracking the
"uses SSC refclk" state in the pipe config, just to make sure we don't
fumble this.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8631fb3..f57b752 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1404,6 +1404,8 @@ struct drm_i915_private {
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> +	bool bios_ssc; /* BIOS had SSC enabled at boot? */
> +
>  	/* overlay */
>  	struct intel_overlay *overlay;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b5cbb28..0e8c9bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5355,7 +5355,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  {
>  	if (i915.panel_use_ssc >= 0)
>  		return i915.panel_use_ssc != 0;
> -	return dev_priv->vbt.lvds_use_ssc
> +	return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> @@ -12478,6 +12478,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  
>  void intel_modeset_gem_init(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *c;
>  	struct intel_framebuffer *fb;
>  
> @@ -12485,6 +12486,14 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  	intel_init_gt_powersave(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> +	/*
> +	 * There may be no VBT; and if the BIOS enabled SSC we can
> +	 * just keep using it to avoid unnecessary flicker.
> +	 */
> +	if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) &&
> +	    (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE))
> +			dev_priv->bios_ssc = true;
> +
>  	intel_modeset_init_hw(dev);
>  
>  	intel_setup_overlay(dev);
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8631fb3..f57b752 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1404,6 +1404,8 @@  struct drm_i915_private {
 	struct intel_opregion opregion;
 	struct intel_vbt_data vbt;
 
+	bool bios_ssc; /* BIOS had SSC enabled at boot? */
+
 	/* overlay */
 	struct intel_overlay *overlay;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b5cbb28..0e8c9bc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5355,7 +5355,7 @@  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
 	if (i915.panel_use_ssc >= 0)
 		return i915.panel_use_ssc != 0;
-	return dev_priv->vbt.lvds_use_ssc
+	return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
@@ -12478,6 +12478,7 @@  void intel_modeset_setup_hw_state(struct drm_device *dev,
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *c;
 	struct intel_framebuffer *fb;
 
@@ -12485,6 +12486,14 @@  void intel_modeset_gem_init(struct drm_device *dev)
 	intel_init_gt_powersave(dev);
 	mutex_unlock(&dev->struct_mutex);
 
+	/*
+	 * There may be no VBT; and if the BIOS enabled SSC we can
+	 * just keep using it to avoid unnecessary flicker.
+	 */
+	if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) &&
+	    (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE))
+			dev_priv->bios_ssc = true;
+
 	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);