diff mbox

[v4,10/12] drm/i915: HSW cdclk support

Message ID 1432282962-3530-11-git-send-email-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kahola, Mika May 22, 2015, 8:22 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Implement support for changing the cdclk frequency during runtime on
HSW. VLV/CHV already have support for this, so we can follow their
example for the most part. Only the actual hardware programming differs,
the rest is pretty much the same.

The pipe pixel rate stuff is handled a bit differently for now due to
the difference in pch vs. gmch pfit handling. Eventually we should unify
that part to eliminate what is essentially duplicated code.

v2: Grab rps.hw_lock around sandybridge_pcode_write()
v3: Rebase due to power well vs. .global_resources() reordering

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

v3: Rebased to the latest
v4: Reformatting 'haswell_modeset_global_pipes' function to
    support atomic state

Signed-off-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   3 +
 drivers/gpu/drm/i915/intel_display.c | 161 +++++++++++++++++++++++++++++++++--
 2 files changed, 159 insertions(+), 5 deletions(-)

Comments

Lespiau, Damien May 29, 2015, 11:30 a.m. UTC | #1
On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Implement support for changing the cdclk frequency during runtime on
> HSW. VLV/CHV already have support for this, so we can follow their
> example for the most part. Only the actual hardware programming differs,
> the rest is pretty much the same.
> 
> The pipe pixel rate stuff is handled a bit differently for now due to
> the difference in pch vs. gmch pfit handling. Eventually we should unify
> that part to eliminate what is essentially duplicated code.
> 
> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> v3: Rebase due to power well vs. .global_resources() reordering
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v3: Rebased to the latest
> v4: Reformatting 'haswell_modeset_global_pipes' function to
>     support atomic state
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Apart from the small remark below, it looks like it does what the spec
tell us to do.

However there are a few extra comments:

  - I remember someone saying changing CDCLK hasn't been validated on
    HSW, that's usually a red flag. Even if it seems like it's working,
    I've no idea what the pcode does with the notification on ULX for
    instance
  - Changing CDCLK means we have to bring whole display down and up
    again, which will make the current display flicker when hotplugging
    a screen that needs a CDCLK bump for instance.
  - it'd be nice to have some idea of the power gain running at a lower
    frequency
Kahola, Mika May 29, 2015, 12:06 p.m. UTC | #2
> -----Original Message-----
> From: Lespiau, Damien
> Sent: Friday, May 29, 2015 2:31 PM
> To: Kahola, Mika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v4 10/12] drm/i915: HSW cdclk support
> 
> On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Implement support for changing the cdclk frequency during runtime on
> > HSW. VLV/CHV already have support for this, so we can follow their
> > example for the most part. Only the actual hardware programming
> > differs, the rest is pretty much the same.
> >
> > The pipe pixel rate stuff is handled a bit differently for now due to
> > the difference in pch vs. gmch pfit handling. Eventually we should
> > unify that part to eliminate what is essentially duplicated code.
> >
> > v2: Grab rps.hw_lock around sandybridge_pcode_write()
> > v3: Rebase due to power well vs. .global_resources() reordering
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > v3: Rebased to the latest
> > v4: Reformatting 'haswell_modeset_global_pipes' function to
> >     support atomic state
> >
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >
> > Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> 
> Apart from the small remark below, it looks like it does what the spec tell us
> to do.
> 
> However there are a few extra comments:
> 
>   - I remember someone saying changing CDCLK hasn't been validated on
>     HSW, that's usually a red flag. Even if it seems like it's working,
>     I've no idea what the pcode does with the notification on ULX for
>     instance
>   - Changing CDCLK means we have to bring whole display down and up
>     again, which will make the current display flicker when hotplugging
>     a screen that needs a CDCLK bump for instance.
>   - it'd be nice to have some idea of the power gain running at a lower
>     frequency
> 
> --
> Damien

Thanks for the comments and review.

