Re: [PATCH v2 16/23] drm/i915: Sanitize the shared DPLL reserve/release interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 25, 2019 at 04:53:01PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 20, 2019 at 05:05:53PM +0300, Imre Deak wrote:
> > For consistency s/intel_get_shared_dpll()/intel_reserve_shared_dplls()/
> > to better match intel_release_shared_dplls(). Also, pass to the
> > reserve/release and get_dplls/put_dplls hooks the intel_atomic_state and
> > CRTC object, that way these functions can look up the old or new state
> > as needed.
> > 
> > Also release the PLLs from the atomic state via a new
> > put_dplls->intel_unreference_shared_dpll() call chain for better
> > symmetry with the reservation via the
> > get_dplls->intel_reference_shared_dpll() call chain.
> > 
> > Since nothing uses the PLL returned by intel_reserve_shared_dplls(),
> > make it return only a bool.
> > 
> > While at it also clarify the reserve/release function docbook headers
> > making it clear that multiple DPLLs will be reserved/released and
> > whether the new or old atomic CRTC state is affected.
> > 
> > This refactoring is also a preparation for a follow-up change that needs
> > to reserve multiple DPLLs.
> > 
> > Kudos to Ville for the idea to pass intel_atomic_state around, to make
> > things clearer locally where an object's old/new atomic state is
> > required.
> > 
> > No functional changes.
> > 
> > v2:
> > - Fix checkpatch issue: typo in code comment.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  19 +-
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 221 +++++++++++-------
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.h |  13 +-
> >  3 files changed, 153 insertions(+), 100 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 468ca6d84bd8..688137524179 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -9504,6 +9504,8 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
> >  				       struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	struct intel_atomic_state *state =
> > +		to_intel_atomic_state(crtc_state->base.state);
> >  	const struct intel_limit *limit;
> >  	int refclk = 120000;
> >  
> > @@ -9545,7 +9547,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
> >  
> >  	ironlake_compute_dpll(crtc, crtc_state, NULL);
> >  
> > -	if (!intel_get_shared_dpll(crtc_state, NULL)) {
> > +	if (!intel_reserve_shared_dplls(state, crtc, NULL)) {
> >  		DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> >  			      pipe_name(crtc->pipe));
> >  		return -EINVAL;
> > @@ -9926,7 +9928,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >  		struct intel_encoder *encoder =
> >  			intel_get_crtc_new_encoder(state, crtc_state);
> >  
> > -		if (!intel_get_shared_dpll(crtc_state, encoder)) {
> > +		if (!intel_reserve_shared_dplls(state, crtc, encoder)) {
> >  			DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
> >  				      pipe_name(crtc->pipe));
> >  			return -EINVAL;
> > @@ -13195,27 +13197,20 @@ static void update_scanline_offset(const struct intel_crtc_state *crtc_state)
> >  static void intel_modeset_clear_plls(struct intel_atomic_state *state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > -	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> > +	struct intel_crtc_state *new_crtc_state;
> >  	struct intel_crtc *crtc;
> >  	int i;
> >  
> >  	if (!dev_priv->display.crtc_compute_clock)
> >  		return;
> >  
> > -	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > -					    new_crtc_state, i) {
> > -		struct intel_shared_dpll *old_dpll =
> > -			old_crtc_state->shared_dpll;
> > -
> > +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> >  		if (!needs_modeset(&new_crtc_state->base))
> >  			continue;
> >  
> >  		new_crtc_state->shared_dpll = NULL;
> >  
> > -		if (!old_dpll)
> > -			continue;
> > -
> > -		intel_release_shared_dpll(old_dpll, crtc, &state->base);
> > +		intel_release_shared_dplls(state, crtc);
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index bf66261c8bf0..3fbc975851fa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -36,9 +36,10 @@
> >   * This file provides an abstraction over display PLLs. The function
> >   * intel_shared_dpll_init() initializes the PLLs for the given platform.  The
> >   * users of a PLL are tracked and that tracking is integrated with the atomic
> > - * modest interface. During an atomic operation, a PLL can be requested for a
> > - * given CRTC and encoder configuration by calling intel_get_shared_dpll() and
> > - * a previously used PLL can be released with intel_release_shared_dpll().
> > + * modset interface. During an atomic operation, required PLLs can be reserved
> > + * for a given CRTC and encoder configuration by calling
> > + * intel_reserve_shared_dplls() and previously reserved PLLs can be released
> > + * with intel_release_shared_dplls().
> >   * Changes to the users are first staged in the atomic state, and then made
> >   * effective by calling intel_shared_dpll_swap_state() during the atomic
> >   * commit phase.
> > @@ -309,6 +310,28 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
> >  	shared_dpll[id].crtc_mask |= 1 << crtc->pipe;
> >  }
> >  
> > +static void intel_unreference_shared_dpll(struct intel_atomic_state *state,
> > +					  const struct intel_crtc *crtc,
> > +					  const struct intel_shared_dpll *pll)
> > +{
> > +	struct intel_shared_dpll_state *shared_dpll;
> > +
> > +	shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
> > +	shared_dpll[pll->info->id].crtc_mask &= ~(1 << crtc->pipe);
> > +}
> > +
> > +static void intel_put_dpll(struct intel_atomic_state *state,
> > +			   struct intel_crtc *crtc)
> > +{
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_old_crtc_state(state, crtc);
> 
> I'm wondering a bit why we're using the old crtc state here. We
> duplicated the state so shouldn't the new crtc state have the
> same dpll still at this point?
> 
> Doesn't really matter I suppose. Not even sure which state would
> be more correct here.

You are right, the new crtc state would be the better choice here. The
two states are equal here yes, but in the upcoming icl_put_dplls() we
should clear the new crtc_state->icl_port_dplls. Not clearing them
happens to not cause a problem, since we reallocate all PLLs in
icl_get_dplls(), but it's better to do the clearing anyway for
consistency.

I'll change that.

> 
> Anyways, the patch seems OK.
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> > +
> > +	if (!crtc_state->shared_dpll)
> > +		return;
> > +
> > +	intel_unreference_shared_dpll(state, crtc, crtc_state->shared_dpll);
> > +}
> > +
> >  /**
> >   * intel_shared_dpll_swap_state - make atomic DPLL configuration effective
> >   * @state: atomic state
> > @@ -421,11 +444,12 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
> >  	udelay(200);
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -ibx_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool ibx_get_dpll(struct intel_atomic_state *state,
> > +			 struct intel_crtc *crtc,
> > +			 struct intel_encoder *encoder)
> >  {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	struct intel_shared_dpll *pll;
> >  	enum intel_dpll_id i;
> > @@ -445,12 +469,12 @@ ibx_get_dpll(struct intel_crtc_state *crtc_state,
> >  	}
> >  
> >  	if (!pll)
> > -		return NULL;
> > +		return false;
> >  
> >  	/* reference the pll */
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static void ibx_dump_hw_state(struct drm_i915_private *dev_priv,
> > @@ -821,10 +845,12 @@ hsw_ddi_dp_get_dpll(struct intel_crtc_state *crtc_state)
> >  	return pll;
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -hsw_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool hsw_get_dpll(struct intel_atomic_state *state,
> > +			 struct intel_crtc *crtc,
> > +			 struct intel_encoder *encoder)
> >  {
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct intel_shared_dpll *pll;
> >  
> >  	memset(&crtc_state->dpll_hw_state, 0,
> > @@ -836,7 +862,7 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
> >  		pll = hsw_ddi_dp_get_dpll(crtc_state);
> >  	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> >  		if (WARN_ON(crtc_state->port_clock / 2 != 135000))
> > -			return NULL;
> > +			return false;
> >  
> >  		crtc_state->dpll_hw_state.spll =
> >  			SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz | SPLL_REF_MUXED_SSC;
> > @@ -844,15 +870,15 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
> >  		pll = intel_find_shared_dpll(crtc_state,
> >  					     DPLL_ID_SPLL, DPLL_ID_SPLL);
> >  	} else {
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	if (!pll)
> > -		return NULL;
> > +		return false;
> >  
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static void hsw_dump_hw_state(struct drm_i915_private *dev_priv,
> > @@ -1385,10 +1411,12 @@ skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> >  	return true;
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -skl_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool skl_get_dpll(struct intel_atomic_state *state,
> > +			 struct intel_crtc *crtc,
> > +			 struct intel_encoder *encoder)
> >  {
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct intel_shared_dpll *pll;
> >  	bool bret;
> >  
> > @@ -1396,16 +1424,16 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
> >  		bret = skl_ddi_hdmi_pll_dividers(crtc_state);
> >  		if (!bret) {
> >  			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
> > -			return NULL;
> > +			return false;
> >  		}
> >  	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
> >  		bret = skl_ddi_dp_set_dpll_hw_state(crtc_state);
> >  		if (!bret) {
> >  			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
> > -			return NULL;
> > +			return false;
> >  		}
> >  	} else {
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> > @@ -1417,11 +1445,11 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
> >  					     DPLL_ID_SKL_DPLL1,
> >  					     DPLL_ID_SKL_DPLL3);
> >  	if (!pll)
> > -		return NULL;
> > +		return false;
> >  
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static void skl_dump_hw_state(struct drm_i915_private *dev_priv,
> > @@ -1827,22 +1855,23 @@ bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> >  	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -bxt_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool bxt_get_dpll(struct intel_atomic_state *state,
> > +			 struct intel_crtc *crtc,
> > +			 struct intel_encoder *encoder)
> >  {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	struct intel_shared_dpll *pll;
> >  	enum intel_dpll_id id;
> >  
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> >  	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc_state))
> > -		return NULL;
> > +		return false;
> >  
> >  	if (intel_crtc_has_dp_encoder(crtc_state) &&
> >  	    !bxt_ddi_dp_set_dpll_hw_state(crtc_state))
> > -		return NULL;
> > +		return false;
> >  
> >  	/* 1:1 mapping between ports and PLLs */
> >  	id = (enum intel_dpll_id) encoder->port;
> > @@ -1853,7 +1882,7 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
> >  
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static void bxt_dump_hw_state(struct drm_i915_private *dev_priv,
> > @@ -1884,8 +1913,11 @@ static const struct intel_shared_dpll_funcs bxt_ddi_pll_funcs = {
> >  struct intel_dpll_mgr {
> >  	const struct dpll_info *dpll_info;
> >  
> > -	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc_state *crtc_state,
> > -					      struct intel_encoder *encoder);
> > +	bool (*get_dplls)(struct intel_atomic_state *state,
> > +			  struct intel_crtc *crtc,
> > +			  struct intel_encoder *encoder);
> > +	void (*put_dplls)(struct intel_atomic_state *state,
> > +			  struct intel_crtc *crtc);
> >  
> >  	void (*dump_hw_state)(struct drm_i915_private *dev_priv,
> >  			      const struct intel_dpll_hw_state *hw_state);
> > @@ -1899,7 +1931,8 @@ static const struct dpll_info pch_plls[] = {
> >  
> >  static const struct intel_dpll_mgr pch_pll_mgr = {
> >  	.dpll_info = pch_plls,
> > -	.get_dpll = ibx_get_dpll,
> > +	.get_dplls = ibx_get_dpll,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = ibx_dump_hw_state,
> >  };
> >  
> > @@ -1915,7 +1948,8 @@ static const struct dpll_info hsw_plls[] = {
> >  
> >  static const struct intel_dpll_mgr hsw_pll_mgr = {
> >  	.dpll_info = hsw_plls,
> > -	.get_dpll = hsw_get_dpll,
> > +	.get_dplls = hsw_get_dpll,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = hsw_dump_hw_state,
> >  };
> >  
> > @@ -1929,7 +1963,8 @@ static const struct dpll_info skl_plls[] = {
> >  
> >  static const struct intel_dpll_mgr skl_pll_mgr = {
> >  	.dpll_info = skl_plls,
> > -	.get_dpll = skl_get_dpll,
> > +	.get_dplls = skl_get_dpll,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = skl_dump_hw_state,
> >  };
> >  
> > @@ -1942,7 +1977,8 @@ static const struct dpll_info bxt_plls[] = {
> >  
> >  static const struct intel_dpll_mgr bxt_pll_mgr = {
> >  	.dpll_info = bxt_plls,
> > -	.get_dpll = bxt_get_dpll,
> > +	.get_dplls = bxt_get_dpll,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = bxt_dump_hw_state,
> >  };
> >  
> > @@ -2332,10 +2368,12 @@ cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> >  	return true;
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -cnl_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool cnl_get_dpll(struct intel_atomic_state *state,
> > +			 struct intel_crtc *crtc,
> > +			 struct intel_encoder *encoder)
> >  {
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct intel_shared_dpll *pll;
> >  	bool bret;
> >  
> > @@ -2343,18 +2381,18 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
> >  		bret = cnl_ddi_hdmi_pll_dividers(crtc_state);
> >  		if (!bret) {
> >  			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
> > -			return NULL;
> > +			return false;
> >  		}
> >  	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
> >  		bret = cnl_ddi_dp_set_dpll_hw_state(crtc_state);
> >  		if (!bret) {
> >  			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
> > -			return NULL;
> > +			return false;
> >  		}
> >  	} else {
> >  		DRM_DEBUG_KMS("Skip DPLL setup for output_types 0x%x\n",
> >  			      crtc_state->output_types);
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	pll = intel_find_shared_dpll(crtc_state,
> > @@ -2362,12 +2400,12 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
> >  				     DPLL_ID_SKL_DPLL2);
> >  	if (!pll) {
> >  		DRM_DEBUG_KMS("No PLL selected\n");
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static void cnl_dump_hw_state(struct drm_i915_private *dev_priv,
> > @@ -2394,7 +2432,8 @@ static const struct dpll_info cnl_plls[] = {
> >  
> >  static const struct intel_dpll_mgr cnl_pll_mgr = {
> >  	.dpll_info = cnl_plls,
> > -	.get_dpll = cnl_get_dpll,
> > +	.get_dplls = cnl_get_dpll,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = cnl_dump_hw_state,
> >  };
> >  
> > @@ -2792,11 +2831,13 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
> >  	return true;
> >  }
> >  
> > -static struct intel_shared_dpll *
> > -icl_get_dpll(struct intel_crtc_state *crtc_state,
> > -	     struct intel_encoder *encoder)
> > +static bool icl_get_dplls(struct intel_atomic_state *state,
> > +			  struct intel_crtc *crtc,
> > +			  struct intel_encoder *encoder)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > +	struct intel_crtc_state *crtc_state =
> > +		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct intel_digital_port *intel_dig_port;
> >  	struct intel_shared_dpll *pll;
> >  	enum port port = encoder->port;
> > @@ -2831,24 +2872,24 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
> >  		}
> >  	} else {
> >  		MISSING_CASE(port);
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	if (!ret) {
> >  		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  
> >  	pll = intel_find_shared_dpll(crtc_state, min, max);
> >  	if (!pll) {
> >  		DRM_DEBUG_KMS("No PLL selected\n");
> > -		return NULL;
> > +		return false;
> >  	}
> >  
> >  	intel_reference_shared_dpll(pll, crtc_state);
> >  
> > -	return pll;
> > +	return true;
> >  }
> >  
> >  static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
> > @@ -3223,7 +3264,8 @@ static const struct dpll_info icl_plls[] = {
> >  
> >  static const struct intel_dpll_mgr icl_pll_mgr = {
> >  	.dpll_info = icl_plls,
> > -	.get_dpll = icl_get_dpll,
> > +	.get_dplls = icl_get_dplls,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = icl_dump_hw_state,
> >  };
> >  
> > @@ -3235,7 +3277,8 @@ static const struct dpll_info ehl_plls[] = {
> >  
> >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> >  	.dpll_info = ehl_plls,
> > -	.get_dpll = icl_get_dpll,
> > +	.get_dplls = icl_get_dplls,
> > +	.put_dplls = intel_put_dpll,
> >  	.dump_hw_state = icl_dump_hw_state,
> >  };
> >  
> > @@ -3287,50 +3330,64 @@ void intel_shared_dpll_init(struct drm_device *dev)
> >  }
> >  
> >  /**
> > - * intel_get_shared_dpll - get a shared DPLL for CRTC and encoder combination
> > - * @crtc_state: atomic state for the crtc
> > + * intel_reserve_shared_dplls - reserve DPLLs for CRTC and encoder combination
> > + * @state: atomic state
> > + * @crtc: CRTC to reserve DPLLs for
> >   * @encoder: encoder
> >   *
> > - * Find an appropriate DPLL for the given CRTC and encoder combination. A
> > - * reference from the @crtc_state to the returned pll is registered in the
> > - * atomic state. That configuration is made effective by calling
> > - * intel_shared_dpll_swap_state(). The reference should be released by calling
> > - * intel_release_shared_dpll().
> > + * This function reserves all required DPLLs for the given CRTC and encoder
> > + * combination in the current atomic commit @state and the new @crtc atomic
> > + * state.
> > + *
> > + * The new configuration in the atomic commit @state is made effective by
> > + * calling intel_shared_dpll_swap_state().
> > + *
> > + * The reserved DPLLs should be released by calling
> > + * intel_release_shared_dplls().
> >   *
> >   * Returns:
> > - * A shared DPLL to be used by @crtc_state and @encoder.
> > + * True if all required DPLLs were successfully reserved.
> >   */
> > -struct intel_shared_dpll *
> > -intel_get_shared_dpll(struct intel_crtc_state *crtc_state,
> > -		      struct intel_encoder *encoder)
> > +bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
> > +				struct intel_crtc *crtc,
> > +				struct intel_encoder *encoder)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> >  	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
> >  
> >  	if (WARN_ON(!dpll_mgr))
> > -		return NULL;
> > +		return false;
> >  
> > -	return dpll_mgr->get_dpll(crtc_state, encoder);
> > +	return dpll_mgr->get_dplls(state, crtc, encoder);
> >  }
> >  
> >  /**
> > - * intel_release_shared_dpll - end use of DPLL by CRTC in atomic state
> > - * @dpll: dpll in use by @crtc
> > - * @crtc: crtc
> > + * intel_release_shared_dplls - end use of DPLLs by CRTC in atomic state
> >   * @state: atomic state
> > + * @crtc: crtc from which the DPLLs are to be released
> >   *
> > - * This function releases the reference from @crtc to @dpll from the
> > - * atomic @state. The new configuration is made effective by calling
> > - * intel_shared_dpll_swap_state().
> > + * This function releases all DPLLs reserved by intel_reserve_shared_dplls()
> > + * from the current atomic commit @state and the old @crtc atomic state.
> > + *
> > + * The new configuration in the atomic commit @state is made effective by
> > + * calling intel_shared_dpll_swap_state().
> >   */
> > -void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
> > -			       struct intel_crtc *crtc,
> > -			       struct drm_atomic_state *state)
> > +void intel_release_shared_dplls(struct intel_atomic_state *state,
> > +				struct intel_crtc *crtc)
> >  {
> > -	struct intel_shared_dpll_state *shared_dpll_state;
> > +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > +	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
> > +
> > +	/*
> > +	 * FIXME: this function is called for every platform having a
> > +	 * compute_clock hook, even though the platform doesn't yet support
> > +	 * the shared DPLL framework and intel_reserve_shared_dplls() is not
> > +	 * called on those.
> > +	 */
> > +	if (!dpll_mgr)
> > +		return;
> >  
> > -	shared_dpll_state = intel_atomic_get_shared_dpll_state(state);
> > -	shared_dpll_state[dpll->info->id].crtc_mask &= ~(1 << crtc->pipe);
> > +	dpll_mgr->put_dplls(state, crtc);
> >  }
> >  
> >  /**
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > index d0570414f3d1..16ddab138574 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> > @@ -39,6 +39,7 @@
> >  struct drm_atomic_state;
> >  struct drm_device;
> >  struct drm_i915_private;
> > +struct intel_atomic_state;
> >  struct intel_crtc;
> >  struct intel_crtc_state;
> >  struct intel_encoder;
> > @@ -195,7 +196,7 @@ struct intel_dpll_hw_state {
> >   * future state which would be applied by an atomic mode set (stored in
> >   * a struct &intel_atomic_state).
> >   *
> > - * See also intel_get_shared_dpll() and intel_release_shared_dpll().
> > + * See also intel_reserve_shared_dplls() and intel_release_shared_dplls().
> >   */
> >  struct intel_shared_dpll_state {
> >  	/**
> > @@ -331,11 +332,11 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
> >  			bool state);
> >  #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
> >  #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
> > -struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc_state *state,
> > -						struct intel_encoder *encoder);
> > -void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
> > -			       struct intel_crtc *crtc,
> > -			       struct drm_atomic_state *state);
> > +bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
> > +				struct intel_crtc *crtc,
> > +				struct intel_encoder *encoder);
> > +void intel_release_shared_dplls(struct intel_atomic_state *state,
> > +				struct intel_crtc *crtc);
> >  void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
> >  void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
> >  void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
> > -- 
> > 2.17.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux