All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
@ 2016-09-26 14:54 Imre Deak
  2016-09-26 15:02 ` Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Imre Deak @ 2016-09-26 14:54 UTC (permalink / raw)
  To: intel-gfx

a277ca7dc01d should've been a no-functional-change commit, but it
removed the initialization of the dpll_hw_state for HDMI outputs,
resulting in state mismatches and a failed modeset with blank
screen. Fix this by reinstating the dpll_hw_state initialization.

Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c26d18a..e8bf838 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock,
 	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
 }
 
+bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
+				    struct intel_crtc_state *crtc_state,
+				    int clock,
+				    struct intel_dpll_hw_state *dpll_hw_state)
+{
+	struct bxt_clk_div clk_div = { };
+
+	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock, &clk_div);
+
+	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
+}
+
 static struct intel_shared_dpll *
 bxt_get_dpll(struct intel_crtc *crtc,
 		struct intel_crtc_state *crtc_state,
 		struct intel_encoder *encoder)
 {
-	struct bxt_clk_div clk_div = {0};
-	struct intel_dpll_hw_state dpll_hw_state = {0};
+	struct intel_dpll_hw_state dpll_hw_state = { };
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_digital_port *intel_dig_port;
 	struct intel_shared_dpll *pll;
 	int i, clock = crtc_state->port_clock;
 
-	if (encoder->type == INTEL_OUTPUT_HDMI
-	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
-					  clock, &clk_div))
+	if (encoder->type == INTEL_OUTPUT_HDMI &&
+	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
+					    &dpll_hw_state))
 		return NULL;
 
 	if ((encoder->type == INTEL_OUTPUT_DP ||
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-26 14:54 [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration Imre Deak
@ 2016-09-26 15:02 ` Jani Nikula
  2016-10-13 13:07   ` Daniel Vetter
  2016-09-26 15:25 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-09-27  8:03 ` [PATCH] " Ander Conselvan De Oliveira
  2 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2016-09-26 15:02 UTC (permalink / raw)
  To: Imre Deak, intel-gfx; +Cc: Ander Conselvan de Oliveira, Rodrigo Vivi

On Mon, 26 Sep 2016, Imre Deak <imre.deak@intel.com> wrote:
> a277ca7dc01d should've been a no-functional-change commit, but it
> removed the initialization of the dpll_hw_state for HDMI outputs,
> resulting in state mismatches and a failed modeset with blank
> screen. Fix this by reinstating the dpll_hw_state initialization.
>
> Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c26d18a..e8bf838 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock,
>  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
>  }
>  
> +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
> +				    struct intel_crtc_state *crtc_state,
> +				    int clock,
> +				    struct intel_dpll_hw_state *dpll_hw_state)
> +{
> +	struct bxt_clk_div clk_div = { };
> +
> +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock, &clk_div);
> +
> +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
> +}
> +
>  static struct intel_shared_dpll *
>  bxt_get_dpll(struct intel_crtc *crtc,
>  		struct intel_crtc_state *crtc_state,
>  		struct intel_encoder *encoder)
>  {
> -	struct bxt_clk_div clk_div = {0};
> -	struct intel_dpll_hw_state dpll_hw_state = {0};
> +	struct intel_dpll_hw_state dpll_hw_state = { };
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_shared_dpll *pll;
>  	int i, clock = crtc_state->port_clock;
>  
> -	if (encoder->type == INTEL_OUTPUT_HDMI
> -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> -					  clock, &clk_div))
> +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
> +					    &dpll_hw_state))
>  		return NULL;
>  
>  	if ((encoder->type == INTEL_OUTPUT_DP ||

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-26 14:54 [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration Imre Deak
  2016-09-26 15:02 ` Jani Nikula
@ 2016-09-26 15:25 ` Patchwork
  2016-09-27 11:07   ` Imre Deak
  2016-09-27  8:03 ` [PATCH] " Ander Conselvan De Oliveira
  2 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2016-09-26 15:25 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/bxt: Fix HDMI DPLL configuration
URL   : https://patchwork.freedesktop.org/series/12930/
State : failure

== Summary ==

Series 12930v1 drm/i915/bxt: Fix HDMI DPLL configuration
https://patchwork.freedesktop.org/api/1.0/series/12930/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-skl-6260u)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:177  dwarn:5   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2577/

aab15c274da587bcab19376d2caa9d6626440335 drm-intel-nightly: 2016y-09m-26d-12h-11m-33s UTC integration manifest
5b1abe9 drm/i915/bxt: Fix HDMI DPLL configuration

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-26 14:54 [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration Imre Deak
  2016-09-26 15:02 ` Jani Nikula
  2016-09-26 15:25 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-09-27  8:03 ` Ander Conselvan De Oliveira
  2016-09-27 11:26   ` Imre Deak
  2 siblings, 1 reply; 12+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-09-27  8:03 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

On Mon, 2016-09-26 at 17:54 +0300, Imre Deak wrote:
> a277ca7dc01d should've been a no-functional-change commit, but it
> removed the initialization of the dpll_hw_state for HDMI outputs,
> resulting in state mismatches and a failed modeset with blank
> screen. Fix this by reinstating the dpll_hw_state initialization.
> 
> Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c26d18a..e8bf838 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock,
>  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
>  }
>  
> +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
> +				    struct intel_crtc_state *crtc_state,
> +				    int clock,
> +				    struct intel_dpll_hw_state
> *dpll_hw_state)
> +{
> +	struct bxt_clk_div clk_div = { };
> +
> +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock, &clk_div);
> +
> +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
> +}
> +
>  static struct intel_shared_dpll *
>  bxt_get_dpll(struct intel_crtc *crtc,
>  		struct intel_crtc_state *crtc_state,
>  		struct intel_encoder *encoder)
>  {
> -	struct bxt_clk_div clk_div = {0};
> -	struct intel_dpll_hw_state dpll_hw_state = {0};
> +	struct intel_dpll_hw_state dpll_hw_state = { };
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_shared_dpll *pll;
>  	int i, clock = crtc_state->port_clock;
>  
> -	if (encoder->type == INTEL_OUTPUT_HDMI
> -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> -					  clock, &clk_div))
> +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
> +					    &dpll_hw_state))

In my original patch there was just a straight call
to bxt_ddi_set_dpll_hw_state() after this and the DP if condition (which also
only calculated dividers). But whatever fixes the issue:

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>


>  		return NULL;
>  
>  	if ((encoder->type == INTEL_OUTPUT_DP ||
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-26 15:25 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-09-27 11:07   ` Imre Deak
  2016-09-27 12:46     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Imre Deak @ 2016-09-27 11:07 UTC (permalink / raw)
  To: intel-gfx

On ma, 2016-09-26 at 15:25 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/bxt: Fix HDMI DPLL configuration
> URL   : https://patchwork.freedesktop.org/series/12930/
> State : failure
> 
> == Summary ==
> 
> Series 12930v1 drm/i915/bxt: Fix HDMI DPLL configuration
> https://patchwork.freedesktop.org/api/1.0/series/12930/revisions/1/mb
> ox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup nonblocking-crc-pipe-a-frame-sequence:
>                 pass       -> DMESG-WARN (fi-ilk-650)
>         Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                 pass       -> DMESG-WARN (fi-ilk-650)
>         Subgroup read-crc-pipe-b:
>                 pass       -> DMESG-WARN (fi-ilk-650)
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (fi-ilk-650)
>         Subgroup suspend-read-crc-pipe-b:
>                 pass       -> DMESG-WARN (fi-ilk-650)

All the above are:
*ERROR* CPU pipe B FIFO underrun
*ERROR* PCH transcoder B FIFO underrun

Unrelated platform:
https://bugs.freedesktop.org/show_bug.cgi?id=93310


>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> INCOMPLETE (fi-skl-6260u)

Unrelated platform:
https://bugs.freedesktop.org/show_bug.cgi?id=91703


> 
> fi-bdw-
> 5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
> fi-bsw-
> n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
> fi-byt-
> n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
> fi-hsw-
> 4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> fi-hsw-
> 4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> fi-ilk-
> 650       total:244  pass:177  dwarn:5   dfail:0   fail:2   skip:60 
> fi-ivb-
> 3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
> fi-ivb-
> 3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> fi-skl-
> 6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:12 
> fi-skl-
> 6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
> fi-skl-
> 6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
> fi-skl-
> 6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
> fi-snb-
> 2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
> fi-snb-
> 2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2577/
> 
> aab15c274da587bcab19376d2caa9d6626440335 drm-intel-nightly: 2016y-
> 09m-26d-12h-11m-33s UTC integration manifest
> 5b1abe9 drm/i915/bxt: Fix HDMI DPLL configuration
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27  8:03 ` [PATCH] " Ander Conselvan De Oliveira
@ 2016-09-27 11:26   ` Imre Deak
  2016-09-27 13:04     ` Ander Conselvan De Oliveira
  0 siblings, 1 reply; 12+ messages in thread
From: Imre Deak @ 2016-09-27 11:26 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira, intel-gfx

On ti, 2016-09-27 at 11:03 +0300, Ander Conselvan De Oliveira wrote:
> On Mon, 2016-09-26 at 17:54 +0300, Imre Deak wrote:
> > a277ca7dc01d should've been a no-functional-change commit, but it
> > removed the initialization of the dpll_hw_state for HDMI outputs,
> > resulting in state mismatches and a failed modeset with blank
> > screen. Fix this by reinstating the dpll_hw_state initialization.
> > 
> > Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index c26d18a..e8bf838 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int
> > clock,
> >  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > dpll_hw_state);
> >  }
> >  
> > +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
> > +				    struct intel_crtc_state
> > *crtc_state,
> > +				    int clock,
> > +				    struct intel_dpll_hw_state
> > *dpll_hw_state)
> > +{
> > +	struct bxt_clk_div clk_div = { };
> > +
> > +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock,
> > &clk_div);
> > +
> > +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > dpll_hw_state);
> > +}
> > +
> >  static struct intel_shared_dpll *
> >  bxt_get_dpll(struct intel_crtc *crtc,
> >  		struct intel_crtc_state *crtc_state,
> >  		struct intel_encoder *encoder)
> >  {
> > -	struct bxt_clk_div clk_div = {0};
> > -	struct intel_dpll_hw_state dpll_hw_state = {0};
> > +	struct intel_dpll_hw_state dpll_hw_state = { };
> >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> > >base.dev);
> >  	struct intel_digital_port *intel_dig_port;
> >  	struct intel_shared_dpll *pll;
> >  	int i, clock = crtc_state->port_clock;
> >  
> > -	if (encoder->type == INTEL_OUTPUT_HDMI
> > -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> > -					  clock, &clk_div))
> > +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> > +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state,
> > clock,
> > +					    &dpll_hw_state))
> 
> In my original patch there was just a straight call
> to bxt_ddi_set_dpll_hw_state() after this and the DP if condition
> (which also only calculated dividers).

That would require exporting bxt_ddi_dp_pll_dividers(), so I just left
it as-is. I forgot to make bxt_ddi_hdmi_set_dpll_hw_state() static, I
fixed that up while applying.

Pushed to -dinq, thanks for the review.

> But whatever fixes the issue:
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> 
> 
> >  		return NULL;
> >  
> >  	if ((encoder->type == INTEL_OUTPUT_DP ||
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ Fi.CI.BAT:  failure for drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27 11:07   ` Imre Deak
@ 2016-09-27 12:46     ` Ville Syrjälä
  2016-09-27 14:08       ` Imre Deak
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-09-27 12:46 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Tue, Sep 27, 2016 at 02:07:11PM +0300, Imre Deak wrote:
> On ma, 2016-09-26 at 15:25 +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915/bxt: Fix HDMI DPLL configuration
> > URL   : https://patchwork.freedesktop.org/series/12930/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 12930v1 drm/i915/bxt: Fix HDMI DPLL configuration
> > https://patchwork.freedesktop.org/api/1.0/series/12930/revisions/1/mb
> > ox/
> > 
> > Test kms_pipe_crc_basic:
> >         Subgroup nonblocking-crc-pipe-a-frame-sequence:
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >         Subgroup nonblocking-crc-pipe-b-frame-sequence:
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >         Subgroup read-crc-pipe-b:
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >         Subgroup suspend-read-crc-pipe-a:
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> >         Subgroup suspend-read-crc-pipe-b:
> >                 pass       -> DMESG-WARN (fi-ilk-650)
> 
> All the above are:
> *ERROR* CPU pipe B FIFO underrun
> *ERROR* PCH transcoder B FIFO underrun

Hmm. All of those should have been fixed long ago. Did we have a
regression? Except I'm not seeing any of these in the CI history.
Weird.

> 
> Unrelated platform:
> https://bugs.freedesktop.org/show_bug.cgi?id=93310
> 
> 
> >         Subgroup suspend-read-crc-pipe-c:
> >                 pass       -> INCOMPLETE (fi-skl-6260u)
> 
> Unrelated platform:
> https://bugs.freedesktop.org/show_bug.cgi?id=91703
> 
> 
> > 
> > fi-bdw-
> > 5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
> > fi-bsw-
> > n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
> > fi-byt-
> > n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
> > fi-hsw-
> > 4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> > fi-hsw-
> > 4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> > fi-ilk-
> > 650       total:244  pass:177  dwarn:5   dfail:0   fail:2   skip:60 
> > fi-ivb-
> > 3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
> > fi-ivb-
> > 3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> > fi-skl-
> > 6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:12 
> > fi-skl-
> > 6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
> > fi-skl-
> > 6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
> > fi-skl-
> > 6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
> > fi-snb-
> > 2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
> > fi-snb-
> > 2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> > 
> > Results at /archive/results/CI_IGT_test/Patchwork_2577/
> > 
> > aab15c274da587bcab19376d2caa9d6626440335 drm-intel-nightly: 2016y-
> > 09m-26d-12h-11m-33s UTC integration manifest
> > 5b1abe9 drm/i915/bxt: Fix HDMI DPLL configuration
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27 11:26   ` Imre Deak
@ 2016-09-27 13:04     ` Ander Conselvan De Oliveira
  2016-09-27 15:54       ` Imre Deak
  0 siblings, 1 reply; 12+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-09-27 13:04 UTC (permalink / raw)
  To: imre.deak, intel-gfx; +Cc: Vivi, Rodrigo

On Tue, 2016-09-27 at 14:26 +0300, Imre Deak wrote:
> On ti, 2016-09-27 at 11:03 +0300, Ander Conselvan De Oliveira wrote:
> > 
> > On Mon, 2016-09-26 at 17:54 +0300, Imre Deak wrote:
> > > 
> > > a277ca7dc01d should've been a no-functional-change commit, but it
> > > removed the initialization of the dpll_hw_state for HDMI outputs,
> > > resulting in state mismatches and a failed modeset with blank
> > > screen. Fix this by reinstating the dpll_hw_state initialization.
> > > 
> > > Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
> > >  1 file changed, 16 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > index c26d18a..e8bf838 100644
> > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int
> > > clock,
> > >  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > > dpll_hw_state);
> > >  }
> > >  
> > > +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
> > > +				    struct intel_crtc_state
> > > *crtc_state,
> > > +				    int clock,
> > > +				    struct intel_dpll_hw_state
> > > *dpll_hw_state)
> > > +{
> > > +	struct bxt_clk_div clk_div = { };
> > > +
> > > +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock,
> > > &clk_div);
> > > +
> > > +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > > dpll_hw_state);
> > > +}
> > > +
> > >  static struct intel_shared_dpll *
> > >  bxt_get_dpll(struct intel_crtc *crtc,
> > >  		struct intel_crtc_state *crtc_state,
> > >  		struct intel_encoder *encoder)
> > >  {
> > > -	struct bxt_clk_div clk_div = {0};
> > > -	struct intel_dpll_hw_state dpll_hw_state = {0};
> > > +	struct intel_dpll_hw_state dpll_hw_state = { };
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > 
> > > > base.dev);
> > >  	struct intel_digital_port *intel_dig_port;
> > >  	struct intel_shared_dpll *pll;
> > >  	int i, clock = crtc_state->port_clock;
> > >  
> > > -	if (encoder->type == INTEL_OUTPUT_HDMI
> > > -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> > > -					  clock, &clk_div))
> > > +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> > > +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state,
> > > clock,
> > > +					    &dpll_hw_state))
> > In my original patch there was just a straight call
> > to bxt_ddi_set_dpll_hw_state() after this and the DP if condition
> > (which also only calculated dividers).
> That would require exporting bxt_ddi_dp_pll_dividers(), so I just left
> it as-is.

Not necessarily, bxt_ddi_dp_set_dpll_hw_state() could still be exported for the
caller. But the current solution is fine.

An even better solution would be to have a somewhat platform independent entry
point for force enabling a DPLL independent of modeset with a given clock for
DP. The caller of bxt_ddi_dp_set_dpll_hw_state() will immediately enable the
DPLL anyway, so it could instead just ask the DPLL code to do the right thing
and enable the PLL and leave as much of the platform specific part out of the
caller.

Ander


>  I forgot to make bxt_ddi_hdmi_set_dpll_hw_state() static, I
> fixed that up while applying.
> 
> Pushed to -dinq, thanks for the review.
> 
> > 
> > But whatever fixes the issue:
> > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> > 
> > 
> > > 
> > >  		return NULL;
> > >  
> > >  	if ((encoder->type == INTEL_OUTPUT_DP ||
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27 12:46     ` Ville Syrjälä
@ 2016-09-27 14:08       ` Imre Deak
  2016-09-27 14:49         ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Imre Deak @ 2016-09-27 14:08 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On ti, 2016-09-27 at 15:46 +0300, Ville Syrjälä wrote:
> On Tue, Sep 27, 2016 at 02:07:11PM +0300, Imre Deak wrote:
> > On ma, 2016-09-26 at 15:25 +0000, Patchwork wrote:
> > > == Series Details ==
> > > 
> > > Series: drm/i915/bxt: Fix HDMI DPLL configuration
> > > URL   : https://patchwork.freedesktop.org/series/12930/
> > > State : failure
> > > 
> > > == Summary ==
> > > 
> > > Series 12930v1 drm/i915/bxt: Fix HDMI DPLL configuration
> > > https://patchwork.freedesktop.org/api/1.0/series/12930/revisions/
> > > 1/mb
> > > ox/
> > > 
> > > Test kms_pipe_crc_basic:
> > >         Subgroup nonblocking-crc-pipe-a-frame-sequence:
> > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > >         Subgroup nonblocking-crc-pipe-b-frame-sequence:
> > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > >         Subgroup read-crc-pipe-b:
> > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > >         Subgroup suspend-read-crc-pipe-a:
> > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > >         Subgroup suspend-read-crc-pipe-b:
> > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > 
> > All the above are:
> > *ERROR* CPU pipe B FIFO underrun
> > *ERROR* PCH transcoder B FIFO underrun
> 
> Hmm. All of those should have been fixed long ago. Did we have a
> regression?

Not sure. It follows a
"Clock recovery check failed, cannot continue channel equalization",
which AFAICS appeared recently, first in CI_DRM_1655. The error message
itself isn't too old either though, it was added in CI_DRM_1626
("drm/i915: Make DP link training channel equalization DP 1.2 Spec
compliant").

> Except I'm not seeing any of these in the CI history.
> Weird.
>
> > 
> > Unrelated platform:
> > https://bugs.freedesktop.org/show_bug.cgi?id=93310
> > 
> > 
> > >         Subgroup suspend-read-crc-pipe-c:
> > >                 pass       -> INCOMPLETE (fi-skl-6260u)
> > 
> > Unrelated platform:
> > https://bugs.freedesktop.org/show_bug.cgi?id=91703
> > 
> > 
> > > 
> > > fi-bdw-
> > > 5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:
> > > 15 
> > > fi-bsw-
> > > n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:
> > > 42 
> > > fi-byt-
> > > n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:
> > > 35 
> > > fi-hsw-
> > > 4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:
> > > 22 
> > > fi-hsw-
> > > 4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:
> > > 22 
> > > fi-ilk-
> > > 650       total:244  pass:177  dwarn:5   dfail:0   fail:2   skip:
> > > 60 
> > > fi-ivb-
> > > 3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:
> > > 25 
> > > fi-ivb-
> > > 3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:
> > > 37 
> > > fi-skl-
> > > 6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:
> > > 12 
> > > fi-skl-
> > > 6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:
> > > 22 
> > > fi-skl-
> > > 6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:
> > > 24 
> > > fi-skl-
> > > 6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:
> > > 14 
> > > fi-snb-
> > > 2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:
> > > 36 
> > > fi-snb-
> > > 2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:
> > > 37 
> > > 
> > > Results at /archive/results/CI_IGT_test/Patchwork_2577/
> > > 
> > > aab15c274da587bcab19376d2caa9d6626440335 drm-intel-nightly:
> > > 2016y-
> > > 09m-26d-12h-11m-33s UTC integration manifest
> > > 5b1abe9 drm/i915/bxt: Fix HDMI DPLL configuration
> > > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ Fi.CI.BAT:  failure for drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27 14:08       ` Imre Deak
@ 2016-09-27 14:49         ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-09-27 14:49 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Tue, Sep 27, 2016 at 05:08:34PM +0300, Imre Deak wrote:
> On ti, 2016-09-27 at 15:46 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 27, 2016 at 02:07:11PM +0300, Imre Deak wrote:
> > > On ma, 2016-09-26 at 15:25 +0000, Patchwork wrote:
> > > > == Series Details ==
> > > > 
> > > > Series: drm/i915/bxt: Fix HDMI DPLL configuration
> > > > URL   : https://patchwork.freedesktop.org/series/12930/
> > > > State : failure
> > > > 
> > > > == Summary ==
> > > > 
> > > > Series 12930v1 drm/i915/bxt: Fix HDMI DPLL configuration
> > > > https://patchwork.freedesktop.org/api/1.0/series/12930/revisions/
> > > > 1/mb
> > > > ox/
> > > > 
> > > > Test kms_pipe_crc_basic:
> > > >         Subgroup nonblocking-crc-pipe-a-frame-sequence:
> > > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > > >         Subgroup nonblocking-crc-pipe-b-frame-sequence:
> > > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > > >         Subgroup read-crc-pipe-b:
> > > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > > >         Subgroup suspend-read-crc-pipe-a:
> > > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > > >         Subgroup suspend-read-crc-pipe-b:
> > > >                 pass       -> DMESG-WARN (fi-ilk-650)
> > > 
> > > All the above are:
> > > *ERROR* CPU pipe B FIFO underrun
> > > *ERROR* PCH transcoder B FIFO underrun
> > 
> > Hmm. All of those should have been fixed long ago. Did we have a
> > regression?
> 
> Not sure. It follows a
> "Clock recovery check failed, cannot continue channel equalization",
> which AFAICS appeared recently, first in CI_DRM_1655. The error message
> itself isn't too old either though, it was added in CI_DRM_1626
> ("drm/i915: Make DP link training channel equalization DP 1.2 Spec
> compliant").

Ah. More importantly it follows
[  439.107783] [drm:intel_dp_check_link_status] DP C: channel EQ not ok, retraining'
which does generate underruns in my experience. I suppose we should just suppress them.

> 
> > Except I'm not seeing any of these in the CI history.
> > Weird.
> >
> > > 
> > > Unrelated platform:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=93310
> > > 
> > > 
> > > >         Subgroup suspend-read-crc-pipe-c:
> > > >                 pass       -> INCOMPLETE (fi-skl-6260u)
> > > 
> > > Unrelated platform:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=91703
> > > 
> > > 
> > > > 
> > > > fi-bdw-
> > > > 5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:
> > > > 15 
> > > > fi-bsw-
> > > > n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:
> > > > 42 
> > > > fi-byt-
> > > > n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:
> > > > 35 
> > > > fi-hsw-
> > > > 4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:
> > > > 22 
> > > > fi-hsw-
> > > > 4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:
> > > > 22 
> > > > fi-ilk-
> > > > 650       total:244  pass:177  dwarn:5   dfail:0   fail:2   skip:
> > > > 60 
> > > > fi-ivb-
> > > > 3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:
> > > > 25 
> > > > fi-ivb-
> > > > 3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:
> > > > 37 
> > > > fi-skl-
> > > > 6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:
> > > > 12 
> > > > fi-skl-
> > > > 6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:
> > > > 22 
> > > > fi-skl-
> > > > 6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:
> > > > 24 
> > > > fi-skl-
> > > > 6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:
> > > > 14 
> > > > fi-snb-
> > > > 2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:
> > > > 36 
> > > > fi-snb-
> > > > 2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:
> > > > 37 
> > > > 
> > > > Results at /archive/results/CI_IGT_test/Patchwork_2577/
> > > > 
> > > > aab15c274da587bcab19376d2caa9d6626440335 drm-intel-nightly:
> > > > 2016y-
> > > > 09m-26d-12h-11m-33s UTC integration manifest
> > > > 5b1abe9 drm/i915/bxt: Fix HDMI DPLL configuration
> > > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-27 13:04     ` Ander Conselvan De Oliveira
@ 2016-09-27 15:54       ` Imre Deak
  0 siblings, 0 replies; 12+ messages in thread
From: Imre Deak @ 2016-09-27 15:54 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira, intel-gfx; +Cc: Vivi, Rodrigo

On ti, 2016-09-27 at 16:04 +0300, Ander Conselvan De Oliveira wrote:
> On Tue, 2016-09-27 at 14:26 +0300, Imre Deak wrote:
> > On ti, 2016-09-27 at 11:03 +0300, Ander Conselvan De Oliveira
> > wrote:
> > > 
> > > On Mon, 2016-09-26 at 17:54 +0300, Imre Deak wrote:
> > > > 
> > > > a277ca7dc01d should've been a no-functional-change commit, but
> > > > it
> > > > removed the initialization of the dpll_hw_state for HDMI
> > > > outputs,
> > > > resulting in state mismatches and a failed modeset with blank
> > > > screen. Fix this by reinstating the dpll_hw_state
> > > > initialization.
> > > > 
> > > > Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++---
> > > > --
> > > >  1 file changed, 16 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > index c26d18a..e8bf838 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int
> > > > clock,
> > > >  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > > > dpll_hw_state);
> > > >  }
> > > >  
> > > > +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc
> > > > *intel_crtc,
> > > > +				    struct intel_crtc_state
> > > > *crtc_state,
> > > > +				    int clock,
> > > > +				    struct intel_dpll_hw_state
> > > > *dpll_hw_state)
> > > > +{
> > > > +	struct bxt_clk_div clk_div = { };
> > > > +
> > > > +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state,
> > > > clock,
> > > > &clk_div);
> > > > +
> > > > +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div,
> > > > dpll_hw_state);
> > > > +}
> > > > +
> > > >  static struct intel_shared_dpll *
> > > >  bxt_get_dpll(struct intel_crtc *crtc,
> > > >  		struct intel_crtc_state *crtc_state,
> > > >  		struct intel_encoder *encoder)
> > > >  {
> > > > -	struct bxt_clk_div clk_div = {0};
> > > > -	struct intel_dpll_hw_state dpll_hw_state = {0};
> > > > +	struct intel_dpll_hw_state dpll_hw_state = { };
> > > >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > > 
> > > > > base.dev);
> > > >  	struct intel_digital_port *intel_dig_port;
> > > >  	struct intel_shared_dpll *pll;
> > > >  	int i, clock = crtc_state->port_clock;
> > > >  
> > > > -	if (encoder->type == INTEL_OUTPUT_HDMI
> > > > -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> > > > -					  clock, &clk_div))
> > > > +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> > > > +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state,
> > > > clock,
> > > > +					    &dpll_hw_state))
> > > In my original patch there was just a straight call
> > > to bxt_ddi_set_dpll_hw_state() after this and the DP if condition
> > > (which also only calculated dividers).
> > That would require exporting bxt_ddi_dp_pll_dividers(), so I just
> > left
> > it as-is.
> 
> Not necessarily, bxt_ddi_dp_set_dpll_hw_state() could still be
> exported for the
> caller.

Ok, that would've worked. I was thinking along having the same calls
from intel_ddi_get_link_dpll() and bxt_get_dpll() for DPLL setup 
to keep things more unified.

> But the current solution is fine.
> 
> An even better solution would be to have a somewhat platform
> independent entry
> point for force enabling a DPLL independent of modeset with a given
> clock for
> DP. The caller of bxt_ddi_dp_set_dpll_hw_state() will immediately
> enable the
> DPLL anyway, so it could instead just ask the DPLL code to do the
> right thing
> and enable the PLL and leave as much of the platform specific part
> out of the
> caller.

There is no user yet for intel_ddi_get_link_dpll(), but having a
platform independent way to turn DPLLs on/off makes sense to me.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration
  2016-09-26 15:02 ` Jani Nikula
@ 2016-10-13 13:07   ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-10-13 13:07 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Ander Conselvan de Oliveira, intel-gfx, Rodrigo Vivi

On Mon, Sep 26, 2016 at 06:02:20PM +0300, Jani Nikula wrote:
> On Mon, 26 Sep 2016, Imre Deak <imre.deak@intel.com> wrote:
> > a277ca7dc01d should've been a no-functional-change commit, but it
> > removed the initialization of the dpll_hw_state for HDMI outputs,
> > resulting in state mismatches and a failed modeset with blank
> > screen. Fix this by reinstating the dpll_hw_state initialization.
> >
> > Fixes: a277ca7dc01d ("drm/i915: Split bxt_ddi_pll_select()")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Aka pls update your dim copy, it'll generate this automatically.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index c26d18a..e8bf838 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -1694,21 +1694,32 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock,
> >  	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
> >  }
> >  
> > +bool bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
> > +				    struct intel_crtc_state *crtc_state,
> > +				    int clock,
> > +				    struct intel_dpll_hw_state *dpll_hw_state)
> > +{
> > +	struct bxt_clk_div clk_div = { };
> > +
> > +	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock, &clk_div);
> > +
> > +	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
> > +}
> > +
> >  static struct intel_shared_dpll *
> >  bxt_get_dpll(struct intel_crtc *crtc,
> >  		struct intel_crtc_state *crtc_state,
> >  		struct intel_encoder *encoder)
> >  {
> > -	struct bxt_clk_div clk_div = {0};
> > -	struct intel_dpll_hw_state dpll_hw_state = {0};
> > +	struct intel_dpll_hw_state dpll_hw_state = { };
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	struct intel_digital_port *intel_dig_port;
> >  	struct intel_shared_dpll *pll;
> >  	int i, clock = crtc_state->port_clock;
> >  
> > -	if (encoder->type == INTEL_OUTPUT_HDMI
> > -	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
> > -					  clock, &clk_div))
> > +	if (encoder->type == INTEL_OUTPUT_HDMI &&
> > +	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
> > +					    &dpll_hw_state))
> >  		return NULL;
> >  
> >  	if ((encoder->type == INTEL_OUTPUT_DP ||
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-10-13 13:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 14:54 [PATCH] drm/i915/bxt: Fix HDMI DPLL configuration Imre Deak
2016-09-26 15:02 ` Jani Nikula
2016-10-13 13:07   ` Daniel Vetter
2016-09-26 15:25 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-09-27 11:07   ` Imre Deak
2016-09-27 12:46     ` Ville Syrjälä
2016-09-27 14:08       ` Imre Deak
2016-09-27 14:49         ` Ville Syrjälä
2016-09-27  8:03 ` [PATCH] " Ander Conselvan De Oliveira
2016-09-27 11:26   ` Imre Deak
2016-09-27 13:04     ` Ander Conselvan De Oliveira
2016-09-27 15:54       ` Imre Deak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.