> >  drivers/gpu/drm/i915/i915_reg.h      |   3 +
> >  drivers/gpu/drm/i915/intel_display.c | 161
> > +++++++++++++++++++++++++++++++++--
> >  2 files changed, 159 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 14366c8..015fe12 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
> >  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> > +#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> >  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> > @@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
> >  #define  LCPLL_PLL_LOCK			(1<<30)
> >  #define  LCPLL_CLK_FREQ_MASK		(3<<26)
> >  #define  LCPLL_CLK_FREQ_450		(0<<26)
> > +#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5
> (ULX) or 540 */
> >  #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
> >  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> > +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index febf993..556d0ec7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct
> > drm_device *dev)  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > -	if (IS_VALLEYVIEW(dev)) {
> > +	if (IS_HASWELL(dev)) {
> > +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else if (IS_HSW_ULX(dev))
> > +			dev_priv->max_cdclk_freq = 337500;
> > +		else if (IS_HSW_ULT(dev))
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 540000;
> > +	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->max_cdclk_freq = 400000;
> >  	} else {
> >  		/* otherwise assume cdclk is fixed */ @@ -9404,6 +9413,139
> @@
> > static void broxton_modeset_global_resources(struct drm_atomic_state
> *old_state)
> >  		broxton_set_cdclk(dev, req_cdclk);
> >  }
> >
> > +/* compute the max rate for new configuration */ static int
> > +ilk_max_pixel_rate(struct drm_i915_private *dev_priv) {
> > +	struct drm_device *dev = dev_priv->dev;
> > +	struct intel_crtc *crtc;
> > +	int max_pixel_rate = 0;
> > +
> > +	for_each_intel_crtc(dev, crtc) {
> > +		if (crtc->new_enabled)
> > +			max_pixel_rate = max((int)max_pixel_rate,
> > +					     (int)ilk_pipe_pixel_rate(crtc-
> >config));
> > +	}
> > +
> > +	return max_pixel_rate;
> > +}
> 
> new_enabled doesn't look like what we want to look at, it looks like a
> temporary field we shouldn't be using in new code. Maybe
> crtc->state->enable instead?

You're right! This was a good catch. I was a bit confused which one to use here. I'll revise the patch

Cheers,
Mika
> > +
> > +static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> > +			      int max_pixel_rate)
> > +{
> > +	int cdclk;
> > +
> > +	/*
> > +	 * FIXME should also account for plane ratio
> > +	 * once 64bpp pixel formats are supported.
> > +	 */
> > +	if (max_pixel_rate > 450000)
> > +		cdclk = 540000;
> > +	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> > +		cdclk = 450000;
> > +	else
> > +		cdclk = 337500;
> > +
> > +	/*
> > +	 * FIXME move the cdclk caclulation to
> > +	 * compute_config() so we can fail gracegully.
> > +	 */
> > +	if (cdclk > dev_priv->max_cdclk_freq) {
> > +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d
> kHz)\n",
> > +			  cdclk, dev_priv->max_cdclk_freq);
> > +		cdclk = dev_priv->max_cdclk_freq;
> > +	}
> > +
> > +	return cdclk;
> > +}
> > +
> > +static void haswell_set_cdclk(struct drm_device *dev, int cdclk) {
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	uint32_t val;
> > +
> > +	if (WARN((I915_READ(LCPLL_CTL) &
> > +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> > +		   LCPLL_CD_CLOCK_DISABLE |
> LCPLL_ROOT_CD_CLOCK_DISABLE |
> > +		   LCPLL_CD2X_CLOCK_DISABLE |
> LCPLL_POWER_DOWN_ALLOW |
> > +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> > +		 "trying to change cdclk frequency with cdclk not
> enabled\n"))
> > +		return;
> > +
> > +	val = I915_READ(LCPLL_CTL);
> > +	val &= ~LCPLL_CLK_FREQ_MASK;
> > +
> > +	switch (cdclk) {
> > +	case 450000:
> > +		val |= LCPLL_CLK_FREQ_450;
> > +		break;
> > +	case 337500:
> > +	case 540000:
> > +		val |= LCPLL_CLK_FREQ_ALT_HSW;
> > +		break;
> > +	default:
> > +		WARN(1, "invalid cdclk frequency\n");
> > +		return;
> > +	}
> > +
> > +	I915_WRITE(LCPLL_CTL, val);
> > +
> > +	if (IS_HSW_ULX(dev)) {
> > +		mutex_lock(&dev_priv->rps.hw_lock);
> > +		sandybridge_pcode_write(dev_priv,
> HSW_PCODE_DE_WRITE_FREQ_REQ,
> > +					cdclk == 337500);
> > +		mutex_unlock(&dev_priv->rps.hw_lock);
> > +	}
> > +
> > +	intel_update_cdclk(dev);
> > +
> > +	WARN(cdclk != dev_priv->cdclk_freq,
> > +	     "cdclk requested %d kHz but got %d kHz\n",
> > +	     cdclk, dev_priv->cdclk_freq);
> > +}
> > +
> > +static int haswell_modeset_global_pipes(struct drm_atomic_state
> > +*state) {
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct drm_crtc *crtc;
> > +	struct drm_crtc_state *crtc_state;
> > +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> > +	int cdclk, i;
> > +
> > +	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> > +
> > +	if (cdclk == dev_priv->cdclk_freq)
> > +		return 0;
> > +
> > +	/* add all active pipes to the state */
> > +	for_each_crtc(state->dev, crtc) {
> > +		if (!crtc->state->enable)
> > +			continue;
> > +
> > +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +		if (IS_ERR(crtc_state))
> > +			return PTR_ERR(crtc_state);
> > +	}
> > +
> > +	/* disable/enable all currently active pipes while we change cdclk */
> > +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> > +		if (crtc_state->enable)
> > +			crtc_state->mode_changed = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void haswell_modeset_global_resources(struct drm_atomic_state
> > +*state) {
> > +	struct drm_device *dev = state->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> > +	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> > +
> > +	if (req_cdclk != dev_priv->cdclk_freq) {
> > +		haswell_set_cdclk(dev, req_cdclk);
> > +	}
> > +}
> > +
> >  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >  				      struct intel_crtc_state *crtc_state)  { @@ -
> 12582,10
> > +12724,16 @@ static int __intel_set_mode_checks(struct
> drm_atomic_state *state)
> >  	 * mode set on this crtc.  For other crtcs we need to use the
> >  	 * adjusted_mode bits in the crtc directly.
> >  	 */
> > -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > -		ret = valleyview_modeset_global_pipes(state);
> > -		if (ret)
> > -			return ret;
> > +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> > +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > +			ret = valleyview_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = haswell_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		}
> >  	}
> >
> >  	ret = __intel_set_mode_setup_plls(state);
> > @@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device
> *dev)
> >  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> > +		if (IS_HASWELL(dev))
> > +			dev_priv->display.modeset_global_resources =
> > +				haswell_modeset_global_resources;
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->display.modeset_global_resources =
> >  			valleyview_modeset_global_resources;
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lespiau, Damien May 29, 2015, 12:56 p.m. UTC | #3
On Fri, May 29, 2015 at 01:06:47PM +0100, Kahola, Mika wrote:
> > > static void broxton_modeset_global_resources(struct drm_atomic_state
> > *old_state)
> > >  		broxton_set_cdclk(dev, req_cdclk);
> > >  }
> > >
> > > +/* compute the max rate for new configuration */ static int
> > > +ilk_max_pixel_rate(struct drm_i915_private *dev_priv) {
> > > +	struct drm_device *dev = dev_priv->dev;
> > > +	struct intel_crtc *crtc;
> > > +	int max_pixel_rate = 0;
> > > +
> > > +	for_each_intel_crtc(dev, crtc) {
> > > +		if (crtc->new_enabled)
> > > +			max_pixel_rate = max((int)max_pixel_rate,
> > > +					     (int)ilk_pipe_pixel_rate(crtc-
> > >config));
> > > +	}
> > > +
> > > +	return max_pixel_rate;
> > > +}
> > 
> > new_enabled doesn't look like what we want to look at, it looks like a
> > temporary field we shouldn't be using in new code. Maybe
> > crtc->state->enable instead?
> 
> You're right! This was a good catch. I was a bit confused which one to
> use here. I'll revise the patch

