From: Ville Syrjälä ville.syrjala@linux.intel.com
On VLV/CHV there is some kind of linkage between the cdclk frequency and the DP link frequency. The spec says: "For DP audio configuration, cdclk frequency shall be set to meet the following requirements: DP Link Frequency(MHz) | Cdclk frequency(MHz) 270 | 320 or higher 162 | 200 or higher"
I suspect that would more accurately be expressed as "cdclk >= DP link clock", and in any case we can express it like that in the code because of the limited set of cdclk and link frequencies we support.
Without this we can end up in a situation where the cdclk is too low and enabling DP audio will kill the pipe. Happens eg. with 2560x1440 modes where the 266MHz cdclk is sufficient to pump the pixels (241.5 MHz dotclock) but is too low for the DP audio due to the link frequency being 270 MHz.
Cc: stable@vger.kernel.org Tested-by: Stefan Gottwald gottwald@igel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111149 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/display/intel_cdclk.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index d0581a1ac243..93b0d190c184 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2262,6 +2262,17 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk);
+ /* + * "For DP audio configuration, cdclk frequency shall be set to + * meet the following requirements: + * DP Link Frequency(MHz) | Cdclk frequency(MHz) + * 270 | 320 or higher + * 162 | 200 or higher" + */ + if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && + intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio) + min_cdclk = max(crtc_state->port_clock, min_cdclk); + /* * On Valleyview some DSI panels lose (v|h)sync when the clock is lower * than 320000KHz.
Quoting Ville Syrjala (2019-07-17 12:45:36)
From: Ville Syrjälä ville.syrjala@linux.intel.com
On VLV/CHV there is some kind of linkage between the cdclk frequency and the DP link frequency. The spec says: "For DP audio configuration, cdclk frequency shall be set to meet the following requirements: DP Link Frequency(MHz) | Cdclk frequency(MHz) 270 | 320 or higher 162 | 200 or higher"
I suspect that would more accurately be expressed as "cdclk >= DP link clock", and in any case we can express it like that in the code because of the limited set of cdclk and link frequencies we support.
Without this we can end up in a situation where the cdclk is too low and enabling DP audio will kill the pipe. Happens eg. with 2560x1440 modes where the 266MHz cdclk is sufficient to pump the pixels (241.5 MHz dotclock) but is too low for the DP audio due to the link frequency being 270 MHz.
Cc: stable@vger.kernel.org Tested-by: Stefan Gottwald gottwald@igel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111149 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_cdclk.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index d0581a1ac243..93b0d190c184 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2262,6 +2262,17 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk);
/** "For DP audio configuration, cdclk frequency shall be set to* meet the following requirements:* DP Link Frequency(MHz) | Cdclk frequency(MHz)* 270 | 320 or higher* 162 | 200 or higher"*/if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio)min_cdclk = max(crtc_state->port_clock, min_cdclk);
I tracked port_clock down to being the dp link clock (162 or 270) so that part of the story checks out.
Judging by the rest of the function, I buy that the cdclk and link clock may be inscrutably tied together, and accept the test result that the cdclk must be at least the link clock with audio enabled.
It may be that a corner case does require a higher frequency (rather than just bumping from 266 to 270), but for here and now Acked-by: Chris Wilson chris@chris-wilson.co.uk -Chris
On Thu, Jul 18, 2019 at 06:11:08PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2019-07-17 12:45:36)
From: Ville Syrjälä ville.syrjala@linux.intel.com
On VLV/CHV there is some kind of linkage between the cdclk frequency and the DP link frequency. The spec says: "For DP audio configuration, cdclk frequency shall be set to meet the following requirements: DP Link Frequency(MHz) | Cdclk frequency(MHz) 270 | 320 or higher 162 | 200 or higher"
I suspect that would more accurately be expressed as "cdclk >= DP link clock", and in any case we can express it like that in the code because of the limited set of cdclk and link frequencies we support.
Without this we can end up in a situation where the cdclk is too low and enabling DP audio will kill the pipe. Happens eg. with 2560x1440 modes where the 266MHz cdclk is sufficient to pump the pixels (241.5 MHz dotclock) but is too low for the DP audio due to the link frequency being 270 MHz.
Cc: stable@vger.kernel.org Tested-by: Stefan Gottwald gottwald@igel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111149 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_cdclk.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index d0581a1ac243..93b0d190c184 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2262,6 +2262,17 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk);
/** "For DP audio configuration, cdclk frequency shall be set to* meet the following requirements:* DP Link Frequency(MHz) | Cdclk frequency(MHz)* 270 | 320 or higher* 162 | 200 or higher"*/if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio)min_cdclk = max(crtc_state->port_clock, min_cdclk);I tracked port_clock down to being the dp link clock (162 or 270) so that part of the story checks out.
Judging by the rest of the function, I buy that the cdclk and link clock may be inscrutably tied together, and accept the test result that the cdclk must be at least the link clock with audio enabled.
It may be that a corner case does require a higher frequency (rather than just bumping from 266 to 270), but for here and now
Yeah there could be some extra headroom required. But our cdclk can only be 200, 266, 320 or 400 MHz (and 200 won't actually get used due to inexplicable display failure when try to use it). So in practice we going to actually get bumped 162->266 and 270->320 here. I should have expressed that better in the commit message.
Acked-by: Chris Wilson chris@chris-wilson.co.uk
Thanks. I amended the explanation a bit and pushed to dinq.
Quoting Ville Syrjälä (2019-07-18 19:04:56)
On Thu, Jul 18, 2019 at 06:11:08PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2019-07-17 12:45:36)
From: Ville Syrjälä ville.syrjala@linux.intel.com
On VLV/CHV there is some kind of linkage between the cdclk frequency and the DP link frequency. The spec says: "For DP audio configuration, cdclk frequency shall be set to meet the following requirements: DP Link Frequency(MHz) | Cdclk frequency(MHz) 270 | 320 or higher 162 | 200 or higher"
I suspect that would more accurately be expressed as "cdclk >= DP link clock", and in any case we can express it like that in the code because of the limited set of cdclk and link frequencies we support.
Without this we can end up in a situation where the cdclk is too low and enabling DP audio will kill the pipe. Happens eg. with 2560x1440 modes where the 266MHz cdclk is sufficient to pump the pixels (241.5 MHz dotclock) but is too low for the DP audio due to the link frequency being 270 MHz.
Cc: stable@vger.kernel.org Tested-by: Stefan Gottwald gottwald@igel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111149 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_cdclk.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index d0581a1ac243..93b0d190c184 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2262,6 +2262,17 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk);
/** "For DP audio configuration, cdclk frequency shall be set to* meet the following requirements:* DP Link Frequency(MHz) | Cdclk frequency(MHz)* 270 | 320 or higher* 162 | 200 or higher"*/if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio)min_cdclk = max(crtc_state->port_clock, min_cdclk);I tracked port_clock down to being the dp link clock (162 or 270) so that part of the story checks out.
Judging by the rest of the function, I buy that the cdclk and link clock may be inscrutably tied together, and accept the test result that the cdclk must be at least the link clock with audio enabled.
It may be that a corner case does require a higher frequency (rather than just bumping from 266 to 270), but for here and now
Yeah there could be some extra headroom required. But our cdclk can only be 200, 266, 320 or 400 MHz (and 200 won't actually get used due to inexplicable display failure when try to use it). So in practice we going to actually get bumped 162->266 and 270->320 here. I should have expressed that better in the commit message.
Also, I didn't find an easy way to confirm the limited set of cdlck. Hmm, actually I was looking at min_cdclk and didn't thin to compare that against any table. Ah, adding a see vlv_calc_cdclk() might have helped me put together the other side. Certainly adding more of a hint that min_cdclk isn't the final clock would help :) -Chris
linux-stable-mirror@lists.linaro.org
-
Chris Wilson -
Ville Syrjala -
Ville Syrjälä