Re: [PATCH v2 2/7] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
On 2/23/2024 1:21 PM, Konrad Dybcio wrote: > + /* Wait 50us for PLL_LOCK_DET bit to go high */ > + 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); Please use EXPORT_SYMBOL_GPL. -- ---Trilok Soni
Re: [PATCH v2 4/7] drm/msm/adreno: Add missing defines for A702
On Fri, 23 Feb 2024 at 23:21, Konrad Dybcio wrote: > > Add some defines required for A702. Can be substituted with a header > sync after merging mesa!27665 [1]. > > [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27665 > Signed-off-by: Konrad Dybcio > --- > drivers/gpu/drm/msm/adreno/a6xx.xml.h | 18 ++ > 1 file changed, 18 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 2/7] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
On Fri, 23 Feb 2024 at 23:21, 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
> ---
> drivers/clk/qcom/clk-alpha-pll.c | 47
>
> drivers/clk/qcom/clk-alpha-pll.h | 3 +++
> 2 files changed, 50 insertions(+)
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
[PATCH v2 7/7] arm64: dts: qcom: qrb2210-rb1: Enable the GPU
Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol).
[1]
https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index 6e9dd0312adc..c9abca5a7e39 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -199,6 +199,14 @@ _dma0 {
status = "okay";
};
+ {
+ status = "okay";
+
+ zap-shader {
+ firmware-name = "qcom/qcm2290/a702_zap.mbn";
+ };
+};
+
{
clock-frequency = <40>;
status = "okay";
--
2.43.2
[PATCH v2 4/7] drm/msm/adreno: Add missing defines for A702
Add some defines required for A702. Can be substituted with a header
sync after merging mesa!27665 [1].
[1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27665
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx.xml.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
index 863b5e3b0e67..1ec4dbc0e746 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
@@ -1945,6 +1945,24 @@ static inline uint32_t
REG_A6XX_RBBM_PERFCTR_RBBM_SEL(uint32_t i0) { return 0x00
#define REG_A6XX_RBBM_CLOCK_HYST_TEX_FCHE 0x0122
+#define REG_A6XX_RBBM_CLOCK_CNTL_FCHE 0x0123
+
+#define REG_A6XX_RBBM_CLOCK_DELAY_FCHE 0x0124
+
+#define REG_A6XX_RBBM_CLOCK_HYST_FCHE 0x0125
+
+#define REG_A6XX_RBBM_CLOCK_CNTL_MHUB 0x0126
+
+#define REG_A6XX_RBBM_CLOCK_DELAY_MHUB 0x0127
+
+#define REG_A6XX_RBBM_CLOCK_HYST_MHUB 0x0128
+
+#define REG_A6XX_RBBM_CLOCK_DELAY_GLC 0x0129
+
+#define REG_A6XX_RBBM_CLOCK_HYST_GLC 0x012a
+
+#define REG_A6XX_RBBM_CLOCK_CNTL_GLC 0x012b
+
#define REG_A7XX_RBBM_CLOCK_HYST2_VFD 0x012f
#define REG_A6XX_RBBM_LPAC_GBIF_CLIENT_QOS_CNTL
0x05ff
--
2.43.2
[PATCH v2 6/7] arm64: dts: qcom: qcm2290: Add GPU nodes
Describe the GPU hardware on the QCM2290.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/qcm2290.dtsi | 154 ++
1 file changed, 154 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
index 89beac833d43..ec5aef5d9c69 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -7,6 +7,7 @@
#include
#include
+#include
#include
#include
#include
@@ -737,6 +738,11 @@ qusb2_hstx_trim: hstx-trim@25b {
reg = <0x25b 0x1>;
bits = <1 4>;
};
+
+ gpu_speed_bin: gpu-speed-bin@2006 {
+ reg = <0x2006 0x2>;
+ bits = <5 8>;
+ };
};
pmu@1b8e300 {
@@ -1383,6 +1389,154 @@ usb_dwc3: usb@4e0 {
};
};
+ gpu: gpu@590 {
+ compatible = "qcom,adreno-07000200", "qcom,adreno";
+ reg = <0x0 0x0590 0x0 0x4>;
+ reg-names = "kgsl_3d0_reg_memory";
+
+ interrupts = ;
+
+ clocks = < GPU_CC_GX_GFX3D_CLK>,
+< GPU_CC_AHB_CLK>,
+< GCC_BIMC_GPU_AXI_CLK>,
+< GCC_GPU_MEMNOC_GFX_CLK>,
+< GPU_CC_CX_GMU_CLK>,
+< GPU_CC_CXO_CLK>;
+ clock-names = "core",
+ "iface",
+ "mem_iface",
+ "alt_mem_iface",
+ "gmu",
+ "xo";
+
+ interconnects = < MASTER_GFX3D RPM_ALWAYS_TAG
+ SLAVE_EBI1 RPM_ALWAYS_TAG>;
+ interconnect-names = "gfx-mem";
+
+ iommus = <_smmu 0 1>,
+<_smmu 2 0>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = < QCM2290_VDDCX>;
+ qcom,gmu = <_wrapper>;
+
+ nvmem-cells = <_speed_bin>;
+ nvmem-cell-names = "speed_bin";
+ #cooling-cells = <2>;
+
+ status = "disabled";
+
+ zap-shader {
+ memory-region = <_gpu_mem>;
+ };
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ /* TODO: Scale RPM_SMD_BIMC_GPU_CLK w/ turbo
freqs */
+ opp-112320 {
+ opp-hz = /bits/ 64 <112320>;
+ required-opps = <_opp_turbo_plus>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ turbo-mode;
+ };
+
+ opp-101760 {
+ opp-hz = /bits/ 64 <101760>;
+ required-opps = <_opp_turbo>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ turbo-mode;
+ };
+
+ opp-92160 {
+ opp-hz = /bits/ 64 <92160>;
+ required-opps = <_opp_nom_plus>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ };
+
+ opp-84480 {
+ opp-hz = /bits/ 64 <84480>;
+ required-opps = <_opp_nom>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x7>;
+ };
+
+ opp-67200 {
+ opp-hz = /bits/ 64 <67200>;
+ required-opps = <_opp_svs_plus>;
+ opp-peak-kBps = <3879000>;
+ opp-supported-hw = <0xf>;
+ };
+
+ opp-53760 {
+ opp-hz = /bits/ 64 <53760>;
+
[PATCH v2 5/7] drm/msm/adreno: Add A702 support
The A702 is a weird mix of 600 and 700 series.. Perhaps even a
testing ground for some A7xx features with good ol' A6xx silicon.
It's basically A610 that's been beefed up with some new registers
and hw features (like APRIV!), that was then cut back in size,
memory bus and some other ways.
Add support for it, tested with QCM2290 / RB1.
Signed-off-by: Konrad Dybcio
---
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 +-
3 files changed, 117 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c9c55e2ea584..2a491a486ca1 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -837,6 +837,65 @@ const struct adreno_reglist a690_hwcg[] = {
{}
};
+const struct adreno_reglist a702_hwcg[] = {
+ { REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0081 },
+ { REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xf3cf },
+ { REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001 },
+ { REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007 },
+ { REG_A6XX_RBBM_CLOCK_CNTL_RB0, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL2_RB0, 0x0120 },
+ { REG_A6XX_RBBM_CLOCK_CNTL_CCU0, 0x2220 },
+ { REG_A6XX_RBBM_CLOCK_HYST_RB_CCU0, 0x00040f00 },
+ { REG_A6XX_RBBM_CLOCK_CNTL_RAC, 0x05522022 },
+ { REG_A6XX_RBBM_CLOCK_CNTL2_RAC, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY_RAC, 0x0011 },
+ { REG_A6XX_RBBM_CLOCK_HYST_RAC, 0x00445044 },
+ { REG_A6XX_RBBM_CLOCK_CNTL_TSE_RAS_RBBM, 0x0422 },
+ { REG_A6XX_RBBM_CLOCK_MODE_VFD, 0x },
+ { REG_A6XX_RBBM_CLOCK_MODE_GPC, 0x0222 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_HLSQ_2, 0x0002 },
+ { REG_A6XX_RBBM_CLOCK_MODE_HLSQ, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY_TSE_RAS_RBBM, 0x4000 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_VFD, 0x },
+ { REG_A6XX_RBBM_CLOCK_DELAY_GPC, 0x0200 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_HLSQ, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_TSE_RAS_RBBM, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_VFD, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_GPC, 0x04104004 },
+ { REG_A6XX_RBBM_CLOCK_HYST_HLSQ, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL_UCHE, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_UCHE, 0x0004 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_UCHE, 0x0002 },
+ { REG_A6XX_RBBM_ISDB_CNT, 0x0182 },
+ { REG_A6XX_RBBM_RAC_THRESHOLD_CNT, 0x },
+ { REG_A6XX_RBBM_SP_HYST_CNT, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x0222 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x0111 },
+ { REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x0555 },
+ { REG_A6XX_RBBM_CLOCK_CNTL_FCHE, 0x0222 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_FCHE, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_FCHE, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL_GLC, 0x0022 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_GLC, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_GLC, 0x },
+ { REG_A6XX_RBBM_CLOCK_CNTL_MHUB, 0x0002 },
+ { REG_A6XX_RBBM_CLOCK_DELAY_MHUB, 0x },
+ { REG_A6XX_RBBM_CLOCK_HYST_MHUB, 0x },
+ {}
+};
+
const struct adreno_reglist a730_hwcg[] = {
{ REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x0222 },
{ REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0202 },
@@ -968,6 +1027,8 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
clock_cntl_on = 0x8aa8aa02;
else if (adreno_is_a610(adreno_gpu))
clock_cntl_on = 0xaaa8aa82;
+ else if (adreno_is_a702(adreno_gpu))
+ clock_cntl_on = 0xaa82;
else
clock_cntl_on = 0x8aa8aa82;
@@ -989,14 +1050,14 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool
state)
return;
/* Disable SP clock before programming HWCG registers */
- if (!adreno_is_a610(adreno_gpu) && !adreno_is_a7xx(adreno_gpu))
+ if (!adreno_is_a610_family(adreno_gpu) && !adreno_is_a7xx(adreno_gpu))
gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 1, 0);
for (i = 0; (reg = _gpu->info->hwcg[i],
[PATCH v2 2/7] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
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
---
drivers/clk/qcom/clk-alpha-pll.c | 47
drivers/clk/qcom/clk-alpha-pll.h | 3 +++
2 files changed, 50 insertions(+)
diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 8a412ef47e16..82b71f24ee7d 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -83,6 +83,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
[PLL_OFF_TEST_CTL_U] = 0x20,
[PLL_OFF_STATUS] = 0x24,
},
+ [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,
+ },
[CLK_ALPHA_PLL_TYPE_BRAMMO] = {
[PLL_OFF_L_VAL] = 0x04,
[PLL_OFF_ALPHA_VAL] = 0x08,
@@ -779,6 +792,40 @@ 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)
+{
+ u32 val;
+
+ 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);
+ regmap_read(regmap, PLL_MODE(pll), );
+
+ /* Wait 5 us between setting BYPASS and deasserting reset */
+ udelay(5);
+
+ /* Take PLL out from reset state */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
+ regmap_read(regmap, PLL_MODE(pll), );
+
+ /* Wait 50us for PLL_LOCK_DET bit to go high */
+ 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..d1cd52158c17 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -15,6 +15,7 @@
enum {
CLK_ALPHA_PLL_TYPE_DEFAULT,
CLK_ALPHA_PLL_TYPE_HUAYRA,
+ CLK_ALPHA_PLL_TYPE_HUAYRA_2290,
CLK_ALPHA_PLL_TYPE_BRAMMO,
CLK_ALPHA_PLL_TYPE_FABIA,
CLK_ALPHA_PLL_TYPE_TRION,
@@ -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
[PATCH v2 3/7] clk: qcom: Add QCM2290 GPU clock controller driver
Add a driver for the GPU clock controller block found on the QCM2290 SoC.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Konrad Dybcio
---
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile| 1 +
drivers/clk/qcom/gpucc-qcm2290.c | 423 +++
3 files changed, 433 insertions(+)
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 4580edbd13ea..d70ea4548755 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -65,6 +65,15 @@ config CLK_X1E80100_TCSRCC
Support for the TCSR clock controller on X1E80100 devices.
Say Y if you want to use peripheral devices such as SD/UFS.
+config CLK_QCM2290_GPUCC
+ tristate "QCM2290 Graphics Clock Controller"
+ depends on ARM64 || COMPILE_TEST
+ select CLK_QCM2290_GCC
+ help
+ Support for the graphics clock controller on QCM2290 devices.
+ Say Y if you want to support graphics controller devices and
+ functionality such as 3D graphics.
+
config QCOM_A53PLL
tristate "MSM8916 A53 PLL"
help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 1da65ca78e24..b8d49c054558 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CLK_X1E80100_DISPCC) += dispcc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_GCC) += gcc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
+obj-$(CONFIG_CLK_QCM2290_GPUCC) += gpucc-qcm2290.o
obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.o
obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
diff --git a/drivers/clk/qcom/gpucc-qcm2290.c b/drivers/clk/qcom/gpucc-qcm2290.c
new file mode 100644
index ..b6e20d63ac85
--- /dev/null
+++ b/drivers/clk/qcom/gpucc-qcm2290.c
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
+#include "clk-regmap-mux.h"
+#include "clk-regmap-phy-mux.h"
+#include "gdsc.h"
+#include "reset.h"
+
+enum {
+ DT_GCC_AHB_CLK,
+ DT_BI_TCXO,
+ DT_GCC_GPU_GPLL0_CLK_SRC,
+ DT_GCC_GPU_GPLL0_DIV_CLK_SRC,
+};
+
+enum {
+ P_BI_TCXO,
+ P_GPLL0_OUT_MAIN,
+ P_GPLL0_OUT_MAIN_DIV,
+ P_GPU_CC_PLL0_2X_DIV_CLK_SRC,
+ P_GPU_CC_PLL0_OUT_AUX,
+ P_GPU_CC_PLL0_OUT_AUX2,
+ P_GPU_CC_PLL0_OUT_MAIN,
+};
+
+static const struct pll_vco huayra_vco[] = {
+ { 6, 33, 0 },
+ { 6, 22, 1 },
+};
+
+static const struct alpha_pll_config gpu_cc_pll0_config = {
+ .l = 0x25,
+ .config_ctl_val = 0x200d4828,
+ .config_ctl_hi_val = 0x6,
+ .test_ctl_val = GENMASK(28, 26),
+ .test_ctl_hi_val = BIT(14),
+ .user_ctl_val = 0xf,
+};
+
+static struct clk_alpha_pll gpu_cc_pll0 = {
+ .offset = 0x0,
+ .vco_table = huayra_vco,
+ .num_vco = ARRAY_SIZE(huayra_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_HUAYRA_2290],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_pll0",
+ .parent_data = &(const struct clk_parent_data) {
+ .index = DT_BI_TCXO,
+ },
+ .num_parents = 1,
+ .ops = _alpha_pll_huayra_ops,
+ },
+ },
+};
+
+static const struct parent_map gpu_cc_parent_map_0[] = {
+ { P_BI_TCXO, 0 },
+ { P_GPU_CC_PLL0_OUT_MAIN, 1 },
+ { P_GPLL0_OUT_MAIN, 5 },
+ { P_GPLL0_OUT_MAIN_DIV, 6 },
+};
+
+static const struct clk_parent_data gpu_cc_parent_data_0[] = {
+ { .index = DT_BI_TCXO, },
+ { .hw = _cc_pll0.clkr.hw, },
+ { .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
+ { .index = DT_GCC_GPU_GPLL0_DIV_CLK_SRC, },
+};
+
+static const struct parent_map gpu_cc_parent_map_1[] = {
+ { P_BI_TCXO, 0 },
+ { P_GPU_CC_PLL0_2X_DIV_CLK_SRC, 1 },
+ { P_GPU_CC_PLL0_OUT_AUX2, 2 },
+ { P_GPU_CC_PLL0_OUT_AUX, 3 },
+ { P_GPLL0_OUT_MAIN, 5 },
+};
+
+static const struct clk_parent_data gpu_cc_parent_data_1[] = {
+ { .index = DT_BI_TCXO, },
+ { .hw = _cc_pll0.clkr.hw, },
+ { .hw = _cc_pll0.clkr.hw, },
+ { .hw = _cc_pll0.clkr.hw, },
+ { .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
+};
+
+static const struct freq_tbl ftbl_gpu_cc_gmu_clk_src[] = {
+ F(2, P_GPLL0_OUT_MAIN, 3, 0, 0),
+ { }
+};
+
+static struct clk_rcg2 gpu_cc_gmu_clk_src = {
+ .cmd_rcgr = 0x1120,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map =
[PATCH v2 1/7] dt-bindings: clock: Add Qcom QCM2290 GPUCC
Add device tree bindings for graphics clock controller for Qualcomm
Technology Inc's QCM2290 SoCs.
Signed-off-by: Konrad Dybcio
---
.../bindings/clock/qcom,qcm2290-gpucc.yaml | 77 ++
include/dt-bindings/clock/qcom,qcm2290-gpucc.h | 32 +
2 files changed, 109 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
new file mode 100644
index ..734880805c1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,qcm2290-gpucc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Graphics Clock & Reset Controller on QCM2290
+
+maintainers:
+ - Konrad Dybcio
+
+description: |
+ Qualcomm graphics clock control module provides the clocks, resets and power
+ domains on Qualcomm SoCs.
+
+ See also::
+include/dt-bindings/clock/qcom,qcm2290-gpucc.h
+
+properties:
+ compatible:
+const: qcom,qcm2290-gpucc
+
+ reg:
+maxItems: 1
+
+ clocks:
+items:
+ - description: AHB interface clock,
+ - description: SoC CXO clock
+ - description: GPLL0 main branch source
+ - description: GPLL0 div branch source
+
+ power-domains:
+description:
+ A phandle and PM domain specifier for the CX power domain.
+maxItems: 1
+
+ required-opps:
+description:
+ A phandle to an OPP node describing required CX performance point.
+maxItems: 1
+
+required:
+ - compatible
+ - clocks
+ - power-domains
+
+allOf:
+ - $ref: qcom,gcc.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+
+soc {
+#address-cells = <2>;
+#size-cells = <2>;
+
+clock-controller@599 {
+compatible = "qcom,qcm2290-gpucc";
+reg = <0x0 0x0599 0x0 0x9000>;
+clocks = < GCC_GPU_CFG_AHB_CLK>,
+ < RPM_SMD_XO_CLK_SRC>,
+ < GCC_GPU_GPLL0_CLK_SRC>,
+ < GCC_GPU_GPLL0_DIV_CLK_SRC>;
+power-domains = < QCM2290_VDDCX>;
+required-opps = <_opp_low_svs>;
+#clock-cells = <1>;
+#reset-cells = <1>;
+#power-domain-cells = <1>;
+};
+};
+...
diff --git a/include/dt-bindings/clock/qcom,qcm2290-gpucc.h
b/include/dt-bindings/clock/qcom,qcm2290-gpucc.h
new file mode 100644
index ..7c76dd05278f
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,qcm2290-gpucc.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_GPU_CC_QCM2290_H
+#define _DT_BINDINGS_CLK_QCOM_GPU_CC_QCM2290_H
+
+/* GPU_CC clocks */
+#define GPU_CC_AHB_CLK 0
+#define GPU_CC_CRC_AHB_CLK 1
+#define GPU_CC_CX_GFX3D_CLK2
+#define GPU_CC_CX_GMU_CLK 3
+#define GPU_CC_CX_SNOC_DVM_CLK 4
+#define GPU_CC_CXO_AON_CLK 5
+#define GPU_CC_CXO_CLK 6
+#define GPU_CC_GMU_CLK_SRC 7
+#define GPU_CC_GX_GFX3D_CLK8
+#define GPU_CC_GX_GFX3D_CLK_SRC9
+#define GPU_CC_PLL010
+#define GPU_CC_SLEEP_CLK 11
+#define GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK 12
+
+/* Resets */
+#define GPU_GX_BCR 0
+
+/* GDSCs */
+#define GPU_CX_GDSC0
+#define GPU_GX_GDSC1
+
+#endif
--
2.43.2
[PATCH v2 0/7] A702 support
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 --- Changes in v2: - Drop applied smmu-bindings patch - Fix the gpucc bindings patch to be even better - Reorder HUAYRA_2290 definitions near HUAYRA (..Add HUAYRA_2290 support..) - Replace weird memory barriers copypasted from msm-5.4 with readback to ensure timely write completion (..Add HUAYRA_2290 support..) - Keep my super amazing commit message referencing the 3D accelerator official naming (dts) - Pick up tags - Link to v1: https://lore.kernel.org/r/20240219-topic-rb1_gpu-v1-0-d260fa854...@linaro.org --- Konrad Dybcio (7): 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 | 77 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 | 47 +++ 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 ++ 13 files changed, 889 insertions(+), 9 deletions(-) --- base-commit: 26d7d52b6253574d5b6fec16a93e1110d1489cef change-id: 20240219-topic-rb1_gpu-3ec8c6830384 Best regards, -- Konrad Dybcio
Re: [PATCH] drm/msm/dpu: add support for 4:2:2 and 4:4:4 planar YCbCr plane formats
On Fri, 23 Feb 2024 at 22:48, Abhinav Kumar wrote:
>
>
>
> On 2/22/2024 3:43 AM, Dmitry Baryshkov wrote:
> > The DPU driver provides support for 4:2:0 planar YCbCr plane formats.
> > Extend it to also support 4:2:2 and 4:4:4 plat formats.
> >
>
> I checked myself and also internally on this. On sm8250, the DPU planes
> do not support YUV444 and YUV422 (and the corresponding YVU formats).
>
> May I know what was the reference to add these formats to DPU
> considering that even downstream sources didn't add them?
No reference. I was interested in checking different YUV formats for
the test. It worked, so I wanted to discuss this. In the end, we are
just changing several bits, which are used for other formats.
>
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > Full-screen (1080p@60) YV24 gave me underruns on SM8250 until I bumped
> > the clock inefficiency factor from 105 to 117. I'm not sure that it is a
> > correct way to handle it, so I'm sending this as an RFC. If we agree
> > that bumping the .clk_inefficiency_factor is a correct way, I'll send
> > v2, including catalog changes.
> >
> > I had no such issues for the YV16/YU16 formats.
>
> We don't support this too on sm8250. But interesting it worked.
As I wrote, YV24 also works if I slightly bump the clock inefficiency.
I think this points out that maybe we should calculate clock factor
dynamically.
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 24
> >
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > index e366ab134249..1b763cd95e5a 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > @@ -475,6 +475,30 @@ static const struct dpu_format dpu_format_map[] = {
> > C1_B_Cb, C2_R_Cr, C0_G_Y,
> > false, DPU_CHROMA_420, 1, DPU_FORMAT_FLAG_YUV,
> > DPU_FETCH_LINEAR, 3),
> > +
> > + PLANAR_YUV_FMT(YUV422,
> > + 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> > + C2_R_Cr, C1_B_Cb, C0_G_Y,
> > + false, DPU_CHROMA_H2V1, 1, DPU_FORMAT_FLAG_YUV,
> > + DPU_FETCH_LINEAR, 3),
> > +
> > + PLANAR_YUV_FMT(YVU422,
> > + 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> > + C1_B_Cb, C2_R_Cr, C0_G_Y,
> > + false, DPU_CHROMA_H2V1, 1, DPU_FORMAT_FLAG_YUV,
> > + DPU_FETCH_LINEAR, 3),
> > +
> > + PLANAR_YUV_FMT(YUV444,
> > + 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> > + C2_R_Cr, C1_B_Cb, C0_G_Y,
> > + false, DPU_CHROMA_RGB, 1, DPU_FORMAT_FLAG_YUV,
> > + DPU_FETCH_LINEAR, 3),
> > +
> > + PLANAR_YUV_FMT(YVU444,
> > + 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> > + C1_B_Cb, C2_R_Cr, C0_G_Y,
> > + false, DPU_CHROMA_RGB, 1, DPU_FORMAT_FLAG_YUV,
> > + DPU_FETCH_LINEAR, 3),
> > };
> >
> > /*
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > index ccbee0f40ad7..949c86a44ec7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -195,6 +195,10 @@ static const uint32_t plane_formats_yuv[] = {
> > DRM_FORMAT_YVYU,
> > DRM_FORMAT_YUV420,
> > DRM_FORMAT_YVU420,
> > + DRM_FORMAT_YUV422,
> > + DRM_FORMAT_YVU422,
> > + DRM_FORMAT_YUV444,
> > + DRM_FORMAT_YVU444,
> > };
> >
> > static const u32 rotation_v2_formats[] = {
> >
> > ---
> > base-commit: ffa0c87f172bf7a0132aa960db412f8d63b2f533
> > change-id: 20240222-fd-dpu-yv16-yv24-6bf152dfa7f3
> >
> > Best regards,
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/dpu: add support for 4:2:2 and 4:4:4 planar YCbCr plane formats
On 2/22/2024 3:43 AM, Dmitry Baryshkov wrote:
The DPU driver provides support for 4:2:0 planar YCbCr plane formats.
Extend it to also support 4:2:2 and 4:4:4 plat formats.
I checked myself and also internally on this. On sm8250, the DPU planes
do not support YUV444 and YUV422 (and the corresponding YVU formats).
May I know what was the reference to add these formats to DPU
considering that even downstream sources didn't add them?
Signed-off-by: Dmitry Baryshkov
---
Full-screen (1080p@60) YV24 gave me underruns on SM8250 until I bumped
the clock inefficiency factor from 105 to 117. I'm not sure that it is a
correct way to handle it, so I'm sending this as an RFC. If we agree
that bumping the .clk_inefficiency_factor is a correct way, I'll send
v2, including catalog changes.
I had no such issues for the YV16/YU16 formats.
We don't support this too on sm8250. But interesting it worked.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 24
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index e366ab134249..1b763cd95e5a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -475,6 +475,30 @@ static const struct dpu_format dpu_format_map[] = {
C1_B_Cb, C2_R_Cr, C0_G_Y,
false, DPU_CHROMA_420, 1, DPU_FORMAT_FLAG_YUV,
DPU_FETCH_LINEAR, 3),
+
+ PLANAR_YUV_FMT(YUV422,
+ 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+ C2_R_Cr, C1_B_Cb, C0_G_Y,
+ false, DPU_CHROMA_H2V1, 1, DPU_FORMAT_FLAG_YUV,
+ DPU_FETCH_LINEAR, 3),
+
+ PLANAR_YUV_FMT(YVU422,
+ 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+ C1_B_Cb, C2_R_Cr, C0_G_Y,
+ false, DPU_CHROMA_H2V1, 1, DPU_FORMAT_FLAG_YUV,
+ DPU_FETCH_LINEAR, 3),
+
+ PLANAR_YUV_FMT(YUV444,
+ 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+ C2_R_Cr, C1_B_Cb, C0_G_Y,
+ false, DPU_CHROMA_RGB, 1, DPU_FORMAT_FLAG_YUV,
+ DPU_FETCH_LINEAR, 3),
+
+ PLANAR_YUV_FMT(YVU444,
+ 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+ C1_B_Cb, C2_R_Cr, C0_G_Y,
+ false, DPU_CHROMA_RGB, 1, DPU_FORMAT_FLAG_YUV,
+ DPU_FETCH_LINEAR, 3),
};
/*
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index ccbee0f40ad7..949c86a44ec7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -195,6 +195,10 @@ static const uint32_t plane_formats_yuv[] = {
DRM_FORMAT_YVYU,
DRM_FORMAT_YUV420,
DRM_FORMAT_YVU420,
+ DRM_FORMAT_YUV422,
+ DRM_FORMAT_YVU422,
+ DRM_FORMAT_YUV444,
+ DRM_FORMAT_YVU444,
};
static const u32 rotation_v2_formats[] = {
---
base-commit: ffa0c87f172bf7a0132aa960db412f8d63b2f533
change-id: 20240222-fd-dpu-yv16-yv24-6bf152dfa7f3
Best regards,
Re: [PATCH] drm: ci: uprev IGT
On Wed, Feb 21, 2024 at 6:36 PM Dmitry Baryshkov wrote: > > On Tue, 20 Feb 2024 at 16:31, Helen Koike wrote: > > > > > > > > On 20/02/2024 09:17, Dmitry Baryshkov wrote: > > > Bump IGT revision to pick up Rob Clark's fixes for the msm driver: > > > > > > - msm_submit@invalid-duplicate-bo-submit,Fail > > > > > > Signed-off-by: Dmitry Baryshkov > > > > Do you have a gitlab pipeline link I can check? > > Before uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109455 > > After uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109501 jfyi a couple more fixes landed after this, for kms_plane_cursor (skips->pass) and kms_universal_plane (fail->pass).. I have additional fixes for kms_bw, and kms_plane_scaling still waiting for review BR, -R
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On 2/23/2024 9:56 AM, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > [ >This is a treewide change. I will likely re-create this patch again in >the second week of the merge window of v6.9 and submit it then. Hoping >to keep the conflicts that it will cause to a minimum. > ] > > With the rework of how the __string() handles dynamic strings where it > saves off the source string in field in the helper structure[1], the > assignment of that value to the trace event field is stored in the helper > value and does not need to be passed in again. Just curious if this could be done piecemeal by first changing the macros to be variadic macros which allows you to ignore the extra argument. The callers could then be modified in their separate trees. And then once all the callers have be merged, the macros could be changed to no longer be variadic.
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, 23 Feb 2024 at 16:55, Neil Armstrong wrote: > > On 23/02/2024 15:52, Johan Hovold wrote: > > On Fri, Feb 23, 2024 at 03:38:13PM +0100, Neil Armstrong wrote: > >> On 23/02/2024 15:21, Johan Hovold wrote: > > > >>> But it is *not* standalone as I tried to explain above. > >>> > >>> So you have to drop it again as the later patches depend on it and > >>> cannot be merged (through a different tree) without it. > >> > >> drm-misc branches cannot be rebased, it must be reverted, but it can still > >> be applied > >> on drm-misc-next and I'll send a revert patch for drm-misc-fixes if > >> needed, not a big deal. > >> > >>> I thought you had all the acks you needed to take this through drm-misc, > >>> but we can wait a bit more if necessary (and there's no rush to get the > >>> first one in). > >> > >> If you want it to be in v6.9, it's too late since the last drm-misc-next > >> PR has been sent > >> yesterday > >> (https://cgit.freedesktop.org/drm/drm-misc/tag/?h=drm-misc-next-2024-02-22) > >> > >> Please ping Thomas or Maxime, perhaps it's not too late since the > >> drm-misc-next tree > >> really closes on sunday. > > > > I don't want this in 6.9, this is needed for *6.8* as this fixes a DRM > > regression in 6.8-rc1 that breaks the display on machines like the X13s. > > > > If you guys can't sort this out in time, then perhaps Bjorn can take > > this through the Qualcomm tree instead (with DRM acks). > > > > But again, this is fixing a severe *regression* in 6.8-rc1. It can not > > wait for 6.9. > > Right, I can't apply them right now, I send a patchset ack so it can be > applied ASAP, Applied and pushed patches 2-4. Patches 5 and 6 can go through the phy/fixes. There is no need for them to go through drm-misc tree. -- With best wishes Dmitry
Re: (subset) [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: > Starting with 6.8-rc1 the internal display sometimes fails to come up on > machines like the Lenovo ThinkPad X13s and the logs indicate that this > is due to a regression in the DRM subsystem [1]. > > This series fixes a race in the pmic_glink_altmode driver which was > exposed / triggered by the transparent DRM bridges rework that went into > 6.8-rc1 and that manifested itself as a bridge failing to attach and > sometimes triggering a NULL-pointer dereference. > > [...] Applied to drm-misc-fixes, thanks! [2/6] drm/bridge: aux-hpd: separate allocation and registration commit: e5ca263508f7e9d2cf711edf3258d11ca087885c [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free commit: b979f2d50a099f3402418d7ff5f26c3952fb08bb [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m commit: f79ee78767ca60e7a2c89eacd2dbdf237d97e838 Note, PHY patches (5,6) do not have dependency on the drm patch, so they can go through the phy/fixes tree. Best regards, -- Dmitry Baryshkov
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On 23/02/2024 15:52, Johan Hovold wrote: On Fri, Feb 23, 2024 at 03:38:13PM +0100, Neil Armstrong wrote: On 23/02/2024 15:21, Johan Hovold wrote: But it is *not* standalone as I tried to explain above. So you have to drop it again as the later patches depend on it and cannot be merged (through a different tree) without it. drm-misc branches cannot be rebased, it must be reverted, but it can still be applied on drm-misc-next and I'll send a revert patch for drm-misc-fixes if needed, not a big deal. I thought you had all the acks you needed to take this through drm-misc, but we can wait a bit more if necessary (and there's no rush to get the first one in). If you want it to be in v6.9, it's too late since the last drm-misc-next PR has been sent yesterday (https://cgit.freedesktop.org/drm/drm-misc/tag/?h=drm-misc-next-2024-02-22) Please ping Thomas or Maxime, perhaps it's not too late since the drm-misc-next tree really closes on sunday. I don't want this in 6.9, this is needed for *6.8* as this fixes a DRM regression in 6.8-rc1 that breaks the display on machines like the X13s. If you guys can't sort this out in time, then perhaps Bjorn can take this through the Qualcomm tree instead (with DRM acks). But again, this is fixing a severe *regression* in 6.8-rc1. It can not wait for 6.9. Right, I can't apply them right now, I send a patchset ack so it can be applied ASAP, Thanks, Neil Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On 17/02/2024 16:02, Johan Hovold wrote: Starting with 6.8-rc1 the internal display sometimes fails to come up on machines like the Lenovo ThinkPad X13s and the logs indicate that this is due to a regression in the DRM subsystem [1]. This series fixes a race in the pmic_glink_altmode driver which was exposed / triggered by the transparent DRM bridges rework that went into 6.8-rc1 and that manifested itself as a bridge failing to attach and sometimes triggering a NULL-pointer dereference. The intermittent hard resets that have also been reported since 6.8-rc1 unfortunately still remains and suggests that we are dealing with two separate regressions. There is some indication that also the hard resets (e.g. due to register accesses to unclocked hardware) are also due to changes in the DRM subsystem as it happens around the time that the eDP panel and display controller would be initialised during boot (the runtime PM rework?). This remains to be verified, however. Included is also a fix for a related OF node reference leak in the aux-hpd driver found through inspection when reworking the driver. The use-after-free bug is triggered by a probe deferral and highlighted some further bugs in the involved drivers, which were registering child devices before deferring probe. This behaviour is not correct and can both trigger probe deferral loops and potentially also further issues with the DRM bridge implementation. This series can either go through the Qualcomm SoC tree (pmic_glink) or the DRM tree. The PHY patches do not depend on the rest of the series and could possibly be merged separately through the PHY tree. Whichever gets this to mainline the fastest. Johan [1] https://lore.kernel.org/lkml/zctvmlk4ztwcp...@hovoldconsulting.com/ Johan Hovold (5): drm/bridge: aux-hpd: fix OF node leaks drm/bridge: aux-hpd: separate allocation and registration soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free phy: qcom-qmp-combo: fix drm bridge registration phy: qcom-qmp-combo: fix type-c switch registration Rob Clark (1): soc: qcom: pmic_glink: Fix boot when QRTR=m drivers/gpu/drm/bridge/aux-hpd-bridge.c | 70 ++- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 16 +++--- drivers/soc/qcom/pmic_glink.c | 21 +++ drivers/soc/qcom/pmic_glink_altmode.c | 16 +- include/drm/bridge/aux-bridge.h | 15 + 5 files changed, 102 insertions(+), 36 deletions(-) For the serie: Acked-by: Neil Armstrong After an offline discussion, Dmitry, it's ok to push the remaining patches to drm-misc-fixes. Thanks, Neil
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, Feb 23, 2024 at 03:38:13PM +0100, Neil Armstrong wrote: > On 23/02/2024 15:21, Johan Hovold wrote: > > But it is *not* standalone as I tried to explain above. > > > > So you have to drop it again as the later patches depend on it and > > cannot be merged (through a different tree) without it. > > drm-misc branches cannot be rebased, it must be reverted, but it can still be > applied > on drm-misc-next and I'll send a revert patch for drm-misc-fixes if needed, > not a big deal. > > > I thought you had all the acks you needed to take this through drm-misc, > > but we can wait a bit more if necessary (and there's no rush to get the > > first one in). > > If you want it to be in v6.9, it's too late since the last drm-misc-next PR > has been sent > yesterday > (https://cgit.freedesktop.org/drm/drm-misc/tag/?h=drm-misc-next-2024-02-22) > > Please ping Thomas or Maxime, perhaps it's not too late since the > drm-misc-next tree > really closes on sunday. I don't want this in 6.9, this is needed for *6.8* as this fixes a DRM regression in 6.8-rc1 that breaks the display on machines like the X13s. If you guys can't sort this out in time, then perhaps Bjorn can take this through the Qualcomm tree instead (with DRM acks). But again, this is fixing a severe *regression* in 6.8-rc1. It can not wait for 6.9. Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On 23/02/2024 15:21, Johan Hovold wrote: On Fri, Feb 23, 2024 at 02:52:28PM +0100, Neil Armstrong wrote: On 23/02/2024 13:51, Johan Hovold wrote: On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: On 23/02/2024 12:02, Neil Armstrong wrote: Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/6] drm/bridge: aux-hpd: fix OF node leaks https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b [2/6] drm/bridge: aux-hpd: separate allocation and registration (no commit info) [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free (no commit info) [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m (no commit info) [5/6] phy: qcom-qmp-combo: fix drm bridge registration (no commit info) [6/6] phy: qcom-qmp-combo: fix type-c switch registration (no commit info) To clarify, I only applied patch 1 to drm-misc-fixes Ok, but can you please not do that? :) These patches should go in through the same tree to avoid conflicts. I discussed this with Bjorn and Dmitry the other day and the conclusion was that it was easiest to take all of these through DRM. I only applied patch 1, which is a standalone fix and goes into a separate tree, for the next patches it would be indeed simpler for them to go via drm-misc when they are properly acked. But it is *not* standalone as I tried to explain above. So you have to drop it again as the later patches depend on it and cannot be merged (through a different tree) without it. drm-misc branches cannot be rebased, it must be reverted, but it can still be applied on drm-misc-next and I'll send a revert patch for drm-misc-fixes if needed, not a big deal. I thought you had all the acks you needed to take this through drm-misc, but we can wait a bit more if necessary (and there's no rush to get the first one in). If you want it to be in v6.9, it's too late since the last drm-misc-next PR has been sent yesterday (https://cgit.freedesktop.org/drm/drm-misc/tag/?h=drm-misc-next-2024-02-22) Please ping Thomas or Maxime, perhaps it's not too late since the drm-misc-next tree really closes on sunday. Neil Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, Feb 23, 2024 at 04:18:08PM +0200, Dmitry Baryshkov wrote: > On Fri, 23 Feb 2024 at 15:52, Neil Armstrong > wrote: > > On 23/02/2024 13:51, Johan Hovold wrote: > > > On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: > > >> On 23/02/2024 12:02, Neil Armstrong wrote: > > >>> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git > > >>> (drm-misc-fixes) > > >>> > > >>> [1/6] drm/bridge: aux-hpd: fix OF node leaks > > >>> > > >>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b > > >>> [2/6] drm/bridge: aux-hpd: separate allocation and registration > > >>> (no commit info) > > >>> [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free > > >>> (no commit info) > > >>> [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m > > >>> (no commit info) > > >>> [5/6] phy: qcom-qmp-combo: fix drm bridge registration > > >>> (no commit info) > > >>> [6/6] phy: qcom-qmp-combo: fix type-c switch registration > > >>> (no commit info) > > >>> > > >> > > >> To clarify, I only applied patch 1 to drm-misc-fixes > > > > > > Ok, but can you please not do that? :) > > > > > > These patches should go in through the same tree to avoid conflicts. > > > > > > I discussed this with Bjorn and Dmitry the other day and the conclusion > > > was that it was easiest to take all of these through DRM. > > > > I only applied patch 1, which is a standalone fix and goes into a separate > > tree, > > for the next patches it would be indeed simpler for them to go via drm-misc > > when > > they are properly acked. > > I think PHY patches can go through a usual route (phy/next or > phy/fixes). They can, but I've explicitly asked Vinod to ack them so that they can go in with the rest of the series through DRM. They also fix a regression that came in through DRM in 6.8-rc1 (the bridge rework which started registering child devices) so it makes sense to also route the fix the same way. And to do it for this cycle. > For patches 3 and 4 I'd need an ack from Bjorn to merge > them through drm-misc-next or drm-misc-fixes. You have Bjorn's ack. He's reviewed all the patches for this purpose and we discussed this in person two days ago. And, again, this has to go in for *this* cycle. You broke the display on the X13s and other machines so this cannot wait for 6.9. Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, Feb 23, 2024 at 02:52:28PM +0100, Neil Armstrong wrote: > On 23/02/2024 13:51, Johan Hovold wrote: > > On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: > >> On 23/02/2024 12:02, Neil Armstrong wrote: > >>> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git > >>> (drm-misc-fixes) > >>> > >>> [1/6] drm/bridge: aux-hpd: fix OF node leaks > >>> > >>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b > >>> [2/6] drm/bridge: aux-hpd: separate allocation and registration > >>> (no commit info) > >>> [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free > >>> (no commit info) > >>> [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m > >>> (no commit info) > >>> [5/6] phy: qcom-qmp-combo: fix drm bridge registration > >>> (no commit info) > >>> [6/6] phy: qcom-qmp-combo: fix type-c switch registration > >>> (no commit info) > >>> > >> > >> To clarify, I only applied patch 1 to drm-misc-fixes > > > > Ok, but can you please not do that? :) > > > > These patches should go in through the same tree to avoid conflicts. > > > > I discussed this with Bjorn and Dmitry the other day and the conclusion > > was that it was easiest to take all of these through DRM. > > I only applied patch 1, which is a standalone fix and goes into a separate > tree, > for the next patches it would be indeed simpler for them to go via drm-misc > when > they are properly acked. But it is *not* standalone as I tried to explain above. So you have to drop it again as the later patches depend on it and cannot be merged (through a different tree) without it. I thought you had all the acks you needed to take this through drm-misc, but we can wait a bit more if necessary (and there's no rush to get the first one in). Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, 23 Feb 2024 at 15:52, Neil Armstrong wrote: > > On 23/02/2024 13:51, Johan Hovold wrote: > > On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: > >> On 23/02/2024 12:02, Neil Armstrong wrote: > >>> Hi, > >>> > >>> On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: > Starting with 6.8-rc1 the internal display sometimes fails to come up on > machines like the Lenovo ThinkPad X13s and the logs indicate that this > is due to a regression in the DRM subsystem [1]. > > This series fixes a race in the pmic_glink_altmode driver which was > exposed / triggered by the transparent DRM bridges rework that went into > 6.8-rc1 and that manifested itself as a bridge failing to attach and > sometimes triggering a NULL-pointer dereference. > > [...] > >>> > >>> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git > >>> (drm-misc-fixes) > >>> > >>> [1/6] drm/bridge: aux-hpd: fix OF node leaks > >>> > >>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b > >>> [2/6] drm/bridge: aux-hpd: separate allocation and registration > >>> (no commit info) > >>> [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free > >>> (no commit info) > >>> [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m > >>> (no commit info) > >>> [5/6] phy: qcom-qmp-combo: fix drm bridge registration > >>> (no commit info) > >>> [6/6] phy: qcom-qmp-combo: fix type-c switch registration > >>> (no commit info) > >>> > >> > >> To clarify, I only applied patch 1 to drm-misc-fixes > > > > Ok, but can you please not do that? :) > > > > These patches should go in through the same tree to avoid conflicts. > > > > I discussed this with Bjorn and Dmitry the other day and the conclusion > > was that it was easiest to take all of these through DRM. > > I only applied patch 1, which is a standalone fix and goes into a separate > tree, > for the next patches it would be indeed simpler for them to go via drm-misc > when > they are properly acked. I think PHY patches can go through a usual route (phy/next or phy/fixes). For patches 3 and 4 I'd need an ack from Bjorn to merge them through drm-misc-next or drm-misc-fixes. -- With best wishes Dmitry
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On 23/02/2024 13:51, Johan Hovold wrote: On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: On 23/02/2024 12:02, Neil Armstrong wrote: Hi, On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: Starting with 6.8-rc1 the internal display sometimes fails to come up on machines like the Lenovo ThinkPad X13s and the logs indicate that this is due to a regression in the DRM subsystem [1]. This series fixes a race in the pmic_glink_altmode driver which was exposed / triggered by the transparent DRM bridges rework that went into 6.8-rc1 and that manifested itself as a bridge failing to attach and sometimes triggering a NULL-pointer dereference. [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/6] drm/bridge: aux-hpd: fix OF node leaks https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b [2/6] drm/bridge: aux-hpd: separate allocation and registration (no commit info) [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free (no commit info) [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m (no commit info) [5/6] phy: qcom-qmp-combo: fix drm bridge registration (no commit info) [6/6] phy: qcom-qmp-combo: fix type-c switch registration (no commit info) To clarify, I only applied patch 1 to drm-misc-fixes Ok, but can you please not do that? :) These patches should go in through the same tree to avoid conflicts. I discussed this with Bjorn and Dmitry the other day and the conclusion was that it was easiest to take all of these through DRM. I only applied patch 1, which is a standalone fix and goes into a separate tree, for the next patches it would be indeed simpler for them to go via drm-misc when they are properly acked. Neil With Vinod acking the PHY patches, I believe you have what you need to merge the whole series now? Johan
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Fri, Feb 23, 2024 at 12:03:10PM +0100, Neil Armstrong wrote: > On 23/02/2024 12:02, Neil Armstrong wrote: > > Hi, > > > > On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: > >> Starting with 6.8-rc1 the internal display sometimes fails to come up on > >> machines like the Lenovo ThinkPad X13s and the logs indicate that this > >> is due to a regression in the DRM subsystem [1]. > >> > >> This series fixes a race in the pmic_glink_altmode driver which was > >> exposed / triggered by the transparent DRM bridges rework that went into > >> 6.8-rc1 and that manifested itself as a bridge failing to attach and > >> sometimes triggering a NULL-pointer dereference. > >> > >> [...] > > > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git > > (drm-misc-fixes) > > > > [1/6] drm/bridge: aux-hpd: fix OF node leaks > > > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b > > [2/6] drm/bridge: aux-hpd: separate allocation and registration > >(no commit info) > > [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free > >(no commit info) > > [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m > >(no commit info) > > [5/6] phy: qcom-qmp-combo: fix drm bridge registration > >(no commit info) > > [6/6] phy: qcom-qmp-combo: fix type-c switch registration > >(no commit info) > > > > To clarify, I only applied patch 1 to drm-misc-fixes Ok, but can you please not do that? :) These patches should go in through the same tree to avoid conflicts. I discussed this with Bjorn and Dmitry the other day and the conclusion was that it was easiest to take all of these through DRM. With Vinod acking the PHY patches, I believe you have what you need to merge the whole series now? Johan
Re: [PATCH 2/6] drm/bridge: aux-hpd: separate allocation and registration
On Thu, Feb 22, 2024 at 10:57:07PM +0200, Dmitry Baryshkov wrote: > On Sat, 17 Feb 2024 at 17:03, Johan Hovold wrote: > > > > Combining allocation and registration is an anti-pattern that should be > > avoided. Add two new functions for allocating and registering an dp-hpd > > bridge with a proper 'devm' prefix so that it is clear that these are > > device managed interfaces. > > > > devm_drm_dp_hpd_bridge_alloc() > > devm_drm_dp_hpd_bridge_add() > > > > The new interface will be used to fix a use-after-free bug in the > > Qualcomm PMIC GLINK driver and may prevent similar issues from being > > introduced elsewhere. > > > > The existing drm_dp_hpd_bridge_register() is reimplemented using the > > above and left in place for now. > > > > Signed-off-by: Johan Hovold > > Reviewed-by: Dmitry Baryshkov Thanks for reviewing. > Minor nit below. > > diff --git a/include/drm/bridge/aux-bridge.h > > b/include/drm/bridge/aux-bridge.h > > index c4c423e97f06..4453906105ca 100644 > > --- a/include/drm/bridge/aux-bridge.h > > +++ b/include/drm/bridge/aux-bridge.h > > @@ -9,6 +9,8 @@ > > > > #include > > > > +struct auxiliary_device; > > + > > #if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE) > > int drm_aux_bridge_register(struct device *parent); > > #else > > @@ -19,10 +21,23 @@ static inline int drm_aux_bridge_register(struct device > > *parent) > > #endif > > > > #if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE) > > +struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device > > *parent, struct device_node *np); > > +int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device > > *adev); > > I had a pretty close idea during prototyping, but I ended up doing it > as a single function for the following reasons: > > First, this exports the implementation detail that internally the code > uses an aux device. That's not an issue. The opposite, with interfaces trying to do too much and hide details from the developers so that they can no longer reason about what is going on, is a real problem though. > Also, by exporting the aux device the code becomes less type-safe. By > mistake one can call devm_drm_dp_hpd_bridge_add() on any aux device, > which is not necessarily the HPD bridge. No. First, that is currently not even an issue either as the registration interface is safe to use with any aux device. Second, if you cared about about type-safety you wouldn't have used a struct device pointer for drm_aux_hpd_bridge_notify() which you back cast to an aux device. > I'd prefer to see an opaque device-specific structure instead. WDYT? That should have been there from the start. But I'm not interested in cleaning up this mess beyond what is minimally required to fix the regressions it caused. This can be reworked for 6.9 or later. > > struct device *drm_dp_hpd_bridge_register(struct device *parent, > > struct device_node *np); > > void drm_aux_hpd_bridge_notify(struct device *dev, enum > > drm_connector_status status); Johan
Re: [PATCH 6/6] phy: qcom-qmp-combo: fix type-c switch registration
On 17-02-24, 16:02, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
>
> Move registration of the typec switch to after looking up clocks and
> other resources.
>
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
Acked-by: Vinod Koul
--
~Vinod
Re: [PATCH 5/6] phy: qcom-qmp-combo: fix drm bridge registration
On 17-02-24, 16:02, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
>
> This could potentially also trigger a bug in the DRM bridge
> implementation which does not expect bridges to go away even if device
> links may avoid triggering this (when enabled).
>
> Move registration of the DRM aux bridge to after looking up clocks and
> other resources.
>
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
Acked-by: Vinod Koul
--
~Vinod
Re: [PATCH 4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m
On 17/02/2024 16:02, Johan Hovold wrote:
From: Rob Clark
We need to bail out before adding/removing devices if we are going to
-EPROBE_DEFER. Otherwise boot can get stuck in a probe deferral loop due
to a long-standing issue in driver core (see fbc35b45f9f6 ("Add
documentation on meaning of -EPROBE_DEFER")).
Deregistering the altmode child device can potentially also trigger bugs
in the DRM bridge implementation, which does not expect bridges to go
away.
Suggested-by: Dmitry Baryshkov
Signed-off-by: Rob Clark
Link: https://lore.kernel.org/r/20231213210644.8702-1-robdcl...@gmail.com
[ johan: rebase on 6.8-rc4, amend commit message and mention DRM ]
Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver")
Cc: sta...@vger.kernel.org # 6.3
Cc: Bjorn Andersson
Signed-off-by: Johan Hovold
---
drivers/soc/qcom/pmic_glink.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index f4bfd24386f1..f913e9bd57ed 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -265,10 +265,17 @@ static int pmic_glink_probe(struct platform_device *pdev)
pg->client_mask = *match_data;
+ pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
+ if (IS_ERR(pg->pdr)) {
+ ret = dev_err_probe(>dev, PTR_ERR(pg->pdr),
+ "failed to initialize pdr\n");
+ return ret;
+ }
+
if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) {
ret = pmic_glink_add_aux_device(pg, >ucsi_aux, "ucsi");
if (ret)
- return ret;
+ goto out_release_pdr_handle;
}
if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
ret = pmic_glink_add_aux_device(pg, >altmode_aux,
"altmode");
@@ -281,17 +288,11 @@ static int pmic_glink_probe(struct platform_device *pdev)
goto out_release_altmode_aux;
}
- pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
- if (IS_ERR(pg->pdr)) {
- ret = dev_err_probe(>dev, PTR_ERR(pg->pdr), "failed to
initialize pdr\n");
- goto out_release_aux_devices;
- }
-
service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd");
if (IS_ERR(service)) {
ret = dev_err_probe(>dev, PTR_ERR(service),
"failed adding pdr lookup for
charger_pd\n");
- goto out_release_pdr_handle;
+ goto out_release_aux_devices;
}
mutex_lock(&__pmic_glink_lock);
@@ -300,8 +301,6 @@ static int pmic_glink_probe(struct platform_device *pdev)
return 0;
-out_release_pdr_handle:
- pdr_handle_release(pg->pdr);
out_release_aux_devices:
if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
pmic_glink_del_aux_device(pg, >ps_aux);
@@ -311,6 +310,8 @@ static int pmic_glink_probe(struct platform_device *pdev)
out_release_ucsi_aux:
if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
pmic_glink_del_aux_device(pg, >ucsi_aux);
+out_release_pdr_handle:
+ pdr_handle_release(pg->pdr);
return ret;
}
Reviewed-by: Neil Armstrong
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On 23/02/2024 12:02, Neil Armstrong wrote: Hi, On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: Starting with 6.8-rc1 the internal display sometimes fails to come up on machines like the Lenovo ThinkPad X13s and the logs indicate that this is due to a regression in the DRM subsystem [1]. This series fixes a race in the pmic_glink_altmode driver which was exposed / triggered by the transparent DRM bridges rework that went into 6.8-rc1 and that manifested itself as a bridge failing to attach and sometimes triggering a NULL-pointer dereference. [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/6] drm/bridge: aux-hpd: fix OF node leaks https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b [2/6] drm/bridge: aux-hpd: separate allocation and registration (no commit info) [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free (no commit info) [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m (no commit info) [5/6] phy: qcom-qmp-combo: fix drm bridge registration (no commit info) [6/6] phy: qcom-qmp-combo: fix type-c switch registration (no commit info) To clarify, I only applied patch 1 to drm-misc-fixes Thanks, Neil
Re: [PATCH 0/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
Hi, On Sat, 17 Feb 2024 16:02:22 +0100, Johan Hovold wrote: > Starting with 6.8-rc1 the internal display sometimes fails to come up on > machines like the Lenovo ThinkPad X13s and the logs indicate that this > is due to a regression in the DRM subsystem [1]. > > This series fixes a race in the pmic_glink_altmode driver which was > exposed / triggered by the transparent DRM bridges rework that went into > 6.8-rc1 and that manifested itself as a bridge failing to attach and > sometimes triggering a NULL-pointer dereference. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/6] drm/bridge: aux-hpd: fix OF node leaks https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9ee485bdda68d6d3f5728cbe3150eb9013d7d22b [2/6] drm/bridge: aux-hpd: separate allocation and registration (no commit info) [3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free (no commit info) [4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m (no commit info) [5/6] phy: qcom-qmp-combo: fix drm bridge registration (no commit info) [6/6] phy: qcom-qmp-combo: fix type-c switch registration (no commit info) -- Neil
Re: [PATCH 1/6] drm/bridge: aux-hpd: fix OF node leaks
On 17/02/2024 16:02, Johan Hovold wrote:
The two device node references taken during allocation need to be
dropped when the auxiliary device is freed.
Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge
drivers")
Cc: Dmitry Baryshkov
Cc: Neil Armstrong
Signed-off-by: Johan Hovold
---
drivers/gpu/drm/bridge/aux-hpd-bridge.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
index bb55f697a181..9e71daf95bde 100644
--- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
@@ -25,6 +25,7 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
ida_free(_aux_hpd_bridge_ida, adev->id);
of_node_put(adev->dev.platform_data);
+ of_node_put(adev->dev.of_node);
kfree(adev);
}
@@ -74,6 +75,8 @@ struct device *drm_dp_hpd_bridge_register(struct device
*parent,
ret = auxiliary_device_init(adev);
if (ret) {
+ of_node_put(adev->dev.platform_data);
+ of_node_put(adev->dev.of_node);
ida_free(_aux_hpd_bridge_ida, adev->id);
kfree(adev);
return ERR_PTR(ret);
Reviewed-by: Neil Armstrong
Re: [PATCH 1/6] drm/bridge: aux-hpd: fix OF node leaks
On 17/02/2024 16:02, Johan Hovold wrote:
The two device node references taken during allocation need to be
dropped when the auxiliary device is freed.
Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge
drivers")
Cc: Dmitry Baryshkov
Cc: Neil Armstrong
Signed-off-by: Johan Hovold
---
drivers/gpu/drm/bridge/aux-hpd-bridge.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
index bb55f697a181..9e71daf95bde 100644
--- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
@@ -25,6 +25,7 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
ida_free(_aux_hpd_bridge_ida, adev->id);
of_node_put(adev->dev.platform_data);
+ of_node_put(adev->dev.of_node);
kfree(adev);
}
@@ -74,6 +75,8 @@ struct device *drm_dp_hpd_bridge_register(struct device
*parent,
ret = auxiliary_device_init(adev);
if (ret) {
+ of_node_put(adev->dev.platform_data);
+ of_node_put(adev->dev.of_node);
ida_free(_aux_hpd_bridge_ida, adev->id);
kfree(adev);
return ERR_PTR(ret);
Reviewed-by: Neil Armstrong