I've noticed that the BDW patch after that has to be rebased on top of
this change as well.
Ville Syrjala May 29, 2015, 1:51 p.m. UTC | #4
On Fri, May 29, 2015 at 12:30:41PM +0100, Damien Lespiau wrote:
> On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Implement support for changing the cdclk frequency during runtime on
> > HSW. VLV/CHV already have support for this, so we can follow their
> > example for the most part. Only the actual hardware programming differs,
> > the rest is pretty much the same.
> > 
> > The pipe pixel rate stuff is handled a bit differently for now due to
> > the difference in pch vs. gmch pfit handling. Eventually we should unify
> > that part to eliminate what is essentially duplicated code.
> > 
> > v2: Grab rps.hw_lock around sandybridge_pcode_write()
> > v3: Rebase due to power well vs. .global_resources() reordering
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > v3: Rebased to the latest
> > v4: Reformatting 'haswell_modeset_global_pipes' function to
> >     support atomic state
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > 
> > Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> 
> Apart from the small remark below, it looks like it does what the spec
> tell us to do.
> 
> However there are a few extra comments:
> 
>   - I remember someone saying changing CDCLK hasn't been validated on
>     HSW, that's usually a red flag. Even if it seems like it's working,
>     I've no idea what the pcode does with the notification on ULX for
>     instance

