On Thu, Dec 22, 2016 at 04:04:41PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> crtc->config is on its way out. Let's reduce our dependence on it a
> little bit by removing it from intel_modeset_readout_hw_state().
>
> Also replace crtc->acttive checks with crtc_state->base.active checks.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Suggested-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Idling musing: I wonder whether we should convert the readout code to use
an entirely free-standing atomic static tracked in drm_atomic_commit. And
then apply it with a directly call to swap_state. That way we could reuse
all the neat iterators.
Not sure that's a good idea though.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++-------------
> 1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d8effd4da034..091c192c6c5c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -17032,7 +17032,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> dev_priv->active_crtcs = 0;
>
> for_each_intel_crtc(dev, crtc) {
> - struct intel_crtc_state *crtc_state = crtc->config;
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
>
> __drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
> memset(crtc_state, 0, sizeof(*crtc_state));
> @@ -17051,7 +17052,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
> crtc->base.base.id, crtc->base.name,
> - enableddisabled(crtc->active));
> + enableddisabled(crtc_state->base.active));
> }
>
> for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> @@ -17061,7 +17062,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> &pll->config.hw_state);
> pll->config.crtc_mask = 0;
> for_each_intel_crtc(dev, crtc) {
> - if (crtc->active && crtc->config->shared_dpll == pll)
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> +
> + if (crtc_state->base.active &&
> + crtc_state->shared_dpll == pll)
> pll->config.crtc_mask |= 1 << crtc->pipe;
> }
> pll->active_mask = pll->config.crtc_mask;
> @@ -17074,11 +17079,14 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> pipe = 0;
>
> if (encoder->get_hw_state(encoder, &pipe)) {
> + struct intel_crtc_state *crtc_state;
> +
> crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> + crtc_state = to_intel_crtc_state(crtc->base.state);
>
> encoder->base.crtc = &crtc->base;
> - crtc->config->output_types |= 1 << encoder->type;
> - encoder->get_config(encoder, crtc->config);
> + crtc_state->output_types |= 1 << encoder->type;
> + encoder->get_config(encoder, crtc_state);
> } else {
> encoder->base.crtc = NULL;
> }
> @@ -17119,14 +17127,16 @@ 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;
>
> - crtc->base.hwmode = crtc->config->base.adjusted_mode;
> + crtc->base.hwmode = crtc_state->base.adjusted_mode;
>
> memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
> - if (crtc->base.state->active) {
> - intel_mode_from_pipe_config(&crtc->base.mode, crtc->config);
> - intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
> + if (crtc_state->base.active) {
> + intel_mode_from_pipe_config(&crtc->base.mode, crtc_state);
> + intel_mode_from_pipe_config(&crtc_state->base.adjusted_mode, crtc_state);
> WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
>
> /*
> @@ -17146,17 +17156,17 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> * mode change, which means it's safe to do a full
> * recalculation.
> */
> - crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
> + crtc_state->base.mode.private_flags = I915_MODE_FLAG_INHERITED;
>
> if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> - pixclk = ilk_pipe_pixel_rate(crtc->config);
> + pixclk = ilk_pipe_pixel_rate(crtc_state);
> else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - pixclk = crtc->config->base.adjusted_mode.crtc_clock;
> + pixclk = crtc_state->base.adjusted_mode.crtc_clock;
> 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->config->ips_enabled)
> + if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> pixclk = DIV_ROUND_UP(pixclk * 100, 95);
>
> drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
> @@ -17165,7 +17175,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> dev_priv->min_pixclk[crtc->pipe] = pixclk;
>
> - intel_pipe_config_sanity_check(dev_priv, crtc->config);
> + intel_pipe_config_sanity_check(dev_priv, crtc_state);
> }
> }
>
> --
> 2.10.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
