On Mon, Jun 22, 2015 at 05:22:33PM +0300, Imre Deak wrote:
> On ma, 2015-06-22 at 16:44 +0300, Ville Syrjälä wrote:
> > On Thu, Jun 18, 2015 at 05:25:57PM +0300, Imre Deak wrote:
> > > Add support for reading out the HW state for DDI ports. Since the actual
> > > programming is very similar to the CHV/VLV DPIO PLL programming we can
> > > reuse much of the logic from there.
> > >
> > > This fixes the state checker failures I saw on my BXT with HDMI output.
> > >
> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++------
> > > drivers/gpu/drm/i915/intel_ddi.c | 22 ++++++++++++++++++++--
> > > 2 files changed, 29 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index bba0691..fcf6ad5 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -1169,10 +1169,12 @@ enum skl_disp_power_wells {
> > > #define _PORT_PLL_EBB_0_A 0x162034
> > > #define _PORT_PLL_EBB_0_B 0x6C034
> > > #define _PORT_PLL_EBB_0_C 0x6C340
> > > -#define PORT_PLL_P1_MASK (0x07 << 13)
> > > -#define PORT_PLL_P1(x) ((x) << 13)
> > > -#define PORT_PLL_P2_MASK (0x1f << 8)
> > > -#define PORT_PLL_P2(x) ((x) << 8)
> > > +#define PORT_PLL_P1_SHIFT 13
> > > +#define PORT_PLL_P1_MASK (0x07 << PORT_PLL_P1_SHIFT)
> > > +#define PORT_PLL_P1(x) ((x) << PORT_PLL_P1_SHIFT)
> > > +#define PORT_PLL_P2_SHIFT 8
> > > +#define PORT_PLL_P2_MASK (0x1f << PORT_PLL_P2_SHIFT)
> > > +#define PORT_PLL_P2(x) ((x) << PORT_PLL_P2_SHIFT)
> > > #define BXT_PORT_PLL_EBB_0(port) _PORT3(port, _PORT_PLL_EBB_0_A, \
> > > _PORT_PLL_EBB_0_B, \
> > > _PORT_PLL_EBB_0_C)
> > > @@ -1192,8 +1194,9 @@ enum skl_disp_power_wells {
> > > /* PORT_PLL_0_A */
> > > #define PORT_PLL_M2_MASK 0xFF
> > > /* PORT_PLL_1_A */
> > > -#define PORT_PLL_N_MASK (0x0F << 8)
> > > -#define PORT_PLL_N(x) ((x) << 8)
> > > +#define PORT_PLL_N_SHIFT 8
> > > +#define PORT_PLL_N_MASK (0x0F << PORT_PLL_N_SHIFT)
> > > +#define PORT_PLL_N(x) ((x) << PORT_PLL_N_SHIFT)
> > > /* PORT_PLL_2_A */
> > > #define PORT_PLL_M2_FRAC_MASK 0x3FFFFF
> > > /* PORT_PLL_3_A */
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index ca970ba..6859068 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -971,8 +971,26 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
> > > static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
> > > enum intel_dpll_id dpll)
> > > {
> > > - /* FIXME formula not available in bspec */
> > > - return 0;
> > > + struct intel_shared_dpll *pll;
> > > + struct intel_dpll_hw_state *state;
> > > + intel_clock_t clock;
> > > +
> > > + /* For DDI ports we always use a shared PLL. */
> > > + if (WARN_ON(dpll == DPLL_ID_PRIVATE))
> > > + return 0;
> > > +
> > > + pll = &dev_priv->shared_dplls[dpll];
> > > + state = &pll->config.hw_state;
> > > +
> > > + clock.m1 = 2;
> >
> > For chv I opted to read out the m1 divider from PLL_DW1 too even though
> > 2 is the only supported value. Doing so might catch some cases where the
> > register gets misprogrammed.
>
> I haven't found the BXT counterpart for this at least when looking
> through BSpec. PORT_PLL_1[2:0] is "i_fbpredivratio" and it reads 0 for
> me.
That's the one.
>
> > But that's a minor issue, so even without this is
> > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > > + clock.m2 = (state->pll0 & PORT_PLL_M2_MASK) << 22;
> > > + if (state->pll3 & PORT_PLL_M2_FRAC_ENABLE)
> > > + clock.m2 |= state->pll2 & PORT_PLL_M2_FRAC_MASK;
> >
> > I just realized we're missing the fracnen check from chv_crtc_clock_get().
> > Care to send a patch to add the check there as well?
>
> Ok.
>
> > > + clock.n = (state->pll1 & PORT_PLL_N_MASK) >> PORT_PLL_N_SHIFT;
>
> Btw, now that I looked through BSpec and the code again, this looks like
> also be tied to 1, so we'd need to check for this in
> bxt_ddi_pll_select(). chv_find_best_dpll() does the right thing already.
You mean WARN if n != 1? I suppose it can't hurt.
>
> > > + clock.p1 = (state->ebb0 & PORT_PLL_P1_MASK) >> PORT_PLL_P1_SHIFT;
> > > + clock.p2 = (state->ebb0 & PORT_PLL_P2_MASK) >> PORT_PLL_P2_SHIFT;
> > > +
> > > + return vlv_calc_port_clock(100000, &clock);
> > > }
> > >
> > > static void bxt_ddi_clock_get(struct intel_encoder *encoder,
> > > --
> > > 2.1.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