Yeah I suppose it's better to reorder things so that the BDW patch comes
first. Then if we decide to drop the HSW one it won't cause problems for
the BDW patch.

>   - Changing CDCLK means we have to bring whole display down and up
>     again, which will make the current display flicker when hotplugging
>     a screen that needs a CDCLK bump for instance.
>   - it'd be nice to have some idea of the power gain running at a lower
>     frequency
> 
> -- 
> Damien
> 
> >  drivers/gpu/drm/i915/i915_reg.h      |   3 +
> >  drivers/gpu/drm/i915/intel_display.c | 161 +++++++++++++++++++++++++++++++++--
> >  2 files changed, 159 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 14366c8..015fe12 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
> >  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> > +#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> >  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> > @@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
> >  #define  LCPLL_PLL_LOCK			(1<<30)
> >  #define  LCPLL_CLK_FREQ_MASK		(3<<26)
> >  #define  LCPLL_CLK_FREQ_450		(0<<26)
> > +#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5 (ULX) or 540 */
> >  #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
> >  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> > +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index febf993..556d0ec7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	if (IS_VALLEYVIEW(dev)) {
> > +	if (IS_HASWELL(dev)) {
> > +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else if (IS_HSW_ULX(dev))
> > +			dev_priv->max_cdclk_freq = 337500;
> > +		else if (IS_HSW_ULT(dev))
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 540000;
> > +	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->max_cdclk_freq = 400000;
> >  	} else {
> >  		/* otherwise assume cdclk is fixed */
> > @@ -9404,6 +9413,139 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
> >  		broxton_set_cdclk(dev, req_cdclk);
> >  }
> >  
> > +/* compute the max rate for new configuration */
> > +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> > +{
> > +	struct drm_device *dev = dev_priv->dev;
> > +	struct intel_crtc *crtc;
> > +	int max_pixel_rate = 0;
> > +
> > +	for_each_intel_crtc(dev, crtc) {
> > +		if (crtc->new_enabled)
> > +			max_pixel_rate = max((int)max_pixel_rate,
> > +					     (int)ilk_pipe_pixel_rate(crtc->config));
> > +	}
> > +
> > +	return max_pixel_rate;
> > +}
> 
> new_enabled doesn't look like what we want to look at, it looks like a
> temporary field we shouldn't be using in new code. Maybe
> crtc->state->enable instead?
> 
> > +
> > +static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> > +			      int max_pixel_rate)
> > +{
> > +	int cdclk;
> > +
> > +	/*
> > +	 * FIXME should also account for plane ratio
> > +	 * once 64bpp pixel formats are supported.
> > +	 */
> > +	if (max_pixel_rate > 450000)
> > +		cdclk = 540000;
> > +	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> > +		cdclk = 450000;
> > +	else
> > +		cdclk = 337500;
> > +
> > +	/*
> > +	 * FIXME move the cdclk caclulation to
> > +	 * compute_config() so we can fail gracegully.
> > +	 */
> > +	if (cdclk > dev_priv->max_cdclk_freq) {
> > +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> > +			  cdclk, dev_priv->max_cdclk_freq);
> > +		cdclk = dev_priv->max_cdclk_freq;
> > +	}
> > +
> > +	return cdclk;
> > +}
> > +
> > +static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	uint32_t val;
> > +
> > +	if (WARN((I915_READ(LCPLL_CTL) &
> > +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> > +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> > +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> > +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> > +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> > +		return;
> > +
> > +	val = I915_READ(LCPLL_CTL);
> > +	val &= ~LCPLL_CLK_FREQ_MASK;
> > +
> > +	switch (cdclk) {
> > +	case 450000:
> > +		val |= LCPLL_CLK_FREQ_450;
> > +		break;
> > +	case 337500:
> > +	case 540000:
> > +		val |= LCPLL_CLK_FREQ_ALT_HSW;
> > +		break;
> > +	default:
> > +		WARN(1, "invalid cdclk frequency\n");
> > +		return;
> > +	}
> > +
> > +	I915_WRITE(LCPLL_CTL, val);
> > +
> > +	if (IS_HSW_ULX(dev)) {
> > +		mutex_lock(&dev_priv->rps.hw_lock);
> > +		sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> > +					cdclk == 337500);
> > +		mutex_unlock(&dev_priv->rps.hw_lock);
> > +	}
> > +
> > +	intel_update_cdclk(dev);
> > +
> > +	WARN(cdclk != dev_priv->cdclk_freq,
> > +	     "cdclk requested %d kHz but got %d kHz\n",
> > +	     cdclk, dev_priv->cdclk_freq);
> > +}
> > +
> > +static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct drm_crtc *crtc;
> > +	struct drm_crtc_state *crtc_state;
> > +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> > +	int cdclk, i;
> > +
> > +	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> > +
> > +	if (cdclk == dev_priv->cdclk_freq)
> > +		return 0;
> > +
> > +	/* add all active pipes to the state */
> > +	for_each_crtc(state->dev, crtc) {
> > +		if (!crtc->state->enable)
> > +			continue;
> > +
> > +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +		if (IS_ERR(crtc_state))
> > +			return PTR_ERR(crtc_state);
> > +	}
> > +
> > +	/* disable/enable all currently active pipes while we change cdclk */
> > +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> > +		if (crtc_state->enable)
> > +			crtc_state->mode_changed = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void haswell_modeset_global_resources(struct drm_atomic_state *state)
> > +{
> > +	struct drm_device *dev = state->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> > +	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> > +
> > +	if (req_cdclk != dev_priv->cdclk_freq) {
> > +		haswell_set_cdclk(dev, req_cdclk);
> > +	}
> > +}
> > +
> >  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >  				      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -12582,10 +12724,16 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> >  	 * mode set on this crtc.  For other crtcs we need to use the
> >  	 * adjusted_mode bits in the crtc directly.
> >  	 */
> > -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > -		ret = valleyview_modeset_global_pipes(state);
> > -		if (ret)
> > -			return ret;
> > +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> > +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > +			ret = valleyview_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = haswell_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		}
> >  	}
> >  
> >  	ret = __intel_set_mode_setup_plls(state);
> > @@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device *dev)
> >  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> > +		if (IS_HASWELL(dev))
> > +			dev_priv->display.modeset_global_resources =
> > +				haswell_modeset_global_resources;
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->display.modeset_global_resources =
> >  			valleyview_modeset_global_resources;
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 14366c8..015fe12 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6701,6 +6701,7 @@  enum skl_disp_power_wells {
 #define	  GEN6_PCODE_READ_RC6VIDS		0x5
 #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
+#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
 #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
 #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
 #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
@@ -7159,10 +7160,12 @@  enum skl_disp_power_wells {
 #define  LCPLL_PLL_LOCK			(1<<30)
 #define  LCPLL_CLK_FREQ_MASK		(3<<26)
 #define  LCPLL_CLK_FREQ_450		(0<<26)
+#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5 (ULX) or 540 */
 #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
 #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
 #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
+#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
 #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
 #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index febf993..556d0ec7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5545,7 +5545,16 @@  static void intel_update_max_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_HASWELL(dev)) {
+		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_HSW_ULX(dev))
+			dev_priv->max_cdclk_freq = 337500;
+		else if (IS_HSW_ULT(dev))
+			dev_priv->max_cdclk_freq = 450000;
+		else
+			dev_priv->max_cdclk_freq = 540000;
+	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->max_cdclk_freq = 400000;
 	} else {
 		/* otherwise assume cdclk is fixed */
@@ -9404,6 +9413,139 @@  static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
 		broxton_set_cdclk(dev, req_cdclk);
 }
 
