Skip to content

Commit bb0f4aa

Browse files
vsyrjalavsyrjala
committed
drm/i915: Track full cdclk state for the logical and actual cdclk frequencies
The current dev_cdclk vs. cdclk vs. atomic_cdclk_freq is quite a mess. So here I'm introducing the "actual" and "logical" naming for our cdclk state. "actual" is what we'll bash into the hardware and "logical" is what everyone should use for state computaion/checking and whatnot. We'll track both using the intel_cdclk_state as both will need other differing parameters than just the actual cdclk frequency. While doing that we can at the same time unify the appearance of the .modeset_calc_cdclk() implementations a little bit. v2: Commit dev_priv->cdclk.actual since that already has the new state by the time .modeset_commit_cdclk() is called. v3: s/locical/logical/ and improve the docs a bit Signed-off-by: Ville Syrjälä <[email protected]> Reviewed-by: Ander Conselvan de Oliveira <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 49cd97a commit bb0f4aa

File tree

6 files changed

+128
-84
lines changed

6 files changed

+128
-84
lines changed

‎drivers/gpu/drm/i915/i915_drv.h

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,18 +2172,26 @@ struct drm_i915_private {
21722172
unsigned int skl_preferred_vco_freq;
21732173
unsigned int max_cdclk_freq;
21742174

2175-
/*
2176-
* For reading holding any crtc lock is sufficient,
2177-
* for writing must hold all of them.
2178-
*/
2179-
unsigned int atomic_cdclk_freq;
2180-
21812175
unsigned int max_dotclk_freq;
21822176
unsigned int rawclk_freq;
21832177
unsigned int hpll_freq;
21842178
unsigned int czclk_freq;
21852179

21862180
struct {
2181+
/*
2182+
* The current logical cdclk state.
2183+
* See intel_atomic_state.cdclk.logical
2184+
*
2185+
* For reading holding any crtc lock is sufficient,
2186+
* for writing must hold all of them.
2187+
*/
2188+
struct intel_cdclk_state logical;
2189+
/*
2190+
* The current actual cdclk state.
2191+
* See intel_atomic_state.cdclk.actual
2192+
*/
2193+
struct intel_cdclk_state actual;
2194+
/* The current hardware cdclk state */
21872195
struct intel_cdclk_state hw;
21882196
} cdclk;
21892197

‎drivers/gpu/drm/i915/intel_cdclk.c

Lines changed: 78 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,12 +1460,26 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
14601460
int max_pixclk = intel_max_pixel_rate(state);
14611461
struct intel_atomic_state *intel_state =
14621462
to_intel_atomic_state(state);
1463+
int cdclk;
1464+
1465+
cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
1466+
1467+
if (cdclk > dev_priv->max_cdclk_freq) {
1468+
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
1469+
cdclk, dev_priv->max_cdclk_freq);
1470+
return -EINVAL;
1471+
}
14631472

1464-
intel_state->cdclk = intel_state->dev_cdclk =
1465-
vlv_calc_cdclk(dev_priv, max_pixclk);
1473+
intel_state->cdclk.logical.cdclk = cdclk;
14661474

1467-
if (!intel_state->active_crtcs)
1468-
intel_state->dev_cdclk = vlv_calc_cdclk(dev_priv, 0);
1475+
if (!intel_state->active_crtcs) {
1476+
cdclk = vlv_calc_cdclk(dev_priv, 0);
1477+
1478+
intel_state->cdclk.actual.cdclk = cdclk;
1479+
} else {
1480+
intel_state->cdclk.actual =
1481+
intel_state->cdclk.logical;
1482+
}
14691483

