* [PATCH 0/4] cdclk fixes
@ 2015-11-24 10:29 Maarten Lankhorst
2015-11-24 10:29 ` [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled Maarten Lankhorst
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2015-11-24 10:29 UTC (permalink / raw)
To: intel-gfx
The first patch fixes a WARN_ON that happens when running kms_flip.
Second patch fixes a FIXME. Now that the code's atomic the cdclk is
calculated in compute_config so -EINVAL can be returned.
The third patch introduces active_crtcs which is useful to have in general,
and also calculates the state without locking other crtc's. This will allow
modesets on newer platforms without locking all planes and crtc's.
The final patch is a small removal of a FIXME for broxton. Hopefully useful
to have.
Maarten Lankhorst (4):
drm/i915/skl: Do not allow scaling when crtc is disabled.
drm/i915: Handle cdclk limits on broadwell.
drm/i915: Do not acquire crtc state to check clock during modeset, v3.
drm/i915/bxt: Use the bypass frequency if there are no active pipes.
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/intel_atomic.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 156 ++++++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 7 +-
4 files changed, 119 insertions(+), 51 deletions(-)
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled. 2015-11-24 10:29 [PATCH 0/4] cdclk fixes Maarten Lankhorst @ 2015-11-24 10:29 ` Maarten Lankhorst 2015-12-14 12:07 ` Mika Kahola 2015-11-24 10:29 ` [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Maarten Lankhorst @ 2015-11-24 10:29 UTC (permalink / raw) To: intel-gfx This fixes a warning when the crtc is turned off. In that case fb will be NULL, and crtc_clock will be 0. Because the crtc is no longer active this is not a bug, and shouldn't trigger the WARN_ON. Also remove handling a null crtc_state, with all transitional helpers gone this can no longer happen. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 764eeb05492d..351ecb69e5eb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13693,7 +13693,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state struct drm_i915_private *dev_priv; int crtc_clock, cdclk; - if (!intel_crtc || !crtc_state) + if (!intel_crtc || !crtc_state->base.enable) return DRM_PLANE_HELPER_NO_SCALING; dev = intel_crtc->base.dev; -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled. 2015-11-24 10:29 ` [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled Maarten Lankhorst @ 2015-12-14 12:07 ` Mika Kahola 0 siblings, 0 replies; 11+ messages in thread From: Mika Kahola @ 2015-12-14 12:07 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, 2015-11-24 at 11:29 +0100, Maarten Lankhorst wrote: > This fixes a warning when the crtc is turned off. In that case fb > will be NULL, and crtc_clock will be 0. Because the crtc is no longer > active this is not a bug, and shouldn't trigger the WARN_ON. > > Also remove handling a null crtc_state, with all transitional helpers > gone this can no longer happen. > Reviewed-by: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 764eeb05492d..351ecb69e5eb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13693,7 +13693,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state > struct drm_i915_private *dev_priv; > int crtc_clock, cdclk; > > - if (!intel_crtc || !crtc_state) > + if (!intel_crtc || !crtc_state->base.enable) > return DRM_PLANE_HELPER_NO_SCALING; > > dev = intel_crtc->base.dev; _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell. 2015-11-24 10:29 [PATCH 0/4] cdclk fixes Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled Maarten Lankhorst @ 2015-11-24 10:29 ` Maarten Lankhorst 2015-11-24 10:36 ` Chris Wilson 2015-11-24 10:29 ` [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3 Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 4/4] drm/i915/bxt: Use the bypass frequency if there are no active pipes Maarten Lankhorst 3 siblings, 1 reply; 11+ messages in thread From: Maarten Lankhorst @ 2015-11-24 10:29 UTC (permalink / raw) To: intel-gfx As the comment indicates this can only fail gracefully when called from compute_config. Fortunately this is now what's happening, so the fixme can be removed and the DRM_ERROR downgraded. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 351ecb69e5eb..3c46037b6e55 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9677,14 +9677,10 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state) else cdclk = 337500; - /* - * FIXME move the cdclk caclulation to - * compute_config() so we can fail gracegully. - */ if (cdclk > dev_priv->max_cdclk_freq) { - DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - cdclk = dev_priv->max_cdclk_freq; + DRM_INFO("requested cdclk (%d kHz) exceeds max (%d kHz)\n", + cdclk, dev_priv->max_cdclk_freq); + return -EINVAL; } to_intel_atomic_state(state)->cdclk = cdclk; -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell. 2015-11-24 10:29 ` [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst @ 2015-11-24 10:36 ` Chris Wilson 0 siblings, 0 replies; 11+ messages in thread From: Chris Wilson @ 2015-11-24 10:36 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, Nov 24, 2015 at 11:29:03AM +0100, Maarten Lankhorst wrote: > As the comment indicates this can only fail gracefully when > called from compute_config. Fortunately this is now what's happening, > so the fixme can be removed and the DRM_ERROR downgraded. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 351ecb69e5eb..3c46037b6e55 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9677,14 +9677,10 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state) > else > cdclk = 337500; > > - /* > - * FIXME move the cdclk caclulation to > - * compute_config() so we can fail gracegully. > - */ > if (cdclk > dev_priv->max_cdclk_freq) { > - DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > - cdclk, dev_priv->max_cdclk_freq); > - cdclk = dev_priv->max_cdclk_freq; > + DRM_INFO("requested cdclk (%d kHz) exceeds max (%d kHz)\n", > + cdclk, dev_priv->max_cdclk_freq); This is a validation step, right? So the invalid value is never applied to hardware and so doesn't really exist for anybody but the calling userspace. Ergo, this shouldn't be logged for the user but only when debugging the application -> DRM_DEBUG_KMS Dreaming of an application-centric debug log, -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3. 2015-11-24 10:29 [PATCH 0/4] cdclk fixes Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst @ 2015-11-24 10:29 ` Maarten Lankhorst 2015-12-15 10:22 ` Mika Kahola 2015-11-24 10:29 ` [PATCH 4/4] drm/i915/bxt: Use the bypass frequency if there are no active pipes Maarten Lankhorst 3 siblings, 1 reply; 11+ messages in thread From: Maarten Lankhorst @ 2015-11-24 10:29 UTC (permalink / raw) To: intel-gfx Parallel modesets are still not allowed, but this will allow updating a different crtc during a modeset if the clock is not changed. Additionally when all pipes are DPMS off the cdclk will be lowered to the minimum allowed. Changes since v1: - Add dev_priv->active_crtcs for tracking which crtcs are active. - Rename min_cdclk to min_pixclk and move to dev_priv. - Add a active_crtcs mask which is updated atomically. - Add intel_atomic_state->modeset which is set on modesets. - Commit new pixclk/active_crtcs right after state swap. Changes since v2: - Make the changes related to max_pixel_rate calculations more readable. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/intel_atomic.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 139 +++++++++++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 7 +- 4 files changed, 112 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a32571f0d018..c7b76c2e8e09 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1821,8 +1821,13 @@ struct drm_i915_private { struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; #endif + /* dpll and cdclk state is protected by connection_mutex */ int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; + + unsigned int active_crtcs; + unsigned int min_pixclk[I915_MAX_PIPES]; + int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; struct i915_workarounds workarounds; diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 643f342de33b..c4eadbc928b7 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -306,5 +306,5 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) { struct intel_atomic_state *state = to_intel_atomic_state(s); drm_atomic_state_default_clear(&state->base); - state->dpll_set = false; + state->dpll_set = state->modeset = false; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c46037b6e55..178a042f917e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5991,22 +5991,31 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, static int intel_mode_max_pixclk(struct drm_device *dev, struct drm_atomic_state *state) { - struct intel_crtc *intel_crtc; - struct intel_crtc_state *crtc_state; - int max_pixclk = 0; + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned max_pixclk = 0, i; + enum pipe pipe; - for_each_intel_crtc(dev, intel_crtc) { - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); - if (IS_ERR(crtc_state)) - return PTR_ERR(crtc_state); + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, + sizeof(intel_state->min_pixclk)); - if (!crtc_state->base.enable) - continue; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + int pixclk = 0; + + if (crtc_state->enable) + pixclk = crtc_state->adjusted_mode.crtc_clock; - max_pixclk = max(max_pixclk, - crtc_state->base.adjusted_mode.crtc_clock); + intel_state->min_pixclk[i] = pixclk; } + if (!intel_state->active_crtcs) + return 0; + + for_each_pipe(dev_priv, pipe) + max_pixclk = max(intel_state->min_pixclk[pipe], max_pixclk); + return max_pixclk; } @@ -6310,6 +6319,9 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) for_each_power_domain(domain, domains) intel_display_power_put(dev_priv, domain); intel_crtc->enabled_power_domains = 0; + + dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe); + dev_priv->min_pixclk[intel_crtc->pipe] = 0; } /* @@ -9555,29 +9567,41 @@ static void broxton_modeset_commit_cdclk(struct drm_atomic_state *old_state) /* compute the max rate for new configuration */ static int ilk_max_pixel_rate(struct drm_atomic_state *state) { - struct intel_crtc *intel_crtc; + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); + struct drm_i915_private *dev_priv = state->dev->dev_private; + struct drm_crtc *crtc; + struct drm_crtc_state *cstate; struct intel_crtc_state *crtc_state; - int max_pixel_rate = 0; + unsigned max_pixel_rate = 0, i; + enum pipe pipe; - for_each_intel_crtc(state->dev, intel_crtc) { - int pixel_rate; + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, + sizeof(intel_state->min_pixclk)); - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); - if (IS_ERR(crtc_state)) - return PTR_ERR(crtc_state); + for_each_crtc_in_state(state, crtc, cstate, i) { + int pixel_rate; - if (!crtc_state->base.enable) + crtc_state = to_intel_crtc_state(cstate); + if (!crtc_state->base.enable) { + intel_state->min_pixclk[i] = 0; continue; + } pixel_rate = ilk_pipe_pixel_rate(crtc_state); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ - if (IS_BROADWELL(state->dev) && crtc_state->ips_enabled) + if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95); - max_pixel_rate = max(max_pixel_rate, pixel_rate); + intel_state->min_pixclk[i] = pixel_rate; } + if (!intel_state->active_crtcs) + return 0; + + for_each_pipe(dev_priv, pipe) + max_pixel_rate = max(intel_state->min_pixclk[pipe], max_pixel_rate); + return max_pixel_rate; } @@ -13069,15 +13093,27 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state) static int intel_modeset_checks(struct drm_atomic_state *state) { - struct drm_device *dev = state->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - int ret; + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); + struct drm_i915_private *dev_priv = state->dev->dev_private; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret = 0, i; if (!check_digital_port_conflicts(state)) { DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); return -EINVAL; } + intel_state->modeset = true; + intel_state->active_crtcs = dev_priv->active_crtcs; + + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (crtc_state->active) + intel_state->active_crtcs |= 1 << i; + else + intel_state->active_crtcs &= ~(1 << i); + } + /* * See if the config requires any additional preparation, e.g. * to adjust global state with pipes off. We need to do this @@ -13101,7 +13137,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state) intel_modeset_clear_plls(state); - if (IS_HASWELL(dev)) + if (IS_HASWELL(dev_priv)) return haswell_mode_set_planes_workaround(state); return 0; @@ -13318,12 +13354,12 @@ static int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool async) { + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; - int ret = 0; - int i; - bool any_ms = false; + int ret = 0, i; + bool hw_check = intel_state->modeset; ret = intel_atomic_prepare_commit(dev, state, async); if (ret) { @@ -13334,13 +13370,18 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_helper_swap_state(dev, state); dev_priv->wm.config = to_intel_atomic_state(state)->wm_config; + if (intel_state->modeset) { + memcpy(dev_priv->min_pixclk, intel_state->min_pixclk, + sizeof(intel_state->min_pixclk)); + dev_priv->active_crtcs = intel_state->active_crtcs; + } + for_each_crtc_in_state(state, crtc, crtc_state, i) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); if (!needs_modeset(crtc->state)) continue; - any_ms = true; intel_pre_plane_update(intel_crtc); if (crtc_state->active) { @@ -13355,7 +13396,7 @@ static int intel_atomic_commit(struct drm_device *dev, * update the the output configuration. */ intel_modeset_update_crtc_state(state); - if (any_ms) { + if (intel_state->modeset) { intel_shared_dpll_commit(state); drm_atomic_helper_update_legacy_modeset_state(state->dev, state); @@ -13382,7 +13423,7 @@ static int intel_atomic_commit(struct drm_device *dev, put_domains = modeset_get_crtc_power_domains(crtc); /* make sure intel_modeset_check_state runs */ - any_ms = true; + hw_check = true; } if (!modeset) @@ -13409,7 +13450,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_helper_cleanup_planes(dev, state); mutex_unlock(&dev->struct_mutex); - if (any_ms) + if (hw_check) intel_modeset_check_state(dev, state); drm_atomic_state_free(state); @@ -15417,16 +15458,36 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) struct intel_connector *connector; int i; + dev_priv->active_crtcs = 0; + for_each_intel_crtc(dev, crtc) { - __drm_atomic_helper_crtc_destroy_state(&crtc->base, crtc->base.state); - memset(crtc->config, 0, sizeof(*crtc->config)); - crtc->config->base.crtc = &crtc->base; + struct intel_crtc_state *crtc_state = crtc->config; + int pixclk = 0; - crtc->active = dev_priv->display.get_pipe_config(crtc, - crtc->config); + __drm_atomic_helper_crtc_destroy_state(&crtc->base, &crtc_state->base); + memset(crtc_state, 0, sizeof(*crtc_state)); + crtc_state->base.crtc = &crtc->base; - crtc->base.state->active = crtc->active; - crtc->base.enabled = crtc->active; + crtc_state->base.active = crtc_state->base.enable = + dev_priv->display.get_pipe_config(crtc, crtc_state); + + crtc->base.enabled = crtc_state->base.enable; + crtc->active = crtc_state->base.active; + + if (crtc_state->base.active) { + dev_priv->active_crtcs |= 1 << crtc->pipe; + + if (IS_BROADWELL(dev_priv)) { + pixclk = ilk_pipe_pixel_rate(crtc_state); + + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ + if (crtc_state->ips_enabled) + pixclk = DIV_ROUND_UP(pixclk * 100, 95); + } else if (IS_BROXTON(dev_priv) || IS_VALLEYVIEW(dev_priv)) + pixclk = crtc_state->base.adjusted_mode.crtc_clock; + } + + dev_priv->min_pixclk[crtc->pipe] = pixclk; readout_plane_state(crtc); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4dfca58d37bf..8cfc599fd413 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -246,7 +246,12 @@ struct intel_atomic_state { struct drm_atomic_state base; unsigned int cdclk; - bool dpll_set; + + bool dpll_set, modeset; + + unsigned int active_crtcs; + unsigned int min_pixclk[I915_MAX_PIPES]; + struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS]; struct intel_wm_config wm_config; }; -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3. 2015-11-24 10:29 ` [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3 Maarten Lankhorst @ 2015-12-15 10:22 ` Mika Kahola 2015-12-15 10:26 ` Chris Wilson 2015-12-16 10:10 ` Maarten Lankhorst 0 siblings, 2 replies; 11+ messages in thread From: Mika Kahola @ 2015-12-15 10:22 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, 2015-11-24 at 11:29 +0100, Maarten Lankhorst wrote: > Parallel modesets are still not allowed, but this will allow updating > a different crtc during a modeset if the clock is not changed. > > Additionally when all pipes are DPMS off the cdclk will be lowered > to the minimum allowed. > > Changes since v1: > - Add dev_priv->active_crtcs for tracking which crtcs are active. > - Rename min_cdclk to min_pixclk and move to dev_priv. > - Add a active_crtcs mask which is updated atomically. > - Add intel_atomic_state->modeset which is set on modesets. > - Commit new pixclk/active_crtcs right after state swap. > Changes since v2: > - Make the changes related to max_pixel_rate calculations more readable. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 5 ++ > drivers/gpu/drm/i915/intel_atomic.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 139 +++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_drv.h | 7 +- > 4 files changed, 112 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a32571f0d018..c7b76c2e8e09 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1821,8 +1821,13 @@ struct drm_i915_private { > struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; > #endif > > + /* dpll and cdclk state is protected by connection_mutex */ > int num_shared_dpll; > struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; > + > + unsigned int active_crtcs; > + unsigned int min_pixclk[I915_MAX_PIPES]; > + > int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; > > struct i915_workarounds workarounds; > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 643f342de33b..c4eadbc928b7 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -306,5 +306,5 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) > { > struct intel_atomic_state *state = to_intel_atomic_state(s); > drm_atomic_state_default_clear(&state->base); > - state->dpll_set = false; > + state->dpll_set = state->modeset = false; > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3c46037b6e55..178a042f917e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5991,22 +5991,31 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, > static int intel_mode_max_pixclk(struct drm_device *dev, > struct drm_atomic_state *state) > { > - struct intel_crtc *intel_crtc; > - struct intel_crtc_state *crtc_state; > - int max_pixclk = 0; > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > + struct drm_i915_private *dev_priv = dev->dev_private; This is a nitpick but we should use nowadays to_i915() > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + unsigned max_pixclk = 0, i; > + enum pipe pipe; > > - for_each_intel_crtc(dev, intel_crtc) { > - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > - if (IS_ERR(crtc_state)) > - return PTR_ERR(crtc_state); > + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, > + sizeof(intel_state->min_pixclk)); > > - if (!crtc_state->base.enable) > - continue; > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + int pixclk = 0; > + > + if (crtc_state->enable) > + pixclk = crtc_state->adjusted_mode.crtc_clock; > > - max_pixclk = max(max_pixclk, > - crtc_state->base.adjusted_mode.crtc_clock); > + intel_state->min_pixclk[i] = pixclk; > } > > + if (!intel_state->active_crtcs) > + return 0; > + > + for_each_pipe(dev_priv, pipe) > + max_pixclk = max(intel_state->min_pixclk[pipe], max_pixclk); > + > return max_pixclk; > } > > @@ -6310,6 +6319,9 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > for_each_power_domain(domain, domains) > intel_display_power_put(dev_priv, domain); > intel_crtc->enabled_power_domains = 0; > + > + dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe); > + dev_priv->min_pixclk[intel_crtc->pipe] = 0; > } > > /* > @@ -9555,29 +9567,41 @@ static void broxton_modeset_commit_cdclk(struct drm_atomic_state *old_state) > /* compute the max rate for new configuration */ > static int ilk_max_pixel_rate(struct drm_atomic_state *state) > { > - struct intel_crtc *intel_crtc; > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > + struct drm_i915_private *dev_priv = state->dev->dev_private; > + struct drm_crtc *crtc; > + struct drm_crtc_state *cstate; > struct intel_crtc_state *crtc_state; > - int max_pixel_rate = 0; > + unsigned max_pixel_rate = 0, i; > + enum pipe pipe; > > - for_each_intel_crtc(state->dev, intel_crtc) { > - int pixel_rate; > + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, > + sizeof(intel_state->min_pixclk)); > > - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > - if (IS_ERR(crtc_state)) > - return PTR_ERR(crtc_state); > + for_each_crtc_in_state(state, crtc, cstate, i) { > + int pixel_rate; > > - if (!crtc_state->base.enable) > + crtc_state = to_intel_crtc_state(cstate); > + if (!crtc_state->base.enable) { > + intel_state->min_pixclk[i] = 0; > continue; > + } > > pixel_rate = ilk_pipe_pixel_rate(crtc_state); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > - if (IS_BROADWELL(state->dev) && crtc_state->ips_enabled) > + if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95); > > - max_pixel_rate = max(max_pixel_rate, pixel_rate); > + intel_state->min_pixclk[i] = pixel_rate; > } > > + if (!intel_state->active_crtcs) > + return 0; > + > + for_each_pipe(dev_priv, pipe) > + max_pixel_rate = max(intel_state->min_pixclk[pipe], max_pixel_rate); > + > return max_pixel_rate; > } > > @@ -13069,15 +13093,27 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state) > > static int intel_modeset_checks(struct drm_atomic_state *state) > { > - struct drm_device *dev = state->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > + struct drm_i915_private *dev_priv = state->dev->dev_private; > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + int ret = 0, i; > > if (!check_digital_port_conflicts(state)) { > DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); > return -EINVAL; > } > > + intel_state->modeset = true; > + intel_state->active_crtcs = dev_priv->active_crtcs; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + if (crtc_state->active) > + intel_state->active_crtcs |= 1 << i; > + else > + intel_state->active_crtcs &= ~(1 << i); > + } > + > /* > * See if the config requires any additional preparation, e.g. > * to adjust global state with pipes off. We need to do this > @@ -13101,7 +13137,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state) > > intel_modeset_clear_plls(state); > > - if (IS_HASWELL(dev)) > + if (IS_HASWELL(dev_priv)) > return haswell_mode_set_planes_workaround(state); > > return 0; > @@ -13318,12 +13354,12 @@ static int intel_atomic_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool async) > { > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc_state *crtc_state; > struct drm_crtc *crtc; > - int ret = 0; > - int i; > - bool any_ms = false; > + int ret = 0, i; > + bool hw_check = intel_state->modeset; > > ret = intel_atomic_prepare_commit(dev, state, async); > if (ret) { > @@ -13334,13 +13370,18 @@ static int intel_atomic_commit(struct drm_device *dev, > drm_atomic_helper_swap_state(dev, state); > dev_priv->wm.config = to_intel_atomic_state(state)->wm_config; > > + if (intel_state->modeset) { > + memcpy(dev_priv->min_pixclk, intel_state->min_pixclk, > + sizeof(intel_state->min_pixclk)); > + dev_priv->active_crtcs = intel_state->active_crtcs; > + } > + > for_each_crtc_in_state(state, crtc, crtc_state, i) { > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > if (!needs_modeset(crtc->state)) > continue; > > - any_ms = true; > intel_pre_plane_update(intel_crtc); > > if (crtc_state->active) { > @@ -13355,7 +13396,7 @@ static int intel_atomic_commit(struct drm_device *dev, > * update the the output configuration. */ > intel_modeset_update_crtc_state(state); > > - if (any_ms) { > + if (intel_state->modeset) { > intel_shared_dpll_commit(state); > > drm_atomic_helper_update_legacy_modeset_state(state->dev, state); > @@ -13382,7 +13423,7 @@ static int intel_atomic_commit(struct drm_device *dev, > put_domains = modeset_get_crtc_power_domains(crtc); > > /* make sure intel_modeset_check_state runs */ > - any_ms = true; > + hw_check = true; > } > > if (!modeset) > @@ -13409,7 +13450,7 @@ static int intel_atomic_commit(struct drm_device *dev, > drm_atomic_helper_cleanup_planes(dev, state); > mutex_unlock(&dev->struct_mutex); > > - if (any_ms) > + if (hw_check) > intel_modeset_check_state(dev, state); > > drm_atomic_state_free(state); > @@ -15417,16 +15458,36 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > struct intel_connector *connector; > int i; > > + dev_priv->active_crtcs = 0; > + > for_each_intel_crtc(dev, crtc) { > - __drm_atomic_helper_crtc_destroy_state(&crtc->base, crtc->base.state); > - memset(crtc->config, 0, sizeof(*crtc->config)); > - crtc->config->base.crtc = &crtc->base; > + struct intel_crtc_state *crtc_state = crtc->config; > + int pixclk = 0; > > - crtc->active = dev_priv->display.get_pipe_config(crtc, > - crtc->config); > + __drm_atomic_helper_crtc_destroy_state(&crtc->base, &crtc_state->base); > + memset(crtc_state, 0, sizeof(*crtc_state)); > + crtc_state->base.crtc = &crtc->base; > > - crtc->base.state->active = crtc->active; > - crtc->base.enabled = crtc->active; > + crtc_state->base.active = crtc_state->base.enable = > + dev_priv->display.get_pipe_config(crtc, crtc_state); > + > + crtc->base.enabled = crtc_state->base.enable; > + crtc->active = crtc_state->base.active; > + > + if (crtc_state->base.active) { > + dev_priv->active_crtcs |= 1 << crtc->pipe; > + > + if (IS_BROADWELL(dev_priv)) { > + pixclk = ilk_pipe_pixel_rate(crtc_state); > + > + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > + if (crtc_state->ips_enabled) > + pixclk = DIV_ROUND_UP(pixclk * 100, 95); > + } else if (IS_BROXTON(dev_priv) || IS_VALLEYVIEW(dev_priv)) > + pixclk = crtc_state->base.adjusted_mode.crtc_clock; What happens with the other platforms? > + } > + > + dev_priv->min_pixclk[crtc->pipe] = pixclk; > > readout_plane_state(crtc); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 4dfca58d37bf..8cfc599fd413 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -246,7 +246,12 @@ struct intel_atomic_state { > struct drm_atomic_state base; > > unsigned int cdclk; > - bool dpll_set; > + > + bool dpll_set, modeset; > + > + unsigned int active_crtcs; > + unsigned int min_pixclk[I915_MAX_PIPES]; > + > struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS]; > struct intel_wm_config wm_config; > }; -- Mika Kahola - Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3. 2015-12-15 10:22 ` Mika Kahola @ 2015-12-15 10:26 ` Chris Wilson 2015-12-16 12:48 ` Mika Kahola 2015-12-16 10:10 ` Maarten Lankhorst 1 sibling, 1 reply; 11+ messages in thread From: Chris Wilson @ 2015-12-15 10:26 UTC (permalink / raw) To: Mika Kahola; +Cc: intel-gfx On Tue, Dec 15, 2015 at 12:22:40PM +0200, Mika Kahola wrote: > On Tue, 2015-11-24 at 11:29 +0100, Maarten Lankhorst wrote: > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3c46037b6e55..178a042f917e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5991,22 +5991,31 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, > > static int intel_mode_max_pixclk(struct drm_device *dev, > > struct drm_atomic_state *state) > > { > > - struct intel_crtc *intel_crtc; > > - struct intel_crtc_state *crtc_state; > > - int max_pixclk = 0; > > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > > + struct drm_i915_private *dev_priv = dev->dev_private; > This is a nitpick but we should use nowadays to_i915() If you're going to bring that up, we should be passing in the right pointer to begin with! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3. 2015-12-15 10:26 ` Chris Wilson @ 2015-12-16 12:48 ` Mika Kahola 0 siblings, 0 replies; 11+ messages in thread From: Mika Kahola @ 2015-12-16 12:48 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, 2015-12-15 at 10:26 +0000, Chris Wilson wrote: > On Tue, Dec 15, 2015 at 12:22:40PM +0200, Mika Kahola wrote: > > On Tue, 2015-11-24 at 11:29 +0100, Maarten Lankhorst wrote: > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 3c46037b6e55..178a042f917e 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -5991,22 +5991,31 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, > > > static int intel_mode_max_pixclk(struct drm_device *dev, > > > struct drm_atomic_state *state) > > > { > > > - struct intel_crtc *intel_crtc; > > > - struct intel_crtc_state *crtc_state; > > > - int max_pixclk = 0; > > > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > This is a nitpick but we should use nowadays to_i915() > > If you're going to bring that up, we should be passing in the right > pointer to begin with! > -Chris This was more meant be as a notification. I was advised that we should use 'to_i915()' in a new code. Personally, I don't have any preference one way or another. Both ways will work. Maybe this would be a topic for a follow up patch? Cheers, Mika _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3. 2015-12-15 10:22 ` Mika Kahola 2015-12-15 10:26 ` Chris Wilson @ 2015-12-16 10:10 ` Maarten Lankhorst 1 sibling, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2015-12-16 10:10 UTC (permalink / raw) To: mika.kahola; +Cc: intel-gfx Op 15-12-15 om 11:22 schreef Mika Kahola: > On Tue, 2015-11-24 at 11:29 +0100, Maarten Lankhorst wrote: >> Parallel modesets are still not allowed, but this will allow updating >> a different crtc during a modeset if the clock is not changed. >> >> Additionally when all pipes are DPMS off the cdclk will be lowered >> to the minimum allowed. >> >> Changes since v1: >> - Add dev_priv->active_crtcs for tracking which crtcs are active. >> - Rename min_cdclk to min_pixclk and move to dev_priv. >> - Add a active_crtcs mask which is updated atomically. >> - Add intel_atomic_state->modeset which is set on modesets. >> - Commit new pixclk/active_crtcs right after state swap. >> Changes since v2: >> - Make the changes related to max_pixel_rate calculations more readable. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 5 ++ >> drivers/gpu/drm/i915/intel_atomic.c | 2 +- >> drivers/gpu/drm/i915/intel_display.c | 139 +++++++++++++++++++++++++---------- >> drivers/gpu/drm/i915/intel_drv.h | 7 +- >> 4 files changed, 112 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index a32571f0d018..c7b76c2e8e09 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1821,8 +1821,13 @@ struct drm_i915_private { >> struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; >> #endif >> >> + /* dpll and cdclk state is protected by connection_mutex */ >> int num_shared_dpll; >> struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; >> + >> + unsigned int active_crtcs; >> + unsigned int min_pixclk[I915_MAX_PIPES]; >> + >> int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; >> >> struct i915_workarounds workarounds; >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >> index 643f342de33b..c4eadbc928b7 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -306,5 +306,5 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) >> { >> struct intel_atomic_state *state = to_intel_atomic_state(s); >> drm_atomic_state_default_clear(&state->base); >> - state->dpll_set = false; >> + state->dpll_set = state->modeset = false; >> } >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 3c46037b6e55..178a042f917e 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -5991,22 +5991,31 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, >> static int intel_mode_max_pixclk(struct drm_device *dev, >> struct drm_atomic_state *state) >> { >> - struct intel_crtc *intel_crtc; >> - struct intel_crtc_state *crtc_state; >> - int max_pixclk = 0; >> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); >> + struct drm_i915_private *dev_priv = dev->dev_private; > This is a nitpick but we should use nowadays to_i915() > >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + unsigned max_pixclk = 0, i; >> + enum pipe pipe; >> >> - for_each_intel_crtc(dev, intel_crtc) { >> - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); >> - if (IS_ERR(crtc_state)) >> - return PTR_ERR(crtc_state); >> + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, >> + sizeof(intel_state->min_pixclk)); >> >> - if (!crtc_state->base.enable) >> - continue; >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { >> + int pixclk = 0; >> + >> + if (crtc_state->enable) >> + pixclk = crtc_state->adjusted_mode.crtc_clock; >> >> - max_pixclk = max(max_pixclk, >> - crtc_state->base.adjusted_mode.crtc_clock); >> + intel_state->min_pixclk[i] = pixclk; >> } >> >> + if (!intel_state->active_crtcs) >> + return 0; >> + >> + for_each_pipe(dev_priv, pipe) >> + max_pixclk = max(intel_state->min_pixclk[pipe], max_pixclk); >> + >> return max_pixclk; >> } >> >> @@ -6310,6 +6319,9 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) >> for_each_power_domain(domain, domains) >> intel_display_power_put(dev_priv, domain); >> intel_crtc->enabled_power_domains = 0; >> + >> + dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe); >> + dev_priv->min_pixclk[intel_crtc->pipe] = 0; >> } >> >> /* >> @@ -9555,29 +9567,41 @@ static void broxton_modeset_commit_cdclk(struct drm_atomic_state *old_state) >> /* compute the max rate for new configuration */ >> static int ilk_max_pixel_rate(struct drm_atomic_state *state) >> { >> - struct intel_crtc *intel_crtc; >> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); >> + struct drm_i915_private *dev_priv = state->dev->dev_private; >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *cstate; >> struct intel_crtc_state *crtc_state; >> - int max_pixel_rate = 0; >> + unsigned max_pixel_rate = 0, i; >> + enum pipe pipe; >> >> - for_each_intel_crtc(state->dev, intel_crtc) { >> - int pixel_rate; >> + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, >> + sizeof(intel_state->min_pixclk)); >> >> - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); >> - if (IS_ERR(crtc_state)) >> - return PTR_ERR(crtc_state); >> + for_each_crtc_in_state(state, crtc, cstate, i) { >> + int pixel_rate; >> >> - if (!crtc_state->base.enable) >> + crtc_state = to_intel_crtc_state(cstate); >> + if (!crtc_state->base.enable) { >> + intel_state->min_pixclk[i] = 0; >> continue; >> + } >> >> pixel_rate = ilk_pipe_pixel_rate(crtc_state); >> >> /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ >> - if (IS_BROADWELL(state->dev) && crtc_state->ips_enabled) >> + if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) >> pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95); >> >> - max_pixel_rate = max(max_pixel_rate, pixel_rate); >> + intel_state->min_pixclk[i] = pixel_rate; >> } >> >> + if (!intel_state->active_crtcs) >> + return 0; >> + >> + for_each_pipe(dev_priv, pipe) >> + max_pixel_rate = max(intel_state->min_pixclk[pipe], max_pixel_rate); >> + >> return max_pixel_rate; >> } >> >> @@ -13069,15 +13093,27 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state) >> >> static int intel_modeset_checks(struct drm_atomic_state *state) >> { >> - struct drm_device *dev = state->dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - int ret; >> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); >> + struct drm_i915_private *dev_priv = state->dev->dev_private; >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + int ret = 0, i; >> >> if (!check_digital_port_conflicts(state)) { >> DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); >> return -EINVAL; >> } >> >> + intel_state->modeset = true; >> + intel_state->active_crtcs = dev_priv->active_crtcs; >> + >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { >> + if (crtc_state->active) >> + intel_state->active_crtcs |= 1 << i; >> + else >> + intel_state->active_crtcs &= ~(1 << i); >> + } >> + >> /* >> * See if the config requires any additional preparation, e.g. >> * to adjust global state with pipes off. We need to do this >> @@ -13101,7 +13137,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state) >> >> intel_modeset_clear_plls(state); >> >> - if (IS_HASWELL(dev)) >> + if (IS_HASWELL(dev_priv)) >> return haswell_mode_set_planes_workaround(state); >> >> return 0; >> @@ -13318,12 +13354,12 @@ static int intel_atomic_commit(struct drm_device *dev, >> struct drm_atomic_state *state, >> bool async) >> { >> + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct drm_crtc_state *crtc_state; >> struct drm_crtc *crtc; >> - int ret = 0; >> - int i; >> - bool any_ms = false; >> + int ret = 0, i; >> + bool hw_check = intel_state->modeset; >> >> ret = intel_atomic_prepare_commit(dev, state, async); >> if (ret) { >> @@ -13334,13 +13370,18 @@ static int intel_atomic_commit(struct drm_device *dev, >> drm_atomic_helper_swap_state(dev, state); >> dev_priv->wm.config = to_intel_atomic_state(state)->wm_config; >> >> + if (intel_state->modeset) { >> + memcpy(dev_priv->min_pixclk, intel_state->min_pixclk, >> + sizeof(intel_state->min_pixclk)); >> + dev_priv->active_crtcs = intel_state->active_crtcs; >> + } >> + >> for_each_crtc_in_state(state, crtc, crtc_state, i) { >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> >> if (!needs_modeset(crtc->state)) >> continue; >> >> - any_ms = true; >> intel_pre_plane_update(intel_crtc); >> >> if (crtc_state->active) { >> @@ -13355,7 +13396,7 @@ static int intel_atomic_commit(struct drm_device *dev, >> * update the the output configuration. */ >> intel_modeset_update_crtc_state(state); >> >> - if (any_ms) { >> + if (intel_state->modeset) { >> intel_shared_dpll_commit(state); >> >> drm_atomic_helper_update_legacy_modeset_state(state->dev, state); >> @@ -13382,7 +13423,7 @@ static int intel_atomic_commit(struct drm_device *dev, >> put_domains = modeset_get_crtc_power_domains(crtc); >> >> /* make sure intel_modeset_check_state runs */ >> - any_ms = true; >> + hw_check = true; >> } >> >> if (!modeset) >> @@ -13409,7 +13450,7 @@ static int intel_atomic_commit(struct drm_device *dev, >> drm_atomic_helper_cleanup_planes(dev, state); >> mutex_unlock(&dev->struct_mutex); >> >> - if (any_ms) >> + if (hw_check) >> intel_modeset_check_state(dev, state); >> >> drm_atomic_state_free(state); >> @@ -15417,16 +15458,36 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) >> struct intel_connector *connector; >> int i; >> >> + dev_priv->active_crtcs = 0; >> + >> for_each_intel_crtc(dev, crtc) { >> - __drm_atomic_helper_crtc_destroy_state(&crtc->base, crtc->base.state); >> - memset(crtc->config, 0, sizeof(*crtc->config)); >> - crtc->config->base.crtc = &crtc->base; >> + struct intel_crtc_state *crtc_state = crtc->config; >> + int pixclk = 0; >> >> - crtc->active = dev_priv->display.get_pipe_config(crtc, >> - crtc->config); >> + __drm_atomic_helper_crtc_destroy_state(&crtc->base, &crtc_state->base); >> + memset(crtc_state, 0, sizeof(*crtc_state)); >> + crtc_state->base.crtc = &crtc->base; >> >> - crtc->base.state->active = crtc->active; >> - crtc->base.enabled = crtc->active; >> + crtc_state->base.active = crtc_state->base.enable = >> + dev_priv->display.get_pipe_config(crtc, crtc_state); >> + >> + crtc->base.enabled = crtc_state->base.enable; >> + crtc->active = crtc_state->base.active; >> + >> + if (crtc_state->base.active) { >> + dev_priv->active_crtcs |= 1 << crtc->pipe; >> + >> + if (IS_BROADWELL(dev_priv)) { >> + pixclk = ilk_pipe_pixel_rate(crtc_state); >> + >> + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ >> + if (crtc_state->ips_enabled) >> + pixclk = DIV_ROUND_UP(pixclk * 100, 95); >> + } else if (IS_BROXTON(dev_priv) || IS_VALLEYVIEW(dev_priv)) >> + pixclk = crtc_state->base.adjusted_mode.crtc_clock; > What happens with the other platforms? > Well the other platforms have no support for cdclk adjustment, but it seems I would need to add a IS_CHERRYVIEW() on that if branch, and perhaps a else WARN_ON(dev_priv->display.modeset_calc_cdclk); as well to make sure any future platform is covered. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] drm/i915/bxt: Use the bypass frequency if there are no active pipes. 2015-11-24 10:29 [PATCH 0/4] cdclk fixes Maarten Lankhorst ` (2 preceding siblings ...) 2015-11-24 10:29 ` [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3 Maarten Lankhorst @ 2015-11-24 10:29 ` Maarten Lankhorst 3 siblings, 0 replies; 11+ messages in thread From: Maarten Lankhorst @ 2015-11-24 10:29 UTC (permalink / raw) To: intel-gfx Now that pixel clock is set to 0 when there are no active pipes it's easy to set the bypass frequency for this case. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 178a042f917e..8a825f24ce51 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5972,7 +5972,6 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, /* * FIXME: * - remove the guardband, it's not needed on BXT - * - set 19.2MHz bypass frequency if there are no active pipes */ if (max_pixclk > 576000*9/10) return 624000; @@ -5982,8 +5981,10 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, return 384000; else if (max_pixclk > 144000*9/10) return 288000; - else + else if (max_pixclk) return 144000; + else + return 19200; } /* Compute the max pixel clock for new configuration. Uses atomic state if -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-12-16 12:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-24 10:29 [PATCH 0/4] cdclk fixes Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 1/4] drm/i915/skl: Do not allow scaling when crtc is disabled Maarten Lankhorst 2015-12-14 12:07 ` Mika Kahola 2015-11-24 10:29 ` [PATCH 2/4] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst 2015-11-24 10:36 ` Chris Wilson 2015-11-24 10:29 ` [PATCH 3/4] drm/i915: Do not acquire crtc state to check clock during modeset, v3 Maarten Lankhorst 2015-12-15 10:22 ` Mika Kahola 2015-12-15 10:26 ` Chris Wilson 2015-12-16 12:48 ` Mika Kahola 2015-12-16 10:10 ` Maarten Lankhorst 2015-11-24 10:29 ` [PATCH 4/4] drm/i915/bxt: Use the bypass frequency if there are no active pipes Maarten Lankhorst
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).