On Tue, Feb 22, 2022 at 06:20:45AM -0800, José Roberto de Souza wrote:
[...]
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> index ba2fdfce15792..4360e1c9266d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> @@ -184,52 +184,74 @@ enum icl_port_dpll_id {
> };
>
> struct intel_dpll_hw_state {
> - /* i9xx, pch plls */
> - u32 dpll;
> - u32 dpll_md;
> - u32 fp0;
> - u32 fp1;
> -
> - /* hsw, bdw */
> - u32 wrpll;
> - u32 spll;
> -
> - /* skl */
> - /*
> - * DPLL_CTRL1 has 6 bits for each each this DPLL. We store those in
> - * lower part of ctrl1 and they get shifted into position when writing
> - * the register. This allows us to easily compare the state to share
> - * the DPLL.
> - */
> - u32 ctrl1;
> - /* HDMI only, 0 when used for DP */
> - u32 cfgcr1, cfgcr2;
> -
> - /* icl */
> - u32 cfgcr0;
> -
> - /* tgl */
> - u32 div0;
> -
> - /* bxt */
> - u32 ebb0, ebb4, pll0, pll1, pll2, pll3, pll6, pll8, pll9, pll10, pcsdw12;
> -
> - /*
> - * ICL uses the following, already defined:
> - * u32 cfgcr0, cfgcr1;
> - */
> - u32 mg_refclkin_ctl;
> - u32 mg_clktop2_coreclkctl1;
> - u32 mg_clktop2_hsclkctl;
> - u32 mg_pll_div0;
> - u32 mg_pll_div1;
> - u32 mg_pll_lf;
> - u32 mg_pll_frac_lock;
> - u32 mg_pll_ssc;
> - u32 mg_pll_bias;
> - u32 mg_pll_tdc_coldst_bias;
> - u32 mg_pll_bias_mask;
> - u32 mg_pll_tdc_coldst_bias_mask;
> + union {
> + /* icl+ combo */
> + struct {
> + u32 icl_cfgcr0;
> + u32 icl_cfgcr1;
> + u32 icl_div0;
At least icl_ddi_combo_pll_get_freq() and icl_dump_hw_state() missed
converting cfgcr1 to icl_cfgcr1. Would it be less error-prone/simpler to
store here all the skl+ combo state? That would also reduce the diff
size.
> + };
> +
> + /* icl+ TC */
> + struct {
> + u32 mg_refclkin_ctl;
> + u32 mg_clktop2_coreclkctl1;
> + u32 mg_clktop2_hsclkctl;
> + u32 mg_pll_div0;
> + u32 mg_pll_div1;
> + u32 mg_pll_lf;
> + u32 mg_pll_frac_lock;
> + u32 mg_pll_ssc;
> + u32 mg_pll_bias;
> + u32 mg_pll_tdc_coldst_bias;
> + u32 mg_pll_bias_mask;
> + u32 mg_pll_tdc_coldst_bias_mask;
> + };
> +
> + /* bxt */
> + struct {
> + /* bxt */
> + u32 ebb0;
> + u32 ebb4;
> + u32 pll0;
> + u32 pll1;
> + u32 pll2;
> + u32 pll3;
> + u32 pll6;
> + u32 pll8;
> + u32 pll9;
> + u32 pll10;
> + u32 pcsdw12;
> + };
> +
> + /* skl */
> + struct {
> + /*
> + * DPLL_CTRL1 has 6 bits for each this DPLL. We store those in
> + * lower part of ctrl1 and they get shifted into position when writing
> + * the register. This allows us to easily compare the state to share
> + * the DPLL.
> + */
> + u32 ctrl1;
> + u32 cfgcr1;
> + /* HDMI only, 0 when used for DP */
> + u32 cfgcr2;
> + };
> +
> + /* hsw, bdw */
> + struct {
> + u32 wrpll;
> + u32 spll;
> + };
> +
> + /* i9xx, pch plls */
> + struct {
> + u32 dpll;
> + u32 dpll_md;
> + u32 fp0;
> + u32 fp1;
> + };
> + };
> };
>
> /**
> --
> 2.35.1
>
