| Message ID | 20240219-topic-rb1_gpu-v1-0-d260fa854707@linaro.org |
|---|---|
| Headers | show |
| Series | A702 support | expand |
On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol). Is it not? > > [1] https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> With the exception of the commit message: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 ++++++++ > 1 file changed, 8 insertions(+)
On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL") > introduced an entry to the alpha offsets array, but diving into QCM2290 > downstream and some documentation, it turned out that the name Huayra > apparently has been used quite liberally across many chips, even with > noticeably different hardware. > > Introduce another set of offsets and a new configure function for the > Huayra PLL found on QCM2290. This is required e.g. for the consumers > of GPUCC_PLL0 to properly start. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/clk-alpha-pll.c | 45 ++++++++++++++++++++++++++++++++++++++++ > drivers/clk/qcom/clk-alpha-pll.h | 3 +++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index 8a412ef47e16..61b5abd13782 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { > [PLL_OFF_OPMODE] = 0x30, > [PLL_OFF_STATUS] = 0x3c, > }, > + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = { > + [PLL_OFF_L_VAL] = 0x04, > + [PLL_OFF_ALPHA_VAL] = 0x08, > + [PLL_OFF_USER_CTL] = 0x0c, > + [PLL_OFF_CONFIG_CTL] = 0x10, > + [PLL_OFF_CONFIG_CTL_U] = 0x14, > + [PLL_OFF_CONFIG_CTL_U1] = 0x18, > + [PLL_OFF_TEST_CTL] = 0x1c, > + [PLL_OFF_TEST_CTL_U] = 0x20, > + [PLL_OFF_TEST_CTL_U1] = 0x24, > + [PLL_OFF_OPMODE] = 0x28, > + [PLL_OFF_STATUS] = 0x38, > + }, Can you please move them next to the standard huayra PLL regs? > }; > EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); > > @@ -779,6 +792,38 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate, > return clamp(rate, min_freq, max_freq); > } > > +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > + const struct alpha_pll_config *config) > +{ > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val); > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val); > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val); > + clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l); > + clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha); > + clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val); > + > + /* Set PLL_BYPASSNL */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL); > + > + /* Wait 5 us between setting BYPASS and deasserting reset */ > + mb(); > + udelay(5); > + > + /* Take PLL out from reset state */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); > + > + /* Wait 50us for PLL_LOCK_DET bit to go high */ > + mb(); > + usleep_range(50, 55); > + > + /* Enable PLL output */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL); > +} > +EXPORT_SYMBOL(clk_huayra_2290_pll_configure); > + > static unsigned long > alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a) > { > diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h > index fb6d50263bb9..91d3d6f19eae 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.h > +++ b/drivers/clk/qcom/clk-alpha-pll.h > @@ -29,6 +29,7 @@ enum { > CLK_ALPHA_PLL_TYPE_BRAMMO_EVO, > CLK_ALPHA_PLL_TYPE_STROMER, > CLK_ALPHA_PLL_TYPE_STROMER_PLUS, > + CLK_ALPHA_PLL_TYPE_HUAYRA_2290, Also next to standard huayra PLL, please. > CLK_ALPHA_PLL_TYPE_MAX, > }; > > @@ -191,6 +192,8 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops; > > void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > const struct alpha_pll_config *config); > +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > + const struct alpha_pll_config *config); > void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > const struct alpha_pll_config *config); > void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > > -- > 2.43.2 >
On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > Add a driver for the GPU clock controller block found on the QCM2290 SoC. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/Kconfig | 9 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/gpucc-qcm2290.c | 423 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 433 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Mon, Feb 19, 2024 at 02:35:48PM +0100, Konrad Dybcio wrote: > Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL") > introduced an entry to the alpha offsets array, but diving into QCM2290 > downstream and some documentation, it turned out that the name Huayra > apparently has been used quite liberally across many chips, even with > noticeably different hardware. > > Introduce another set of offsets and a new configure function for the > Huayra PLL found on QCM2290. This is required e.g. for the consumers > of GPUCC_PLL0 to properly start. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/clk-alpha-pll.c | 45 ++++++++++++++++++++++++++++++++++++++++ > drivers/clk/qcom/clk-alpha-pll.h | 3 +++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index 8a412ef47e16..61b5abd13782 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { > [PLL_OFF_OPMODE] = 0x30, > [PLL_OFF_STATUS] = 0x3c, > }, > + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = { > + [PLL_OFF_L_VAL] = 0x04, > + [PLL_OFF_ALPHA_VAL] = 0x08, > + [PLL_OFF_USER_CTL] = 0x0c, > + [PLL_OFF_CONFIG_CTL] = 0x10, > + [PLL_OFF_CONFIG_CTL_U] = 0x14, > + [PLL_OFF_CONFIG_CTL_U1] = 0x18, > + [PLL_OFF_TEST_CTL] = 0x1c, > + [PLL_OFF_TEST_CTL_U] = 0x20, > + [PLL_OFF_TEST_CTL_U1] = 0x24, > + [PLL_OFF_OPMODE] = 0x28, > + [PLL_OFF_STATUS] = 0x38, > + }, > }; > EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); > > @@ -779,6 +792,38 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate, > return clamp(rate, min_freq, max_freq); > } > > +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > + const struct alpha_pll_config *config) > +{ > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val); > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val); > + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val); > + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val); > + clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l); > + clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha); > + clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val); > + > + /* Set PLL_BYPASSNL */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL); > + > + /* Wait 5 us between setting BYPASS and deasserting reset */ > + mb(); > + udelay(5); > + > + /* Take PLL out from reset state */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); > + > + /* Wait 50us for PLL_LOCK_DET bit to go high */ > + mb(); > + usleep_range(50, 55); I *think* you'd want to use a read to ensure your write goes through prior to your sleep... from memory-barriers.txt: 5. A readX() by a CPU thread from the peripheral will complete before any subsequent delay() loop can begin execution on the same thread. This ensures that two MMIO register writes by the CPU to a peripheral will arrive at least 1us apart if the first write is immediately read back with readX() and udelay(1) is called prior to the second writeX(): writel(42, DEVICE_REGISTER_0); // Arrives at the device... readl(DEVICE_REGISTER_0); udelay(1); writel(42, DEVICE_REGISTER_1); // ...at least 1us before this. also https://youtu.be/i6DayghhA8Q?si=7lp0be35q1HRmlnV&t=1677 for more references on this topic. > + > + /* Enable PLL output */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL); > +} > +EXPORT_SYMBOL(clk_huayra_2290_pll_configure); > + > static unsigned long > alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a) > { > diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h > index fb6d50263bb9..91d3d6f19eae 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.h > +++ b/drivers/clk/qcom/clk-alpha-pll.h > @@ -29,6 +29,7 @@ enum { > CLK_ALPHA_PLL_TYPE_BRAMMO_EVO, > CLK_ALPHA_PLL_TYPE_STROMER, > CLK_ALPHA_PLL_TYPE_STROMER_PLUS, > + CLK_ALPHA_PLL_TYPE_HUAYRA_2290, > CLK_ALPHA_PLL_TYPE_MAX, > }; > > @@ -191,6 +192,8 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops; > > void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > const struct alpha_pll_config *config); > +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > + const struct alpha_pll_config *config); > void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > const struct alpha_pll_config *config); > void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > > -- > 2.43.2 > >
On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > Describe the GPU hardware on the QCM2290. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > arch/arm64/boot/dts/qcom/qcm2290.dtsi | 154 ++++++++++++++++++++++++++++++++++ > 1 file changed, 154 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 19.02.2024 15:54, Andrew Halaney wrote: > On Mon, Feb 19, 2024 at 02:35:48PM +0100, Konrad Dybcio wrote: >> Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL") >> introduced an entry to the alpha offsets array, but diving into QCM2290 >> downstream and some documentation, it turned out that the name Huayra >> apparently has been used quite liberally across many chips, even with >> noticeably different hardware. >> >> Introduce another set of offsets and a new configure function for the >> Huayra PLL found on QCM2290. This is required e.g. for the consumers >> of GPUCC_PLL0 to properly start. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/clk/qcom/clk-alpha-pll.c | 45 ++++++++++++++++++++++++++++++++++++++++ >> drivers/clk/qcom/clk-alpha-pll.h | 3 +++ >> 2 files changed, 48 insertions(+) >> >> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c >> index 8a412ef47e16..61b5abd13782 100644 >> --- a/drivers/clk/qcom/clk-alpha-pll.c >> +++ b/drivers/clk/qcom/clk-alpha-pll.c >> @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { >> [PLL_OFF_OPMODE] = 0x30, >> [PLL_OFF_STATUS] = 0x3c, >> }, >> + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = { >> + [PLL_OFF_L_VAL] = 0x04, >> + [PLL_OFF_ALPHA_VAL] = 0x08, >> + [PLL_OFF_USER_CTL] = 0x0c, >> + [PLL_OFF_CONFIG_CTL] = 0x10, >> + [PLL_OFF_CONFIG_CTL_U] = 0x14, >> + [PLL_OFF_CONFIG_CTL_U1] = 0x18, >> + [PLL_OFF_TEST_CTL] = 0x1c, >> + [PLL_OFF_TEST_CTL_U] = 0x20, >> + [PLL_OFF_TEST_CTL_U1] = 0x24, >> + [PLL_OFF_OPMODE] = 0x28, >> + [PLL_OFF_STATUS] = 0x38, >> + }, >> }; >> EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); >> >> @@ -779,6 +792,38 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate, >> return clamp(rate, min_freq, max_freq); >> } >> >> +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, >> + const struct alpha_pll_config *config) >> +{ >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val); >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val); >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val); >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val); >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val); >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val); >> + clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l); >> + clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha); >> + clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val); >> + >> + /* Set PLL_BYPASSNL */ >> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL); >> + >> + /* Wait 5 us between setting BYPASS and deasserting reset */ >> + mb(); >> + udelay(5); >> + >> + /* Take PLL out from reset state */ >> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); >> + >> + /* Wait 50us for PLL_LOCK_DET bit to go high */ >> + mb(); >> + usleep_range(50, 55); > > I *think* you'd want to use a read to ensure your write goes through > prior to your sleep... from memory-barriers.txt: > > 5. A readX() by a CPU thread from the peripheral will complete before > any subsequent delay() loop can begin execution on the same thread. > This ensures that two MMIO register writes by the CPU to a peripheral > will arrive at least 1us apart if the first write is immediately read > back with readX() and udelay(1) is called prior to the second > writeX(): > > writel(42, DEVICE_REGISTER_0); // Arrives at the device... > readl(DEVICE_REGISTER_0); > udelay(1); > writel(42, DEVICE_REGISTER_1); // ...at least 1us before this. > > also https://youtu.be/i6DayghhA8Q?si=7lp0be35q1HRmlnV&t=1677 > for more references on this topic. I mentioned this feels very iffy in the cover letter, but it's a combination of two things: 1. i followed what qualcomm downstream code did 2. qualcomm downstream code is not known for being always correct I suppose a readback would be the correct solution, but then it should be done for all similar calls in this driver. Although this code has shipped in literally hundreds of millions of devices and it didn't explode badly :P (i'm not defending it, though) Konrad
On 19.02.2024 15:53, Dmitry Baryshkov wrote: > On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL") >> introduced an entry to the alpha offsets array, but diving into QCM2290 >> downstream and some documentation, it turned out that the name Huayra >> apparently has been used quite liberally across many chips, even with >> noticeably different hardware. >> >> Introduce another set of offsets and a new configure function for the >> Huayra PLL found on QCM2290. This is required e.g. for the consumers >> of GPUCC_PLL0 to properly start. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/clk/qcom/clk-alpha-pll.c | 45 ++++++++++++++++++++++++++++++++++++++++ >> drivers/clk/qcom/clk-alpha-pll.h | 3 +++ >> 2 files changed, 48 insertions(+) >> >> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c >> index 8a412ef47e16..61b5abd13782 100644 >> --- a/drivers/clk/qcom/clk-alpha-pll.c >> +++ b/drivers/clk/qcom/clk-alpha-pll.c >> @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { >> [PLL_OFF_OPMODE] = 0x30, >> [PLL_OFF_STATUS] = 0x3c, >> }, >> + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = { >> + [PLL_OFF_L_VAL] = 0x04, >> + [PLL_OFF_ALPHA_VAL] = 0x08, >> + [PLL_OFF_USER_CTL] = 0x0c, >> + [PLL_OFF_CONFIG_CTL] = 0x10, >> + [PLL_OFF_CONFIG_CTL_U] = 0x14, >> + [PLL_OFF_CONFIG_CTL_U1] = 0x18, >> + [PLL_OFF_TEST_CTL] = 0x1c, >> + [PLL_OFF_TEST_CTL_U] = 0x20, >> + [PLL_OFF_TEST_CTL_U1] = 0x24, >> + [PLL_OFF_OPMODE] = 0x28, >> + [PLL_OFF_STATUS] = 0x38, >> + }, > > Can you please move them next to the standard huayra PLL regs? Sure, no prob Konrad
On Mon, 19 Feb 2024 at 17:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 19.02.2024 15:54, Andrew Halaney wrote: > > On Mon, Feb 19, 2024 at 02:35:48PM +0100, Konrad Dybcio wrote: > >> Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL") > >> introduced an entry to the alpha offsets array, but diving into QCM2290 > >> downstream and some documentation, it turned out that the name Huayra > >> apparently has been used quite liberally across many chips, even with > >> noticeably different hardware. > >> > >> Introduce another set of offsets and a new configure function for the > >> Huayra PLL found on QCM2290. This is required e.g. for the consumers > >> of GPUCC_PLL0 to properly start. > >> > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >> --- > >> drivers/clk/qcom/clk-alpha-pll.c | 45 ++++++++++++++++++++++++++++++++++++++++ > >> drivers/clk/qcom/clk-alpha-pll.h | 3 +++ > >> 2 files changed, 48 insertions(+) > >> > >> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > >> index 8a412ef47e16..61b5abd13782 100644 > >> --- a/drivers/clk/qcom/clk-alpha-pll.c > >> +++ b/drivers/clk/qcom/clk-alpha-pll.c > >> @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { > >> [PLL_OFF_OPMODE] = 0x30, > >> [PLL_OFF_STATUS] = 0x3c, > >> }, > >> + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = { > >> + [PLL_OFF_L_VAL] = 0x04, > >> + [PLL_OFF_ALPHA_VAL] = 0x08, > >> + [PLL_OFF_USER_CTL] = 0x0c, > >> + [PLL_OFF_CONFIG_CTL] = 0x10, > >> + [PLL_OFF_CONFIG_CTL_U] = 0x14, > >> + [PLL_OFF_CONFIG_CTL_U1] = 0x18, > >> + [PLL_OFF_TEST_CTL] = 0x1c, > >> + [PLL_OFF_TEST_CTL_U] = 0x20, > >> + [PLL_OFF_TEST_CTL_U1] = 0x24, > >> + [PLL_OFF_OPMODE] = 0x28, > >> + [PLL_OFF_STATUS] = 0x38, > >> + }, > >> }; > >> EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); > >> > >> @@ -779,6 +792,38 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate, > >> return clamp(rate, min_freq, max_freq); > >> } > >> > >> +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > >> + const struct alpha_pll_config *config) > >> +{ > >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val); > >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val); > >> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val); > >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val); > >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val); > >> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val); > >> + clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l); > >> + clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha); > >> + clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val); > >> + > >> + /* Set PLL_BYPASSNL */ > >> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL); > >> + > >> + /* Wait 5 us between setting BYPASS and deasserting reset */ > >> + mb(); > >> + udelay(5); > >> + > >> + /* Take PLL out from reset state */ > >> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); > >> + > >> + /* Wait 50us for PLL_LOCK_DET bit to go high */ > >> + mb(); > >> + usleep_range(50, 55); > > > > I *think* you'd want to use a read to ensure your write goes through > > prior to your sleep... from memory-barriers.txt: > > > > 5. A readX() by a CPU thread from the peripheral will complete before > > any subsequent delay() loop can begin execution on the same thread. > > This ensures that two MMIO register writes by the CPU to a peripheral > > will arrive at least 1us apart if the first write is immediately read > > back with readX() and udelay(1) is called prior to the second > > writeX(): > > > > writel(42, DEVICE_REGISTER_0); // Arrives at the device... > > readl(DEVICE_REGISTER_0); > > udelay(1); > > writel(42, DEVICE_REGISTER_1); // ...at least 1us before this. > > > > also https://youtu.be/i6DayghhA8Q?si=7lp0be35q1HRmlnV&t=1677 > > for more references on this topic. > > I mentioned this feels very iffy in the cover letter, but it's a combination > of two things: > > 1. i followed what qualcomm downstream code did > > 2. qualcomm downstream code is not known for being always correct > > > > I suppose a readback would be the correct solution, but then it should be > done for all similar calls in this driver. > > Although this code has shipped in literally hundreds of millions of devices > and it didn't explode badly :P (i'm not defending it, though) I think the idea behind the code looks pretty close to clk_alpha_pll_stromer_plus_set_rate(), which uses neither wmb() nor read-back.
On 19.02.2024 15:49, Dmitry Baryshkov wrote: > On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol). > > Is it not? Sure, every electronic device is also a heater, I suppose.. I found this wording extremely funny though > >> >> [1] https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > With the exception of the commit message: :( Konrad > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> --- >> arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 ++++++++ >> 1 file changed, 8 insertions(+) > >
On Mon, 19 Feb 2024 at 23:37, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 19.02.2024 15:49, Dmitry Baryshkov wrote: > > On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >> > >> Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol). > > > > Is it not? > > Sure, every electronic device is also a heater, I suppose.. I found > this wording extremely funny though 3D accelerator is a bit outdated term, but it's still valid. Well, unless using A702 makes 3D applications run slower than when using llvmpipe. > >> [1] https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > > > With the exception of the commit message: > > :( > > Konrad > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > >> --- > >> arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > > > >
On 19/02/2024 22:37, Konrad Dybcio wrote: > On 19.02.2024 15:49, Dmitry Baryshkov wrote: >> On Mon, 19 Feb 2024 at 15:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>> >>> Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol). >> >> Is it not? > > Sure, every electronic device is also a heater, I suppose.. I found > this wording extremely funny though GPU (Graphics Processing Unit) isn't accurate either since it can also run compute shaders do to non graphics processing, so 3D Accelerator isn't that different. A proper term would be Vectorized Mathematical Computing Complex or something similar. Neil > >> >>> >>> [1] https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> >> With the exception of the commit message: > > :( > > Konrad > >> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> >>> --- >>> arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >> >> >
On Mon, 19 Feb 2024 14:35:45 +0100, Konrad Dybcio wrote: > Bit of a megaseries, bunched together for your testing convenience.. > Needs mesa!27665 [1] on the userland part, kmscube happily spins. > > I'm feeling quite lukewarm about the memory barriers in patch 3.. > > Patch 1 for Will/smmu, 5-6 for drm/msm, rest for qcom > > [...] Applied SMMU bindings patch to will (for-joerg/arm-smmu/bindings), thanks! [1/8] dt-bindings: arm-smmu: Add QCM2290 GPU SMMU https://git.kernel.org/will/c/0eca305f8e0d Cheers,
Bit of a megaseries, bunched together for your testing convenience.. Needs mesa!27665 [1] on the userland part, kmscube happily spins. I'm feeling quite lukewarm about the memory barriers in patch 3.. Patch 1 for Will/smmu, 5-6 for drm/msm, rest for qcom [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27665 Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- Konrad Dybcio (8): dt-bindings: arm-smmu: Add QCM2290 GPU SMMU dt-bindings: clock: Add Qcom QCM2290 GPUCC clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support clk: qcom: Add QCM2290 GPU clock controller driver drm/msm/adreno: Add missing defines for A702 drm/msm/adreno: Add A702 support arm64: dts: qcom: qcm2290: Add GPU nodes arm64: dts: qcom: qrb2210-rb1: Enable the GPU .../bindings/clock/qcom,qcm2290-gpucc.yaml | 76 ++++ .../devicetree/bindings/iommu/arm,smmu.yaml | 3 +- arch/arm64/boot/dts/qcom/qcm2290.dtsi | 154 ++++++++ arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 + drivers/clk/qcom/Kconfig | 9 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-alpha-pll.c | 45 +++ drivers/clk/qcom/clk-alpha-pll.h | 3 + drivers/clk/qcom/gpucc-qcm2290.c | 423 +++++++++++++++++++++ drivers/gpu/drm/msm/adreno/a6xx.xml.h | 18 + drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 92 ++++- drivers/gpu/drm/msm/adreno/adreno_device.c | 18 + drivers/gpu/drm/msm/adreno/adreno_gpu.h | 16 +- include/dt-bindings/clock/qcom,qcm2290-gpucc.h | 32 ++ 14 files changed, 888 insertions(+), 10 deletions(-) --- base-commit: 26d7d52b6253574d5b6fec16a93e1110d1489cef change-id: 20240219-topic-rb1_gpu-3ec8c6830384 Best regards,