14701484
return 0;
14711485
}
@@ -1474,9 +1488,7 @@ static void vlv_modeset_commit_cdclk(struct drm_atomic_state *old_state)
14741488
{
14751489
struct drm_device *dev = old_state->dev;
14761490
struct drm_i915_private *dev_priv = to_i915(dev);
1477-
struct intel_atomic_state *old_intel_state =
1478-
to_intel_atomic_state(old_state);
1479-
unsigned int req_cdclk = old_intel_state->dev_cdclk;
1491+
unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
14801492

14811493
/*
14821494
* FIXME: We can end up here with all power domains off, yet
@@ -1518,19 +1530,24 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
15181530
return -EINVAL;
15191531
}
15201532

1521-
intel_state->cdclk = intel_state->dev_cdclk = cdclk;
1522-
if (!intel_state->active_crtcs)
1523-
intel_state->dev_cdclk = bdw_calc_cdclk(0);
1533+
intel_state->cdclk.logical.cdclk = cdclk;
1534+
1535+
if (!intel_state->active_crtcs) {
1536+
cdclk = bdw_calc_cdclk(0);
1537+
1538+
intel_state->cdclk.actual.cdclk = cdclk;
1539+
} else {
1540+
intel_state->cdclk.actual =
1541+
intel_state->cdclk.logical;
1542+
}
15241543

15251544
return 0;
15261545
}
15271546

15281547
static void bdw_modeset_commit_cdclk(struct drm_atomic_state *old_state)
15291548
{
15301549
struct drm_device *dev = old_state->dev;
1531-
struct intel_atomic_state *old_intel_state =
1532-
to_intel_atomic_state(old_state);
1533-
unsigned int req_cdclk = old_intel_state->dev_cdclk;
1550+
unsigned int req_cdclk = to_i915(dev)->cdclk.actual.cdclk;
15341551

15351552
bdw_set_cdclk(dev, req_cdclk);
15361553
}
@@ -1540,39 +1557,45 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
15401557
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
15411558
struct drm_i915_private *dev_priv = to_i915(state->dev);
15421559
const int max_pixclk = intel_max_pixel_rate(state);
1543-
int vco = intel_state->cdclk_pll_vco;
1544-
int cdclk;
1560+
int cdclk, vco;
1561+
1562+
vco = intel_state->cdclk.logical.vco;
1563+
if (!vco)
1564+
vco = dev_priv->skl_preferred_vco_freq;
15451565

15461566
/*
15471567
* FIXME should also account for plane ratio
15481568
* once 64bpp pixel formats are supported.
15491569
*/
15501570
cdclk = skl_calc_cdclk(max_pixclk, vco);
15511571

1552-
/*
1553-
* FIXME move the cdclk caclulation to
1554-
* compute_config() so we can fail gracegully.
1555-
*/
15561572
if (cdclk > dev_priv->max_cdclk_freq) {
1557-
DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
1558-
cdclk, dev_priv->max_cdclk_freq);
1559-
cdclk = dev_priv->max_cdclk_freq;
1573+
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
1574+
cdclk, dev_priv->max_cdclk_freq);
1575+
return -EINVAL;
15601576
}
15611577

1562-
intel_state->cdclk = intel_state->dev_cdclk = cdclk;
1563-
if (!intel_state->active_crtcs)
1564-
intel_state->dev_cdclk = skl_calc_cdclk(0, vco);
1578+
intel_state->cdclk.logical.vco = vco;
1579+
intel_state->cdclk.logical.cdclk = cdclk;
1580+
1581+
if (!intel_state->active_crtcs) {
1582+
cdclk = skl_calc_cdclk(0, vco);
1583+
1584+
intel_state->cdclk.actual.vco = vco;
1585+
intel_state->cdclk.actual.cdclk = cdclk;
1586+
} else {
1587+
intel_state->cdclk.actual =
1588+
intel_state->cdclk.logical;
1589+
}
15651590

15661591
return 0;
15671592
}
15681593

15691594
static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
15701595
{
15711596
struct drm_i915_private *dev_priv = to_i915(old_state->dev);
1572-
struct intel_atomic_state *intel_state =
1573-
to_intel_atomic_state(old_state);
1574-
unsigned int req_cdclk = intel_state->dev_cdclk;
1575-
unsigned int req_vco = intel_state->cdclk_pll_vco;
1597+
unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
1598+
unsigned int req_vco = dev_priv->cdclk.actual.vco;
15761599

15771600
skl_set_cdclk(dev_priv, req_cdclk, req_vco);
15781601
}
@@ -1583,22 +1606,39 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
15831606
int max_pixclk = intel_max_pixel_rate(state);
15841607
struct intel_atomic_state *intel_state =
15851608
to_intel_atomic_state(state);
1586-
int cdclk;
1609+
int cdclk, vco;
15871610

1588-
if (IS_GEMINILAKE(dev_priv))
1611+
if (IS_GEMINILAKE(dev_priv)) {
15891612
cdclk = glk_calc_cdclk(max_pixclk);
1590-
else
1613+
vco = glk_de_pll_vco(dev_priv, cdclk);
1614+
} else {
15911615
cdclk = bxt_calc_cdclk(max_pixclk);
1616+
vco = bxt_de_pll_vco(dev_priv, cdclk);
1617+
}
1618+
1619+
if (cdclk > dev_priv->max_cdclk_freq) {
1620+
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
1621+
cdclk, dev_priv->max_cdclk_freq);
1622+
return -EINVAL;
1623+
}
15921624

1593-
intel_state->cdclk = intel_state->dev_cdclk = cdclk;
1625+
intel_state->cdclk.logical.vco = vco;
1626+
intel_state->cdclk.logical.cdclk = cdclk;
15941627

15951628
if (!intel_state->active_crtcs) {
1596-
if (IS_GEMINILAKE(dev_priv))
1629+
if (IS_GEMINILAKE(dev_priv)) {
15971630
cdclk = glk_calc_cdclk(0);
1598-
else
1631+
vco = glk_de_pll_vco(dev_priv, cdclk);
1632+
} else {
15991633
cdclk = bxt_calc_cdclk(0);
1634+
vco = bxt_de_pll_vco(dev_priv, cdclk);
1635+
}
16001636

1601-
intel_state->dev_cdclk = cdclk;
1637+
intel_state->cdclk.actual.vco = vco;
1638+
intel_state->cdclk.actual.cdclk = cdclk;
1639+
} else {
1640+
intel_state->cdclk.actual =
1641+
intel_state->cdclk.logical;
16021642
}
16031643

16041644
return 0;
@@ -1607,15 +1647,8 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
16071647
static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
16081648
{
16091649
struct drm_i915_private *dev_priv = to_i915(old_state->dev);
1610-
struct intel_atomic_state *old_intel_state =
1611-
to_intel_atomic_state(old_state);
1612-
unsigned int req_cdclk = old_intel_state->dev_cdclk;
1613-
unsigned int req_vco;
1614-
1615-
if (IS_GEMINILAKE(dev_priv))
1616-
req_vco = glk_de_pll_vco(dev_priv, req_cdclk);
1617-
else
1618-
req_vco = bxt_de_pll_vco(dev_priv, req_cdclk);
1650+
unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
1651+
unsigned int req_vco = dev_priv->cdclk.actual.vco;
16191652

16201653
bxt_set_cdclk(dev_priv, req_cdclk, req_vco);
16211654
}

‎drivers/gpu/drm/i915/intel_display.c

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12393,6 +12393,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
1239312393

1239412394
intel_state->modeset = true;
1239512395
intel_state->active_crtcs = dev_priv->active_crtcs;
12396+
intel_state->cdclk.logical = dev_priv->cdclk.logical;
12397+
intel_state->cdclk.actual = dev_priv->cdclk.actual;
1239612398

1239712399
for_each_crtc_in_state(state, crtc, crtc_state, i) {
1239812400
if (crtc_state->active)
@@ -12412,38 +12414,35 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
1241212414
* adjusted_mode bits in the crtc directly.
1241312415
*/
1241412416
if (dev_priv->display.modeset_calc_cdclk) {
12415-
if (!intel_state->cdclk_pll_vco)
12416-
intel_state->cdclk_pll_vco = dev_priv->cdclk.hw.vco;
12417-
if (!intel_state->cdclk_pll_vco)
12418-
intel_state->cdclk_pll_vco = dev_priv->skl_preferred_vco_freq;
12419-
1242012417
ret = dev_priv->display.modeset_calc_cdclk(state);
1242112418
if (ret < 0)
1242212419
return ret;
1242312420

1242412421
/*
12425-
* Writes to dev_priv->atomic_cdclk_freq must protected by
12422+
* Writes to dev_priv->cdclk.logical must protected by
1242612423
* holding all the crtc locks, even if we don't end up
1242712424
* touching the hardware
1242812425
*/
12429-
if (intel_state->cdclk != dev_priv->atomic_cdclk_freq) {
12426+
if (!intel_cdclk_state_compare(&dev_priv->cdclk.logical,
12427+
&intel_state->cdclk.logical)) {
1243012428
ret = intel_lock_all_pipes(state);
1243112429
if (ret < 0)
1243212430
return ret;
1243312431
}
1243412432

1243512433
/* All pipes must be switched off while we change the cdclk. */
12436-
if (intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
12437-
intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco) {
12434+
if (!intel_cdclk_state_compare(&dev_priv->cdclk.actual,
12435+
&intel_state->cdclk.actual)) {
1243812436
ret = intel_modeset_all_pipes(state);
1243912437
if (ret < 0)
1244012438
return ret;
1244112439
}
1244212440

12443-
DRM_DEBUG_KMS("New cdclk calculated to be atomic %u, actual %u\n",
12444-
intel_state->cdclk, intel_state->dev_cdclk);
12441+
DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
12442+
intel_state->cdclk.logical.cdclk,
12443+
intel_state->cdclk.actual.cdclk);
1244512444
} else {
12446-
to_intel_atomic_state(state)->cdclk = dev_priv->atomic_cdclk_freq;
12445+
to_intel_atomic_state(state)->cdclk.logical = dev_priv->cdclk.logical;
1244712446
}
1244812447

1244912448
intel_modeset_clear_plls(state);
@@ -12546,7 +12545,7 @@ static int intel_atomic_check(struct drm_device *dev,
1254612545
if (ret)
1254712546
return ret;
1254812547
} else {
12549-
intel_state->cdclk = dev_priv->atomic_cdclk_freq;
12548+
intel_state->cdclk.logical = dev_priv->cdclk.logical;
1255012549
}
1255112550

1255212551
ret = drm_atomic_helper_check_planes(dev, state);
@@ -12869,8 +12868,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
1286912868
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
1287012869

1287112870
if (dev_priv->display.modeset_commit_cdclk &&
12872-
(intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
12873-
intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco))
12871+
!intel_cdclk_state_compare(&dev_priv->cdclk.hw,
12872+
&dev_priv->cdclk.actual))
1287412873
dev_priv->display.modeset_commit_cdclk(state);
1287512874

1287612875
/*
@@ -13059,7 +13058,8 @@ static int intel_atomic_commit(struct drm_device *dev,
1305913058
memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
1306013059
sizeof(intel_state->min_pixclk));
1306113060
dev_priv->active_crtcs = intel_state->active_crtcs;
13062-
dev_priv->atomic_cdclk_freq = intel_state->cdclk;
13061+
dev_priv->cdclk.logical = intel_state->cdclk.logical;
13062+
dev_priv->cdclk.actual = intel_state->cdclk.actual;
1306313063
}
1306413064

1306513065
drm_atomic_state_get(state);
@@ -13297,7 +13297,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
1329713297
return DRM_PLANE_HELPER_NO_SCALING;
1329813298

1329913299
crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
13300-
cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk;
13300+
cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
1330113301

1330213302
if (WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock))
1330313303
return DRM_PLANE_HELPER_NO_SCALING;
@@ -14854,8 +14854,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
1485414854
struct drm_i915_private *dev_priv = to_i915(dev);
1485514855

1485614856
intel_update_cdclk(dev_priv);
14857-
14858-
dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
14857+
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
1485914858

1486014859
intel_init_clock_gating(dev_priv);
1486114860
}
@@ -15031,7 +15030,7 @@ int intel_modeset_init(struct drm_device *dev)
1503115030

1503215031
intel_update_czclk(dev_priv);
1503315032
intel_update_cdclk(dev_priv);
15034-
dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
15033+
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
1503515034

1503615035
intel_shared_dpll_init(dev);
1503715036

‎drivers/gpu/drm/i915/intel_dp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1785,7 +1785,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
17851785
break;
17861786
}
17871787

1788-
to_intel_atomic_state(pipe_config->base.state)->cdclk_pll_vco = vco;
1788+
to_intel_atomic_state(pipe_config->base.state)->cdclk.logical.vco = vco;
17891789
}
17901790

17911791
if (!HAS_DDI(dev_priv))

‎drivers/gpu/drm/i915/intel_drv.h

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -333,13 +333,20 @@ struct dpll {
333333
struct intel_atomic_state {
334334
struct drm_atomic_state base;
335335

336-
unsigned int cdclk;
337-
338-
/*
339-
* Calculated device cdclk, can be different from cdclk
340-
* only when all crtc's are DPMS off.
341-
*/
342-
unsigned int dev_cdclk;
336+
struct {
337+
/*
338+
* Logical state of cdclk (used for all scaling, watermark,
339+
* etc. calculations and checks). This is computed as if all
340+
* enabled crtcs were active.
341+
*/
342+
struct intel_cdclk_state logical;
343+
344+
/*
345+
* Actual state of cdclk, can be different from the logical
346+
* state only when all crtc's are DPMS off.
347+
*/
348+
struct intel_cdclk_state actual;
349+
} cdclk;
343350

344351
bool dpll_set, modeset;
345352

@@ -356,9 +363,6 @@ struct intel_atomic_state {
356363
unsigned int active_crtcs;
357364
unsigned int min_pixclk[I915_MAX_PIPES];
358365

359-
/* SKL/KBL Only */
360-
unsigned int cdclk_pll_vco;
361-
362366
struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
363367

364368
/*

0 commit comments

Comments
 (0)