* [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
@ 2017-07-07 10:24 ville.syrjala
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Follow the GLK path when computing cdclk and related limits. CNL
pipes also produce two pixels per clock, so that's what we should
use.
For the HBR2 vs. audio issue the limit should more correctly be 336
MHz, but the GLK limit of 316.8 MHz works just as well and results
in picking at least 336 MHz. Also toss in some related w/a numbers.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 1241e5891b29..9e0deebae279 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
crtc_state->has_audio &&
crtc_state->port_clock >= 540000 &&
crtc_state->lane_count == 4) {
- if (IS_CANNONLAKE(dev_priv))
- pixel_rate = max(316800, pixel_rate);
- else if (IS_GEMINILAKE(dev_priv))
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+ /* Display WA #1145: glk,cnl */
pixel_rate = max(2 * 316800, pixel_rate);
- else
+ } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
+ /* Display WA #1144: skl,bxt */
pixel_rate = max(432000, pixel_rate);
+ }
}
/* According to BSpec, "The CD clock frequency must be at least twice
@@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
* two pixels per clock.
*/
if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
- if (IS_GEMINILAKE(dev_priv))
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
pixel_rate = max(2 * 2 * 96000, pixel_rate);
else
pixel_rate = max(2 * 96000, pixel_rate);
@@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
{
int max_cdclk_freq = dev_priv->max_cdclk_freq;
- if (IS_GEMINILAKE(dev_priv))
+ if (IS_CANNONLAKE(dev_priv))
+ return 2 * max_cdclk_freq;
+ else if (IS_GEMINILAKE(dev_priv))
/*
* FIXME: Limiting to 99% as a temporary workaround. See
* glk_calc_cdclk() for details.
--
2.13.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
@ 2017-07-07 10:24 ` ville.syrjala
2017-07-07 18:28 ` Ville Syrjälä
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
` (4 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.
The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 12 ++-
drivers/gpu/drm/i915/intel_cdclk.c | 179 +++++++++++++++++------------------
drivers/gpu/drm/i915/intel_display.c | 21 ++--
drivers/gpu/drm/i915/intel_drv.h | 4 +-
4 files changed, 107 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..c80176424d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,15 @@ struct i915_hotplug {
(__i)++) \
for_each_if (plane_state)
+#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
+ for ((__i) = 0; \
+ (__i) < (__state)->base.dev->mode_config.num_crtc && \
+ ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+ (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+ (__i)++) \
+ for_each_if (crtc)
+
+
struct drm_i915_private;
struct i915_mm_struct;
struct i915_mmu_object;
@@ -2268,7 +2277,8 @@ struct drm_i915_private {
struct mutex dpll_lock;
unsigned int active_crtcs;
- unsigned int min_pixclk[I915_MAX_PIPES];
+ /* minimum acceptable cdclk for each pipe */
+ int min_cdclk[I915_MAX_PIPES];
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9e0deebae279..82e5bc967cca 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -417,24 +417,21 @@ static void hsw_get_cdclk(struct drm_i915_private *dev_priv,
cdclk_state->cdclk = 540000;
}
-static int vlv_calc_cdclk(struct drm_i915_private *dev_priv,
- int max_pixclk)
+static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk)
{
int freq_320 = (dev_priv->hpll_freq << 1) % 320000 != 0 ?
333333 : 320000;
- int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
/*
* We seem to get an unstable or solid color picture at 200MHz.
* Not sure what's wrong. For now use 200MHz only when all pipes
* are off.
*/
- if (!IS_CHERRYVIEW(dev_priv) &&
- max_pixclk > freq_320*limit/100)
+ if (IS_VALLEYVIEW(dev_priv) && min_cdclk > freq_320)
return 400000;
- else if (max_pixclk > 266667*limit/100)
+ else if (min_cdclk > 266667)
return freq_320;
- else if (max_pixclk > 0)
+ else if (min_cdclk > 0)
return 266667;
else
return 200000;
@@ -612,13 +609,13 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
}
-static int bdw_calc_cdclk(int max_pixclk)
+static int bdw_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 675000;
- else if (max_pixclk > 450000)
+ else if (min_cdclk > 450000)
return 540000;
- else if (max_pixclk > 337500)
+ else if (min_cdclk > 337500)
return 450000;
else
return 337500;
@@ -724,23 +721,23 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
cdclk, dev_priv->cdclk.hw.cdclk);
}
-static int skl_calc_cdclk(int max_pixclk, int vco)
+static int skl_calc_cdclk(int min_cdclk, int vco)
{
if (vco == 8640000) {
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 617143;
- else if (max_pixclk > 432000)
+ else if (min_cdclk > 432000)
return 540000;
- else if (max_pixclk > 308571)
+ else if (min_cdclk > 308571)
return 432000;
else
return 308571;
} else {
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 675000;
- else if (max_pixclk > 450000)
+ else if (min_cdclk > 450000)
return 540000;
- else if (max_pixclk > 337500)
+ else if (min_cdclk > 337500)
return 450000;
else
return 337500;
@@ -1075,31 +1072,25 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state);
}
-static int bxt_calc_cdclk(int max_pixclk)
+static int bxt_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 576000)
+ if (min_cdclk > 576000)
return 624000;
- else if (max_pixclk > 384000)
+ else if (min_cdclk > 384000)
return 576000;
- else if (max_pixclk > 288000)
+ else if (min_cdclk > 288000)
return 384000;
- else if (max_pixclk > 144000)
+ else if (min_cdclk > 144000)
return 288000;
else
return 144000;
}
-static int glk_calc_cdclk(int max_pixclk)
+static int glk_calc_cdclk(int min_cdclk)
{
- /*
- * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
- * as a temporary workaround. Use a higher cdclk instead. (Note that
- * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
- * cdclk.)
- */
- if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
+ if (min_cdclk > 158400)
return 316800;
- else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
+ else if (min_cdclk > 79200)
return 158400;
else
return 79200;
@@ -1420,11 +1411,11 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state);
}
-static int cnl_calc_cdclk(int max_pixclk)
+static int cnl_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 336000)
+ if (min_cdclk > 336000)
return 528000;
- else if (max_pixclk > 168000)
+ else if (min_cdclk > 168000)
return 336000;
else
return 168000;
@@ -1732,21 +1723,47 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
dev_priv->display.set_cdclk(dev_priv, cdclk_state);
}
-static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
- int pixel_rate)
+static int intel_min_cdclk(struct drm_i915_private *dev_priv,
+ int pixel_rate)
+{
+ if (INTEL_GEN(dev_priv) >= 10)
+ return DIV_ROUND_UP(pixel_rate, 2);
+ else if (IS_GEMINILAKE(dev_priv))
+ /*
+ * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
+ * as a temporary workaround. Use a higher cdclk instead. (Note that
+ * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
+ * cdclk.)
+ */
+ return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
+ else if (IS_GEN9(dev_priv) ||
+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+ return pixel_rate;
+ else if (IS_CHERRYVIEW(dev_priv))
+ return DIV_ROUND_UP(pixel_rate * 100, 95);
+ else
+ return DIV_ROUND_UP(pixel_rate * 100, 90);
+}
+
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv =
to_i915(crtc_state->base.crtc->dev);
+ int min_cdclk;
+
+ if (!crtc_state->base.enable)
+ return 0;
+
+ min_cdclk = intel_min_cdclk(dev_priv, crtc_state->pixel_rate);
/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
- pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+ min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95);
/* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz,
* audio enabled, port width x4, and link rate HBR2 (5.4 GHz), or else
* there may be audio corruption or screen corruption." This cdclk
- * restriction for GLK is 316.8 MHz and since GLK can output two
- * pixels per clock, the pixel rate becomes 2 * 316.8 MHz.
+ * restriction for GLK is 316.8 MHz.
*/
if (intel_crtc_has_dp_encoder(crtc_state) &&
crtc_state->has_audio &&
@@ -1754,77 +1771,53 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
crtc_state->lane_count == 4) {
if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
/* Display WA #1145: glk,cnl */
- pixel_rate = max(2 * 316800, pixel_rate);
+ min_cdclk = max(316800, min_cdclk);
} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
/* Display WA #1144: skl,bxt */
- pixel_rate = max(432000, pixel_rate);
+ min_cdclk = max(432000, min_cdclk);
}
}
/* 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.
- * The check for GLK has to be adjusted as the platform can output
- * two pixels per clock.
*/
- if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
- if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
- pixel_rate = max(2 * 2 * 96000, pixel_rate);
- else
- pixel_rate = max(2 * 96000, pixel_rate);
- }
+ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+ min_cdclk = max(2 * 96000, min_cdclk);
- return pixel_rate;
+ return min_cdclk;
}
-/* compute the max rate for new configuration */
-static int intel_max_pixel_rate(struct drm_atomic_state *state)
+static int intel_compute_min_cdclk(struct drm_atomic_state *state)
{
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_i915_private *dev_priv = to_i915(state->dev);
- struct drm_crtc *crtc;
- struct drm_crtc_state *cstate;
+ struct intel_crtc *crtc;
struct intel_crtc_state *crtc_state;
- unsigned int max_pixel_rate = 0, i;
+ int min_cdclk = 0, i;
enum pipe pipe;
- memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
- sizeof(intel_state->min_pixclk));
-
- for_each_new_crtc_in_state(state, crtc, cstate, i) {
- int pixel_rate;
+ memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
+ sizeof(intel_state->min_cdclk));
- crtc_state = to_intel_crtc_state(cstate);
- if (!crtc_state->base.enable) {
- intel_state->min_pixclk[i] = 0;
- continue;
- }
-
- pixel_rate = crtc_state->pixel_rate;
-
- if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
- pixel_rate =
- bdw_adjust_min_pipe_pixel_rate(crtc_state,
- pixel_rate);
-
- intel_state->min_pixclk[i] = pixel_rate;
- }
+ for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
+ intel_state->min_cdclk[i] =
+ intel_crtc_compute_min_cdclk(crtc_state);
for_each_pipe(dev_priv, pipe)
- max_pixel_rate = max(intel_state->min_pixclk[pipe],
- max_pixel_rate);
+ min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
- return max_pixel_rate;
+ return min_cdclk;
}
static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
int cdclk;
- cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
+ cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1850,14 +1843,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk;
/*
* FIXME should also account for plane ratio
* once 64bpp pixel formats are supported.
*/
- cdclk = bdw_calc_cdclk(max_pixclk);
+ cdclk = bdw_calc_cdclk(min_cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1883,7 +1876,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_i915_private *dev_priv = to_i915(state->dev);
- const int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk, vco;
vco = intel_state->cdclk.logical.vco;
@@ -1894,7 +1887,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
* FIXME should also account for plane ratio
* once 64bpp pixel formats are supported.
*/
- cdclk = skl_calc_cdclk(max_pixclk, vco);
+ cdclk = skl_calc_cdclk(min_cdclk, vco);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1921,16 +1914,16 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
int cdclk, vco;
if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(max_pixclk);
+ cdclk = glk_calc_cdclk(min_cdclk);
vco = glk_de_pll_vco(dev_priv, cdclk);
} else {
- cdclk = bxt_calc_cdclk(max_pixclk);
+ cdclk = bxt_calc_cdclk(min_cdclk);
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
@@ -1967,10 +1960,10 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk, vco;
- cdclk = cnl_calc_cdclk(max_pixclk);
+ cdclk = cnl_calc_cdclk(min_cdclk);
vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
@@ -2005,11 +1998,11 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
else if (IS_GEMINILAKE(dev_priv))
/*
* FIXME: Limiting to 99% as a temporary workaround. See
- * glk_calc_cdclk() for details.
+ * intel_min_cdclk() for details.
*/
return 2 * max_cdclk_freq * 99 / 100;
- else if (INTEL_INFO(dev_priv)->gen >= 9 ||
- IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ else if (IS_GEN9(dev_priv) ||
+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
return max_cdclk_freq;
else if (IS_CHERRYVIEW(dev_priv))
return max_cdclk_freq*95/100;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2144adc5b1d5..b47535f5d95d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5920,7 +5920,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
intel_crtc->enabled_power_domains = 0;
dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
- dev_priv->min_pixclk[intel_crtc->pipe] = 0;
+ dev_priv->min_cdclk[intel_crtc->pipe] = 0;
}
/*
@@ -13292,8 +13292,8 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_atomic_track_fbs(state);
if (intel_state->modeset) {
- memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
- sizeof(intel_state->min_pixclk));
+ memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
+ sizeof(intel_state->min_cdclk));
dev_priv->active_crtcs = intel_state->active_crtcs;
dev_priv->cdclk.logical = intel_state->cdclk.logical;
dev_priv->cdclk.actual = intel_state->cdclk.actual;
@@ -15569,7 +15569,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
for_each_intel_crtc(dev, crtc) {
struct intel_crtc_state *crtc_state =
to_intel_crtc_state(crtc->base.state);
- int pixclk = 0;
+ int min_cdclk = 0;
memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
if (crtc_state->base.active) {
@@ -15590,22 +15590,15 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
intel_crtc_compute_pixel_rate(crtc_state);
- if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv) ||
- IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- pixclk = crtc_state->pixel_rate;
- else
- WARN_ON(dev_priv->display.modeset_calc_cdclk);
-
- /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
- if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
- pixclk = DIV_ROUND_UP(pixclk * 100, 95);
+ if (dev_priv->display.modeset_calc_cdclk)
+ min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
drm_calc_timestamping_constants(&crtc->base,
&crtc_state->base.adjusted_mode);
update_scanline_offset(crtc);
}
- dev_priv->min_pixclk[crtc->pipe] = pixclk;
+ dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
intel_pipe_config_sanity_check(dev_priv, crtc_state);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..8cc1b86b799a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,7 +383,8 @@ struct intel_atomic_state {
unsigned int active_pipe_changes;
unsigned int active_crtcs;
- unsigned int min_pixclk[I915_MAX_PIPES];
+ /* minimum acceptable cdclk for each pipe */
+ int min_cdclk[I915_MAX_PIPES];
struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
@@ -1308,6 +1309,7 @@ void intel_audio_init(struct drm_i915_private *dev_priv);
void intel_audio_deinit(struct drm_i915_private *dev_priv);
/* intel_cdclk.c */
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
void skl_init_cdclk(struct drm_i915_private *dev_priv);
void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
void cnl_init_cdclk(struct drm_i915_private *dev_priv);
--
2.13.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
@ 2017-07-07 10:24 ` ville.syrjala
2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Currently the .modeset_calc_cdclk() hooks check the final cdclk value
against the max allowed. That's not really sufficient since the low
level calc_cdclk() functions effectively clamp the minimum required
cdclk to the max supported by the platform. Hence if the minimum
required exceeds the platforms capabilities we'd keep going anyway
using the max cdclk frequency.
To fix that let's move the check earlier into
intel_crtc_compute_min_cdclk() and we'll check the minimum required
cdclk of the pipe against the maximum supported by the platform.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_cdclk.c | 96 +++++++++++++++++-------------------
drivers/gpu/drm/i915/intel_display.c | 5 +-
2 files changed, 48 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 82e5bc967cca..d7a77123a7e5 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1784,6 +1784,12 @@ 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);
+ if (min_cdclk > dev_priv->max_cdclk_freq) {
+ DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
+ min_cdclk, dev_priv->max_cdclk_freq);
+ return -EINVAL;
+ }
+
return min_cdclk;
}
@@ -1793,16 +1799,21 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_crtc *crtc;
struct intel_crtc_state *crtc_state;
- int min_cdclk = 0, i;
+ int min_cdclk, i;
enum pipe pipe;
memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
sizeof(intel_state->min_cdclk));
- for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
- intel_state->min_cdclk[i] =
- intel_crtc_compute_min_cdclk(crtc_state);
+ for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
+ min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+ if (min_cdclk < 0)
+ return min_cdclk;
+
+ intel_state->min_cdclk[i] = min_cdclk;
+ }
+ min_cdclk = 0;
for_each_pipe(dev_priv, pipe)
min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
@@ -1812,18 +1823,14 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int min_cdclk = intel_compute_min_cdclk(state);
- struct intel_atomic_state *intel_state =
- to_intel_atomic_state(state);
- int cdclk;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ int min_cdclk, cdclk;
- cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
+ min_cdclk = intel_compute_min_cdclk(state);
+ if (min_cdclk < 0)
+ return min_cdclk;
- if (cdclk > dev_priv->max_cdclk_freq) {
- DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
- return -EINVAL;
- }
+ cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
intel_state->cdclk.logical.cdclk = cdclk;
@@ -1841,10 +1848,12 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
{
- struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- int min_cdclk = intel_compute_min_cdclk(state);
- int cdclk;
+ int min_cdclk, cdclk;
+
+ min_cdclk = intel_compute_min_cdclk(state);
+ if (min_cdclk < 0)
+ return min_cdclk;
/*
* FIXME should also account for plane ratio
@@ -1852,12 +1861,6 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
*/
cdclk = bdw_calc_cdclk(min_cdclk);
- if (cdclk > dev_priv->max_cdclk_freq) {
- DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
- return -EINVAL;
- }
-
intel_state->cdclk.logical.cdclk = cdclk;
if (!intel_state->active_crtcs) {
@@ -1874,10 +1877,13 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
{
- struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int min_cdclk = intel_compute_min_cdclk(state);
- int cdclk, vco;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ int min_cdclk, cdclk, vco;
+
+ min_cdclk = intel_compute_min_cdclk(state);
+ if (min_cdclk < 0)
+ return min_cdclk;
vco = intel_state->cdclk.logical.vco;
if (!vco)
@@ -1889,12 +1895,6 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
*/
cdclk = skl_calc_cdclk(min_cdclk, vco);
- if (cdclk > dev_priv->max_cdclk_freq) {
- DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
- return -EINVAL;
- }
-
intel_state->cdclk.logical.vco = vco;
intel_state->cdclk.logical.cdclk = cdclk;
@@ -1914,10 +1914,12 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int min_cdclk = intel_compute_min_cdclk(state);
- struct intel_atomic_state *intel_state =
- to_intel_atomic_state(state);
- int cdclk, vco;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ int min_cdclk, cdclk, vco;
+
+ min_cdclk = intel_compute_min_cdclk(state);
+ if (min_cdclk < 0)
+ return min_cdclk;
if (IS_GEMINILAKE(dev_priv)) {
cdclk = glk_calc_cdclk(min_cdclk);
@@ -1927,12 +1929,6 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
- if (cdclk > dev_priv->max_cdclk_freq) {
- DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
- return -EINVAL;
- }
-
intel_state->cdclk.logical.vco = vco;
intel_state->cdclk.logical.cdclk = cdclk;
@@ -1958,20 +1954,16 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- struct intel_atomic_state *intel_state =
- to_intel_atomic_state(state);
- int min_cdclk = intel_compute_min_cdclk(state);
- int cdclk, vco;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ int min_cdclk, cdclk, vco;
+
+ min_cdclk = intel_compute_min_cdclk(state);
+ if (min_cdclk < 0)
+ return min_cdclk;
cdclk = cnl_calc_cdclk(min_cdclk);
vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
- if (cdclk > dev_priv->max_cdclk_freq) {
- DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
- cdclk, dev_priv->max_cdclk_freq);
- return -EINVAL;
- }
-
intel_state->cdclk.logical.vco = vco;
intel_state->cdclk.logical.cdclk = cdclk;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b47535f5d95d..1caf0ef82e36 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15590,8 +15590,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
intel_crtc_compute_pixel_rate(crtc_state);
- if (dev_priv->display.modeset_calc_cdclk)
+ if (dev_priv->display.modeset_calc_cdclk) {
min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+ if (WARN_ON(min_cdclk < 0))
+ min_cdclk = 0;
+ }
drm_calc_timestamping_constants(&crtc->base,
&crtc_state->base.adjusted_mode);
--
2.13.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
@ 2017-07-07 11:13 ` Patchwork
2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-07-07 11:13 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915: Fix up CNL cdclk related limits
URL : https://patchwork.freedesktop.org/series/26988/
State : success
== Summary ==
Series 26988v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/26988/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
fail -> PASS (fi-snb-2600) fdo#100215
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip -> PASS (fi-skl-x1585l)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-byt-n2820) fdo#101705
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:442s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:427s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:350s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:531s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:513s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:489s
fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:478s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:593s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:433s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:419s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:419s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:484s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:470s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:460s
fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:572s
fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:584s
fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:561s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:460s
fi-skl-6700hq total:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:585s
fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:467s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:481s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:439s
fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:488s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:540s
fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:407s
c07f01228c2240ec9604cb9fa4647ccfe575b8a6 drm-tip: 2017y-07m-07d-10h-10m-58s UTC integration manifest
565b523 drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
78592a4b drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
11d4ee5 drm/i915: Fix up CNL cdclk related limits
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5142/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
` (2 preceding siblings ...)
2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
@ 2017-07-07 17:54 ` Rodrigo Vivi
2017-07-07 18:24 ` Ville Syrjälä
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork
5 siblings, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2017-07-07 17:54 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni
I will review the series more carefully later,
but my concern is that with this series applied I got a blank screen on eDP...
On Fri, Jul 7, 2017 at 3:24 AM, <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> use.
>
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..9e0deebae279 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> crtc_state->has_audio &&
> crtc_state->port_clock >= 540000 &&
> crtc_state->lane_count == 4) {
> - if (IS_CANNONLAKE(dev_priv))
> - pixel_rate = max(316800, pixel_rate);
> - else if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> + /* Display WA #1145: glk,cnl */
> pixel_rate = max(2 * 316800, pixel_rate);
> - else
> + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> + /* Display WA #1144: skl,bxt */
> pixel_rate = max(432000, pixel_rate);
> + }
> }
>
> /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> * two pixels per clock.
> */
> if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> - if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> pixel_rate = max(2 * 2 * 96000, pixel_rate);
> else
> pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> {
> int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> - if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv))
> + return 2 * max_cdclk_freq;
> + else if (IS_GEMINILAKE(dev_priv))
> /*
> * FIXME: Limiting to 99% as a temporary workaround. See
> * glk_calc_cdclk() for details.
> --
> 2.13.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
@ 2017-07-07 18:24 ` Ville Syrjälä
2017-07-07 18:33 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-07 18:24 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni
On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...
Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.
>
>
> On Fri, Jul 7, 2017 at 3:24 AM, <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 540000 &&
> > crtc_state->lane_count == 4) {
> > - if (IS_CANNONLAKE(dev_priv))
> > - pixel_rate = max(316800, pixel_rate);
> > - else if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > + /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > - else
> > + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > + /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > + }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > * two pixels per clock.
> > */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv))
> > + return 2 * max_cdclk_freq;
> > + else if (IS_GEMINILAKE(dev_priv))
> > /*
> > * FIXME: Limiting to 99% as a temporary workaround. See
> > * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
@ 2017-07-07 18:28 ` Ville Syrjälä
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
1 sibling, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-07 18:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
On Fri, Jul 07, 2017 at 01:24:49PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the min_pixclk thing less confusing by changing it to track
> the minimum acceptable cdclk frequency instead. This means moving
> the application of the guardbands to a slightly higher level from
> the low level platform specific calc_cdclk() functions.
>
> The immediate benefit is elimination of the confusing 2x factors
> on GLK/CNL+ in the audio workarounds (which stems from the fact
> that the pipes produce two pixels per clock).
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 12 ++-
> drivers/gpu/drm/i915/intel_cdclk.c | 179 +++++++++++++++++------------------
> drivers/gpu/drm/i915/intel_display.c | 21 ++--
> drivers/gpu/drm/i915/intel_drv.h | 4 +-
> 4 files changed, 107 insertions(+), 109 deletions(-)
>
<snip>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 9e0deebae279..82e5bc967cca 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1732,21 +1723,47 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> }
>
> -static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> - int pixel_rate)
> +static int intel_min_cdclk(struct drm_i915_private *dev_priv,
> + int pixel_rate)
> +{
> + if (INTEL_GEN(dev_priv) >= 10)
> + return DIV_ROUND_UP(pixel_rate, 2);
Rodrigo, so this part here could be why your CNL no longer works.
If you have time to try it, then I think changing this to just
'return pixel_rate;' should get us back to the old behaviour.
> + else if (IS_GEMINILAKE(dev_priv))
> + /*
> + * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
> + * as a temporary workaround. Use a higher cdclk instead. (Note that
> + * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
> + * cdclk.)
> + */
> + return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
> + else if (IS_GEN9(dev_priv) ||
> + IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> + return pixel_rate;
> + else if (IS_CHERRYVIEW(dev_priv))
> + return DIV_ROUND_UP(pixel_rate * 100, 95);
> + else
> + return DIV_ROUND_UP(pixel_rate * 100, 90);
> +}
> +
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-07 18:24 ` Ville Syrjälä
@ 2017-07-07 18:33 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-07 18:33 UTC (permalink / raw)
To: Ville Syrjälä, Rodrigo Vivi
Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo
iirc I assumed 1 pixel per clk for CNL when I originally submitted the workaround patch because cnl_xxx_calc_cdclk() functions assume that. Are we missing something in the cdclk setup code to enable 2 pixels per clk?
-DK
________________________________________
From: Ville Syrjälä [ville.syrjala@linux.intel.com]
Sent: Friday, July 07, 2017 11:24 AM
To: Rodrigo Vivi
Cc: intel-gfx; Pandiyan, Dhinakaran; Zanoni, Paulo R; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...
Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.
>
>
> On Fri, Jul 7, 2017 at 3:24 AM, <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 540000 &&
> > crtc_state->lane_count == 4) {
> > - if (IS_CANNONLAKE(dev_priv))
> > - pixel_rate = max(316800, pixel_rate);
> > - else if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > + /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > - else
> > + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > + /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > + }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > * two pixels per clock.
> > */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv))
> > + return 2 * max_cdclk_freq;
> > + else if (IS_GEMINILAKE(dev_priv))
> > /*
> > * FIXME: Limiting to 99% as a temporary workaround. See
> > * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
` (3 preceding siblings ...)
2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
@ 2017-07-10 13:02 ` ville.syrjala
2017-07-10 17:34 ` Pandiyan, Dhinakaran
2017-07-10 17:34 ` Rodrigo Vivi
2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork
5 siblings, 2 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-10 13:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Follow the GLK path when computing cdclk and related limits. CNL
pipes also produce two pixels per clock, so that's what we should
really use. However for the purposes of pixel rate calculations we
will assume one pixel per clock to keep the voltage higher, at least
until the missing voltage scaling for DDI clocks is implemented.
For the HBR2 vs. audio issue the limit should more correctly be 336
MHz, but the GLK limit of 316.8 MHz works just as well and results
in picking at least 336 MHz. Also toss in some related w/a numbers.
v2: Assume 1 pixel per clock for the purposes of max pixel rate
calculation until DDI clock voltage scaling is handled
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 1241e5891b29..4b8eb6a7d852 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
crtc_state->has_audio &&
crtc_state->port_clock >= 540000 &&
crtc_state->lane_count == 4) {
- if (IS_CANNONLAKE(dev_priv))
- pixel_rate = max(316800, pixel_rate);
- else if (IS_GEMINILAKE(dev_priv))
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+ /* Display WA #1145: glk,cnl */
pixel_rate = max(2 * 316800, pixel_rate);
- else
+ } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
+ /* Display WA #1144: skl,bxt */
pixel_rate = max(432000, pixel_rate);
+ }
}
/* According to BSpec, "The CD clock frequency must be at least twice
@@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
* two pixels per clock.
*/
if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
- if (IS_GEMINILAKE(dev_priv))
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
pixel_rate = max(2 * 2 * 96000, pixel_rate);
else
pixel_rate = max(2 * 96000, pixel_rate);
@@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
{
int max_cdclk_freq = dev_priv->max_cdclk_freq;
- if (IS_GEMINILAKE(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 10)
+ /*
+ * FIXME: Allow '2 * max_cdclk_freq'
+ * once DDI clock voltage requirements are
+ * handled correctly.
+ */
+ return max_cdclk_freq;
+ else if (IS_GEMINILAKE(dev_priv))
/*
* FIXME: Limiting to 99% as a temporary workaround. See
* glk_calc_cdclk() for details.
--
2.13.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
2017-07-07 18:28 ` Ville Syrjälä
@ 2017-07-10 13:02 ` ville.syrjala
1 sibling, 0 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-10 13:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.
The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).
v2: Keep cdclk higher on CNL to workaround missing DDI clock voltage handling
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 12 ++-
drivers/gpu/drm/i915/intel_cdclk.c | 184 +++++++++++++++++------------------
drivers/gpu/drm/i915/intel_display.c | 21 ++--
drivers/gpu/drm/i915/intel_drv.h | 4 +-
4 files changed, 112 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..c80176424d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,15 @@ struct i915_hotplug {
(__i)++) \
for_each_if (plane_state)
+#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
+ for ((__i) = 0; \
+ (__i) < (__state)->base.dev->mode_config.num_crtc && \
+ ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+ (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+ (__i)++) \
+ for_each_if (crtc)
+
+
struct drm_i915_private;
struct i915_mm_struct;
struct i915_mmu_object;
@@ -2268,7 +2277,8 @@ struct drm_i915_private {
struct mutex dpll_lock;
unsigned int active_crtcs;
- unsigned int min_pixclk[I915_MAX_PIPES];
+ /* minimum acceptable cdclk for each pipe */
+ int min_cdclk[I915_MAX_PIPES];
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9e0deebae279..9c0b3ca5256c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -417,24 +417,21 @@ static void hsw_get_cdclk(struct drm_i915_private *dev_priv,
cdclk_state->cdclk = 540000;
}
-static int vlv_calc_cdclk(struct drm_i915_private *dev_priv,
- int max_pixclk)
+static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk)
{
int freq_320 = (dev_priv->hpll_freq << 1) % 320000 != 0 ?
333333 : 320000;
- int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
/*
* We seem to get an unstable or solid color picture at 200MHz.
* Not sure what's wrong. For now use 200MHz only when all pipes
* are off.
*/
- if (!IS_CHERRYVIEW(dev_priv) &&
- max_pixclk > freq_320*limit/100)
+ if (IS_VALLEYVIEW(dev_priv) && min_cdclk > freq_320)
return 400000;
- else if (max_pixclk > 266667*limit/100)
+ else if (min_cdclk > 266667)
return freq_320;
- else if (max_pixclk > 0)
+ else if (min_cdclk > 0)
return 266667;
else
return 200000;
@@ -612,13 +609,13 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
}
-static int bdw_calc_cdclk(int max_pixclk)
+static int bdw_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 675000;
- else if (max_pixclk > 450000)
+ else if (min_cdclk > 450000)
return 540000;
- else if (max_pixclk > 337500)
+ else if (min_cdclk > 337500)
return 450000;
else
return 337500;
@@ -724,23 +721,23 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
cdclk, dev_priv->cdclk.hw.cdclk);
}
-static int skl_calc_cdclk(int max_pixclk, int vco)
+static int skl_calc_cdclk(int min_cdclk, int vco)
{
if (vco == 8640000) {
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 617143;
- else if (max_pixclk > 432000)
+ else if (min_cdclk > 432000)
return 540000;
- else if (max_pixclk > 308571)
+ else if (min_cdclk > 308571)
return 432000;
else
return 308571;
} else {
- if (max_pixclk > 540000)
+ if (min_cdclk > 540000)
return 675000;
- else if (max_pixclk > 450000)
+ else if (min_cdclk > 450000)
return 540000;
- else if (max_pixclk > 337500)
+ else if (min_cdclk > 337500)
return 450000;
else
return 337500;
@@ -1075,31 +1072,25 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state);
}
-static int bxt_calc_cdclk(int max_pixclk)
+static int bxt_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 576000)
+ if (min_cdclk > 576000)
return 624000;
- else if (max_pixclk > 384000)
+ else if (min_cdclk > 384000)
return 576000;
- else if (max_pixclk > 288000)
+ else if (min_cdclk > 288000)
return 384000;
- else if (max_pixclk > 144000)
+ else if (min_cdclk > 144000)
return 288000;
else
return 144000;
}
-static int glk_calc_cdclk(int max_pixclk)
+static int glk_calc_cdclk(int min_cdclk)
{
- /*
- * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
- * as a temporary workaround. Use a higher cdclk instead. (Note that
- * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
- * cdclk.)
- */
- if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
+ if (min_cdclk > 158400)
return 316800;
- else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
+ else if (min_cdclk > 79200)
return 158400;
else
return 79200;
@@ -1420,11 +1411,11 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state);
}
-static int cnl_calc_cdclk(int max_pixclk)
+static int cnl_calc_cdclk(int min_cdclk)
{
- if (max_pixclk > 336000)
+ if (min_cdclk > 336000)
return 528000;
- else if (max_pixclk > 168000)
+ else if (min_cdclk > 168000)
return 336000;
else
return 168000;
@@ -1732,21 +1723,52 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
dev_priv->display.set_cdclk(dev_priv, cdclk_state);
}
-static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
- int pixel_rate)
+static int intel_min_cdclk(struct drm_i915_private *dev_priv,
+ int pixel_rate)
+{
+ if (INTEL_GEN(dev_priv) >= 10)
+ /*
+ * FIXME: Switch to DIV_ROUND_UP(pixel_rate, 2)
+ * once DDI clock voltage requirements are
+ * handled correctly.
+ */
+ return pixel_rate;
+ else if (IS_GEMINILAKE(dev_priv))
+ /*
+ * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
+ * as a temporary workaround. Use a higher cdclk instead. (Note that
+ * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
+ * cdclk.)
+ */
+ return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
+ else if (IS_GEN9(dev_priv) ||
+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+ return pixel_rate;
+ else if (IS_CHERRYVIEW(dev_priv))
+ return DIV_ROUND_UP(pixel_rate * 100, 95);
+ else
+ return DIV_ROUND_UP(pixel_rate * 100, 90);
+}
+
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv =
to_i915(crtc_state->base.crtc->dev);
+ int min_cdclk;
+
+ if (!crtc_state->base.enable)
+ return 0;
+
+ min_cdclk = intel_min_cdclk(dev_priv, crtc_state->pixel_rate);
/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
- pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+ min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95);
/* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz,
* audio enabled, port width x4, and link rate HBR2 (5.4 GHz), or else
* there may be audio corruption or screen corruption." This cdclk
- * restriction for GLK is 316.8 MHz and since GLK can output two
- * pixels per clock, the pixel rate becomes 2 * 316.8 MHz.
+ * restriction for GLK is 316.8 MHz.
*/
if (intel_crtc_has_dp_encoder(crtc_state) &&
crtc_state->has_audio &&
@@ -1754,77 +1776,53 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
crtc_state->lane_count == 4) {
if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
/* Display WA #1145: glk,cnl */
- pixel_rate = max(2 * 316800, pixel_rate);
+ min_cdclk = max(316800, min_cdclk);
} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
/* Display WA #1144: skl,bxt */
- pixel_rate = max(432000, pixel_rate);
+ min_cdclk = max(432000, min_cdclk);
}
}
/* 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.
- * The check for GLK has to be adjusted as the platform can output
- * two pixels per clock.
*/
- if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
- if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
- pixel_rate = max(2 * 2 * 96000, pixel_rate);
- else
- pixel_rate = max(2 * 96000, pixel_rate);
- }
+ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+ min_cdclk = max(2 * 96000, min_cdclk);
- return pixel_rate;
+ return min_cdclk;
}
-/* compute the max rate for new configuration */
-static int intel_max_pixel_rate(struct drm_atomic_state *state)
+static int intel_compute_min_cdclk(struct drm_atomic_state *state)
{
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_i915_private *dev_priv = to_i915(state->dev);
- struct drm_crtc *crtc;
- struct drm_crtc_state *cstate;
+ struct intel_crtc *crtc;
struct intel_crtc_state *crtc_state;
- unsigned int max_pixel_rate = 0, i;
+ int min_cdclk = 0, i;
enum pipe pipe;
- memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
- sizeof(intel_state->min_pixclk));
-
- for_each_new_crtc_in_state(state, crtc, cstate, i) {
- int pixel_rate;
+ memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
+ sizeof(intel_state->min_cdclk));
- crtc_state = to_intel_crtc_state(cstate);
- if (!crtc_state->base.enable) {
- intel_state->min_pixclk[i] = 0;
- continue;
- }
-
- pixel_rate = crtc_state->pixel_rate;
-
- if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
- pixel_rate =
- bdw_adjust_min_pipe_pixel_rate(crtc_state,
- pixel_rate);
-
- intel_state->min_pixclk[i] = pixel_rate;
- }
+ for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
+ intel_state->min_cdclk[i] =
+ intel_crtc_compute_min_cdclk(crtc_state);
for_each_pipe(dev_priv, pipe)
- max_pixel_rate = max(intel_state->min_pixclk[pipe],
- max_pixel_rate);
+ min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
- return max_pixel_rate;
+ return min_cdclk;
}
static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
int cdclk;
- cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
+ cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1850,14 +1848,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk;
/*
* FIXME should also account for plane ratio
* once 64bpp pixel formats are supported.
*/
- cdclk = bdw_calc_cdclk(max_pixclk);
+ cdclk = bdw_calc_cdclk(min_cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1883,7 +1881,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_i915_private *dev_priv = to_i915(state->dev);
- const int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk, vco;
vco = intel_state->cdclk.logical.vco;
@@ -1894,7 +1892,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
* FIXME should also account for plane ratio
* once 64bpp pixel formats are supported.
*/
- cdclk = skl_calc_cdclk(max_pixclk, vco);
+ cdclk = skl_calc_cdclk(min_cdclk, vco);
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1921,16 +1919,16 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
int cdclk, vco;
if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(max_pixclk);
+ cdclk = glk_calc_cdclk(min_cdclk);
vco = glk_de_pll_vco(dev_priv, cdclk);
} else {
- cdclk = bxt_calc_cdclk(max_pixclk);
+ cdclk = bxt_calc_cdclk(min_cdclk);
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
@@ -1967,10 +1965,10 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_atomic_state *intel_state =
to_intel_atomic_state(state);
- int max_pixclk = intel_max_pixel_rate(state);
+ int min_cdclk = intel_compute_min_cdclk(state);
int cdclk, vco;
- cdclk = cnl_calc_cdclk(max_pixclk);
+ cdclk = cnl_calc_cdclk(min_cdclk);
vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
if (cdclk > dev_priv->max_cdclk_freq) {
@@ -2005,11 +2003,11 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
else if (IS_GEMINILAKE(dev_priv))
/*
* FIXME: Limiting to 99% as a temporary workaround. See
- * glk_calc_cdclk() for details.
+ * intel_min_cdclk() for details.
*/
return 2 * max_cdclk_freq * 99 / 100;
- else if (INTEL_INFO(dev_priv)->gen >= 9 ||
- IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ else if (IS_GEN9(dev_priv) ||
+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
return max_cdclk_freq;
else if (IS_CHERRYVIEW(dev_priv))
return max_cdclk_freq*95/100;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2144adc5b1d5..b47535f5d95d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5920,7 +5920,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
intel_crtc->enabled_power_domains = 0;
dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
- dev_priv->min_pixclk[intel_crtc->pipe] = 0;
+ dev_priv->min_cdclk[intel_crtc->pipe] = 0;
}
/*
@@ -13292,8 +13292,8 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_atomic_track_fbs(state);
if (intel_state->modeset) {
- memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
- sizeof(intel_state->min_pixclk));
+ memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
+ sizeof(intel_state->min_cdclk));
dev_priv->active_crtcs = intel_state->active_crtcs;
dev_priv->cdclk.logical = intel_state->cdclk.logical;
dev_priv->cdclk.actual = intel_state->cdclk.actual;
@@ -15569,7 +15569,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
for_each_intel_crtc(dev, crtc) {
struct intel_crtc_state *crtc_state =
to_intel_crtc_state(crtc->base.state);
- int pixclk = 0;
+ int min_cdclk = 0;
memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
if (crtc_state->base.active) {
@@ -15590,22 +15590,15 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
intel_crtc_compute_pixel_rate(crtc_state);
- if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv) ||
- IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- pixclk = crtc_state->pixel_rate;
- else
- WARN_ON(dev_priv->display.modeset_calc_cdclk);
-
- /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
- if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
- pixclk = DIV_ROUND_UP(pixclk * 100, 95);
+ if (dev_priv->display.modeset_calc_cdclk)
+ min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
drm_calc_timestamping_constants(&crtc->base,
&crtc_state->base.adjusted_mode);
update_scanline_offset(crtc);
}
- dev_priv->min_pixclk[crtc->pipe] = pixclk;
+ dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
intel_pipe_config_sanity_check(dev_priv, crtc_state);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..8cc1b86b799a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,7 +383,8 @@ struct intel_atomic_state {
unsigned int active_pipe_changes;
unsigned int active_crtcs;
- unsigned int min_pixclk[I915_MAX_PIPES];
+ /* minimum acceptable cdclk for each pipe */
+ int min_cdclk[I915_MAX_PIPES];
struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
@@ -1308,6 +1309,7 @@ void intel_audio_init(struct drm_i915_private *dev_priv);
void intel_audio_deinit(struct drm_i915_private *dev_priv);
/* intel_cdclk.c */
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
void skl_init_cdclk(struct drm_i915_private *dev_priv);
void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
void cnl_init_cdclk(struct drm_i915_private *dev_priv);
--
2.13.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3)
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
` (4 preceding siblings ...)
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
@ 2017-07-10 13:21 ` Patchwork
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-07-10 13:21 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3)
URL : https://patchwork.freedesktop.org/series/26988/
State : success
== Summary ==
Series 26988v3 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/26988/revisions/3/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:445s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:427s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:355s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:520s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:507s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:489s
fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:484s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:596s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:435s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:414s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:498s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:471s
fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:576s
fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:582s
fi-pnv-d510 total:279 pass:221 dwarn:3 dfail:0 fail:0 skip:55 time:562s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:462s
fi-skl-6700hq total:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:587s
fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:462s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:479s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:431s
fi-skl-x1585l total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:481s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:544s
fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:410s
7484481f41b972c21f2d69c7b53df0d2522d9c70 drm-tip: 2017y-07m-10d-12h-14m-03s UTC integration manifest
9ad3738 drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
3be1884 drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
e14e106 drm/i915: Fix up CNL cdclk related limits
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5154/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
@ 2017-07-10 17:34 ` Pandiyan, Dhinakaran
2017-07-10 18:11 ` Ville Syrjälä
2017-07-10 17:34 ` Rodrigo Vivi
1 sibling, 1 reply; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-10 17:34 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo
On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> really use. However for the purposes of pixel rate calculations we
> will assume one pixel per clock to keep the voltage higher, at least
> until the missing voltage scaling for DDI clocks is implemented.
>
Does the lack of correct voltage scaling implementation affect only
intel_compute_max_dotclk()? i.e., allowing a pixel rate of
2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
account pixel_rate <= 2*cdclk_freq for any frequency?
With this patch,
bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
Isn't that a discrepancy?
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
isn't the correct workaround cdclk 336 MHz?
>
> v2: Assume 1 pixel per clock for the purposes of max pixel rate
> calculation until DDI clock voltage scaling is handled
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..4b8eb6a7d852 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> crtc_state->has_audio &&
> crtc_state->port_clock >= 540000 &&
> crtc_state->lane_count == 4) {
> - if (IS_CANNONLAKE(dev_priv))
> - pixel_rate = max(316800, pixel_rate);
> - else if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> + /* Display WA #1145: glk,cnl */
> pixel_rate = max(2 * 316800, pixel_rate);
> - else
> + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> + /* Display WA #1144: skl,bxt */
> pixel_rate = max(432000, pixel_rate);
> + }
> }
>
> /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> * two pixels per clock.
> */
> if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> - if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> pixel_rate = max(2 * 2 * 96000, pixel_rate);
> else
> pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> {
> int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> - if (IS_GEMINILAKE(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 10)
> + /*
> + * FIXME: Allow '2 * max_cdclk_freq'
> + * once DDI clock voltage requirements are
> + * handled correctly.
> + */
> + return max_cdclk_freq;
> + else if (IS_GEMINILAKE(dev_priv))
> /*
> * FIXME: Limiting to 99% as a temporary workaround. See
> * glk_calc_cdclk() for details.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-10 17:34 ` Pandiyan, Dhinakaran
@ 2017-07-10 17:34 ` Rodrigo Vivi
2017-07-10 17:55 ` Ville Syrjälä
1 sibling, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2017-07-10 17:34 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni
cool, with
v2 of patch 1
v2 of patch 2
patch 3
display works properly here on cnl.
On Mon, Jul 10, 2017 at 6:02 AM, <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> really use. However for the purposes of pixel rate calculations we
> will assume one pixel per clock to keep the voltage higher, at least
> until the missing voltage scaling for DDI clocks is implemented.
>
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> v2: Assume 1 pixel per clock for the purposes of max pixel rate
> calculation until DDI clock voltage scaling is handled
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..4b8eb6a7d852 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> crtc_state->has_audio &&
> crtc_state->port_clock >= 540000 &&
> crtc_state->lane_count == 4) {
> - if (IS_CANNONLAKE(dev_priv))
> - pixel_rate = max(316800, pixel_rate);
> - else if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> + /* Display WA #1145: glk,cnl */
> pixel_rate = max(2 * 316800, pixel_rate);
> - else
> + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> + /* Display WA #1144: skl,bxt */
> pixel_rate = max(432000, pixel_rate);
> + }
> }
>
> /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> * two pixels per clock.
> */
> if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> - if (IS_GEMINILAKE(dev_priv))
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> pixel_rate = max(2 * 2 * 96000, pixel_rate);
> else
> pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> {
> int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> - if (IS_GEMINILAKE(dev_priv))
> + if (INTEL_GEN(dev_priv) >= 10)
> + /*
> + * FIXME: Allow '2 * max_cdclk_freq'
> + * once DDI clock voltage requirements are
> + * handled correctly.
> + */
> + return max_cdclk_freq;
> + else if (IS_GEMINILAKE(dev_priv))
> /*
> * FIXME: Limiting to 99% as a temporary workaround. See
> * glk_calc_cdclk() for details.
Are you sure we don't want this workaround also? With so similar
display engines I wonder if we would end with similar issues.
But I'm just asking... because honestly I didn't check that 99%
workaround closely enough yet...
The rest of the patch makes sense for me and works so feel free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
rv-b only for this test for now, but tested-by you could use in all 3
patches mentioned above...
> --
> 2.13.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-10 17:34 ` Rodrigo Vivi
@ 2017-07-10 17:55 ` Ville Syrjälä
0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-10 17:55 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni
On Mon, Jul 10, 2017 at 10:34:22AM -0700, Rodrigo Vivi wrote:
> cool, with
>
> v2 of patch 1
> v2 of patch 2
> patch 3
>
> display works properly here on cnl.
>
> On Mon, Jul 10, 2017 at 6:02 AM, <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > really use. However for the purposes of pixel rate calculations we
> > will assume one pixel per clock to keep the voltage higher, at least
> > until the missing voltage scaling for DDI clocks is implemented.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> > calculation until DDI clock voltage scaling is handled
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..4b8eb6a7d852 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 540000 &&
> > crtc_state->lane_count == 4) {
> > - if (IS_CANNONLAKE(dev_priv))
> > - pixel_rate = max(316800, pixel_rate);
> > - else if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > + /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > - else
> > + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > + /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > + }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > * two pixels per clock.
> > */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (INTEL_GEN(dev_priv) >= 10)
> > + /*
> > + * FIXME: Allow '2 * max_cdclk_freq'
> > + * once DDI clock voltage requirements are
> > + * handled correctly.
> > + */
> > + return max_cdclk_freq;
> > + else if (IS_GEMINILAKE(dev_priv))
> > /*
> > * FIXME: Limiting to 99% as a temporary workaround. See
> > * glk_calc_cdclk() for details.
>
> Are you sure we don't want this workaround also? With so similar
> display engines I wonder if we would end with similar issues.
> But I'm just asking... because honestly I didn't check that 99%
> workaround closely enough yet...
That workaround seems to be based on empirical evidence and I'd like to
people to get to the bottom of it before it spreads all over the place.
One theory I have is that it might be caused by simply using the wrong
pixel clock when we calculate the pipe's pixel rate. That code currently
assumes that the DPLL can generate a 100% accurate clock which
definitely isn't always the case. IIRC I have code somewhere that
tries to correct that by moving the DPLL computation to happen earlier
than the cdclk computation. That would mean we could then update
adjusted_mode.crtc_clock with the actual clock produced by the DPLL.
I think that patch is stuck in my IVB bifurcation branch and I don't
recall if some of the modeset code reorganization I have there is needed
as well. I guess I'd have to try and extract it and see how much I
need to pull out with it.
>
> The rest of the patch makes sense for me and works so feel free to use:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> rv-b only for this test for now, but tested-by you could use in all 3
> patches mentioned above...
>
>
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-10 17:34 ` Pandiyan, Dhinakaran
@ 2017-07-10 18:11 ` Ville Syrjälä
2017-07-10 18:36 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-10 18:11 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo
On Mon, Jul 10, 2017 at 05:34:11PM +0000, Pandiyan, Dhinakaran wrote:
>
>
>
> On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > really use. However for the purposes of pixel rate calculations we
> > will assume one pixel per clock to keep the voltage higher, at least
> > until the missing voltage scaling for DDI clocks is implemented.
> >
>
> Does the lack of correct voltage scaling implementation affect only
> intel_compute_max_dotclk()? i.e., allowing a pixel rate of
> 2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
> account pixel_rate <= 2*cdclk_freq for any frequency?
>
>
> With this patch,
> bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
> cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
> Isn't that a discrepancy?
Hmm. Yeah. I suppose I should just squash this with patch 2/3. My
original intention for this separate patch was to respect the 2x
limit rather than the 1x limit. But since we couldn't do that I
suppose the justification for this patch has pretty much gone
away, and as you point out, it just leads to a mess.
The combination of patches 1/3+2/3 should still do the right thing
because we no longer use the pixel rate in the audio workarounds.
>
>
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
> 633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
> isn't the correct workaround cdclk 336 MHz?
>
>
> >
> > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> > calculation until DDI clock voltage scaling is handled
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..4b8eb6a7d852 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > crtc_state->has_audio &&
> > crtc_state->port_clock >= 540000 &&
> > crtc_state->lane_count == 4) {
> > - if (IS_CANNONLAKE(dev_priv))
> > - pixel_rate = max(316800, pixel_rate);
> > - else if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > + /* Display WA #1145: glk,cnl */
> > pixel_rate = max(2 * 316800, pixel_rate);
> > - else
> > + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > + /* Display WA #1144: skl,bxt */
> > pixel_rate = max(432000, pixel_rate);
> > + }
> > }
> >
> > /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > * two pixels per clock.
> > */
> > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > else
> > pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > {
> > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > - if (IS_GEMINILAKE(dev_priv))
> > + if (INTEL_GEN(dev_priv) >= 10)
> > + /*
> > + * FIXME: Allow '2 * max_cdclk_freq'
> > + * once DDI clock voltage requirements are
> > + * handled correctly.
> > + */
> > + return max_cdclk_freq;
> > + else if (IS_GEMINILAKE(dev_priv))
> > /*
> > * FIXME: Limiting to 99% as a temporary workaround. See
> > * glk_calc_cdclk() for details.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
2017-07-10 18:11 ` Ville Syrjälä
@ 2017-07-10 18:36 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-10 18:36 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo
On Mon, 2017-07-10 at 21:11 +0300, Ville Syrjälä wrote:
> On Mon, Jul 10, 2017 at 05:34:11PM +0000, Pandiyan, Dhinakaran wrote:
> >
> >
> >
> > On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Follow the GLK path when computing cdclk and related limits. CNL
> > > pipes also produce two pixels per clock, so that's what we should
> > > really use. However for the purposes of pixel rate calculations we
> > > will assume one pixel per clock to keep the voltage higher, at least
> > > until the missing voltage scaling for DDI clocks is implemented.
> > >
> >
> > Does the lack of correct voltage scaling implementation affect only
> > intel_compute_max_dotclk()? i.e., allowing a pixel rate of
> > 2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
> > account pixel_rate <= 2*cdclk_freq for any frequency?
> >
> >
> > With this patch,
> > bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
> > cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
> > Isn't that a discrepancy?
>
> Hmm. Yeah. I suppose I should just squash this with patch 2/3. My
> original intention for this separate patch was to respect the 2x
> limit rather than the 1x limit. But since we couldn't do that I
> suppose the justification for this patch has pretty much gone
> away, and as you point out, it just leads to a mess.
>
> The combination of patches 1/3+2/3 should still do the right thing
> because we no longer use the pixel rate in the audio workarounds.
>
I just took a look at 2/3, squashing does solve the problem. I'll review
the combined patch when you send it.
-DK
> >
> >
> > > For the HBR2 vs. audio issue the limit should more correctly be 336
> > > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
> > 633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
> > isn't the correct workaround cdclk 336 MHz?
> >
> >
> > >
> > > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> > > calculation until DDI clock voltage scaling is handled
> > >
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> > > 1 file changed, 14 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 1241e5891b29..4b8eb6a7d852 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > crtc_state->has_audio &&
> > > crtc_state->port_clock >= 540000 &&
> > > crtc_state->lane_count == 4) {
> > > - if (IS_CANNONLAKE(dev_priv))
> > > - pixel_rate = max(316800, pixel_rate);
> > > - else if (IS_GEMINILAKE(dev_priv))
> > > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > + /* Display WA #1145: glk,cnl */
> > > pixel_rate = max(2 * 316800, pixel_rate);
> > > - else
> > > + } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > + /* Display WA #1144: skl,bxt */
> > > pixel_rate = max(432000, pixel_rate);
> > > + }
> > > }
> > >
> > > /* According to BSpec, "The CD clock frequency must be at least twice
> > > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > * two pixels per clock.
> > > */
> > > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > > - if (IS_GEMINILAKE(dev_priv))
> > > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > > else
> > > pixel_rate = max(2 * 96000, pixel_rate);
> > > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > > {
> > > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > >
> > > - if (IS_GEMINILAKE(dev_priv))
> > > + if (INTEL_GEN(dev_priv) >= 10)
> > > + /*
> > > + * FIXME: Allow '2 * max_cdclk_freq'
> > > + * once DDI clock voltage requirements are
> > > + * handled correctly.
> > > + */
> > > + return max_cdclk_freq;
> > > + else if (IS_GEMINILAKE(dev_priv))
> > > /*
> > > * FIXME: Limiting to 99% as a temporary workaround. See
> > > * glk_calc_cdclk() for details.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-07-10 18:36 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
2017-07-07 18:28 ` Ville Syrjälä
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
2017-07-07 18:24 ` Ville Syrjälä
2017-07-07 18:33 ` Pandiyan, Dhinakaran
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-10 17:34 ` Pandiyan, Dhinakaran
2017-07-10 18:11 ` Ville Syrjälä
2017-07-10 18:36 ` Pandiyan, Dhinakaran
2017-07-10 17:34 ` Rodrigo Vivi
2017-07-10 17:55 ` Ville Syrjälä
2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.