[RFC,1/3] clock: aspeed: Setup video engine clocking

Message ID 1533676303-305-2-git-send-email-eajames@linux.vnet.ibm.com
State Not Applicable, archived
Headers show
Series
  • media: platform: Add Aspeed Video Engine driver
Related show

Commit Message

Eddie James Aug. 7, 2018, 9:11 p.m.
Add the video engine reset bit and add the eclk clock divider table. The
eclk can be muxed, but for the sake of simplicity, force the eclk source
to be the mpll clock.

Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
---
 drivers/clk/clk-aspeed.c                 | 61 +++++++++++++++++++++++++++++++-
 include/dt-bindings/clock/aspeed-clock.h |  1 +
 2 files changed, 61 insertions(+), 1 deletion(-)

Comments

Andrew Jeffery Aug. 13, 2018, 12:45 a.m. | #1
On Wed, 8 Aug 2018, at 06:41, Eddie James wrote:
> Add the video engine reset bit and add the eclk clock divider table. The
> eclk can be muxed, but for the sake of simplicity, force the eclk source
> to be the mpll clock.
> 
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
>  drivers/clk/clk-aspeed.c                 | 61 +++++++++++++++++++++++++++++++-
>  include/dt-bindings/clock/aspeed-clock.h |  1 +
>  2 files changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 8796b8a..c1a64ae 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -20,6 +20,7 @@
>  
>  #define ASPEED_RESET_CTRL	0x04
>  #define ASPEED_CLK_SELECTION	0x08
> +#define  ASPEED_CLK_SEL_VE	GENMASK(3, 2)
>  #define ASPEED_CLK_STOP_CTRL	0x0c
>  #define ASPEED_MPLL_PARAM	0x20
>  #define ASPEED_HPLL_PARAM	0x24
> @@ -87,7 +88,7 @@ struct aspeed_clk_gate {
>  /* TODO: ask Aspeed about the actual parent data */
>  static const struct aspeed_gate_data aspeed_gates[] = {
>  	/*				 clk rst   name			parent	flags */
> -	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video 
> Engine */
> +	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video 
> Engine */
>  	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
>  	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	
> CLK_IS_CRITICAL }, /* SDRAM */
>  	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video 
> Capture */
> @@ -113,6 +114,18 @@ struct aspeed_clk_gate {
>  	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC 
> master/LPC+ */
>  };
>  
> +static const struct clk_div_table ast2500_eclk_div_table[] = {
> +	{ 0x0, 2 },
> +	{ 0x1, 2 },
> +	{ 0x2, 3 },
> +	{ 0x3, 4 },
> +	{ 0x4, 5 },
> +	{ 0x5, 6 },
> +	{ 0x6, 7 },
> +	{ 0x7, 8 },
> +	{ 0 }
> +};
> +
>  static const struct clk_div_table ast2500_mac_div_table[] = {
>  	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
>  	{ 0x1, 4 },
> @@ -192,18 +205,21 @@ static struct clk_hw 
> *aspeed_ast2500_calc_pll(const char *name, u32 val)
>  
>  struct aspeed_clk_soc_data {
>  	const struct clk_div_table *div_table;
> +	const struct clk_div_table *eclk_div_table;
>  	const struct clk_div_table *mac_div_table;
>  	struct clk_hw *(*calc_pll)(const char *name, u32 val);
>  };
>  
>  static const struct aspeed_clk_soc_data ast2500_data = {
>  	.div_table = ast2500_div_table,
> +	.eclk_div_table = ast2500_eclk_div_table,
>  	.mac_div_table = ast2500_mac_div_table,
>  	.calc_pll = aspeed_ast2500_calc_pll,
>  };
>  
>  static const struct aspeed_clk_soc_data ast2400_data = {
>  	.div_table = ast2400_div_table,
> +	.eclk_div_table = ast2400_div_table,
>  	.mac_div_table = ast2400_div_table,
>  	.calc_pll = aspeed_ast2400_calc_pll,
>  };
> @@ -317,6 +333,7 @@ struct aspeed_reset {
>  	[ASPEED_RESET_PECI]	= 10,
>  	[ASPEED_RESET_I2C]	=  2,
>  	[ASPEED_RESET_AHB]	=  1,
> +	[ASPEED_RESET_VIDEO]	=  6,
>  
>  	/*
>  	 * SCUD4 resets start at an offset to separate them from
> @@ -522,6 +539,48 @@ static int aspeed_clk_probe(struct platform_device *pdev)
>  		return PTR_ERR(hw);
>  	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
>  
> +	/* Video Engine (ECLK) clock divider (ignore mux; set parent MPLL) */
> +	regmap_read(map, ASPEED_CLK_SELECTION, &val);
> +	if (val & ASPEED_CLK_SEL_VE) {
> +		/* ECLK clock select isn't MPLL; reset to MPLL */
> +		u32 clk = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].clock_idx);
> +		u32 rst = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].reset_idx);
> +		u32 stop;
> +
> +		dev_info(dev, "setting eclk mux to mpll\n");
> +
> +		/* Put Video Engine in reset */
> +		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, rst);
> +		udelay(100);
> +
> +		/* Stop the clock if it's running */
> +		regmap_read(map, ASPEED_CLK_STOP_CTRL, &stop);
> +		if (!(stop & clk))
> +			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk,
> +					   clk);
> +
> +		/* Set clock select to MPLL */
> +		regmap_update_bits(map, ASPEED_CLK_SELECTION,
> +				   ASPEED_VE_CLK_SEL, 0);
> +
> +		/* Start the clock if it was originally running */
> +		if (!(stop & clk)) {
> +			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk, 0);
> +			mdelay(10);
> +		}
> +
> +		/* Take Video Engine out of reset */
> +		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, 0);
> +	}

This seems like a bit of hack? Shouldn't we describe the clock tree
including the mux and let it sort itself out?

> +
> +	hw = clk_hw_register_divider_table(dev, "eclk", "mpll", 0,
> +			scu_base + ASPEED_CLK_SELECTION, 28, 3, 0,
> +			soc_data->eclk_div_table,
> +			&aspeed_clk_lock);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
> +
>  	/*
>  	 * TODO: There are a number of clocks that not included in this driver
>  	 * as more information is required:
> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-
> bindings/clock/aspeed-clock.h
> index 4476184..f1813db 100644
> --- a/include/dt-bindings/clock/aspeed-clock.h
> +++ b/include/dt-bindings/clock/aspeed-clock.h
> @@ -50,5 +50,6 @@
>  #define ASPEED_RESET_I2C		7
>  #define ASPEED_RESET_AHB		8
>  #define ASPEED_RESET_CRT1		9
> +#define ASPEED_RESET_VIDEO		10
>  
>  #endif
> -- 
> 1.8.3.1
>
Eddie James Aug. 16, 2018, 7:11 p.m. | #2
On 08/12/2018 07:45 PM, Andrew Jeffery wrote:
> On Wed, 8 Aug 2018, at 06:41, Eddie James wrote:
>> Add the video engine reset bit and add the eclk clock divider table. The
>> eclk can be muxed, but for the sake of simplicity, force the eclk source
>> to be the mpll clock.
>>
>> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
>> ---
>>   drivers/clk/clk-aspeed.c                 | 61 +++++++++++++++++++++++++++++++-
>>   include/dt-bindings/clock/aspeed-clock.h |  1 +
>>   2 files changed, 61 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>> index 8796b8a..c1a64ae 100644
>> --- a/drivers/clk/clk-aspeed.c
>> +++ b/drivers/clk/clk-aspeed.c
>> @@ -20,6 +20,7 @@
>>   
>>   #define ASPEED_RESET_CTRL	0x04
>>   #define ASPEED_CLK_SELECTION	0x08
>> +#define  ASPEED_CLK_SEL_VE	GENMASK(3, 2)
>>   #define ASPEED_CLK_STOP_CTRL	0x0c
>>   #define ASPEED_MPLL_PARAM	0x20
>>   #define ASPEED_HPLL_PARAM	0x24
>> @@ -87,7 +88,7 @@ struct aspeed_clk_gate {
>>   /* TODO: ask Aspeed about the actual parent data */
>>   static const struct aspeed_gate_data aspeed_gates[] = {
>>   	/*				 clk rst   name			parent	flags */
>> -	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video
>> Engine */
>> +	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video
>> Engine */
>>   	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
>>   	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	
>> CLK_IS_CRITICAL }, /* SDRAM */
>>   	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video
>> Capture */
>> @@ -113,6 +114,18 @@ struct aspeed_clk_gate {
>>   	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC
>> master/LPC+ */
>>   };
>>   
>> +static const struct clk_div_table ast2500_eclk_div_table[] = {
>> +	{ 0x0, 2 },
>> +	{ 0x1, 2 },
>> +	{ 0x2, 3 },
>> +	{ 0x3, 4 },
>> +	{ 0x4, 5 },
>> +	{ 0x5, 6 },
>> +	{ 0x6, 7 },
>> +	{ 0x7, 8 },
>> +	{ 0 }
>> +};
>> +
>>   static const struct clk_div_table ast2500_mac_div_table[] = {
>>   	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
>>   	{ 0x1, 4 },
>> @@ -192,18 +205,21 @@ static struct clk_hw
>> *aspeed_ast2500_calc_pll(const char *name, u32 val)
>>   
>>   struct aspeed_clk_soc_data {
>>   	const struct clk_div_table *div_table;
>> +	const struct clk_div_table *eclk_div_table;
>>   	const struct clk_div_table *mac_div_table;
>>   	struct clk_hw *(*calc_pll)(const char *name, u32 val);
>>   };
>>   
>>   static const struct aspeed_clk_soc_data ast2500_data = {
>>   	.div_table = ast2500_div_table,
>> +	.eclk_div_table = ast2500_eclk_div_table,
>>   	.mac_div_table = ast2500_mac_div_table,
>>   	.calc_pll = aspeed_ast2500_calc_pll,
>>   };
>>   
>>   static const struct aspeed_clk_soc_data ast2400_data = {
>>   	.div_table = ast2400_div_table,
>> +	.eclk_div_table = ast2400_div_table,
>>   	.mac_div_table = ast2400_div_table,
>>   	.calc_pll = aspeed_ast2400_calc_pll,
>>   };
>> @@ -317,6 +333,7 @@ struct aspeed_reset {
>>   	[ASPEED_RESET_PECI]	= 10,
>>   	[ASPEED_RESET_I2C]	=  2,
>>   	[ASPEED_RESET_AHB]	=  1,
>> +	[ASPEED_RESET_VIDEO]	=  6,
>>   
>>   	/*
>>   	 * SCUD4 resets start at an offset to separate them from
>> @@ -522,6 +539,48 @@ static int aspeed_clk_probe(struct platform_device *pdev)
>>   		return PTR_ERR(hw);
>>   	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
>>   
>> +	/* Video Engine (ECLK) clock divider (ignore mux; set parent MPLL) */
>> +	regmap_read(map, ASPEED_CLK_SELECTION, &val);
>> +	if (val & ASPEED_CLK_SEL_VE) {
>> +		/* ECLK clock select isn't MPLL; reset to MPLL */
>> +		u32 clk = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].clock_idx);
>> +		u32 rst = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].reset_idx);
>> +		u32 stop;
>> +
>> +		dev_info(dev, "setting eclk mux to mpll\n");
>> +
>> +		/* Put Video Engine in reset */
>> +		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, rst);
>> +		udelay(100);
>> +
>> +		/* Stop the clock if it's running */
>> +		regmap_read(map, ASPEED_CLK_STOP_CTRL, &stop);
>> +		if (!(stop & clk))
>> +			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk,
>> +					   clk);
>> +
>> +		/* Set clock select to MPLL */
>> +		regmap_update_bits(map, ASPEED_CLK_SELECTION,
>> +				   ASPEED_VE_CLK_SEL, 0);
>> +
>> +		/* Start the clock if it was originally running */
>> +		if (!(stop & clk)) {
>> +			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk, 0);
>> +			mdelay(10);
>> +		}
>> +
>> +		/* Take Video Engine out of reset */
>> +		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, 0);
>> +	}
> This seems like a bit of hack? Shouldn't we describe the clock tree
> including the mux and let it sort itself out?

Yes. I literally couldn't figure out how to do that... any help would be 
great :D

Thanks,
Eddie

>
>> +
>> +	hw = clk_hw_register_divider_table(dev, "eclk", "mpll", 0,
>> +			scu_base + ASPEED_CLK_SELECTION, 28, 3, 0,
>> +			soc_data->eclk_div_table,
>> +			&aspeed_clk_lock);
>> +	if (IS_ERR(hw))
>> +		return PTR_ERR(hw);
>> +	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
>> +
>>   	/*
>>   	 * TODO: There are a number of clocks that not included in this driver
>>   	 * as more information is required:
>> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-
>> bindings/clock/aspeed-clock.h
>> index 4476184..f1813db 100644
>> --- a/include/dt-bindings/clock/aspeed-clock.h
>> +++ b/include/dt-bindings/clock/aspeed-clock.h
>> @@ -50,5 +50,6 @@
>>   #define ASPEED_RESET_I2C		7
>>   #define ASPEED_RESET_AHB		8
>>   #define ASPEED_RESET_CRT1		9
>> +#define ASPEED_RESET_VIDEO		10
>>   
>>   #endif
>> -- 
>> 1.8.3.1
>>

Patch

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 8796b8a..c1a64ae 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -20,6 +20,7 @@ 
 
 #define ASPEED_RESET_CTRL	0x04
 #define ASPEED_CLK_SELECTION	0x08
+#define  ASPEED_CLK_SEL_VE	GENMASK(3, 2)
 #define ASPEED_CLK_STOP_CTRL	0x0c
 #define ASPEED_MPLL_PARAM	0x20
 #define ASPEED_HPLL_PARAM	0x24
@@ -87,7 +88,7 @@  struct aspeed_clk_gate {
 /* TODO: ask Aspeed about the actual parent data */
 static const struct aspeed_gate_data aspeed_gates[] = {
 	/*				 clk rst   name			parent	flags */
-	[ASPEED_CLK_GATE_ECLK] =	{  0, -1, "eclk-gate",		"eclk",	0 }, /* Video Engine */
+	[ASPEED_CLK_GATE_ECLK] =	{  0,  6, "eclk-gate",		"eclk",	0 }, /* Video Engine */
 	[ASPEED_CLK_GATE_GCLK] =	{  1,  7, "gclk-gate",		NULL,	0 }, /* 2D engine */
 	[ASPEED_CLK_GATE_MCLK] =	{  2, -1, "mclk-gate",		"mpll",	CLK_IS_CRITICAL }, /* SDRAM */
 	[ASPEED_CLK_GATE_VCLK] =	{  3,  6, "vclk-gate",		NULL,	0 }, /* Video Capture */
@@ -113,6 +114,18 @@  struct aspeed_clk_gate {
 	[ASPEED_CLK_GATE_LHCCLK] =	{ 28, -1, "lhclk-gate",		"lhclk", 0 }, /* LPC master/LPC+ */
 };
 
+static const struct clk_div_table ast2500_eclk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
 static const struct clk_div_table ast2500_mac_div_table[] = {
 	{ 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
 	{ 0x1, 4 },
@@ -192,18 +205,21 @@  static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
 
 struct aspeed_clk_soc_data {
 	const struct clk_div_table *div_table;
+	const struct clk_div_table *eclk_div_table;
 	const struct clk_div_table *mac_div_table;
 	struct clk_hw *(*calc_pll)(const char *name, u32 val);
 };
 
 static const struct aspeed_clk_soc_data ast2500_data = {
 	.div_table = ast2500_div_table,
+	.eclk_div_table = ast2500_eclk_div_table,
 	.mac_div_table = ast2500_mac_div_table,
 	.calc_pll = aspeed_ast2500_calc_pll,
 };
 
 static const struct aspeed_clk_soc_data ast2400_data = {
 	.div_table = ast2400_div_table,
+	.eclk_div_table = ast2400_div_table,
 	.mac_div_table = ast2400_div_table,
 	.calc_pll = aspeed_ast2400_calc_pll,
 };
@@ -317,6 +333,7 @@  struct aspeed_reset {
 	[ASPEED_RESET_PECI]	= 10,
 	[ASPEED_RESET_I2C]	=  2,
 	[ASPEED_RESET_AHB]	=  1,
+	[ASPEED_RESET_VIDEO]	=  6,
 
 	/*
 	 * SCUD4 resets start at an offset to separate them from
@@ -522,6 +539,48 @@  static int aspeed_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
 
+	/* Video Engine (ECLK) clock divider (ignore mux; set parent MPLL) */
+	regmap_read(map, ASPEED_CLK_SELECTION, &val);
+	if (val & ASPEED_CLK_SEL_VE) {
+		/* ECLK clock select isn't MPLL; reset to MPLL */
+		u32 clk = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].clock_idx);
+		u32 rst = BIT(aspeed_gates[ASPEED_CLK_GATE_ECLK].reset_idx);
+		u32 stop;
+
+		dev_info(dev, "setting eclk mux to mpll\n");
+
+		/* Put Video Engine in reset */
+		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, rst);
+		udelay(100);
+
+		/* Stop the clock if it's running */
+		regmap_read(map, ASPEED_CLK_STOP_CTRL, &stop);
+		if (!(stop & clk))
+			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk,
+					   clk);
+
+		/* Set clock select to MPLL */
+		regmap_update_bits(map, ASPEED_CLK_SELECTION,
+				   ASPEED_VE_CLK_SEL, 0);
+
+		/* Start the clock if it was originally running */
+		if (!(stop & clk)) {
+			regmap_update_bits(map, ASPEED_CLK_STOP_CTRL, clk, 0);
+			mdelay(10);
+		}
+
+		/* Take Video Engine out of reset */
+		regmap_update_bits(map, ASPEED_RESET_CTRL, rst, 0);
+	}
+
+	hw = clk_hw_register_divider_table(dev, "eclk", "mpll", 0,
+			scu_base + ASPEED_CLK_SELECTION, 28, 3, 0,
+			soc_data->eclk_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw;
+
 	/*
 	 * TODO: There are a number of clocks that not included in this driver
 	 * as more information is required:
diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
index 4476184..f1813db 100644
--- a/include/dt-bindings/clock/aspeed-clock.h
+++ b/include/dt-bindings/clock/aspeed-clock.h
@@ -50,5 +50,6 @@ 
 #define ASPEED_RESET_I2C		7
 #define ASPEED_RESET_AHB		8
 #define ASPEED_RESET_CRT1		9
+#define ASPEED_RESET_VIDEO		10
 
 #endif