On Tue, 2016-03-08 at 11:11 +0000, Conselvan De Oliveira, Ander wrote:
> On Tue, 2016-03-08 at 12:05 +0100, Maarten Lankhorst wrote:
> > Op 26-02-16 om 14:54 schreef Ander Conselvan de Oliveira:
> > > Manage the LCPLLs used with DisplayPort, so that all the HSW/BDW DPLLs
> > > are managed by the shared dpll code.
> > >
> > > Signed-off-by: Ander Conselvan de Oliveira <
> > > ander.conselvan.de.oliveira@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_ddi.c | 18 ++++----
> > > drivers/gpu/drm/i915/intel_display.c | 35 ++++++++++++----
> > > drivers/gpu/drm/i915/intel_dp.c | 23 +---------
> > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 --
> > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 79
> > > +++++++++++++++++++++++++++++++-
> > > ---
> > > drivers/gpu/drm/i915/intel_dpll_mgr.h | 5 ++-
> > > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > > 7 files changed, 110 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index ad7888c..3cb9f36 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -992,17 +992,13 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
> > > {
> > > struct intel_shared_dpll *pll;
> > >
> > > - if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> > > - intel_encoder->type == INTEL_OUTPUT_ANALOG) {
> > > - pll = intel_get_shared_dpll(intel_crtc, crtc_state,
> > > - intel_encoder);
> > > - if (!pll)
> > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe
> > > %c\n",
> > > - pipe_name(intel_crtc->pipe));
> > > - return pll;
> > > - } else {
> > > - return true;
> > > - }
> > > + pll = intel_get_shared_dpll(intel_crtc, crtc_state,
> > > + intel_encoder);
> > > + if (!pll)
> > > + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> > > + pipe_name(intel_crtc->pipe));
> > > +
> > > + return pll;
> > > }
> > Eventually the reliance on intel_encoder->type should end here, and based on
> > the connector.
>
> Do you mean that the parameter to get_shared_dpll() should be connector
> instead
> of encoder?
>
> > It would fix some kms tests, but that would be better to do in a future
> > patch.
> > > ...
> > >
> > > +static bool hsw_ddi_lcpll_get_hw_state(struct drm_i915_private *dev_priv,
> > > + struct intel_shared_dpll *pll,
> > > + struct intel_dpll_hw_state
> > > *hw_state)
> > > +{
> > > + /*
> > > + * LC PLL is kept enabled all the time since it drives CDCLK. The
> > > + * state checker still expects it to be disabled when it is not
> > > used
> > > + * by any crtc. To avoid adding a case to LC PLL, just tell the
> > > + * state checker what it expects.
> > > + */
> > > + if (pll->config.crtc_mask)
> > > + return true;
> > > + else
> > > + return false;
> > > +}
> > Wouldn't it be better to return true or the real hardware state then, and
> > set
> > the ALWAYS_ON flag from the next patch?
>
> That's exactly what v2 of this patch does. :)
Well, actually, I think I fumbled git add or something, so I need to send a v3
of that patch. I also have a v3 of the SKL DPLL0 one, now that I managed to test
it. So I'll resend the series later today.
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
