[PATCH 0/4] SKL eDP clocks

[PATCH 0/4] SKL eDP clocks

[Damien Lespiau]

The previous clock series didn't include the eDP side of it. This should
address most of it, for now.

Note that I have some issues with HBR2 and link training here and I'm trying to
find more information about this. So depending on the configuration (number of
lanes wired, panel bw) this series may not be enough to light up an eDP panel.

-- 
Damien

Damien Lespiau (4):
  drm/i915/skl: Remove spurious warn in get_ddi_pll()
  drm/i915/skl: Set the eDP link rate on DPLL0
  drm/i915/skl: Use the pipe config DPLL tracking to query the link
    clock
  drm/i915/skl: Read out crtl1 for eDP/DPLL0

 drivers/gpu/drm/i915/intel_ddi.c     | 34 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_display.c |  2 --
 drivers/gpu/drm/i915/intel_dp.c      | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 8 deletions(-)

-- 
1.8.3.1

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

[PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll()

[Damien Lespiau]

When reading out a DDI config that uses a PLL that is not part of the
shared_dpll scheme (DPLL0), it's totally normal to end up in the
default: case of that switch.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dda97b3..466a7ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8029,8 +8029,6 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
 	case SKL_DPLL3:
 		pipe_config->shared_dpll = DPLL_ID_SKL_DPLL3;
 		break;
-	default:
-		WARN(1, "Unknown DPLL programmed\n");
 	}
 }
 
-- 
1.8.3.1

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

[PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0

[Damien Lespiau]

On SKL DPLL0 is used to derive CDCLK but can also be used to drive an
eDP port (as long as we don't want SSC). DPLL0 is special enough to not
be handled by the shared DPLL framework (drives CDCLK, not supposed to
enable the HDMI mode), So we need to compute the configuration
separately from the other DPLLs.

Note that we don't need to reprogram DPLL0 (which would mean bringing
down CDCLK) to support the various eDP 1.3 link rates as they all share
the same VCO (8100).

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ca33ee9..596bdc1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1484,6 +1484,25 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		uint32_t dpll = crtc->config.ddi_pll_sel;
 		uint32_t val;
 
+		/*
+		 * DPLL0 is used for eDP and is the only "private" DPLL (as
+		 * opposed to shared) on SKL
+		 */
+		if (type == INTEL_OUTPUT_EDP) {
+			WARN_ON(dpll != SKL_DPLL0);
+
+			val = I915_READ(DPLL_CTRL1);
+
+			val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) |
+				 DPLL_CTRL1_SSC(dpll) |
+				 DPLL_CRTL1_LINK_RATE_MASK(dpll));
+			val |= crtc->config.dpll_hw_state.ctrl1 << (dpll * 6);
+
+			I915_WRITE(DPLL_CTRL1, val);
+			POSTING_READ(DPLL_CTRL1);
+		}
+
+		/* DDI -> PLL mapping  */
 		val = I915_READ(DPLL_CTRL2);
 
 		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
@@ -1492,6 +1511,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
 
 		I915_WRITE(DPLL_CTRL2, val);
+
 	} else {
 		WARN_ON(crtc->config.ddi_pll_sel == PORT_CLK_SEL_NONE);
 		I915_WRITE(PORT_CLK_SEL(port), crtc->config.ddi_pll_sel);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 45b53ff..5dde84f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1075,6 +1075,33 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
 }
 
 static void
+skl_edp_set_pll_config(struct intel_crtc_config *pipe_config, int link_bw)
+{
+	u32 ctrl1;
+
+	pipe_config->ddi_pll_sel = SKL_DPLL0;
+	pipe_config->dpll_hw_state.cfgcr1 = 0;
+	pipe_config->dpll_hw_state.cfgcr2 = 0;
+
+	ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
+	switch (link_bw) {
+	case DP_LINK_BW_1_62:
+		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_810,
+					      SKL_DPLL0);
+		break;
+	case DP_LINK_BW_2_7:
+		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1350,
+					      SKL_DPLL0);
+		break;
+	case DP_LINK_BW_5_4:
+		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2700,
+					      SKL_DPLL0);
+		break;
+	}
+	pipe_config->dpll_hw_state.ctrl1 = ctrl1;
+}
+
+static void
 hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
 {
 	switch (link_bw) {
@@ -1251,7 +1278,9 @@ found:
 				&pipe_config->dp_m2_n2);
 	}
 
-	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+	if (IS_SKYLAKE(dev) && is_edp(intel_dp))
+		skl_edp_set_pll_config(pipe_config, intel_dp->link_bw);
+	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
 	else
 		intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
-- 
1.8.3.1

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

[PATCH 3/4] drm/i915/skl: Use the pipe config DPLL tracking to query the link clock

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 596bdc1..b5a279a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -735,14 +735,10 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 				struct intel_crtc_config *pipe_config)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
-	enum port port = intel_ddi_get_encoder_port(encoder);
 	int link_clock = 0;
 	uint32_t dpll_ctl1, dpll;
 
-	/* FIXME: This should be tracked in the pipe config. */
-	dpll = I915_READ(DPLL_CTRL2);
-	dpll &= DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
-	dpll >>= DPLL_CTRL2_DDI_CLK_SEL_SHIFT(port);
+	dpll = pipe_config->ddi_pll_sel;
 
 	dpll_ctl1 = I915_READ(DPLL_CTRL1);
 
-- 
1.8.3.1

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

[PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b5a279a..924f1ec 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -767,12 +767,20 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 
 	pipe_config->port_clock = link_clock;
 
+	/*
+	 * On SKL the eDP DPLL (DPLL0 as we don't use SSC) is not part of the
+	 * shared DPLL framework and thus needs to be read out separately
+	 */
+	if (encoder->type == INTEL_OUTPUT_EDP)
+		pipe_config->dpll_hw_state.ctrl1 = (dpll_ctl1 >> (dpll * 6)) & 0x3f;
+
 	if (pipe_config->has_dp_encoder)
 		pipe_config->adjusted_mode.crtc_clock =
 			intel_dotclock_calculate(pipe_config->port_clock,
 						 &pipe_config->dp_m_n);
 	else
 		pipe_config->adjusted_mode.crtc_clock = pipe_config->port_clock;
+
 }
 
 static void hsw_ddi_clock_get(struct intel_encoder *encoder,
-- 
1.8.3.1

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

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=290/291->290/291
PNV: pass/total=352/356->356/356
ILK: pass/total=371/372->364/372
IVB: pass/total=545/546->545/546
SNB: pass/total=424/425->424/425
HSW: pass/total=579/579->579/579
BDW: pass/total=434/435->434/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) -> PASS(4, M7)
PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) -> PASS(1, M7)
PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1, M7)
PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1, M7)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible, PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Damien Lespiau]

Hi Shuang,

You wanted suggestions, so how about:

For both examples, to determine the size of the column, I'd take the
length of the longest value of that column (including the title) and add
4 to account for spacing. Left alignment for text, right alignement for
numbers (of course, all of that is debatable).

On Fri, Nov 14, 2014 at 09:28:03PM -0800, shuang.he@intel.com wrote:
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
> -------------------------------------Summary-------------------------------------
> Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> BYT: pass/total=290/291->290/291
> PNV: pass/total=352/356->356/356
> ILK: pass/total=371/372->364/372
> IVB: pass/total=545/546->545/546
> SNB: pass/total=424/425->424/425
> HSW: pass/total=579/579->579/579
> BDW: pass/total=434/435->434/435

For this one:

  - a bit more spacing
  - some table-like alignment. That's assuming the mail client will use
    monospaced fonts for text emails, it really should.
  - add the delta so it's easier to parse the interesting information
  - sort by gens
  - baseline_drm_intel_nightly_pass_rate can really just be
    drm-intel-nightly
  - it's not just a single patch applied, so series applied?

Platform    Delta    drm-intel-nightly    Series Applied
--------------------------------------------------------
PNV            +4              352/356           356/356
ILK            -7              371/372           364/372
SNB             0              424/425           424/425
IVB             0              545/546           545/546
BYT             0              290/291           290/291
HSW             0              579/579           579/579
BDW             0              434/435           434/435


> -------------------------------------Detailed-------------------------------------
> test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
> PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) -> PASS(4, M7)
> PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) -> PASS(1, M7)
> PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1, M7)
> PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1, M7)
> ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible, PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)

And for this one:

  - it's not nessary to repeat the test suite name for every single row,
    if needed at all (I don't think it is when replying to kernel
    patches), it can be put beforehand.
  - sorted by gen
  - The machine id isn't useful in this report
  - It'd be nice to add a some explanations on how to decipher the
    result column (what is the count, what the various states mean).
    Maybe after the table as it only needs to be read a few times to get
    it.

Platform    Test                                                     drm-intel-nightly        Series Applied
-------------------------------------------------------------------------------------------------------------------
ILK         igt_kms_flip_flip-vs-absolute-wf_vblank                  DMESG_WARN(1) PASS(3)    DMESG_WARN(2) PASS(2)
ILK         igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible    DMESG_WARN(1) PASS(3)    DMESG_WARN(1) PASS(3)
..

HTH,

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

Re: [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll()

[Paulo Zanoni]

2014-11-14 15:24 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> When reading out a DDI config that uses a PLL that is not part of the
> shared_dpll scheme (DPLL0), it's totally normal to end up in the
> default: case of that switch.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dda97b3..466a7ae 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8029,8 +8029,6 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
>         case SKL_DPLL3:
>                 pipe_config->shared_dpll = DPLL_ID_SKL_DPLL3;
>                 break;
> -       default:
> -               WARN(1, "Unknown DPLL programmed\n");
>         }
>  }
>
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Daniel Vetter]

On Sat, Nov 15, 2014 at 10:54:47AM +0000, Damien Lespiau wrote:
> Hi Shuang,
> 
> You wanted suggestions, so how about:
> 
> For both examples, to determine the size of the column, I'd take the
> length of the longest value of that column (including the title) and add
> 4 to account for spacing. Left alignment for text, right alignement for
> numbers (of course, all of that is debatable).
> 
> On Fri, Nov 14, 2014 at 09:28:03PM -0800, shuang.he@intel.com wrote:
> > Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
> > -------------------------------------Summary-------------------------------------
> > Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> > BYT: pass/total=290/291->290/291
> > PNV: pass/total=352/356->356/356
> > ILK: pass/total=371/372->364/372
> > IVB: pass/total=545/546->545/546
> > SNB: pass/total=424/425->424/425
> > HSW: pass/total=579/579->579/579
> > BDW: pass/total=434/435->434/435
> 
> For this one:
> 
>   - a bit more spacing
>   - some table-like alignment. That's assuming the mail client will use
>     monospaced fonts for text emails, it really should.
>   - add the delta so it's easier to parse the interesting information

I think the delta should mention both + and - counts separately so that
it's easy to see when a patch regressed the same amount of tests as it
fixed. Iirc there's been a few cases before where this was important.

Otherwise I agree with damien, the column aligment and headers make the
results a lot easier to read.
-Daniel

>   - sort by gens
>   - baseline_drm_intel_nightly_pass_rate can really just be
>     drm-intel-nightly
>   - it's not just a single patch applied, so series applied?
> 
> Platform    Delta    drm-intel-nightly    Series Applied
> --------------------------------------------------------
> PNV            +4              352/356           356/356
> ILK            -7              371/372           364/372
> SNB             0              424/425           424/425
> IVB             0              545/546           545/546
> BYT             0              290/291           290/291
> HSW             0              579/579           579/579
> BDW             0              434/435           434/435
> 
> 
> > -------------------------------------Detailed-------------------------------------
> > test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
> > PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) -> PASS(4, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) -> PASS(1, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1, M7)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible, PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> 
> And for this one:
> 
>   - it's not nessary to repeat the test suite name for every single row,
>     if needed at all (I don't think it is when replying to kernel
>     patches), it can be put beforehand.
>   - sorted by gen
>   - The machine id isn't useful in this report
>   - It'd be nice to add a some explanations on how to decipher the
>     result column (what is the count, what the various states mean).
>     Maybe after the table as it only needs to be read a few times to get
>     it.
> 
> Platform    Test                                                     drm-intel-nightly        Series Applied
> -------------------------------------------------------------------------------------------------------------------
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank                  DMESG_WARN(1) PASS(3)    DMESG_WARN(2) PASS(2)
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible    DMESG_WARN(1) PASS(3)    DMESG_WARN(1) PASS(3)
> ..
> 
> HTH,
> 
> -- 
> Damien
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] SKL eDP clocks

[Paulo Zanoni]

2014-11-14 15:24 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> The previous clock series didn't include the eDP side of it. This should
> address most of it, for now.
>
> Note that I have some issues with HBR2 and link training here and I'm trying to
> find more information about this. So depending on the configuration (number of
> lanes wired, panel bw) this series may not be enough to light up an eDP panel.

I couldn't spot anything obvious here...

For all patches: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

My bikeshedding would be to replace the magic "(id * 6)" - from
patches 2 and 4 - and "0x3f" - from patch 4 - with some nice macro
definitions.

>
> --
> Damien
>
> Damien Lespiau (4):
>   drm/i915/skl: Remove spurious warn in get_ddi_pll()
>   drm/i915/skl: Set the eDP link rate on DPLL0
>   drm/i915/skl: Use the pipe config DPLL tracking to query the link
>     clock
>   drm/i915/skl: Read out crtl1 for eDP/DPLL0
>
>  drivers/gpu/drm/i915/intel_ddi.c     | 34 +++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_display.c |  2 --
>  drivers/gpu/drm/i915/intel_dp.c      | 31 ++++++++++++++++++++++++++++++-
>  3 files changed, 59 insertions(+), 8 deletions(-)
>
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 0/4] SKL eDP clocks

[Damien Lespiau]

On Mon, Nov 17, 2014 at 03:04:19PM -0200, Paulo Zanoni wrote:
> 2014-11-14 15:24 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> > The previous clock series didn't include the eDP side of it. This should
> > address most of it, for now.
> >
> > Note that I have some issues with HBR2 and link training here and I'm trying to
> > find more information about this. So depending on the configuration (number of
> > lanes wired, panel bw) this series may not be enough to light up an eDP panel.
> 
> I couldn't spot anything obvious here...
> 
> For all patches: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thanks!

> My bikeshedding would be to replace the magic "(id * 6)" - from
> patches 2 and 4 - and "0x3f" - from patch 4 - with some nice macro
> definitions.

Yes, that's also the case for the rest of the SKL DPLL code, the plan is
to fix that on top.

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

Re: [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0

[Daniel Vetter]

On Fri, Nov 14, 2014 at 05:24:33PM +0000, Damien Lespiau wrote:
> On SKL DPLL0 is used to derive CDCLK but can also be used to drive an
> eDP port (as long as we don't want SSC). DPLL0 is special enough to not
> be handled by the shared DPLL framework (drives CDCLK, not supposed to
> enable the HDMI mode), So we need to compute the configuration
> separately from the other DPLLs.
> 
> Note that we don't need to reprogram DPLL0 (which would mean bringing
> down CDCLK) to support the various eDP 1.3 link rates as they all share
> the same VCO (8100).
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ca33ee9..596bdc1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1484,6 +1484,25 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>  		uint32_t dpll = crtc->config.ddi_pll_sel;
>  		uint32_t val;
>  
> +		/*
> +		 * DPLL0 is used for eDP and is the only "private" DPLL (as
> +		 * opposed to shared) on SKL
> +		 */
> +		if (type == INTEL_OUTPUT_EDP) {

Are you sure you really want to check for edp here and not for something
more specific like port A? Same with the is_edp check below.

In the past we've had piles of fun with code that checked for is_edp, but
which should have checked for port A or something similar - on desktop
platforms (at least in all chips thus far, dunno about skl) edp ends up on
port D.

Oh and someone bored might want to replace the pile of type == DP || type
== EDP checks in intel_ddi.c with config->has_dp_encoder.

I'll hold off on merging this for now just out of curiosity ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Daniel Vetter]

On Fri, Nov 14, 2014 at 05:24:35PM +0000, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b5a279a..924f1ec 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -767,12 +767,20 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
>  
>  	pipe_config->port_clock = link_clock;
>  
> +	/*
> +	 * On SKL the eDP DPLL (DPLL0 as we don't use SSC) is not part of the
> +	 * shared DPLL framework and thus needs to be read out separately
> +	 */
> +	if (encoder->type == INTEL_OUTPUT_EDP)

Hw state readout shouldn't depend upon our sw state, and given the
multiple personality nature of DDI ports I think this is the case here: We
might have a different opinion than the bios guys about what's edp (it
happened) or how consistently to apply this clock selection algo (also
happened). So might be better to stuff this into the ddi clock readout
code (the one where you patched away the WARN for DPLL0 I guess).

Merged patch 3 meanwhile, thanks.
-Daniel

> +		pipe_config->dpll_hw_state.ctrl1 = (dpll_ctl1 >> (dpll * 6)) & 0x3f;
> +
>  	if (pipe_config->has_dp_encoder)
>  		pipe_config->adjusted_mode.crtc_clock =
>  			intel_dotclock_calculate(pipe_config->port_clock,
>  						 &pipe_config->dp_m_n);
>  	else
>  		pipe_config->adjusted_mode.crtc_clock = pipe_config->port_clock;
> +
>  }
>  
>  static void hsw_ddi_clock_get(struct intel_encoder *encoder,
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[He, Shuang]

> -----Original Message-----
> From: Lespiau, Damien
> Sent: Saturday, November 15, 2014 6:55 PM
> To: He, Shuang
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0
> 
> Hi Shuang,
> 
> You wanted suggestions, so how about:
> 
> For both examples, to determine the size of the column, I'd take the
> length of the longest value of that column (including the title) and add
> 4 to account for spacing. Left alignment for text, right alignement for
> numbers (of course, all of that is debatable).
[He, Shuang] Hello, Damien, Thanks for your great input, we'll check how those suggestion could be integrated, I've created one jira task to track it: https://jira01.devtools.intel.com/browse/VIZ-4672

> 
> On Fri, Nov 14, 2014 at 09:28:03PM -0800, shuang.he@intel.com wrote:
> > Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang.he@intel.com)
> > -------------------------------------Summary-------------------------------------
> > Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> > BYT: pass/total=290/291->290/291
> > PNV: pass/total=352/356->356/356
> > ILK: pass/total=371/372->364/372
> > IVB: pass/total=545/546->545/546
> > SNB: pass/total=424/425->424/425
> > HSW: pass/total=579/579->579/579
> > BDW: pass/total=434/435->434/435
> 
> For this one:
> 
>   - a bit more spacing
>   - some table-like alignment. That's assuming the mail client will use
>     monospaced fonts for text emails, it really should.
>   - add the delta so it's easier to parse the interesting information
>   - sort by gens
[He, Shuang] yes, this makes sense

>   - baseline_drm_intel_nightly_pass_rate can really just be
>     drm-intel-nightly
[He, Shuang] yes, this makes sense

>   - it's not just a single patch applied, so series applied?
[He, Shuang] yes, it's series applied

> 
> Platform    Delta    drm-intel-nightly    Series Applied
> --------------------------------------------------------
> PNV            +4              352/356           356/356
> ILK            -7              371/372           364/372
> SNB             0              424/425           424/425
> IVB             0              545/546           545/546
> BYT             0              290/291           290/291
> HSW             0              579/579           579/579
> BDW             0              434/435           434/435
> 
> 
> > -------------------------------------Detailed-------------------------------------
> > test_platform: test_suite, test_case, result_with_drm_intel_nightly(count,
> machine_id...)...->result_with_patch_applied(count, machine_id)...
> > PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) ->
> PASS(4, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) ->
> PASS(1, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1,
> M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1,
> M7)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible,
> PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) ->
> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26) ->
> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4,
> M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> 
> And for this one:
> 
>   - it's not nessary to repeat the test suite name for every single row,
>     if needed at all (I don't think it is when replying to kernel
>     patches), it can be put beforehand.
[He, Shuang] this makes sense

>   - sorted by gen
>   - The machine id isn't useful in this report
[He, Shuang] Some time, test result diffs because of Machine have changed, machine id is put there to help understand the issue

>   - It'd be nice to add a some explanations on how to decipher the
>     result column (what is the count, what the various states mean).
>     Maybe after the table as it only needs to be read a few times to get
>     it.
[He, Shuang] yes, it may help. I'm always be hesitating on how to put these things in the right position. I'll see if your way could help

