Re: [PATCH 6/7] drm/i915: Add dpll entrypoint for dumping hw state

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 04, 2016 at 03:32:16PM +0300, Ander Conselvan de Oliveira wrote:
> Remove the IS_PLATFORM() macros from intel_dump_pipe_config() and split
> that logic in platform specific implementations inside the dpll code,
> accessed through a platform independent interface.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Real pretty ;-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_display.c  | 35 +---------------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  3 ++
>  3 files changed, 84 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e6fe85b..8ecaf18 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12744,6 +12744,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				   const char *context)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_plane *plane;
>  	struct intel_plane *intel_plane;
>  	struct intel_plane_state *state;
> @@ -12805,39 +12806,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>  
> -	if (IS_BROXTON(dev)) {
> -		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
> -			      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
> -			      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x, pcsdw12: 0x%x\n",
> -			      pipe_config->dpll_hw_state.ebb0,
> -			      pipe_config->dpll_hw_state.ebb4,
> -			      pipe_config->dpll_hw_state.pll0,
> -			      pipe_config->dpll_hw_state.pll1,
> -			      pipe_config->dpll_hw_state.pll2,
> -			      pipe_config->dpll_hw_state.pll3,
> -			      pipe_config->dpll_hw_state.pll6,
> -			      pipe_config->dpll_hw_state.pll8,
> -			      pipe_config->dpll_hw_state.pll9,
> -			      pipe_config->dpll_hw_state.pll10,
> -			      pipe_config->dpll_hw_state.pcsdw12);
> -	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> -		DRM_DEBUG_KMS("dpll_hw_state: "
> -			      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
> -			      pipe_config->dpll_hw_state.ctrl1,
> -			      pipe_config->dpll_hw_state.cfgcr1,
> -			      pipe_config->dpll_hw_state.cfgcr2);
> -	} else if (HAS_DDI(dev)) {
> -		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
> -			      pipe_config->dpll_hw_state.wrpll,
> -			      pipe_config->dpll_hw_state.spll);
> -	} else {
> -		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
> -			      "fp0: 0x%x, fp1: 0x%x\n",
> -			      pipe_config->dpll_hw_state.dpll,
> -			      pipe_config->dpll_hw_state.dpll_md,
> -			      pipe_config->dpll_hw_state.fp0,
> -			      pipe_config->dpll_hw_state.fp1);
> -	}
> +	intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state);
>  
>  	DRM_DEBUG_KMS("planes on this crtc\n");
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 8c4efa9..9b02d9c 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -447,6 +447,17 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	return pll;
>  }
>  
> +static void ibx_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state)
> +{
> +	DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
> +		      "fp0: 0x%x, fp1: 0x%x\n",
> +		      hw_state->dpll,
> +		      hw_state->dpll_md,
> +		      hw_state->fp0,
> +		      hw_state->fp1);
> +}
> +
>  static const struct intel_shared_dpll_funcs ibx_pch_dpll_funcs = {
>  	.prepare = ibx_pch_dpll_prepare,
>  	.enable = ibx_pch_dpll_enable,
> @@ -833,6 +844,13 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	return pll;
>  }
>  
> +static void hsw_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state)
> +{
> +	DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
> +		      hw_state->wrpll, hw_state->spll);
> +}
> +
>  static const struct intel_shared_dpll_funcs hsw_ddi_wrpll_funcs = {
>  	.enable = hsw_ddi_wrpll_enable,
>  	.disable = hsw_ddi_wrpll_disable,
> @@ -1388,6 +1406,16 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	return pll;
>  }
>  
> +static void skl_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state)
> +{
> +	DRM_DEBUG_KMS("dpll_hw_state: "
> +		      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
> +		      hw_state->ctrl1,
> +		      hw_state->cfgcr1,
> +		      hw_state->cfgcr2);
> +}
> +
>  static const struct intel_shared_dpll_funcs skl_ddi_pll_funcs = {
>  	.enable = skl_ddi_pll_enable,
>  	.disable = skl_ddi_pll_disable,
> @@ -1785,6 +1813,25 @@ bxt_get_dpll(struct intel_crtc *crtc,
>  	return pll;
>  }
>  
> +static void bxt_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state)
> +{
> +	DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
> +		      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
> +		      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x, pcsdw12: 0x%x\n",
> +		      hw_state->ebb0,
> +		      hw_state->ebb4,
> +		      hw_state->pll0,
> +		      hw_state->pll1,
> +		      hw_state->pll2,
> +		      hw_state->pll3,
> +		      hw_state->pll6,
> +		      hw_state->pll8,
> +		      hw_state->pll9,
> +		      hw_state->pll10,
> +		      hw_state->pcsdw12);
> +}
> +
>  static const struct intel_shared_dpll_funcs bxt_ddi_pll_funcs = {
>  	.enable = bxt_ddi_pll_enable,
>  	.disable = bxt_ddi_pll_disable,
> @@ -1825,6 +1872,9 @@ struct intel_dpll_mgr {
>  	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc *crtc,
>  					      struct intel_crtc_state *crtc_state,
>  					      struct intel_encoder *encoder);
> +
> +	void (*dump_hw_state)(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state);
>  };
>  
>  static const struct dpll_info pch_plls[] = {
> @@ -1836,6 +1886,7 @@ static const struct dpll_info pch_plls[] = {
>  static const struct intel_dpll_mgr pch_pll_mgr = {
>  	.dpll_info = pch_plls,
>  	.get_dpll = ibx_get_dpll,
> +	.dump_hw_state = ibx_dump_hw_state,
>  };
>  
>  static const struct dpll_info hsw_plls[] = {
> @@ -1851,6 +1902,7 @@ static const struct dpll_info hsw_plls[] = {
>  static const struct intel_dpll_mgr hsw_pll_mgr = {
>  	.dpll_info = hsw_plls,
>  	.get_dpll = hsw_get_dpll,
> +	.dump_hw_state = hsw_dump_hw_state,
>  };
>  
>  static const struct dpll_info skl_plls[] = {
> @@ -1864,6 +1916,7 @@ static const struct dpll_info skl_plls[] = {
>  static const struct intel_dpll_mgr skl_pll_mgr = {
>  	.dpll_info = skl_plls,
>  	.get_dpll = skl_get_dpll,
> +	.dump_hw_state = skl_dump_hw_state,
>  };
>  
>  static const struct dpll_info bxt_plls[] = {
> @@ -1876,6 +1929,7 @@ static const struct dpll_info bxt_plls[] = {
>  static const struct intel_dpll_mgr bxt_pll_mgr = {
>  	.dpll_info = bxt_plls,
>  	.get_dpll = bxt_get_dpll,
> +	.dump_hw_state = bxt_dump_hw_state,
>  };
>  
>  /**
> @@ -1975,3 +2029,28 @@ void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
>  	shared_dpll_state = intel_atomic_get_shared_dpll_state(state);
>  	shared_dpll_state[dpll->id].crtc_mask &= ~(1 << crtc->pipe);
>  }
> +
> +/**
> + * intel_shared_dpll_dump_hw_state - write hw_state to dmesg
> + * @dev_priv: i915 drm device
> + * @hw_state: hw state to be written to the log
> + *
> + * Write the relevant values in @hw_state to dmesg using DRM_DEBUG_KMS.
> + */
> +void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state)
> +{
> +	if (dev_priv->dpll_mgr) {
> +		dev_priv->dpll_mgr->dump_hw_state(dev_priv, hw_state);
> +	} else {
> +		/* fallback for platforms that don't use the shared dpll
> +		 * infrastructure
> +		 */
> +		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
> +			      "fp0: 0x%x, fp1: 0x%x\n",
> +			      hw_state->dpll,
> +			      hw_state->dpll_md,
> +			      hw_state->fp0,
> +			      hw_state->fp1);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index 40f1a6f..76111a4 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -280,6 +280,9 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc);
>  void intel_shared_dpll_swap_state(struct drm_atomic_state *state);
>  void intel_shared_dpll_init(struct drm_device *dev);
>  
> +void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
> +			      struct intel_dpll_hw_state *hw_state);
> +
>  /* BXT dpll related functions */
>  bool bxt_ddi_dp_set_dpll_hw_state(int clock,
>  			  struct intel_dpll_hw_state *dpll_hw_state);
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx