diff mbox

[2/2] clk: imx6: initialize GPU clocks

Message ID 1474017371-28966-2-git-send-email-l.stach@pengutronix.de (mailing list archive)
State Accepted, archived
Delegated to: Stephen Boyd
Headers show

Commit Message

Lucas Stach Sept. 16, 2016, 9:16 a.m. UTC
Initialize the GPU clock muxes to sane inputs. Until now they have
not been changed from their default values, which means that both
GPU3D shader and GPU2D core were fed by clock inputs whose rates
exceed the maximium allowed frequency of the cores by as much as
200MHz.

This fixes a severe GPU stability issue on i.MX6DL.

Cc: stable@vger.kernel.org
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/clk/imx/clk-imx6q.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Shawn Guo Sept. 18, 2016, 12:17 a.m. UTC | #1
On Fri, Sep 16, 2016 at 11:16:11AM +0200, Lucas Stach wrote:
> Initialize the GPU clock muxes to sane inputs. Until now they have
> not been changed from their default values, which means that both
> GPU3D shader and GPU2D core were fed by clock inputs whose rates
> exceed the maximium allowed frequency of the cores by as much as
> 200MHz.
> 
> This fixes a severe GPU stability issue on i.MX6DL.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/clk/imx/clk-imx6q.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index 64c243173395..751c3e7d5843 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -633,6 +633,24 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  	if (IS_ENABLED(CONFIG_PCI_IMX6))
>  		clk_set_parent(clk[IMX6QDL_CLK_LVDS1_SEL], clk[IMX6QDL_CLK_SATA_REF_100M]);
>  
> +	/*
> +	 * Initialize the GPU clock muxes, so that the maximum specified clock
> +	 * rates for the respective SoC are not exceeded.
> +	 */
> +	if (clk_on_imx6dl()) {
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +	} else if (clk_on_imx6q()) {
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_MMDC_CH0_AXI]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_SHADER_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL3_USB_OTG]);
> +	}
> +

Can we handle these with assigned-clock-parents from device tree?

Shawn

>  	imx_register_uart_clocks(uart_clks);
>  }
>  CLK_OF_DECLARE(imx6q, "fsl,imx6q-ccm", imx6q_clocks_init);
> -- 
> 2.8.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach Sept. 19, 2016, 9:07 a.m. UTC | #2
Am Sonntag, den 18.09.2016, 08:17 +0800 schrieb Shawn Guo:
> On Fri, Sep 16, 2016 at 11:16:11AM +0200, Lucas Stach wrote:
> > Initialize the GPU clock muxes to sane inputs. Until now they have
> > not been changed from their default values, which means that both
> > GPU3D shader and GPU2D core were fed by clock inputs whose rates
> > exceed the maximium allowed frequency of the cores by as much as
> > 200MHz.
> > 
> > This fixes a severe GPU stability issue on i.MX6DL.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/clk/imx/clk-imx6q.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> > index 64c243173395..751c3e7d5843 100644
> > --- a/drivers/clk/imx/clk-imx6q.c
> > +++ b/drivers/clk/imx/clk-imx6q.c
> > @@ -633,6 +633,24 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> >  	if (IS_ENABLED(CONFIG_PCI_IMX6))
> >  		clk_set_parent(clk[IMX6QDL_CLK_LVDS1_SEL], clk[IMX6QDL_CLK_SATA_REF_100M]);
> >  
> > +	/*
> > +	 * Initialize the GPU clock muxes, so that the maximum specified clock
> > +	 * rates for the respective SoC are not exceeded.
> > +	 */
> > +	if (clk_on_imx6dl()) {
> > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > +	} else if (clk_on_imx6q()) {
> > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> > +		               clk[IMX6QDL_CLK_MMDC_CH0_AXI]);
> > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_SHADER_SEL],
> > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> > +		               clk[IMX6QDL_CLK_PLL3_USB_OTG]);
> > +	}
> > +
> 
> Can we handle these with assigned-clock-parents from device tree?
> 
No, we want to get rid of the GPU overclocking even with old DTs. DT
stability rules and all that...

Regards,
Lucas

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo Sept. 20, 2016, 1 p.m. UTC | #3
On Mon, Sep 19, 2016 at 11:07:11AM +0200, Lucas Stach wrote:
> > > diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> > > index 64c243173395..751c3e7d5843 100644
> > > --- a/drivers/clk/imx/clk-imx6q.c
> > > +++ b/drivers/clk/imx/clk-imx6q.c
> > > @@ -633,6 +633,24 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> > >  	if (IS_ENABLED(CONFIG_PCI_IMX6))
> > >  		clk_set_parent(clk[IMX6QDL_CLK_LVDS1_SEL], clk[IMX6QDL_CLK_SATA_REF_100M]);
> > >  
> > > +	/*
> > > +	 * Initialize the GPU clock muxes, so that the maximum specified clock
> > > +	 * rates for the respective SoC are not exceeded.
> > > +	 */
> > > +	if (clk_on_imx6dl()) {
> > > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> > > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > > +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> > > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > > +	} else if (clk_on_imx6q()) {
> > > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> > > +		               clk[IMX6QDL_CLK_MMDC_CH0_AXI]);
> > > +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_SHADER_SEL],
> > > +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> > > +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> > > +		               clk[IMX6QDL_CLK_PLL3_USB_OTG]);
> > > +	}
> > > +
> > 
> > Can we handle these with assigned-clock-parents from device tree?
> > 
> No, we want to get rid of the GPU overclocking even with old DTs. DT
> stability rules and all that...

Fair point.  For both patches,

Acked-by: Shawn Guo <shawnguo@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Sept. 20, 2016, 11:58 p.m. UTC | #4
On 09/16, Lucas Stach wrote:
> Initialize the GPU clock muxes to sane inputs. Until now they have
> not been changed from their default values, which means that both
> GPU3D shader and GPU2D core were fed by clock inputs whose rates
> exceed the maximium allowed frequency of the cores by as much as
> 200MHz.
> 
> This fixes a severe GPU stability issue on i.MX6DL.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---

Applied to clk-next
diff mbox

Patch

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 64c243173395..751c3e7d5843 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -633,6 +633,24 @@  static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	if (IS_ENABLED(CONFIG_PCI_IMX6))
 		clk_set_parent(clk[IMX6QDL_CLK_LVDS1_SEL], clk[IMX6QDL_CLK_SATA_REF_100M]);
 
+	/*
+	 * Initialize the GPU clock muxes, so that the maximum specified clock
+	 * rates for the respective SoC are not exceeded.
+	 */
+	if (clk_on_imx6dl()) {
+		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
+		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
+		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
+		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
+	} else if (clk_on_imx6q()) {
+		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
+		               clk[IMX6QDL_CLK_MMDC_CH0_AXI]);
+		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_SHADER_SEL],
+		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
+		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
+		               clk[IMX6QDL_CLK_PLL3_USB_OTG]);
+	}
+
 	imx_register_uart_clocks(uart_clks);
 }
 CLK_OF_DECLARE(imx6q, "fsl,imx6q-ccm", imx6q_clocks_init);