> 
> Platform    Test
> drm-intel-nightly        Series Applied
> ----------------------------------------------------------------------------------------------------------------
> ---
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank
> DMESG_WARN(1) PASS(3)    DMESG_WARN(2) PASS(2)
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible
> DMESG_WARN(1) PASS(3)    DMESG_WARN(1) PASS(3)
> ..
> 
> HTH,
[He, Shuang] By the way, What is HTH? "Hope to help"? :)

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

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[He, Shuang]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Monday, November 17, 2014 11:03 PM
> To: Lespiau, Damien
> Cc: He, Shuang; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0
> 
> On Sat, Nov 15, 2014 at 10:54:47AM +0000, Damien Lespiau wrote:
> > Hi Shuang,
> >
> > You wanted suggestions, so how about:
> >
> > For both examples, to determine the size of the column, I'd take the
> > length of the longest value of that column (including the title) and add
> > 4 to account for spacing. Left alignment for text, right alignement for
> > numbers (of course, all of that is debatable).
> >
> > On Fri, Nov 14, 2014 at 09:28:03PM -0800, shuang.he@intel.com wrote:
> > > Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang.he@intel.com)
> > > -------------------------------------Summary-------------------------------------
> > > Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> > > BYT: pass/total=290/291->290/291
> > > PNV: pass/total=352/356->356/356
> > > ILK: pass/total=371/372->364/372
> > > IVB: pass/total=545/546->545/546
> > > SNB: pass/total=424/425->424/425
> > > HSW: pass/total=579/579->579/579
> > > BDW: pass/total=434/435->434/435
> >
> > For this one:
> >
> >   - a bit more spacing
> >   - some table-like alignment. That's assuming the mail client will use
> >     monospaced fonts for text emails, it really should.
> >   - add the delta so it's easier to parse the interesting information
> 
> I think the delta should mention both + and - counts separately so that
> it's easy to see when a patch regressed the same amount of tests as it
> fixed. Iirc there's been a few cases before where this was important.
[He, Shuang] Agree with that, it's kind of like patch

Thanks
	--Shuang
> 
> Otherwise I agree with damien, the column aligment and headers make the
> results a lot easier to read.
> -Daniel
> 
> >   - sort by gens
> >   - baseline_drm_intel_nightly_pass_rate can really just be
> >     drm-intel-nightly
> >   - it's not just a single patch applied, so series applied?
> >
> > Platform    Delta    drm-intel-nightly    Series Applied
> > --------------------------------------------------------
> > PNV            +4              352/356           356/356
> > ILK            -7              371/372           364/372
> > SNB             0              424/425           424/425
> > IVB             0              545/546           545/546
> > BYT             0              290/291           290/291
> > HSW             0              579/579           579/579
> > BDW             0              434/435           434/435
> >
> >
> > > -------------------------------------Detailed-------------------------------------
> > > test_platform: test_suite, test_case, result_with_drm_intel_nightly(count,
> machine_id...)...->result_with_patch_applied(count, machine_id)...
> > > PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) ->
> PASS(4, M7)
> > > PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) ->
> PASS(1, M7)
> > > PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) ->
> PASS(1, M7)
> > > PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) ->
> PASS(1, M7)
> > > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2,
> M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible,
> PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) ->
> DMESG_WARN(2, M26)PASS(2, M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26)
> -> DMESG_WARN(2, M26)PASS(2, M26)
> > > ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4,
> M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> >
> > And for this one:
> >
> >   - it's not nessary to repeat the test suite name for every single row,
> >     if needed at all (I don't think it is when replying to kernel
> >     patches), it can be put beforehand.
> >   - sorted by gen
> >   - The machine id isn't useful in this report
> >   - It'd be nice to add a some explanations on how to decipher the
> >     result column (what is the count, what the various states mean).
> >     Maybe after the table as it only needs to be read a few times to get
> >     it.
> >
> > Platform    Test
> drm-intel-nightly        Series Applied
> >
> ----------------------------------------------------------------------------------------------------------------
> ---
> > ILK         igt_kms_flip_flip-vs-absolute-wf_vblank
> DMESG_WARN(1) PASS(3)    DMESG_WARN(2) PASS(2)
> > ILK         igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible
> DMESG_WARN(1) PASS(3)    DMESG_WARN(1) PASS(3)
> > ..
> >
> > HTH,
> >
> > --
> > Damien
> > _______________________________________________
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Daniel Vetter]

