On Fri, Apr 05, 2019 at 04:33:30PM -0700, Vivek Kasireddy wrote:
> On Fri, 5 Apr 2019 21:39:11 +0300
> Ville Syrjälä <[email protected]> wrote:
> Hi Ville,
>
> > On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:
> > > On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:
> > > > This patch adds support for DPLL4 on EHL that include the
> > > > following restrictions:
> > > >
> > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP
> > > > usage). DPLL4 can be used with other DDIs, including DDID
> > > > (combo port A external usage).
> > > >
> > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > > >
> > > > - The DPLL4 enable, lock, power enabled, and power state are
> > > > connected to the MGPLL1_ENABLE register.
> > > >
> > > > v2: (suggestions from Bob Paauwe)
> > > > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > > > and iterate twice: once for Combo plls and once for MG plls.
> > > >
> > > > - Use MG pll funcs for DPLL4 instead of creating new ones and
> > > > modify mg_pll_enable to include the restrictions for EHL.
> > > >
> > > > v3: Fix compilation error
> > > >
> > > > Cc: Lucas De Marchi <[email protected]>
> > > > Signed-off-by: Vivek Kasireddy <[email protected]>
> > > > Reviewed-by: Bob Paauwe <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > > > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > > > insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > > > e01c057ce50b..c3f0b9720c54 100644 ---
> > > > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> > > > icl_get_dpll(struct intel_crtc_state *crtc_state, return pll;
> > > > }
> > > >
> > > > +static struct intel_shared_dpll *
> > > > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > > > + struct intel_encoder *encoder)
> > > > +{
> > > > + struct drm_i915_private *dev_priv =
> > > > to_i915(crtc_state->base.crtc->dev);
> > > > + struct intel_shared_dpll *pll;
> > > > + enum port port = encoder->port;
> > > > + enum intel_dpll_id min, max;
> > > > + bool ret;
> > > > +
> > > > + if (!intel_port_is_combophy(dev_priv, port)) {
> > > > + MISSING_CASE(port);
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + min = DPLL_ID_ICL_DPLL0;
> > > > + max = DPLL_ID_ICL_DPLL1;
> > > > + ret = icl_calc_dpll_state(crtc_state, encoder);
> > > > + if (ret) {
> > > > + pll = intel_find_shared_dpll(crtc_state, min,
> > > > max);
> > > > + if (pll) {
> > > > + intel_reference_shared_dpll(pll,
> > > > crtc_state);
> > > > + return pll;
> > > > + }
> > > > + } else {
> > > > + DRM_DEBUG_KMS("Could not calculate PLL
> > > > state.\n");
> > > > + }
> > > > +
> > > > + if (encoder->type == INTEL_OUTPUT_EDP) {
> > > > + DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + min = max = DPLL_ID_ICL_MGPLL1;
> > > > + ret = icl_calc_mg_pll_state(crtc_state);
> > > > + if (!ret) {
> > > > + DRM_DEBUG_KMS("Could not calculate PLL
> > > > state.\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + pll = intel_find_shared_dpll(crtc_state, min, max);
> > > > + if (!pll) {
> > > > + DRM_DEBUG_KMS("No PLL selected\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + intel_reference_shared_dpll(pll, crtc_state);
> > > > + return pll;
> > > > +}
> > > > +
> > > > static bool mg_pll_get_hw_state(struct drm_i915_private
> > > > *dev_priv, struct intel_shared_dpll *pll,
> > > > struct intel_dpll_hw_state
> > > > *hw_state) @@ -3115,6 +3165,13 @@ static void
> > > > mg_pll_enable(struct drm_i915_private *dev_priv, i915_reg_t
> > > > enable_reg = MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> > > >
> > > > + if (IS_ELKHARTLAKE(dev_priv) &&
> > > > + (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > > > + I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > > > + DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are
> > > > enabled\n");
> > > > + return;
> > > > + }
> > >
> > > This looks like dead code, or we messed up earlier already (in which
> > > case it should just be some kind of assert IMO).
> Are you suggesting that I put this in a ->prepare() hook or much earlier
> in the atomic check/prepare phase? FYI, I am just adding a restriction
> mentioned in the spec concerning this DPLL.
>
> >
> > Does DMC handle other MG PLLs? I would have thought not, so if we keep
> > the assert then it should perhaps be unconditional. Hmm. But aren't we
> > still hodling the modeset power domain when this gets called? If so
> > this is definitely dead code.
> I am not sure whether DMC handles other MG PLLs but the spec explicitly
> says that DMC will not handle this PLL. Where do you suggest is an
> appropriate place to put this assert?
DC5/6 enable probably is the best, if we even want the assert. And I'm
not convinced that we do since we don't have it for any other PLL
either.
>
> >
> > >
> > > Also what is this EHL thing anyway? I can't even find it in the
> > > spec.
> EHL = Elkhart Lake, a new Gen 11 platform.
>
> Thanks,
> Vivek
>
> > > > +
> > > > icl_pll_power_enable(dev_priv, pll, enable_reg);
> > > >
> > > > icl_mg_pll_write(dev_priv, pll);
> > > > @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr
> > > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> > > > { "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > > > { "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > > + { "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> > > > { },
> > > > };
> > > >
> > > > static const struct intel_dpll_mgr ehl_pll_mgr = {
> > > > .dpll_info = ehl_plls,
> > > > - .get_dpll = icl_get_dpll,
> > > > + .get_dpll = ehl_get_dpll,
> > > > .dump_hw_state = icl_dump_hw_state,
> > > > };
> > > >
> > > > --
> > > > 2.14.5
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > [email protected]
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
