On ti, 2016-03-15 at 09:50 +0200, Ander Conselvan De Oliveira wrote:
> On Mon, 2016-03-14 at 19:55 +0200, Imre Deak wrote:
> > After the commit below the Broxton PLL IDs had an off-by-one error,
> > so
> > fix this up. Also add a missing brace at intel_shared_dpll_init(),
> > it
> > happened to compile only due to the way the IS_BROXTON macro is
> > defined.
> >
> > v2:
> > - remove debugging left-over
> >
> > Fixes: a3c988ea068c ("drm/i915: Make SKL/KBL DPLL0 managed by the
> > shared dpll
> > code")
> > CC: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.
> > com>
> > CC: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 6 +++---
> > drivers/gpu/drm/i915/intel_dpll_mgr.c | 8 ++++----
> > 2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index a720628..3cbb02b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9762,15 +9762,15 @@ static void bxt_get_ddi_pll(struct
> > drm_i915_private
> > *dev_priv,
> > switch (port) {
> > case PORT_A:
> > pipe_config->ddi_pll_sel = SKL_DPLL0;
> > - id = DPLL_ID_SKL_DPLL1;
> > + id = DPLL_ID_SKL_DPLL0;
> > break;
> > case PORT_B:
> > pipe_config->ddi_pll_sel = SKL_DPLL1;
> > - id = DPLL_ID_SKL_DPLL2;
> > + id = DPLL_ID_SKL_DPLL1;
> > break;
> > case PORT_C:
> > pipe_config->ddi_pll_sel = SKL_DPLL2;
> > - id = DPLL_ID_SKL_DPLL3;
> > + id = DPLL_ID_SKL_DPLL2;
>
> Hmm, I think the mistake is that BXT relied on SKL DPLL ids in the
> first place.
> This is fine as a fix up, but maybe it would be better to just add
> separate ids
> for broxton too. Or even better, get rid of pll->id.
Yes, SKL_DPLLx and DPLL_ID_SKL_DPLLx that were defined originally had
confusingly different indexes already, not sure for the reason for
that. Removing 'id' if possible seems like a good idea.
>
> > break;
> > default:
> > DRM_ERROR("Incorrect port type\n");
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 4b636c4..74d5aec 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -1706,9 +1706,9 @@ static const struct intel_dpll_mgr
> > skl_pll_mgr = {
> > };
> >
> > static const struct dpll_info bxt_plls[] = {
> > - { "PORT PLL A", 0, &bxt_ddi_pll_funcs, 0 },
> > - { "PORT PLL B", 1, &bxt_ddi_pll_funcs, 0 },
> > - { "PORT PLL C", 2, &bxt_ddi_pll_funcs, 0 },
> > + { "PORT PLL A", DPLL_ID_SKL_DPLL0, &bxt_ddi_pll_funcs, 0
> > },
> > + { "PORT PLL B", DPLL_ID_SKL_DPLL1, &bxt_ddi_pll_funcs, 0
> > },
> > + { "PORT PLL C", DPLL_ID_SKL_DPLL2, &bxt_ddi_pll_funcs, 0
> > },
> > { NULL, -1, NULL, },
> > };
> >
> > @@ -1726,7 +1726,7 @@ void intel_shared_dpll_init(struct drm_device
> > *dev)
> >
> > if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> > dpll_mgr = &skl_pll_mgr;
> > - else if IS_BROXTON(dev)
> > + else if (IS_BROXTON(dev))
>
> Wow, really don't know how I managed to do this. Thanks for fixing.
>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx>
Thanks for the review.
>
> > dpll_mgr = &bxt_pll_mgr;
> > else if (HAS_DDI(dev))
> > dpll_mgr = &hsw_pll_mgr;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
