| Message ID | 20240219-topic-rb1_gpu-v1-0-d260fa854707@linaro.org |
|---|---|
| Headers | show |
| Series | A702 support | expand |
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 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 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,