* [PATCH 0/4] SKL eDP clocks
@ 2014-11-14 17:24 Damien Lespiau
2014-11-14 17:24 ` [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() Damien Lespiau
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Damien Lespiau @ 2014-11-14 17:24 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau @ 2014-11-14 17:24 ` Damien Lespiau 2014-11-17 12:40 ` Paulo Zanoni 2014-11-14 17:24 ` [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 Damien Lespiau ` (3 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Damien Lespiau @ 2014-11-14 17:24 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() 2014-11-14 17:24 ` [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() Damien Lespiau @ 2014-11-17 12:40 ` Paulo Zanoni 0 siblings, 0 replies; 19+ messages in thread From: Paulo Zanoni @ 2014-11-17 12:40 UTC (permalink / raw) To: Damien Lespiau; +Cc: Intel Graphics Development 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau 2014-11-14 17:24 ` [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() Damien Lespiau @ 2014-11-14 17:24 ` Damien Lespiau 2014-11-17 18:24 ` Daniel Vetter 2014-11-14 17:24 ` [PATCH 3/4] drm/i915/skl: Use the pipe config DPLL tracking to query the link clock Damien Lespiau ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Damien Lespiau @ 2014-11-14 17:24 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 2014-11-14 17:24 ` [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 Damien Lespiau @ 2014-11-17 18:24 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2014-11-17 18:24 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/4] drm/i915/skl: Use the pipe config DPLL tracking to query the link clock 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau 2014-11-14 17:24 ` [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() Damien Lespiau 2014-11-14 17:24 ` [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 Damien Lespiau @ 2014-11-14 17:24 ` Damien Lespiau 2014-11-14 17:24 ` [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 Damien Lespiau 2014-11-17 17:04 ` [PATCH 0/4] SKL eDP clocks Paulo Zanoni 4 siblings, 0 replies; 19+ messages in thread From: Damien Lespiau @ 2014-11-14 17:24 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau ` (2 preceding siblings ...) 2014-11-14 17:24 ` [PATCH 3/4] drm/i915/skl: Use the pipe config DPLL tracking to query the link clock Damien Lespiau @ 2014-11-14 17:24 ` Damien Lespiau 2014-11-15 5:28 ` shuang.he 2014-11-17 18:28 ` Daniel Vetter 2014-11-17 17:04 ` [PATCH 0/4] SKL eDP clocks Paulo Zanoni 4 siblings, 2 replies; 19+ messages in thread From: Damien Lespiau @ 2014-11-14 17:24 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-14 17:24 ` [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 Damien Lespiau @ 2014-11-15 5:28 ` shuang.he 2014-11-15 10:54 ` Damien Lespiau 2014-11-17 18:28 ` Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: shuang.he @ 2014-11-15 5:28 UTC (permalink / raw) To: shuang.he, intel-gfx, damien.lespiau 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-15 5:28 ` shuang.he @ 2014-11-15 10:54 ` Damien Lespiau 2014-11-17 15:03 ` Daniel Vetter 2014-11-18 8:11 ` He, Shuang 0 siblings, 2 replies; 19+ messages in thread From: Damien Lespiau @ 2014-11-15 10:54 UTC (permalink / raw) To: shuang.he; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-15 10:54 ` Damien Lespiau @ 2014-11-17 15:03 ` Daniel Vetter 2014-11-18 8:14 ` He, Shuang 2014-11-18 8:11 ` He, Shuang 1 sibling, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2014-11-17 15:03 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-17 15:03 ` Daniel Vetter @ 2014-11-18 8:14 ` He, Shuang 0 siblings, 0 replies; 19+ messages in thread From: He, Shuang @ 2014-11-18 8:14 UTC (permalink / raw) To: Daniel Vetter, Lespiau, Damien; +Cc: intel-gfx@lists.freedesktop.org > -----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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-15 10:54 ` Damien Lespiau 2014-11-17 15:03 ` Daniel Vetter @ 2014-11-18 8:11 ` He, Shuang 1 sibling, 0 replies; 19+ messages in thread From: He, Shuang @ 2014-11-18 8:11 UTC (permalink / raw) To: Lespiau, Damien; +Cc: intel-gfx@lists.freedesktop.org > -----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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-14 17:24 ` [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 Damien Lespiau 2014-11-15 5:28 ` shuang.he @ 2014-11-17 18:28 ` Daniel Vetter 2014-11-18 8:18 ` Daniel Vetter 2014-11-21 16:00 ` [PATCH v2] " Damien Lespiau 1 sibling, 2 replies; 19+ messages in thread From: Daniel Vetter @ 2014-11-17 18:28 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-17 18:28 ` Daniel Vetter @ 2014-11-18 8:18 ` Daniel Vetter 2014-11-21 16:00 ` [PATCH v2] " Damien Lespiau 1 sibling, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2014-11-18 8:18 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-17 18:28 ` Daniel Vetter 2014-11-18 8:18 ` Daniel Vetter @ 2014-11-21 16:00 ` Damien Lespiau 2014-11-21 16:14 ` [PATCH v3] " Damien Lespiau 1 sibling, 1 reply; 19+ messages in thread From: Damien Lespiau @ 2014-11-21 16:00 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-21 16:00 ` [PATCH v2] " Damien Lespiau @ 2014-11-21 16:14 ` Damien Lespiau 2014-11-21 17:46 ` Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Damien Lespiau @ 2014-11-21 16:14 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3] drm/i915/skl: Read out crtl1 for eDP/DPLL0 2014-11-21 16:14 ` [PATCH v3] " Damien Lespiau @ 2014-11-21 17:46 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2014-11-21 17:46 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] SKL eDP clocks 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau ` (3 preceding siblings ...) 2014-11-14 17:24 ` [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 Damien Lespiau @ 2014-11-17 17:04 ` Paulo Zanoni 2014-11-17 17:08 ` Damien Lespiau 4 siblings, 1 reply; 19+ messages in thread From: Paulo Zanoni @ 2014-11-17 17:04 UTC (permalink / raw) To: Damien Lespiau; +Cc: Intel Graphics Development 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] SKL eDP clocks 2014-11-17 17:04 ` [PATCH 0/4] SKL eDP clocks Paulo Zanoni @ 2014-11-17 17:08 ` Damien Lespiau 0 siblings, 0 replies; 19+ messages in thread From: Damien Lespiau @ 2014-11-17 17:08 UTC (permalink / raw) To: Paulo Zanoni; +Cc: Intel Graphics Development 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2014-11-21 17:46 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-14 17:24 [PATCH 0/4] SKL eDP clocks Damien Lespiau 2014-11-14 17:24 ` [PATCH 1/4] drm/i915/skl: Remove spurious warn in get_ddi_pll() Damien Lespiau 2014-11-17 12:40 ` Paulo Zanoni 2014-11-14 17:24 ` [PATCH 2/4] drm/i915/skl: Set the eDP link rate on DPLL0 Damien Lespiau 2014-11-17 18:24 ` Daniel Vetter 2014-11-14 17:24 ` [PATCH 3/4] drm/i915/skl: Use the pipe config DPLL tracking to query the link clock Damien Lespiau 2014-11-14 17:24 ` [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0 Damien Lespiau 2014-11-15 5:28 ` shuang.he 2014-11-15 10:54 ` Damien Lespiau 2014-11-17 15:03 ` Daniel Vetter 2014-11-18 8:14 ` He, Shuang 2014-11-18 8:11 ` He, Shuang 2014-11-17 18:28 ` Daniel Vetter 2014-11-18 8:18 ` Daniel Vetter 2014-11-21 16:00 ` [PATCH v2] " Damien Lespiau 2014-11-21 16:14 ` [PATCH v3] " Damien Lespiau 2014-11-21 17:46 ` Daniel Vetter 2014-11-17 17:04 ` [PATCH 0/4] SKL eDP clocks Paulo Zanoni 2014-11-17 17:08 ` Damien Lespiau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox