Re: [PATCH 2/2] drm/i915: Implement BXT and GLK cdclk restriction based on Azalia BCLK

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

On Tue, 2017-02-28 at 18:57 -0800, Dhinakaran Pandiyan wrote:
> According to BSpec, "The CD clock frequency must be at least twice the
> frequency of the Azalia BCLK." and BCLK is configured to 96 MHz by
> default. BXT and GLK both have cdclk frequencies that are less han 192 MHz,
> so apply the check conditionally for these platforms.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 8fc0f72..89027fa 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1444,6 +1444,8 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
>  		struct intel_crtc_state *crtc_state;
>  
>  		crtc_state = to_intel_crtc_state(cstate);
> +		if (!crtc_state->has_audio)
> +			continue;
>  
>  		/* According to BSpec, "Do not use DisplayPort with CDCLK less
>  		 * than 432 MHz, audio enabled, port width x4, and link rate
> @@ -1452,7 +1454,6 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
>  		 * for GLK is at 316.8 MHz
>  		 */
>  		if (intel_crtc_has_dp_encoder(crtc_state) &&
> -		    crtc_state->has_audio &&
>  		    crtc_state->port_clock >= 540000 &&
>  		    crtc_state->lane_count == 4) {
>  			if (IS_GEMINILAKE(dev_priv))
> @@ -1460,6 +1461,13 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
>  			else if (IS_BROADWELL(dev_priv) || IS_GEN9(dev_priv))
>  				min_cdclk = 432000;
>  		}
> +
> +		/* According to BSpec, "The CD clock frequency must be at least
> +		 * twice the frequency of the Azalia BCLK." and BCLK is 96 MHz
> +		 * by default.
> +		 */
> +		if (IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			min_cdclk = max(min_cdclk, 2 * 96000);

Hmm, my assumption from reviewing patch 1 of "this returns a valid cdclk" is
broken here. Maybe with the idea of splitting the calc_cdclk_state() in two from
my comment to Paulo's series this could be just:

	min_cdclk = max(min_cdcdlk, choose_cdclk(2 * 96000));

?

On second thought, it does make sense for this kind of platform specific
restrictions to be dealt with in platform specific hook, so duplicating that in
calc_cdclk_state() is not that bad?

Ander


>  	}
>  
>  	return min_cdclk;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx