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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 14-03-16 om 12:53 schreef Ander Conselvan De Oliveira:
> 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.
>
Ah, no idea why I missed it. But indeed, best get rid of it.

On that you can add my r-b, same for this series if CI is happy. :)

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