Re: [PATCH 3/8] drm/i915: Fold intel_ironlake_limit() into clock computation function

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Mon, 2016-03-14 at 12:46 +0100, Maarten Lankhorst wrote:
> Op 14-03-16 om 09:55 schreef Ander Conselvan de Oliveira:
> > The funcion intel_ironlake_limit() is only called by the crtc compute
> > clock path. By merging it into ironlake_compute_clocks(), the code gets
> > clearer, since there's no more if-ladders to follow.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 56 +++++++++++++++------------------
> > ---
> >  1 file changed, 23 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 07b5244..ea71430 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -566,30 +566,6 @@ static bool intel_pipe_will_have_type(const struct
> > intel_crtc_state *crtc_state,
> >  }
> >  
> >  static const intel_limit_t *
> > -intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
> > -{
> > -	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	const intel_limit_t *limit;
> > -
> > -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> > -		if (intel_is_dual_link_lvds(dev)) {
> > -			if (refclk == 100000)
> > -				limit =
> > &intel_limits_ironlake_dual_lvds_100m;
> > -			else
> > -				limit = &intel_limits_ironlake_dual_lvds;
> > -		} else {
> > -			if (refclk == 100000)
> > -				limit =
> > &intel_limits_ironlake_single_lvds_100m;
> > -			else
> > -				limit = &intel_limits_ironlake_single_lvds;
> > -		}
> > -	} else
> > -		limit = &intel_limits_ironlake_dac;
> > -
> > -	return limit;
> > -}
> > -
> > -static const intel_limit_t *
> >  intel_g4x_limit(struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_device *dev = crtc_state->base.crtc->dev;
> > @@ -619,8 +595,8 @@ intel_limit(struct intel_crtc_state *crtc_state, int
> > refclk)
> >  
> >  	if (IS_BROXTON(dev))
> >  		limit = &intel_limits_bxt;
> > -	else if (HAS_PCH_SPLIT(dev))
> > -		limit = intel_ironlake_limit(crtc_state, refclk);
> > +	else if (WARN_ON(HAS_PCH_SPLIT(dev)))
> > +		limit = NULL;
> >  	else if (IS_G4X(dev)) {
> >  		limit = intel_g4x_limit(crtc_state);
> >  	} else if (IS_PINEVIEW(dev)) {
> I'm curious, when is intel_limits_bxt ever used? Seems like dead code..
> 
> It would appear it uses haswell_crtc_compute_clock, which never calls into
> intel_limit().

It is called from bxt_find_best_dpll(), which is called form the broxton shared
dpll code. I just wrote a patch this morning to make that function reference
 intel_limits_bxt directly. I want to get rid of intel_limit() altogether if
possible, since those if-ladders get confusing really fast.

Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx