lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 9 Apr 2019 11:19:35 +0200
From:   Neil Armstrong <narmstrong@...libre.com>
To:     Jerome Brunet <jbrunet@...libre.com>,
        dri-devel@...ts.freedesktop.org
Cc:     linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/11] drm/meson: Add G12A Video Clock setup

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 <narmstrong@...libre.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

Powered by blists - more mailing lists