+/* compute the max rate for new configuration */
+static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *crtc;
+	int max_pixel_rate = 0;
+
+	for_each_intel_crtc(dev, crtc) {
+		if (crtc->new_enabled)
+			max_pixel_rate = max((int)max_pixel_rate,
+					     (int)ilk_pipe_pixel_rate(crtc->config));
+	}
+
+	return max_pixel_rate;
+}
+
+static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
+			      int max_pixel_rate)
+{
+	int cdclk;
+
+	/*
+	 * FIXME should also account for plane ratio
+	 * once 64bpp pixel formats are supported.
+	 */
+	if (max_pixel_rate > 450000)
+		cdclk = 540000;
+	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
+		cdclk = 450000;
+	else
+		cdclk = 337500;
+
+	/*
+	 * FIXME move the cdclk caclulation to
+	 * compute_config() so we can fail gracegully.
+	 */
+	if (cdclk > dev_priv->max_cdclk_freq) {
+		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+			  cdclk, dev_priv->max_cdclk_freq);
+		cdclk = dev_priv->max_cdclk_freq;
+	}
+
+	return cdclk;
+}
+
+static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val;
+
+	if (WARN((I915_READ(LCPLL_CTL) &
+		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
+		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
+		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
+		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
+		 "trying to change cdclk frequency with cdclk not enabled\n"))
+		return;
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CLK_FREQ_MASK;
+
+	switch (cdclk) {
+	case 450000:
+		val |= LCPLL_CLK_FREQ_450;
+		break;
+	case 337500:
+	case 540000:
+		val |= LCPLL_CLK_FREQ_ALT_HSW;
+		break;
+	default:
+		WARN(1, "invalid cdclk frequency\n");
+		return;
+	}
+
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (IS_HSW_ULX(dev)) {
+		mutex_lock(&dev_priv->rps.hw_lock);
+		sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
+					cdclk == 337500);
+		mutex_unlock(&dev_priv->rps.hw_lock);
+	}
+
+	intel_update_cdclk(dev);
+
+	WARN(cdclk != dev_priv->cdclk_freq,
+	     "cdclk requested %d kHz but got %d kHz\n",
+	     cdclk, dev_priv->cdclk_freq);
+}
+
+static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int max_pixclk = ilk_max_pixel_rate(dev_priv);
+	int cdclk, i;
+
+	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
+
+	if (cdclk == dev_priv->cdclk_freq)
+		return 0;
+
+	/* add all active pipes to the state */
+	for_each_crtc(state->dev, crtc) {
+		if (!crtc->state->enable)
+			continue;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	/* disable/enable all currently active pipes while we change cdclk */
+	for_each_crtc_in_state(state, crtc, crtc_state, i)
+		if (crtc_state->enable)
+			crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+static void haswell_modeset_global_resources(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
+	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
+
+	if (req_cdclk != dev_priv->cdclk_freq) {
+		haswell_set_cdclk(dev, req_cdclk);
+	}
+}
+
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
@@ -12582,10 +12724,16 @@  static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	 * mode set on this crtc.  For other crtcs we need to use the
 	 * adjusted_mode bits in the crtc directly.
 	 */
-	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
-		ret = valleyview_modeset_global_pipes(state);
-		if (ret)
-			return ret;
+	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
+		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
+			ret = valleyview_modeset_global_pipes(state);
+			if (ret)
+				return ret;
+		} else {
+			ret = haswell_modeset_global_pipes(state);
+			if (ret)
+				return ret;
+		}
 	}
 
 	ret = __intel_set_mode_setup_plls(state);
@@ -14468,6 +14616,9 @@  static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
+		if (IS_HASWELL(dev))
+			dev_priv->display.modeset_global_resources =
+				haswell_modeset_global_resources;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.modeset_global_resources =
 			valleyview_modeset_global_resources;