On Mon, Nov 17, 2014 at 07:28:28PM +0100, Daniel Vetter wrote:
> On Fri, Nov 14, 2014 at 05:24:35PM +0000, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index b5a279a..924f1ec 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -767,12 +767,20 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
> >  
> >  	pipe_config->port_clock = link_clock;
> >  
> > +	/*
> > +	 * On SKL the eDP DPLL (DPLL0 as we don't use SSC) is not part of the
> > +	 * shared DPLL framework and thus needs to be read out separately
> > +	 */
> > +	if (encoder->type == INTEL_OUTPUT_EDP)
> 
> Hw state readout shouldn't depend upon our sw state, and given the
> multiple personality nature of DDI ports I think this is the case here: We
> might have a different opinion than the bios guys about what's edp (it
> happened) or how consistently to apply this clock selection algo (also
> happened). So might be better to stuff this into the ddi clock readout
> code (the one where you patched away the WARN for DPLL0 I guess).

And after a night of pondering I stand correct on our irc discussion about
putting dpll0 into the shared dpll infrastructure - if the bios indeed
does something we don't expect and we don't properly track the use count
for dpll0 it'll blow up. So I guess we'll need indeed need that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Damien Lespiau]

v2: Put the DPLL0 state readout in skylake_get_ddi_pll(), closer to
where the PLL assignement read out is done rather than the frequency
readout function. (Daniel)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  1 +
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6b45cd..c624eeb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -773,6 +773,7 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 						 &pipe_config->dp_m_n);
 	else
 		pipe_config->adjusted_mode.crtc_clock = pipe_config->port_clock;
+
 }
 
 static void hsw_ddi_clock_get(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 853697f..1623178 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,12 +7994,21 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
 				enum port port,
 				struct intel_crtc_config *pipe_config)
 {
-	u32 temp;
+	u32 temp, dpll_ctl1;
 
 	temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
 	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
 
 	switch (pipe_config->ddi_pll_sel) {
+	case SKL_DPLL0:
+		/*
+		 * On SKL the eDP DPLL (DPLL0 as we don't use SSC) is not part
+		 * of the shared DPLL framework and thus needs to be read out
+		 * separately
+		 */
+		dpll_ctl1 = I915_READ(DPLL_CTRL1);
+		pipe_config->dpll_hw_state.ctrl1 = dpll_ctl1 & 0x3f;
+		break;
 	case SKL_DPLL1:
 		pipe_config->shared_dpll = DPLL_ID_SKL_DPLL1;
 		break;
-- 
1.8.3.1

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

[PATCH v3] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Damien Lespiau]

v2: Put the DPLL0 state readout in skylake_get_ddi_pll(), closer to
where the PLL assignement read out is done rather than the frequency
readout function. (Daniel)

v3: Remove stray new line (Damien)
    Add Paulo's r-b tag for v1

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v1)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 853697f..1623178 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,12 +7994,21 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
 				enum port port,
 				struct intel_crtc_config *pipe_config)
 {
-	u32 temp;
+	u32 temp, dpll_ctl1;
 
 	temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
 	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
 
 	switch (pipe_config->ddi_pll_sel) {
+	case SKL_DPLL0:
+		/*
+		 * On SKL the eDP DPLL (DPLL0 as we don't use SSC) is not part
+		 * of the shared DPLL framework and thus needs to be read out
+		 * separately
+		 */
+		dpll_ctl1 = I915_READ(DPLL_CTRL1);
+		pipe_config->dpll_hw_state.ctrl1 = dpll_ctl1 & 0x3f;
+		break;
 	case SKL_DPLL1:
 		pipe_config->shared_dpll = DPLL_ID_SKL_DPLL1;
 		break;
-- 
1.8.3.1

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

Re: [PATCH v3] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[Daniel Vetter]

On Fri, Nov 21, 2014 at 04:14:56PM +0000, Damien Lespiau wrote:
> v2: Put the DPLL0 state readout in skylake_get_ddi_pll(), closer to
> where the PLL assignement read out is done rather than the frequency
> readout function. (Daniel)
> 
> v3: Remove stray new line (Damien)
>     Add Paulo's r-b tag for v1
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v1)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

I still think it might be safer to put DPLL0 into the shared
infrastructure too just because. But this looks good as-is.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx