diff mbox

[5/9] drm/i915: drop unnecessary clearing of pch dp transcoder timings

Message ID 1352120854-16533-6-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Nov. 5, 2012, 1:07 p.m. UTC
This has originally been added in

commit 8db9d77b1b14fd730561f64beea8c00e4478d7c5
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date:   Wed Apr 7 16:15:54 2010 +0800

    drm/i915: Support for Cougarpoint PCH display pipeline

probably to combat issues with hw state left behind by the BIOS. And
indeed, I've checked out that specific revision, and there is no DP
support yet. So the pch dp transcoder won't be correctly disabled, and
that's important since it requires a rather special disable dance:
Just writing 0 to TRANS_DP_CTL won't cut it, since we need to select
the NONE port when disabling, too.

And indeed, things seem to still work, so let's just remove this.

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

Comments

Paulo Zanoni Nov. 16, 2012, 7:52 p.m. UTC | #1
Hi

2012/11/5 Daniel Vetter <daniel.vetter@ffwll.ch>:
> This has originally been added in
>
> commit 8db9d77b1b14fd730561f64beea8c00e4478d7c5
> Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> Date:   Wed Apr 7 16:15:54 2010 +0800
>
>     drm/i915: Support for Cougarpoint PCH display pipeline
>
> probably to combat issues with hw state left behind by the BIOS. And
> indeed, I've checked out that specific revision, and there is no DP
> support yet. So the pch dp transcoder won't be correctly disabled, and
> that's important since it requires a rather special disable dance:
> Just writing 0 to TRANS_DP_CTL won't cut it, since we need to select
> the NONE port when disabling, too.
>
> And indeed, things seem to still work, so let's just remove this.

Notice that we have the CPU M/N registers and the PCH M/N registers.
And the PCH M/N registers are only used by DisplayPort, so it makes
sense to zero them for non-DisplayPort. If we really want this, how
about we add this patch only in the beginning of the 3.9 development
cycle?

>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a221eb9..0c923a6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5349,15 +5349,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>         } else
>                 intel_put_pch_pll(intel_crtc);
>
> -       if (is_dp && !is_cpu_edp) {
> +       if (is_dp && !is_cpu_edp)
>                 intel_dp_set_m_n(crtc, mode, adjusted_mode);
> -       } else {
> -               /* For non-DP output, clear any trans DP clock recovery setting.*/
> -               I915_WRITE(TRANSDATA_M1(pipe), 0);
> -               I915_WRITE(TRANSDATA_N1(pipe), 0);
> -               I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> -               I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> -       }
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_pll_enable)
> @@ -5472,18 +5465,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>         DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
>         drm_mode_debug_printmodeline(mode);
>
> -       if (is_dp && !is_cpu_edp) {
> +       if (is_dp && !is_cpu_edp)
>                 intel_dp_set_m_n(crtc, mode, adjusted_mode);
> -       } else {
> -               if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> -                       /* For non-DP output, clear any trans DP clock recovery
> -                        * setting.*/
> -                       I915_WRITE(TRANSDATA_M1(pipe), 0);
> -                       I915_WRITE(TRANSDATA_N1(pipe), 0);
> -                       I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> -                       I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> -               }
> -       }

This is the only code chunk I'm sure won't cause any problems, so you
might add it to the LVDS patch that already removes IBX/CPT stuff from
haswell_crtc_mode_set :)

>
>         intel_crtc->lowfreq_avail = false;
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Nov. 16, 2012, 8:01 p.m. UTC | #2
On Fri, Nov 16, 2012 at 8:52 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> Notice that we have the CPU M/N registers and the PCH M/N registers.
> And the PCH M/N registers are only used by DisplayPort, so it makes
> sense to zero them for non-DisplayPort. If we really want this, how
> about we add this patch only in the beginning of the 3.9 development
> cycle?

Well, all that "let's clear things harder" is imo all a symptom of our
pre-modeset-rework days where we simply didn't keep track of the hw
state well enough. Hence I'm on a holy killing spree against them all
;-)

And yes, this is all for 3.9
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a221eb9..0c923a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5349,15 +5349,8 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	} else
 		intel_put_pch_pll(intel_crtc);
 
-	if (is_dp && !is_cpu_edp) {
+	if (is_dp && !is_cpu_edp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
-	} else {
-		/* For non-DP output, clear any trans DP clock recovery setting.*/
-		I915_WRITE(TRANSDATA_M1(pipe), 0);
-		I915_WRITE(TRANSDATA_N1(pipe), 0);
-		I915_WRITE(TRANSDPLINK_M1(pipe), 0);
-		I915_WRITE(TRANSDPLINK_N1(pipe), 0);
-	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_pll_enable)
@@ -5472,18 +5465,8 @@  static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
 	drm_mode_debug_printmodeline(mode);
 
-	if (is_dp && !is_cpu_edp) {
+	if (is_dp && !is_cpu_edp)
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
-	} else {
-		if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
-			/* For non-DP output, clear any trans DP clock recovery
-			 * setting.*/
-			I915_WRITE(TRANSDATA_M1(pipe), 0);
-			I915_WRITE(TRANSDATA_N1(pipe), 0);
-			I915_WRITE(TRANSDPLINK_M1(pipe), 0);
-			I915_WRITE(TRANSDPLINK_N1(pipe), 0);
-		}
-	}
 
 	intel_crtc->lowfreq_avail = false;