On 09/04/2019 10:46, Jerome Brunet wrote:
> On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote:
>> While switching to the Common Clock Framework is still Work In Progress,
>> this patch adds the corresponding G12A HDMI PLL setup to be on-par
>> with the other SoCs support.
>>
>> The G12A has only a single tweak about the high frequency setup,
>> where the HDMI PLL needs a specific setup to handle correctly the
>> 5.94GHz DCO frequency.
>>
>> Apart that, it handle correctly all the other HDMI frequencies
>> and can achieve even better DMT clock frequency precision with
>> the larger fractional dividier width.
>>
>> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com>
>> ---
>> drivers/gpu/drm/meson/meson_vclk.c | 119 ++++++++++++++++++++++++++---
>> 1 file changed, 108 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_vclk.c
>> b/drivers/gpu/drm/meson/meson_vclk.c
>> index c15a5a5df633..b39034745444 100644
>> --- a/drivers/gpu/drm/meson/meson_vclk.c
>> +++ b/drivers/gpu/drm/meson/meson_vclk.c
>> @@ -113,9 +113,12 @@
>> #define HHI_HDMI_PLL_CNTL4 0x32C /* 0xcb offset in data sheet */
>> #define HHI_HDMI_PLL_CNTL5 0x330 /* 0xcc offset in data sheet */
>> #define HHI_HDMI_PLL_CNTL6 0x334 /* 0xcd offset in data sheet */
>> +#define HHI_HDMI_PLL_CNTL7 0x338 /* 0xce offset in data sheet */
>>
>> #define HDMI_PLL_RESET BIT(28)
>> +#define HDMI_PLL_RESET_G12A BIT(29)
>> #define HDMI_PLL_LOCK BIT(31)
>> +#define HDMI_PLL_LOCK_G12A (3 << 30)
>
> GENMASK(31, 30) ?
Ack
>
>>
>> #define FREQ_1000_1001(_freq) DIV_ROUND_CLOSEST(_freq * 1000, 1001)
>>
>> @@ -257,6 +260,10 @@ static void meson_venci_cvbs_clock_config(struct
>> meson_drm *priv)
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5, 0x71486980);
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL6, 0x00000e55);
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x4800023d);
>> +
>> + /* Poll for lock bit */
>> + regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>> + (val & HDMI_PLL_LOCK), 10, 0);
>> } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x4000027b);
>> @@ -271,11 +278,26 @@ static void meson_venci_cvbs_clock_config(struct
>> meson_drm *priv)
>> HDMI_PLL_RESET, HDMI_PLL_RESET);
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> HDMI_PLL_RESET, 0);
>> - }
>>
>> - /* Poll for lock bit */
>> - regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>> - (val & HDMI_PLL_LOCK), 10, 0);
>> + /* Poll for lock bit */
>> + regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>> + (val & HDMI_PLL_LOCK), 10, 0);
>> + } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x1a0504f7);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00010000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x00000000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4, 0x6a28dc00);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5, 0x65771290);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL6, 0x39272000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL7, 0x56540000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x3a0504f7);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x1a0504f7);
>> +
>> + /* Poll for lock bit */
>> + regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>> + ((val & HDMI_PLL_LOCK_G12A) == HDMI_PLL_LOCK_G12A),
>> + 10, 0);
>> + }
>>
>> /* Disable VCLK2 */
>> regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, 0);
>> @@ -288,8 +310,13 @@ static void meson_venci_cvbs_clock_config(struct
>> meson_drm *priv)
>> VCLK2_DIV_MASK, (55 - 1));
>>
>> /* select vid_pll for vclk2 */
>> - regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
>> - VCLK2_SEL_MASK, (4 << VCLK2_SEL_SHIFT));
>> + if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
>> + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
>> + VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT));
>> + else
>> + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
>> + VCLK2_SEL_MASK, (4 << VCLK2_SEL_SHIFT));
>> +
>> /* enable vclk2 gate */
>> regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, VCLK2_EN);
>>
>> @@ -476,32 +503,80 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv,
>> unsigned int m,
>> /* Poll for lock bit */
>> regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>> (val & HDMI_PLL_LOCK), 10, 0);
>> + } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x0b3a0400 | m);
>> +
>> + /* Enable and reset */
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + 0x3 << 28, 0x3 << 28);
>
> Could you use define of the enable and reset bit instead of 0x3 << 28 ?
Ack
>
>> +
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, frac);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x00000000);
>> +
>> + /* G12A HDMI PLL Needs specific parameters for 5.4GHz */
>> + if (m >= 0xf7) {
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4, 0xea68dc00);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5, 0x65771290);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL6, 0x39272000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL7, 0x55540000);
>> + } else {
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4, 0x0a691c00);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5, 0x33771290);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL6, 0x39270000);
>> + regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL7, 0x50540000);
>> + }
>> +
>> + do {
>> + /* Reset PLL */
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + HDMI_PLL_RESET_G12A,
>> HDMI_PLL_RESET_G12A);
>> +
>> + /* UN-Reset PLL */
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + HDMI_PLL_RESET_G12A, 0);
>> +
>> + /* Poll for lock bits */
>> + if (!regmap_read_poll_timeout(priv->hhi,
>> + HHI_HDMI_PLL_CNTL, val,
>> + ((val &
>> HDMI_PLL_LOCK_G12A)
>> + == HDMI_PLL_LOCK_G12A),
>> + 10, 100))
>> + break;
>> + } while(1);
>> }
>>
>> if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>> 3 << 16, pll_od_to_reg(od1) << 16);
>> else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> + meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>> 3 << 21, pll_od_to_reg(od1) << 21);
>> + else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + 3 << 16, pll_od_to_reg(od1) << 16);
>>
>> if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>> 3 << 22, pll_od_to_reg(od2) << 22);
>> else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> + meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>> 3 << 23, pll_od_to_reg(od2) << 23);
>> + else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + 3 << 18, pll_od_to_reg(od2) << 18);
>>
>> if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>> 3 << 18, pll_od_to_reg(od3) << 18);
>> else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> + meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
>> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>> 3 << 19, pll_od_to_reg(od3) << 19);
>> -
>> + else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
>> + regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>> + 3 << 20, pll_od_to_reg(od3) << 20);
>
> Could you remove the constants above and define the shift and mask of the ODs
> ?
Ack
>
>> }
>>
>> #define XTAL_FREQ 24000
>> @@ -518,6 +593,7 @@ static unsigned int meson_hdmi_pll_get_m(struct
>> meson_drm *priv,
>>
>> #define HDMI_FRAC_MAX_GXBB 4096
>> #define HDMI_FRAC_MAX_GXL 1024
>> +#define HDMI_FRAC_MAX_G12A 131072
>>
>> static unsigned int meson_hdmi_pll_get_frac(struct meson_drm *priv,
>> unsigned int m,
>> @@ -534,6 +610,9 @@ static unsigned int meson_hdmi_pll_get_frac(struct
>> meson_drm *priv,
>> parent_freq *= 2;
>> }
>>
>> + if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
>> + frac_max = HDMI_FRAC_MAX_G12A;
>> +
>> /* We can have a perfect match !*/
>> if (pll_freq / m == parent_freq &&
>> pll_freq % m == 0)
>> @@ -559,7 +638,8 @@ static bool meson_hdmi_pll_validate_params(struct
>> meson_drm *priv,
>> if (frac >= HDMI_FRAC_MAX_GXBB)
>> return false;
>> } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
>> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
>> + meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu") ||
>> + meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> /* Empiric supported min/max dividers */
>> if (m < 106 || m > 247)
>> return false;
>> @@ -713,6 +793,23 @@ static void meson_vclk_set(struct meson_drm *priv,
>> unsigned int pll_base_freq,
>> break;
>> }
>>
>> + meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
>> + } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
>> + switch (pll_base_freq) {
>> + case 2970000:
>> + m = 0x7b;
>> + frac = vic_alternate_clock ? 0x140b4 : 0x18000;
>> + break;
>> + case 4320000:
>> + m = vic_alternate_clock ? 0xb3 : 0xb4;
>> + frac = vic_alternate_clock ? 0x1a3ee : 0;
>> + break;
>> + case 5940000:
>> + m = 0xf7;
>> + frac = vic_alternate_clock ? 0x8148 : 0x10000;
>> + break;
>> + }
>> +
>> meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
>> }
>>
>
>
Will fix in a follow-up patch, including GXBB/GXL/GXM.
Neil
