Re: [PATCH 2/8] drm/i915: Merge ironlake_get_refclk() into its only caller

["Conselvan De Oliveira, Ander" <ander.conselvan.de.oliveira@intel.com>]

On Mon, 2016-03-14 at 15:55 +0200, Ville Syrjälä wrote:
> On Mon, Mar 14, 2016 at 10:55:41AM +0200, Ander Conselvan de Oliveira wrote:
> > A previous patche made ironlake_get_refclk() very simple, so merge
> > it into its only caller.
> 
> Again I'd like to keep the pch and gmch code as similar as possible.
> So could do the same for the gmch code.
> 
> I already had a patch in my lvds_downclock branch that moved some of
> the gmch platform differences out from i9xx_get_refclk() into the
> caller, so I guess might as well move the whole thing I suppose.

I think we should just split i9xx_crtc_compute_clock() into more platform
specific functions and kill i9xx_get_reclk(), intel_limit() and ->find_dpll().
We are jumping through hoops to make the code look like it's generic, but there
is a lot of platform specific details. IMO it would be a lot easier to read that
way.

I wrote some patches going into that direction today. The end result looks like
the following:

static int chv_crtc_compute_clock(struct intel_crtc *crtc,
                                  struct intel_crtc_state *crtc_state)
{
        int refclk;
        bool ok;
        const intel_limit_t *limit;

        memset(&crtc_state->dpll_hw_state, 0,
               sizeof(crtc_state->dpll_hw_state));

        if (crtc_state->has_dsi_encoder)
                return 0;

        limit = &intel_limits_chv;

        if (!crtc_state->clock_set) {
                refclk = 100000;

                ok = chv_find_best_dpll(limit, crtc_state,
                                        crtc_state->port_clock, refclk,
                                        NULL, &crtc_state->dpll);
                if (!ok) {
                        DRM_ERROR("Couldn't find PLL settings for mode!\n");
                        return -EINVAL;
                }
        }

        chv_compute_dpll(crtc, crtc_state);

        return 0;
}

static int vlv_crtc_compute_clock(struct intel_crtc *crtc,
                                  struct intel_crtc_state *crtc_state)
{
        int refclk;
        bool ok;
        const intel_limit_t *limit;

        memset(&crtc_state->dpll_hw_state, 0,
               sizeof(crtc_state->dpll_hw_state));

        if (crtc_state->has_dsi_encoder)
                return 0;

        limit = &intel_limits_vlv;

        if (!crtc_state->clock_set) {
                refclk = 100000;

                ok = vlv_find_best_dpll(limit, crtc_state,
                                        crtc_state->port_clock, refclk,
                                        NULL, &crtc_state->dpll);
                if (!ok) {
                        DRM_ERROR("Couldn't find PLL settings for mode!\n");
                        return -EINVAL;
                }
        }

        vlv_compute_dpll(crtc, crtc_state);

        return 0;
}

static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
                                   struct intel_crtc_state *crtc_state)
{
        struct drm_device *dev = crtc->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
        int refclk;
        const intel_limit_t *limit;

        memset(&crtc_state->dpll_hw_state, 0,
               sizeof(crtc_state->dpll_hw_state));

        if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
            intel_panel_use_ssc(dev_priv)) {
                refclk = dev_priv->vbt.lvds_ssc_freq;
                DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
        } else {
                refclk = 48000;
        }

        if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
                limit = &intel_limits_i8xx_lvds;
        else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
                limit = &intel_limits_i8xx_dvo;
        else
                limit = &intel_limits_i8xx_dac;

        if (!crtc_state->clock_set &&
            !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
                                 refclk, NULL, &crtc_state->dpll)) {
                DRM_ERROR("Couldn't find PLL settings for mode!\n");
                return -EINVAL;
        }

        i8xx_compute_dpll(crtc, crtc_state, NULL);

        return 0;
}

static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
                                   struct intel_crtc_state *crtc_state)
{
        struct drm_device *dev = crtc->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
        int refclk;
        bool ok;
        const intel_limit_t *limit;

        memset(&crtc_state->dpll_hw_state, 0,
               sizeof(crtc_state->dpll_hw_state));

        if (crtc_state->has_dsi_encoder)
                return 0;

        if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
            intel_panel_use_ssc(dev_priv)) {
                refclk = dev_priv->vbt.lvds_ssc_freq;
                DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
        } else {
                refclk = 96000;
        }

        if (!crtc_state->clock_set) {
                /*
                 * Returns a set of divisors for the desired target clock with
                 * the given refclk, or FALSE.  The returned values represent
                 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
                 * 2) / p1 / p2.
                 */
                limit = intel_limit(crtc_state, refclk);
                ok = dev_priv->display.find_dpll(limit, crtc_state,
                                                 crtc_state->port_clock,
                                                 refclk, NULL,
                                                 &crtc_state->dpll);
                if (!ok) {
                        DRM_ERROR("Couldn't find PLL settings for mode!\n");
                        return -EINVAL;
                }
        }

        i9xx_compute_dpll(crtc, crtc_state, NULL);

        return 0;
}


Ander
> 
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++----------------
> >  1 file changed, 8 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index e7d6584..07b5244 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8589,21 +8589,6 @@ void intel_init_pch_refclk(struct drm_device *dev)
> >  		lpt_init_pch_refclk(dev);
> >  }
> >  
> > -static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
> > -{
> > -	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > -	    intel_panel_use_ssc(dev_priv)) {
> > -		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> > -			      dev_priv->vbt.lvds_ssc_freq);
> > -		return dev_priv->vbt.lvds_ssc_freq;
> > -	}
> > -
> > -	return 120000;
> > -}
> > -
> >  static void ironlake_set_pipeconf(struct drm_crtc *crtc)
> >  {
> >  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > @@ -8775,7 +8760,14 @@ static bool ironlake_compute_clocks(struct drm_crtc
> > *crtc,
> >  	const intel_limit_t *limit;
> >  	bool ret;
> >  
> > -	refclk = ironlake_get_refclk(crtc_state);
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > +	    intel_panel_use_ssc(dev_priv)) {
> > +		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> > +			      dev_priv->vbt.lvds_ssc_freq);
> > +		refclk = dev_priv->vbt.lvds_ssc_freq;
> > +	} else {
> > +		refclk = 120000;
> > +	}
> >  
> >  	/*
> >  	 * Returns a set of divisors for the desired target clock with the
> > given
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx