Re: [PATCH] drm/i915/icl: Enable SSC for ICL using panel_use_ssc

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 05 Jun 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
> To enable SSC for DPLL, we need to set the bit DPLL_CFGCR0_SSC_ENABLE_ICL
> while configuring cfgcr0 register. This bit should be set only when we
> are enabling SSC using kernel mod parameter panel_use_ssc.

Well, we should not be relying on the kernel parameter for any normal
operation. By default we rely on VBT.

> Also, moving intel_panel_use_ssc() declaration to intel_drv.h.

Please don't add new inline functions to intel_drv.h. We're trying to
clean up our headers.

One further comment inline.

> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Clinton Taylor <Clinton.A.Taylor@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c  | 8 --------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 4 ++++
>  drivers/gpu/drm/i915/intel_drv.h      | 8 ++++++++
>  3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 012ad08f38c3..34c82a17ab1b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7377,14 +7377,6 @@ intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
>  		    constant_n);
>  }
>  
> -static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> -{
> -	if (i915_modparams.panel_use_ssc >= 0)
> -		return i915_modparams.panel_use_ssc != 0;
> -	return dev_priv->vbt.lvds_use_ssc
> -		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
> -}
> -
>  static u32 pnv_dpll_compute_fp(struct dpll *dpll)
>  {
>  	return (1 << dpll->n) << 16 | dpll->m2;
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 897d93537414..6c460fb81d6a 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2527,6 +2527,10 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
>  	cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(pll_params.dco_fraction) |
>  		 pll_params.dco_integer;
>  
> +	if (intel_panel_use_ssc(dev_priv)) {
> +		cfgcr0 |= DPLL_CFGCR0_SSC_ENABLE_ICL;
> +	}
> +

The VBT field and the module parameter reference an internal panel. Why
should this be enabled for all outputs?

BR,
Jani.


>  	cfgcr1 = DPLL_CFGCR1_QDIV_RATIO(pll_params.qdiv_ratio) |
>  		 DPLL_CFGCR1_QDIV_MODE(pll_params.qdiv_mode) |
>  		 DPLL_CFGCR1_KDIV(pll_params.kdiv) |
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 270f5bb43d9b..e9af27e841b3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1497,6 +1497,14 @@ intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
>  	drm_wait_one_vblank(&dev_priv->drm, pipe);
>  }
> +static inline bool
> +intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> +{
> +	if (i915_modparams.panel_use_ssc >= 0)
> +		return i915_modparams.panel_use_ssc != 0;
> +	return dev_priv->vbt.lvds_use_ssc
> +		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
> +}
>  static inline void
>  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, int pipe)
>  {

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx