[15/19] drm/i915: HSW cdclk support

Message ID	1427800454-5236-1-git-send-email-mika.kahola@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Mika Kahola <mika.kahola@intel.com> To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2015 14:14:14 +0300 Message-Id: <1427800454-5236-1-git-send-email-mika.kahola@intel.com> In-Reply-To: <20150331110535.GB3154@sorvi> References: <20150331110535.GB3154@sorvi> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 15/19] drm/i915: HSW cdclk support Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Mika Kahola March 31, 2015, 11:14 a.m. UTC

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
v4: Rebase due to .global_resources() reordering for Haswell

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   3 +
 drivers/gpu/drm/i915/intel_display.c | 161 ++++++++++++++++++++++++++++++++++-
 2 files changed, 161 insertions(+), 3 deletions(-)

Sivakumar Thulasimani April 7, 2015, 6:27 a.m. UTC | #1

On 3/31/2015 4:44 PM, Mika Kahola wrote:
> 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
> v4: Rebase due to .global_resources() reordering for Haswell
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      |   3 +
>   drivers/gpu/drm/i915/intel_display.c | 161 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 161 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f26ebd2..b25f712 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6244,6 +6244,7 @@ enum skl_disp_power_wells {
>   #define   GEN6_PCODE_WRITE_D_COMP		0x11
>   #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   DISPLAY_IPS_CONTROL			0x19
>   #define	  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
>   #define GEN6_PCODE_DATA				0x138128
> @@ -6698,10 +6699,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 3752d5e..cce7103 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5012,7 +5012,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 */
> @@ -8773,6 +8782,144 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv)
>   	hsw_disable_lcpll(dev_priv, true, true);
>   }
>   
> +/* 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(max_pixel_rate,
> +					     ilk_pipe_pixel_rate(crtc->new_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;
> +}
won't this return 337MHz even for platforms that have HSW_CDCLK_LIMIT is 
set ? this should return 450MHz if this fuse is set

> +
> +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 void haswell_modeset_global_pipes(struct drm_device *dev,
> +					 unsigned *prepare_pipes)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *crtc;
> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> +
> +	if (haswell_calc_cdclk(dev_priv, max_pixel_rate) ==
> +	    dev_priv->cdclk_freq)
> +		return;
> +
> +	/* disable/enable all currently active pipes while we change cdclk */
> +	for_each_intel_crtc(dev, crtc)
> +		if (crtc->base.enabled)
> +			*prepare_pipes |= 1 << crtc->pipe;
> +}
> +
> +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);
> +
> +	if (req_cdclk != dev_priv->cdclk_freq) {
> +		/*
> +		 * FIXME: We can end up here with all power domains off, yet
> +		 * with a CDCLK frequency other than the minimum. To account
> +		 * for this take the PIPE-A power domain, which covers the HW
> +		 * blocks needed for the following programming. This can be
> +		 * removed once it's guaranteed that we get here either with
> +		 * the minimum CDCLK set, or the required power domains
> +		 * enabled.
> +		 */
> +		intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
> +
> +		haswell_set_cdclk(dev, req_cdclk);
> +
> +		vlv_program_pfi_credits(dev_priv);
> +
> +		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> +	}
> +
> +}
> +
>   void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>   {
>   	struct drm_device *dev = dev_priv->dev;
> @@ -8792,6 +8939,8 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>   	intel_prepare_ddi(dev);
>   }
>   
> +
> +
>   static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>   				      struct intel_crtc_state *crtc_state)
>   {
> @@ -11866,8 +12015,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>   	 * mode set on this crtc.  For other crtcs we need to use the
>   	 * adjusted_mode bits in the crtc directly.
>   	 */
> -	if (IS_VALLEYVIEW(dev)) {
> -		valleyview_modeset_global_pipes(dev, &prepare_pipes);
> +	if (IS_VALLEYVIEW(dev) || IS_HASWELL(dev)) {
> +		if (IS_VALLEYVIEW(dev))
> +			valleyview_modeset_global_pipes(dev, &prepare_pipes);
> +		else
> +			haswell_modeset_global_pipes(dev, &prepare_pipes);
>   
>   		/* may have added more to prepare_pipes than we should */
>   		prepare_pipes &= ~disable_pipes;
> @@ -13892,6 +14044,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;

Sivakumar Thulasimani April 7, 2015, 7:03 a.m. UTC | #2

sorry if i am missing something, HSW and BDW requires display audio 
controller to be updated with new values once CD clock is modified. how 
is this accomplished here ?

regards,
Sivakumar
On 4/7/2015 11:57 AM, Sivakumar Thulasimani wrote:
>
>
> On 3/31/2015 4:44 PM, Mika Kahola wrote:
>> 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
>> v4: Rebase due to .global_resources() reordering for Haswell
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h      |   3 +
>>   drivers/gpu/drm/i915/intel_display.c | 161 
>> ++++++++++++++++++++++++++++++++++-
>>   2 files changed, 161 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index f26ebd2..b25f712 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6244,6 +6244,7 @@ enum skl_disp_power_wells {
>>   #define   GEN6_PCODE_WRITE_D_COMP        0x11
>>   #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   DISPLAY_IPS_CONTROL            0x19
>>   #define      HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL    0x1A
>>   #define GEN6_PCODE_DATA                0x138128
>> @@ -6698,10 +6699,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 3752d5e..cce7103 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5012,7 +5012,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 */
>> @@ -8773,6 +8782,144 @@ void hsw_enable_pc8(struct drm_i915_private 
>> *dev_priv)
>>       hsw_disable_lcpll(dev_priv, true, true);
>>   }
>>   +/* 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(max_pixel_rate,
>> + ilk_pipe_pixel_rate(crtc->new_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;
>> +}
> won't this return 337MHz even for platforms that have HSW_CDCLK_LIMIT 
> is set ? this should return 450MHz if this fuse is set
>
>> +
>> +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 void haswell_modeset_global_pipes(struct drm_device *dev,
>> +                     unsigned *prepare_pipes)
>> +{
>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>> +    struct intel_crtc *crtc;
>> +    int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
>> +
>> +    if (haswell_calc_cdclk(dev_priv, max_pixel_rate) ==
>> +        dev_priv->cdclk_freq)
>> +        return;
>> +
>> +    /* disable/enable all currently active pipes while we change 
>> cdclk */
>> +    for_each_intel_crtc(dev, crtc)
>> +        if (crtc->base.enabled)
>> +            *prepare_pipes |= 1 << crtc->pipe;
>> +}
>> +
>> +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);
>> +
>> +    if (req_cdclk != dev_priv->cdclk_freq) {
>> +        /*
>> +         * FIXME: We can end up here with all power domains off, yet
>> +         * with a CDCLK frequency other than the minimum. To account
>> +         * for this take the PIPE-A power domain, which covers the HW
>> +         * blocks needed for the following programming. This can be
>> +         * removed once it's guaranteed that we get here either with
>> +         * the minimum CDCLK set, or the required power domains
>> +         * enabled.
>> +         */
>> +        intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
>> +
>> +        haswell_set_cdclk(dev, req_cdclk);
>> +
>> +        vlv_program_pfi_credits(dev_priv);
>> +
>> +        intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
>> +    }
>> +
>> +}
>> +
>>   void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>>   {
>>       struct drm_device *dev = dev_priv->dev;
>> @@ -8792,6 +8939,8 @@ void hsw_disable_pc8(struct drm_i915_private 
>> *dev_priv)
>>       intel_prepare_ddi(dev);
>>   }
>>   +
>> +
>>   static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>>                         struct intel_crtc_state *crtc_state)
>>   {
>> @@ -11866,8 +12015,11 @@ static int __intel_set_mode(struct drm_crtc 
>> *crtc,
>>        * mode set on this crtc.  For other crtcs we need to use the
>>        * adjusted_mode bits in the crtc directly.
>>        */
>> -    if (IS_VALLEYVIEW(dev)) {
>> -        valleyview_modeset_global_pipes(dev, &prepare_pipes);
>> +    if (IS_VALLEYVIEW(dev) || IS_HASWELL(dev)) {
>> +        if (IS_VALLEYVIEW(dev))
>> +            valleyview_modeset_global_pipes(dev, &prepare_pipes);
>> +        else
>> +            haswell_modeset_global_pipes(dev, &prepare_pipes);
>>             /* may have added more to prepare_pipes than we should */
>>           prepare_pipes &= ~disable_pipes;
>> @@ -13892,6 +14044,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;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjälä April 7, 2015, 8:28 a.m. UTC | #3

On Tue, Apr 07, 2015 at 11:57:23AM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 3/31/2015 4:44 PM, Mika Kahola wrote:
> > 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
> > v4: Rebase due to .global_resources() reordering for Haswell
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h      |   3 +
> >   drivers/gpu/drm/i915/intel_display.c | 161 ++++++++++++++++++++++++++++++++++-
> >   2 files changed, 161 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index f26ebd2..b25f712 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6244,6 +6244,7 @@ enum skl_disp_power_wells {
> >   #define   GEN6_PCODE_WRITE_D_COMP		0x11
> >   #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   DISPLAY_IPS_CONTROL			0x19
> >   #define	  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
> >   #define GEN6_PCODE_DATA				0x138128
> > @@ -6698,10 +6699,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 3752d5e..cce7103 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5012,7 +5012,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 */
> > @@ -8773,6 +8782,144 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv)
> >   	hsw_disable_lcpll(dev_priv, true, true);
> >   }
> >   
> > +/* 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(max_pixel_rate,
> > +					     ilk_pipe_pixel_rate(crtc->new_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;
> > +}
> won't this return 337MHz even for platforms that have HSW_CDCLK_LIMIT is 
> set ? this should return 450MHz if this fuse is set

I'm not sure ULX machines would ever have HSW_CDCLK_LIMIT set. But
having a check for it should do no harm so might as well I suppose.

Ville Syrjälä April 7, 2015, 8:29 a.m. UTC | #4

On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
> sorry if i am missing something, HSW and BDW requires display audio 
> controller to be updated with new values once CD clock is modified. how 
> is this accomplished here ?

I'm hoping the audio driver will query the cdclk frequency after every
modeset.

Sivakumar Thulasimani April 7, 2015, 8:36 a.m. UTC | #5

where can i check this (audio driver) ? since there was no need for them 
to check CD clock value till now i don't think they will be doing it. 
also this needs to be changed in Display Audio controller, so not sure 
if audio driver has access to it in the first place. will be good to 
confirm this before merging as it will break display audio if it is not 
programmed.

if any one has setup this can be confirmed by just switching CD clock 
after boot and playing an audio file.

regards,
Sivakumar

On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
> On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
>> sorry if i am missing something, HSW and BDW requires display audio
>> controller to be updated with new values once CD clock is modified. how
>> is this accomplished here ?
> I'm hoping the audio driver will query the cdclk frequency after every
> modeset.
>

Mika Kahola April 7, 2015, 9:29 a.m. UTC | #6

Definitely a good idea to check the audio part as well if there is
a doubt that by changing CD clock the audio would fail. I can check
this and I'll get back once I have the results.

Cheers,
Mika
  
On Tue, Apr 07, 2015 at 02:06:50PM +0530, Sivakumar Thulasimani wrote:
> where can i check this (audio driver) ? since there was no need for
> them to check CD clock value till now i don't think they will be
> doing it. also this needs to be changed in Display Audio controller,
> so not sure if audio driver has access to it in the first place.
> will be good to confirm this before merging as it will break display
> audio if it is not programmed.
> 
> if any one has setup this can be confirmed by just switching CD
> clock after boot and playing an audio file.
> 
> regards,
> Sivakumar
> 
> On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
> >On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
> >>sorry if i am missing something, HSW and BDW requires display audio
> >>controller to be updated with new values once CD clock is modified. how
> >>is this accomplished here ?
> >I'm hoping the audio driver will query the cdclk frequency after every
> >modeset.
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Daniel Vetter April 7, 2015, 1:52 p.m. UTC | #7

On Tue, Apr 07, 2015 at 12:29:25PM +0300, Mika Kahola wrote:
> Definitely a good idea to check the audio part as well if there is
> a doubt that by changing CD clock the audio would fail. I can check
> this and I'll get back once I have the results.

We force a full modeset, which should result in an interrupt on the audio
side, which should result in the audio driver re-reading the current
cdclk. If that's no the case it's buggy already.
-Daniel

Mika Kahola April 9, 2015, 7:24 a.m. UTC | #8

I did some testing on audio part with HDMI-HDMI and DP-HDMI cables
connected to my Haswell box. Before applying the patch I tested as a
reference with the latest -nightly (04-08-2015), 4.0-rc7. Unfortunately,
I failed to get any audio over HDMI cable. For a reference I tested with
the very same setup the vanillla kernel from Linus tree 4.0-rc7 and with
that kernel the audio worked ok. Then I did some GIT bisecting and it
turned out that the first commit that I failed to get audio working was
aa2fee4286e43b4784982b17669b02cc99c1ae55.

To test this patch audio functionality I checkout the -nightly version
that works for me and apply the patch and test it. I'll come back with
the results later on. 

I had the module option i915.disable_power_well=0

The test routine that I used for audio testing was
speaker-test -c 2 -r 48000 -f S16_LE -t pink --device=plughw:0,3

On Tue, 2015-04-07 at 15:52 +0200, Daniel Vetter wrote:
> On Tue, Apr 07, 2015 at 12:29:25PM +0300, Mika Kahola wrote:
> > Definitely a good idea to check the audio part as well if there is
> > a doubt that by changing CD clock the audio would fail. I can check
> > this and I'll get back once I have the results.
> 
> We force a full modeset, which should result in an interrupt on the audio
> side, which should result in the audio driver re-reading the current
> cdclk. If that's no the case it's buggy already.
> -Daniel

Daniel Vetter April 9, 2015, 9:32 a.m. UTC | #9

On Thu, Apr 09, 2015 at 10:24:24AM +0300, Mika Kahola wrote:
> I did some testing on audio part with HDMI-HDMI and DP-HDMI cables
> connected to my Haswell box. Before applying the patch I tested as a
> reference with the latest -nightly (04-08-2015), 4.0-rc7. Unfortunately,
> I failed to get any audio over HDMI cable. For a reference I tested with
> the very same setup the vanillla kernel from Linus tree 4.0-rc7 and with
> that kernel the audio worked ok. Then I did some GIT bisecting and it
> turned out that the first commit that I failed to get audio working was
> aa2fee4286e43b4784982b17669b02cc99c1ae55.

I don't have this sha1 anywhere. Can you please double-check?

Also to avoid such issues please always add at least the commit subject
and author, so that I can find it if it's somehow rebased.
-Daniel

> 
> To test this patch audio functionality I checkout the -nightly version
> that works for me and apply the patch and test it. I'll come back with
> the results later on. 
> 
> I had the module option i915.disable_power_well=0
> 
> The test routine that I used for audio testing was
> speaker-test -c 2 -r 48000 -f S16_LE -t pink --device=plughw:0,3
> 
> On Tue, 2015-04-07 at 15:52 +0200, Daniel Vetter wrote:
> > On Tue, Apr 07, 2015 at 12:29:25PM +0300, Mika Kahola wrote:
> > > Definitely a good idea to check the audio part as well if there is
> > > a doubt that by changing CD clock the audio would fail. I can check
> > > this and I'll get back once I have the results.
> > 
> > We force a full modeset, which should result in an interrupt on the audio
> > side, which should result in the audio driver re-reading the current
> > cdclk. If that's no the case it's buggy already.
> > -Daniel
> 
> -- 
> Mika Kahola, Intel OTC
> 
> 
>

Mika Kahola April 9, 2015, 1:41 p.m. UTC | #10

On Thu, 2015-04-09 at 11:32 +0200, Daniel Vetter wrote:
> On Thu, Apr 09, 2015 at 10:24:24AM +0300, Mika Kahola wrote:
> > I did some testing on audio part with HDMI-HDMI and DP-HDMI cables
> > connected to my Haswell box. Before applying the patch I tested as a
> > reference with the latest -nightly (04-08-2015), 4.0-rc7. Unfortunately,
> > I failed to get any audio over HDMI cable. For a reference I tested with
> > the very same setup the vanillla kernel from Linus tree 4.0-rc7 and with
> > that kernel the audio worked ok. Then I did some GIT bisecting and it
> > turned out that the first commit that I failed to get audio working was
> > aa2fee4286e43b4784982b17669b02cc99c1ae55.
> 
I rerun the bisecting and this time the result was

commit 0a599838737a2527c35e4d94f794aefe59df1781
Merge: 2d846c7 a59d719
Author: Takashi Iwai <tiwai@suse.de>
Date:   Wed Apr 8 11:29:56 2015 +0200

    Merge branch 'for-linus' into for-next
    
    Back merge HD-audio quirks to for-next branch, so that we can apply
    a couple of more quirks.
    
    Signed-off-by: Takashi Iwai <tiwai@suse.de>

> I don't have this sha1 anywhere. Can you please double-check?
> 
> Also to avoid such issues please always add at least the commit subject
> and author, so that I can find it if it's somehow rebased.
> -Daniel
> 
> > 
> > To test this patch audio functionality I checkout the -nightly version
> > that works for me and apply the patch and test it. I'll come back with
> > the results later on. 
> > 
> > I had the module option i915.disable_power_well=0
> > 
> > The test routine that I used for audio testing was
> > speaker-test -c 2 -r 48000 -f S16_LE -t pink --device=plughw:0,3
> > 
> > On Tue, 2015-04-07 at 15:52 +0200, Daniel Vetter wrote:
> > > On Tue, Apr 07, 2015 at 12:29:25PM +0300, Mika Kahola wrote:
> > > > Definitely a good idea to check the audio part as well if there is
> > > > a doubt that by changing CD clock the audio would fail. I can check
> > > > this and I'll get back once I have the results.
> > > 
> > > We force a full modeset, which should result in an interrupt on the audio
> > > side, which should result in the audio driver re-reading the current
> > > cdclk. If that's no the case it's buggy already.
> > > -Daniel
> > 
> > -- 
> > Mika Kahola, Intel OTC
> > 
> > 
> > 
>

Daniel Vetter April 9, 2015, 1:51 p.m. UTC | #11

On Thu, Apr 09, 2015 at 04:41:26PM +0300, Mika Kahola wrote:
> On Thu, 2015-04-09 at 11:32 +0200, Daniel Vetter wrote:
> > On Thu, Apr 09, 2015 at 10:24:24AM +0300, Mika Kahola wrote:
> > > I did some testing on audio part with HDMI-HDMI and DP-HDMI cables
> > > connected to my Haswell box. Before applying the patch I tested as a
> > > reference with the latest -nightly (04-08-2015), 4.0-rc7. Unfortunately,
> > > I failed to get any audio over HDMI cable. For a reference I tested with
> > > the very same setup the vanillla kernel from Linus tree 4.0-rc7 and with
> > > that kernel the audio worked ok. Then I did some GIT bisecting and it
> > > turned out that the first commit that I failed to get audio working was
> > > aa2fee4286e43b4784982b17669b02cc99c1ae55.
> > 
> I rerun the bisecting and this time the result was
> 
> commit 0a599838737a2527c35e4d94f794aefe59df1781
> Merge: 2d846c7 a59d719
> Author: Takashi Iwai <tiwai@suse.de>
> Date:   Wed Apr 8 11:29:56 2015 +0200
> 
>     Merge branch 'for-linus' into for-next
>     
>     Back merge HD-audio quirks to for-next branch, so that we can apply
>     a couple of more quirks.
>     
>     Signed-off-by: Takashi Iwai <tiwai@suse.de>

Adding Takashi and intel audio folks.
-Daniel

> 
> > I don't have this sha1 anywhere. Can you please double-check?
> > 
> > Also to avoid such issues please always add at least the commit subject
> > and author, so that I can find it if it's somehow rebased.
> > -Daniel
> > 
> > > 
> > > To test this patch audio functionality I checkout the -nightly version
> > > that works for me and apply the patch and test it. I'll come back with
> > > the results later on. 
> > > 
> > > I had the module option i915.disable_power_well=0
> > > 
> > > The test routine that I used for audio testing was
> > > speaker-test -c 2 -r 48000 -f S16_LE -t pink --device=plughw:0,3
> > > 
> > > On Tue, 2015-04-07 at 15:52 +0200, Daniel Vetter wrote:
> > > > On Tue, Apr 07, 2015 at 12:29:25PM +0300, Mika Kahola wrote:
> > > > > Definitely a good idea to check the audio part as well if there is
> > > > > a doubt that by changing CD clock the audio would fail. I can check
> > > > > this and I'll get back once I have the results.
> > > > 
> > > > We force a full modeset, which should result in an interrupt on the audio
> > > > side, which should result in the audio driver re-reading the current
> > > > cdclk. If that's no the case it's buggy already.
> > > > -Daniel
> > > 
> > > -- 
> > > Mika Kahola, Intel OTC
> > > 
> > > 
> > > 
> > 
> 
> -- 
> Mika Kahola, Intel OTC
> 
> 
>

Mika Kahola April 14, 2015, 6:36 a.m. UTC | #12

I tested this patch with the audio in place. With this setup in my HSW
machine I can hear the pink noise played back with DP-HDMI cable
attatched.

speaker-test -c 2 -r 48000 -F S16_LE -t pink --device=plughw:0,7

Cheers,
Mika

On Tue, 2015-04-07 at 14:06 +0530, Sivakumar Thulasimani wrote:
> where can i check this (audio driver) ? since there was no need for them 
> to check CD clock value till now i don't think they will be doing it. 
> also this needs to be changed in Display Audio controller, so not sure 
> if audio driver has access to it in the first place. will be good to 
> confirm this before merging as it will break display audio if it is not 
> programmed.
> 
> if any one has setup this can be confirmed by just switching CD clock 
> after boot and playing an audio file.
> 
> regards,
> Sivakumar
> 
> On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
> > On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
> >> sorry if i am missing something, HSW and BDW requires display audio
> >> controller to be updated with new values once CD clock is modified. how
> >> is this accomplished here ?
> > I'm hoping the audio driver will query the cdclk frequency after every
> > modeset.
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Sivakumar Thulasimani April 14, 2015, 6:57 a.m. UTC | #13

Thanks for the update Mika. The issue will be that audio plays faster or 
slower than normal. i.e it will be < 1x or > 1x. can you confirm if 
audible sound plays after CD Clock change at 1x speed ?

regards,
Sivakumar
On 4/14/2015 12:06 PM, Mika Kahola wrote:
> I tested this patch with the audio in place. With this setup in my HSW
> machine I can hear the pink noise played back with DP-HDMI cable
> attatched.
>
> speaker-test -c 2 -r 48000 -F S16_LE -t pink --device=plughw:0,7
>
> Cheers,
> Mika
>
> On Tue, 2015-04-07 at 14:06 +0530, Sivakumar Thulasimani wrote:
>> where can i check this (audio driver) ? since there was no need for them
>> to check CD clock value till now i don't think they will be doing it.
>> also this needs to be changed in Display Audio controller, so not sure
>> if audio driver has access to it in the first place. will be good to
>> confirm this before merging as it will break display audio if it is not
>> programmed.
>>
>> if any one has setup this can be confirmed by just switching CD clock
>> after boot and playing an audio file.
>>
>> regards,
>> Sivakumar
>>
>> On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
>>> On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
>>>> sorry if i am missing something, HSW and BDW requires display audio
>>>> controller to be updated with new values once CD clock is modified. how
>>>> is this accomplished here ?
>>> I'm hoping the audio driver will query the cdclk frequency after every
>>> modeset.
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>

Mika Kahola April 14, 2015, 7:06 a.m. UTC | #14

With pink noise you can't tell if the audio is played faster or slower.

I tested this by playing ITU speech codec test sample

aplay --device=plughw:0,7 male.wav

and audio played back on 1x speed.

Cheers,
Mika

On Tue, 2015-04-14 at 12:27 +0530, Sivakumar Thulasimani wrote:
> Thanks for the update Mika. The issue will be that audio plays faster or 
> slower than normal. i.e it will be < 1x or > 1x. can you confirm if 
> audible sound plays after CD Clock change at 1x speed ?
> 
> regards,
> Sivakumar
> On 4/14/2015 12:06 PM, Mika Kahola wrote:
> > I tested this patch with the audio in place. With this setup in my HSW
> > machine I can hear the pink noise played back with DP-HDMI cable
> > attatched.
> >
> > speaker-test -c 2 -r 48000 -F S16_LE -t pink --device=plughw:0,7
> >
> > Cheers,
> > Mika
> >
> > On Tue, 2015-04-07 at 14:06 +0530, Sivakumar Thulasimani wrote:
> >> where can i check this (audio driver) ? since there was no need for them
> >> to check CD clock value till now i don't think they will be doing it.
> >> also this needs to be changed in Display Audio controller, so not sure
> >> if audio driver has access to it in the first place. will be good to
> >> confirm this before merging as it will break display audio if it is not
> >> programmed.
> >>
> >> if any one has setup this can be confirmed by just switching CD clock
> >> after boot and playing an audio file.
> >>
> >> regards,
> >> Sivakumar
> >>
> >> On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
> >>> On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
> >>>> sorry if i am missing something, HSW and BDW requires display audio
> >>>> controller to be updated with new values once CD clock is modified. how
> >>>> is this accomplished here ?
> >>> I'm hoping the audio driver will query the cdclk frequency after every
> >>> modeset.
> >>>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
>

Sivakumar Thulasimani April 14, 2015, 7:54 a.m. UTC | #15

Thanks for the confirmation, Mika :). then this change is fine
not sure if it is still relevant but you can add rb if possible

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On 4/14/2015 12:36 PM, Mika Kahola wrote:
> With pink noise you can't tell if the audio is played faster or slower.
>
> I tested this by playing ITU speech codec test sample
>
> aplay --device=plughw:0,7 male.wav
>
> and audio played back on 1x speed.
>
> Cheers,
> Mika
>
> On Tue, 2015-04-14 at 12:27 +0530, Sivakumar Thulasimani wrote:
>> Thanks for the update Mika. The issue will be that audio plays faster or
>> slower than normal. i.e it will be < 1x or > 1x. can you confirm if
>> audible sound plays after CD Clock change at 1x speed ?
>>
>> regards,
>> Sivakumar
>> On 4/14/2015 12:06 PM, Mika Kahola wrote:
>>> I tested this patch with the audio in place. With this setup in my HSW
>>> machine I can hear the pink noise played back with DP-HDMI cable
>>> attatched.
>>>
>>> speaker-test -c 2 -r 48000 -F S16_LE -t pink --device=plughw:0,7
>>>
>>> Cheers,
>>> Mika
>>>
>>> On Tue, 2015-04-07 at 14:06 +0530, Sivakumar Thulasimani wrote:
>>>> where can i check this (audio driver) ? since there was no need for them
>>>> to check CD clock value till now i don't think they will be doing it.
>>>> also this needs to be changed in Display Audio controller, so not sure
>>>> if audio driver has access to it in the first place. will be good to
>>>> confirm this before merging as it will break display audio if it is not
>>>> programmed.
>>>>
>>>> if any one has setup this can be confirmed by just switching CD clock
>>>> after boot and playing an audio file.
>>>>
>>>> regards,
>>>> Sivakumar
>>>>
>>>> On 4/7/2015 1:59 PM, Ville Syrjälä wrote:
>>>>> On Tue, Apr 07, 2015 at 12:33:40PM +0530, Sivakumar Thulasimani wrote:
>>>>>> sorry if i am missing something, HSW and BDW requires display audio
>>>>>> controller to be updated with new values once CD clock is modified. how
>>>>>> is this accomplished here ?
>>>>> I'm hoping the audio driver will query the cdclk frequency after every
>>>>> modeset.
>>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>>
>
>
>

[15/19] drm/i915: HSW cdclk support

Commit Message

Comments

Patch