RE: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
[Public] Hi, After investigating quite some time on this issue, found freeze problem is not with the amdgpu part of buddy allocator patch as the patch doesn’t throw any issues when applied separately on top of the stable base of drm-next. After digging more into this issue, the below patch seems to be the cause of this problem, drm/ttm: rework bulk move handling v5 https://cgit.freedesktop.org/drm/drm/commit/?id=fee2ede155423b0f7a559050a39750b98fe9db69 when this patch applied on top of the stable (working version) of drm-next without buddy allocator patch, we can see multiple issues listed below, each thrown randomly at every GravityMark run, 1. general protection fault at ttm_lru_bulk_move_tail() 2. NULL pointer deference at ttm_lru_bulk_move_tail() 3. NULL pointer deference at ttm_resource_init(). Regards, Arun. -Original Message- From: Alex Deucher Sent: Monday, May 16, 2022 8:36 PM To: Mike Lothian Cc: Paneer Selvam, Arunpravin ; Intel Graphics Development ; amd-gfx list ; Maling list - DRI developers ; Deucher, Alexander ; Koenig, Christian ; Matthew Auld Subject: Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu On Mon, May 16, 2022 at 8:40 AM Mike Lothian wrote: > > Hi > > The merge window for 5.19 will probably be opening next week, has > there been any progress with this bug? It took a while to find a combination of GPUs that would repro the issue, but now that we can, it is still being investigated. Alex > > Thanks > > Mike > > On Mon, 2 May 2022 at 17:31, Mike Lothian wrote: > > > > On Mon, 2 May 2022 at 16:54, Arunpravin Paneer Selvam > > wrote: > > > > > > > > > > > > On 5/2/2022 8:41 PM, Mike Lothian wrote: > > > > On Wed, 27 Apr 2022 at 12:55, Mike Lothian wrote: > > > >> On Tue, 26 Apr 2022 at 17:36, Christian König > > > >> wrote: > > > >>> Hi Mike, > > > >>> > > > >>> sounds like somehow stitching together the SG table for PRIME > > > >>> doesn't work any more with this patch. > > > >>> > > > >>> Can you try with P2P DMA disabled? > > > >> -CONFIG_PCI_P2PDMA=y > > > >> +# CONFIG_PCI_P2PDMA is not set > > > >> > > > >> If that's what you're meaning, then there's no difference, I'll > > > >> upload my dmesg to the gitlab issue > > > >> > > > >>> Apart from that can you take a look Arun? > > > >>> > > > >>> Thanks, > > > >>> Christian. > > > > Hi > > > > > > > > Have you had any success in replicating this? > > > Hi Mike, > > > I couldn't replicate on my Raven APU machine. I see you have 2 > > > cards initialized, one is Renoir and the other is Navy Flounder. > > > Could you give some more details, are you running Gravity Mark on > > > Renoir and what is your system RAM configuration? > > > > > > > > Cheers > > > > > > > > Mike > > > > > Hi > > > > It's a PRIME laptop, it failed on the RENOIR too, it caused a > > lockup, but systemd managed to capture it, I'll attach it to the > > issue > > > > I've got 64GB RAM, the 6800M has 12GB VRAM > > > > Cheers > > > > Mike
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
Hi Linus,
On Fri, May 27, 2022 at 06:04:14PM -0700, Linus Torvalds wrote:
> On Fri, May 27, 2022 at 4:41 PM Sudip Mukherjee
> wrote:
> >
> > I just tested with various values, sizeof(*edid) is 144 bytes at that place.
>
> Hmm. What compiler do you have? Because it seems very broken.
I am using gcc version 11.3.1 20220517 (GCC). And I am not just building
spear3xx_defconfig, I am building all the arm configs with the same
compiler in the same setup and only spear3xx_defconfig started failing.
I am attaching a build summary report generated on 26th May, all arm builds
passed, even allmodconfig.
>
>
> But it obviously doesn't happen for me or for most other people, so
> it's something in your setup. Unusual compiler?
And, just to verify its not my setup or the compiler I use, I took a fresh
Debian Bullseye docker, installed 'gcc-arm-linux-gnueabi' from Debian and I see
the same build failure with spear3xx_defconfig. This gcc from Debian Bullseye
is: gcc version 10.2.1 20210110 (Debian 10.2.1-6).
--
Regards
Sudip
HEAD -> babf0bb978e3 ("Merge tag 'xfs-5.19-for-linus' of
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux")
allmodconfig -> pass
am200epdkit_defconfig -> pass
aspeed_g4_defconfig -> pass
aspeed_g5_defconfig -> pass
assabet_defconfig -> pass
at91_dt_defconfig -> pass
axm55xx_defconfig -> pass
badge4_defconfig -> pass
bcm2835_defconfig -> pass
cerfcube_defconfig -> pass
clps711x_defconfig -> pass
cm_x300_defconfig -> pass
cns3420vb_defconfig -> pass
colibri_pxa270_defconfig -> pass
colibri_pxa300_defconfig -> pass
collie_defconfig -> pass
corgi_defconfig -> pass
davinci_all_defconfig -> pass
dove_defconfig -> pass
ep93xx_defconfig -> pass
eseries_pxa_defconfig -> pass
exynos_defconfig -> pass
ezx_defconfig -> pass
footbridge_defconfig -> pass
gemini_defconfig -> pass
h3600_defconfig -> pass
h5000_defconfig -> pass
hackkit_defconfig -> pass
hisi_defconfig -> pass
imx_v4_v5_defconfig -> pass
imx_v6_v7_defconfig -> pass
imxrt_defconfig -> pass
integrator_defconfig -> pass
iop32x_defconfig -> pass
ixp4xx_defconfig -> pass
jornada720_defconfig -> pass
keystone_defconfig -> pass
lart_defconfig -> pass
lpc18xx_defconfig -> pass
lpc32xx_defconfig -> pass
lpd270_defconfig -> pass
lubbock_defconfig -> pass
magician_defconfig -> pass
mainstone_defconfig -> pass
milbeaut_m10v_defconfig -> pass
mini2440_defconfig -> pass
mmp2_defconfig -> pass
moxart_defconfig -> pass
mps2_defconfig -> pass
multi_v4t_defconfig -> pass
multi_v5_defconfig -> pass
multi_v7_defconfig -> pass
mv78xx0_defconfig -> pass
mvebu_v5_defconfig -> pass
mvebu_v7_defconfig -> pass
mxs_defconfig -> pass
neponset_defconfig -> pass
netwinder_defconfig -> pass
nhk8815_defconfig -> pass
omap1_defconfig -> pass
omap2plus_defconfig -> pass
orion5x_defconfig -> pass
oxnas_v6_defconfig -> pass
palmz72_defconfig -> pass
pcm027_defconfig -> pass
pleb_defconfig -> pass
pxa168_defconfig -> pass
pxa255-idp_defconfig -> pass
pxa3xx_defconfig -> pass
pxa910_defconfig -> pass
pxa_defconfig -> pass
qcom_defconfig -> pass
realview_defconfig -> pass
rpc_defconfig -> pass
s3c2410_defconfig -> pass
s3c6400_defconfig -> pass
s5pv210_defconfig -> pass
sama5_defconfig -> pass
sama7_defconfig -> pass
shannon_defconfig -> pass
shmobile_defconfig -> pass
simpad_defconfig -> pass
socfpga_defconfig -> pass
spear13xx_defconfig -> pass
spear3xx_defconfig -> failed
spear6xx_defconfig -> pass
spitz_defconfig -> pass
stm32_defconfig -> pass
sunxi_defconfig -> pass
tct_hammer_defconfig -> pass
tegra_defconfig -> pass
trizeps4_defconfig -> pass
u8500_defconfig -> pass
versatile_defconfig -> pass
vexpress_defconfig -> pass
vf610m4_defconfig -> pass
viper_defconfig -> pass
vt8500_v6_v7_defconfig -> pass
xcep_defconfig -> pass
zeus_defconfig -> pass
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Fri, May 27, 2022 at 06:04:14PM -0700, Linus Torvalds wrote:
> On Fri, May 27, 2022 at 4:41 PM Sudip Mukherjee
> wrote:
> >
> > I just tested with various values, sizeof(*edid) is 144 bytes at that place.
>
> Hmm. What compiler do you have? Because it seems very broken.
>
> You don't actually have to try with various sizes, you could have just
> done something like
>
> int size_of_edid(const struct edid *edid)
> {
> return sizeof(*edid);
> }
>
> and then "make drivers/gpu/drm/drm_edid.s" to generate assembly and
> see what it looks like (obviously removing the BUG_ON() in order to
> build).
just tried this with
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- drivers/gpu/drm/drm_edid.s
>
> That obviously generates code like
>
> movl$128, %eax
> ret
and for me it looks like:
.L1030:
.word .LC40
.word .LC41
.word -1431655765
.word .LC39
.size drm_edid_to_sad, .-drm_edid_to_sad
.align 2
.global size_of_edid
.syntax unified
.arm
.type size_of_edid, %function
size_of_edid:
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 1, uses_anonymous_args = 0
mov ip, sp @,
push{fp, ip, lr, pc}@
sub fp, ip, #4 @,,
@ drivers/gpu/drm/drm_edid.c:1573: }
mov r0, #144@,
ldmfd sp, {fp, sp, pc}@
.size size_of_edid, .-size_of_edid
--
Regards
Sudip
[PATCH] drm/nouveau/fifo/gv100-: set gv100_fifo_runlist storage-class to static
sparse reports
drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c:56:1: warning: symbol
'gv100_fifo_runlist' was not declared. Should it be static?
gv100_fifo_runlist is only used in gv100.c, so change it to static.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c
b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c
index 70e16a91ac12..faf0fe9f704c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gv100.c
@@ -52,7 +52,7 @@ gv100_fifo_runlist_cgrp(struct nvkm_fifo_cgrp *cgrp,
nvkm_wo32(memory, offset + 0xc, 0x);
}
-const struct gk104_fifo_runlist_func
+static const struct gk104_fifo_runlist_func
gv100_fifo_runlist = {
.size = 16,
.cgrp = gv100_fifo_runlist_cgrp,
--
2.27.0
[PATCH v3 1/4] drm/msm/adreno: Remove dead code
This BUG_ON will never be reached, and there is a comment 20 above
explaining why.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 3e325e2a2b1b..b1c876c339d0 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1548,8 +1548,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct
device_node *node)
if (ret)
goto err_memory;
} else {
- BUG_ON(adreno_is_a660_family(adreno_gpu));
-
/* HFI v1, has sptprac */
gmu->legacy = true;
--
2.36.1
[PATCH v3 3/4] drm/msm/a6xx: Add speedbin support for A619 GPU
There are various SKUs of A619, ranging from 565 MHz to 850 MHz, depending
on the bin. Add support for distinguishing them, so that proper frequency
ranges can be applied, depending on the HW.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 331cd2f6b9e3..a2a30a9ab677 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1836,6 +1836,22 @@ static u32 a618_get_speed_bin(u32 fuse)
return UINT_MAX;
}
+static u32 a619_get_speed_bin(u32 fuse)
+{
+ if (fuse == 0)
+ return 0;
+ else if (fuse == 120)
+ return 4;
+ else if (fuse == 138)
+ return 3;
+ else if (fuse == 169)
+ return 2;
+ else if (fuse == 180)
+ return 1;
+
+ return UINT_MAX;
+}
+
static u32 adreno_7c3_get_speed_bin(u32 fuse)
{
if (fuse == 0)
@@ -1855,6 +1871,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
+ if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
+ val = a619_get_speed_bin(fuse);
+
if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
val = adreno_7c3_get_speed_bin(fuse);
--
2.36.1
[PATCH v3 2/4] drm/msm/adreno: Add A619 support
Add support for the Adreno 619 GPU, as found in Snapdragon 690 (SM6350),
480 (SM4350) and 750G (SM7225).
Signed-off-by: Konrad Dybcio
---
Changes since v2:
- Fix some values in a619_build_bw_table (I think I miscopied things last
time around..)
- Add a comment about icache/dcache allocation for future porters
- Resolve Dmitry's comment about BUG_ON (check patch 1/4)
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 13 +++-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 70 +-
drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 65 +++-
drivers/gpu/drm/msm/adreno/adreno_device.c | 14 +
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 13 +++-
5 files changed, 169 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index b1c876c339d0..0a7c3514e51b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -527,6 +527,8 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
pdc_in_aop = true;
else if (adreno_is_a618(adreno_gpu) ||
adreno_is_a640_family(adreno_gpu))
pdc_address_offset = 0x30090;
+ else if (adreno_is_a619(adreno_gpu))
+ pdc_address_offset = 0x300a0;
else
pdc_address_offset = 0x30080;
@@ -601,7 +603,8 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
pdc_write(pdcptr, REG_A6XX_PDC_GPU_TCS3_CMD0_MSGID + 4, 0x10108);
pdc_write(pdcptr, REG_A6XX_PDC_GPU_TCS3_CMD0_ADDR + 4, 0x3);
- if (adreno_is_a618(adreno_gpu) || adreno_is_a650_family(adreno_gpu))
+ if (adreno_is_a618(adreno_gpu) || adreno_is_a619(adreno_gpu) ||
+ adreno_is_a650_family(adreno_gpu))
pdc_write(pdcptr, REG_A6XX_PDC_GPU_TCS3_CMD0_DATA + 4, 0x2);
else
pdc_write(pdcptr, REG_A6XX_PDC_GPU_TCS3_CMD0_DATA + 4, 0x3);
@@ -1537,6 +1540,12 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct
device_node *node)
SZ_16M - SZ_16K, 0x04000, "icache");
if (ret)
goto err_memory;
+ /*
+* NOTE: when porting legacy ("pre-650-family") GPUs you may be tempted
to add a condition
+* to allocate icache/dcache here, as per downstream code flow, but it
may not actually be
+* necessary. If you omit this step and you don't get random
pagefaults, you are likely
+* good to go without this!
+*/
} else if (adreno_is_a640_family(adreno_gpu)) {
ret = a6xx_gmu_memory_alloc(gmu, &gmu->icache,
SZ_256K - SZ_16K, 0x04000, "icache");
@@ -1547,7 +1556,7 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct
device_node *node)
SZ_256K - SZ_16K, 0x44000, "dcache");
if (ret)
goto err_memory;
- } else {
+ } else if (adreno_is_a630(adreno_gpu) ||
adreno_is_a615_family(adreno_gpu)) {
/* HFI v1, has sptprac */
gmu->legacy = true;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ccc4fcf7a630..331cd2f6b9e3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -252,6 +252,74 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
msm_gem_submit *submit)
a6xx_flush(gpu, ring);
}
+/* For a615 family (a615, a616, a618 and a619) */
+const struct adreno_reglist a615_hwcg[] = {
+ {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x0222},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220},
+ {REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0080},
+ {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xF3CF},
+ {REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x0222},
+ {REG_A6XX_RBBM_CLOCK_CNTL_TP1, 0x0222},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL3_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002},
+ {REG_A6XX_RBBM_CLOCK_CNTL4_TP1, 0x0002},
+ {REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST2_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST3_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007},
+ {REG_A6XX_RBBM_CLOCK_HYST4_TP1, 0x0007},
+ {REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY2_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY3_TP1, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001},
+ {REG
[PATCH v3 4/4] drm/msm/adreno: Fix up formatting
Leading spaces are not something checkpatch likes, and it says so when
they are present. Use tabs consistently to indent function body and
unwrap a 83-char-long line, as 100 is cool nowadays.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index a13a3e5a294b..f73f7b5dfd10 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -199,7 +199,7 @@ static inline int adreno_is_a420(struct adreno_gpu *gpu)
static inline int adreno_is_a430(struct adreno_gpu *gpu)
{
- return gpu->revn == 430;
+ return gpu->revn == 430;
}
static inline int adreno_is_a506(struct adreno_gpu *gpu)
@@ -239,7 +239,7 @@ static inline int adreno_is_a540(struct adreno_gpu *gpu)
static inline int adreno_is_a618(struct adreno_gpu *gpu)
{
- return gpu->revn == 618;
+ return gpu->revn == 618;
}
static inline int adreno_is_a619(struct adreno_gpu *gpu)
@@ -249,7 +249,7 @@ static inline int adreno_is_a619(struct adreno_gpu *gpu)
static inline int adreno_is_a630(struct adreno_gpu *gpu)
{
- return gpu->revn == 630;
+ return gpu->revn == 630;
}
static inline int adreno_is_a640_family(struct adreno_gpu *gpu)
@@ -259,18 +259,18 @@ static inline int adreno_is_a640_family(struct adreno_gpu
*gpu)
static inline int adreno_is_a650(struct adreno_gpu *gpu)
{
- return gpu->revn == 650;
+ return gpu->revn == 650;
}
static inline int adreno_is_7c3(struct adreno_gpu *gpu)
{
/* The order of args is important here to handle ANY_ID correctly */
- return adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), gpu->rev);
+ return adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), gpu->rev);
}
static inline int adreno_is_a660(struct adreno_gpu *gpu)
{
- return gpu->revn == 660;
+ return gpu->revn == 660;
}
/* check for a615, a616, a618, a619 or any derivatives */
@@ -281,14 +281,13 @@ static inline int adreno_is_a615_family(struct adreno_gpu
*gpu)
static inline int adreno_is_a660_family(struct adreno_gpu *gpu)
{
- return adreno_is_a660(gpu) || adreno_is_7c3(gpu);
+ return adreno_is_a660(gpu) || adreno_is_7c3(gpu);
}
/* check for a650, a660, or any derivatives */
static inline int adreno_is_a650_family(struct adreno_gpu *gpu)
{
- return gpu->revn == 650 || gpu->revn == 620 ||
- adreno_is_a660_family(gpu);
+ return gpu->revn == 650 || gpu->revn == 620 ||
adreno_is_a660_family(gpu);
}
int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
--
2.36.1
Re: [PATCH v3 4/4] drm/msm/adreno: Fix up formatting
On Sat, 2022-05-28 at 18:03 +0200, Konrad Dybcio wrote:
> Leading spaces are not something checkpatch likes, and it says so when
> they are present. Use tabs consistently to indent function body and
> unwrap a 83-char-long line, as 100 is cool nowadays.
unassociated trivia:
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
[]
> @@ -199,7 +199,7 @@ static inline int adreno_is_a420(struct adreno_gpu *gpu)
>
> static inline int adreno_is_a430(struct adreno_gpu *gpu)
> {
> - return gpu->revn == 430;
> + return gpu->revn == 430;
> }
looks like these could/should return bool
> static inline int adreno_is_a506(struct adreno_gpu *gpu)
> @@ -239,7 +239,7 @@ static inline int adreno_is_a540(struct adreno_gpu *gpu)
>
> static inline int adreno_is_a618(struct adreno_gpu *gpu)
> {
> - return gpu->revn == 618;
> + return gpu->revn == 618;
> }
etc...
Re: [PATCH] fbdev: vesafb: Fix a use-after-free due early fb_info cleanup
On 5/26/22 21:47, Javier Martinez Canillas wrote:
> Commit b3c9a924aab6 ("fbdev: vesafb: Cleanup fb_info in .fb_destroy rather
> than .remove") fixed a use-after-free error due the vesafb driver freeing
> the fb_info in the .remove handler instead of doing it in .fb_destroy.
>
> This can happen if the .fb_destroy callback is executed after the .remove
> callback, since the former tries to access a pointer freed by the latter.
>
> But that change didn't take into account that another possible scenario is
> that .fb_destroy is called before the .remove callback. For example, if no
> process has the fbdev chardev opened by the time the driver is removed.
>
> If that's the case, fb_info will be freed when unregister_framebuffer() is
> called, making the fb_info pointer accessed in vesafb_remove() after that
> to no longer be valid.
>
> To prevent that, move the expression containing the info->par to happen
> before the unregister_framebuffer() function call.
>
> Fixes: b3c9a924aab6 ("fbdev: vesafb: Cleanup fb_info in .fb_destroy rather
> than .remove")
> Reported-by: Pascal Ernster
> Signed-off-by: Javier Martinez Canillas
applied to the fbdev git tree.
Thanks!
Helge
> ---
>
> drivers/video/fbdev/vesafb.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
> index e25e8de5ff67..929d4775cb4b 100644
> --- a/drivers/video/fbdev/vesafb.c
> +++ b/drivers/video/fbdev/vesafb.c
> @@ -490,11 +490,12 @@ static int vesafb_remove(struct platform_device *pdev)
> {
> struct fb_info *info = platform_get_drvdata(pdev);
>
> - /* vesafb_destroy takes care of info cleanup */
> - unregister_framebuffer(info);
> if (((struct vesafb_par *)(info->par))->region)
> release_region(0x3c0, 32);
>
> + /* vesafb_destroy takes care of info cleanup */
> + unregister_framebuffer(info);
> +
> return 0;
> }
>
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 5:13 AM Sudip Mukherjee
wrote:
>
> just tried this with
> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- drivers/gpu/drm/drm_edid.s
>
> size_of_edid:
> mov r0, #144@,
> ldmfd sp, {fp, sp, pc}@
So digging a bit deeper - since I have am arm compiler after all - I
note that 'sizeof(detailed_timings)' is 88.
Which is completely wrong. It should be 72 bytes (an array of 4
structures, each 18 bytes in size).
I have not dug deeper, but that is clearly the issue.
Now, why that only happens on that spear3xx_defconfig, I have no idea.
Linus
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 10:40 AM Linus Torvalds
wrote:
>
> So digging a bit deeper - since I have am arm compiler after all - I
> note that 'sizeof(detailed_timings)' is 88.
Hmm.
sizeof() both
detailed_timings[0].data.other_data.data.range.formula.gtf2
and
detailed_timings[0].data.other_data.data.range.formula.cvt
is 7.
But the *union* of those things is
detailed_timings[0].data.other_data.data.range.formula
and its size is 8 (despite having an alignment of just 1).
The attached patch would seem to fix it for me.
Not very much tested, and I have no idea what it is that triggers this
only on spear3xx_defconfig.
Some ARM ABI issue that is triggered by some very particular ARM
compiler flag enabled by that config?
Adding some ARM (and SPEAR, and SoC) people in case they have any idea.
This smells like a compiler bug triggered by "there's a 16-bit member
field in that gtf2 structure, and despite it being packed and aligned
to 1, we somehow still align the size to 2".
I dunno. But marking those unions packed too doesn't seem wrong, and
does seem to fix it.
Linus
include/drm/drm_edid.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index c3204a58fb09..b2756753370b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -121,7 +121,7 @@ struct detailed_data_monitor_range {
u8 supported_scalings;
u8 preferred_refresh;
} __attribute__((packed)) cvt;
- } formula;
+ } __attribute__((packed)) formula;
} __attribute__((packed));
struct detailed_data_wpindex {
@@ -154,7 +154,7 @@ struct detailed_non_pixel {
struct detailed_data_wpindex color;
struct std_timing timings[6];
struct cvt_timing cvt[4];
- } data;
+ } __attribute__((packed)) data;
} __attribute__((packed));
#define EDID_DETAIL_EST_TIMINGS 0xf7
@@ -172,7 +172,7 @@ struct detailed_timing {
union {
struct detailed_pixel_timing pixel_data;
struct detailed_non_pixel other_data;
- } data;
+ } __attribute__((packed)) data;
} __attribute__((packed));
#define DRM_EDID_INPUT_SERRATION_VSYNC (1 << 0)
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 8:08 PM Linus Torvalds wrote: > > Not very much tested, and I have no idea what it is that triggers this > only on spear3xx_defconfig. > > Some ARM ABI issue that is triggered by some very particular ARM > compiler flag enabled by that config? It's CONFIG_ARM_AEABI, which is normally set everywhere. Without this option, you the kernel is built for the old 'OABI' that forces all non-packed struct members to be at least 16-bit aligned. I think Russell still uses OABI kernels on his oldest machines, but it is incompatible with all modern user space and should probably not be in the defconfig. Your patch looks like the correct solution to me. Arnd
Re: [PATCH v2 09/15] drm/fbconv: Mode-setting pipeline enable / disable
Hi Thomas,
On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann wrote:
> The display mode is set by converting the DRM display mode to an
> fb_info state and handling it to the fbdev driver's fb_setvar()
> function. This also requires a color depth, which we take from the
> value of struct drm_mode_config.preferred_depth
>
> Signed-off-by: Thomas Zimmermann
> --- a/drivers/gpu/drm/drm_fbconv_helper.c
> +++ b/drivers/gpu/drm/drm_fbconv_helper.c
> @@ -919,6 +919,24 @@ static int
> drm_fbconv_update_fb_var_screeninfo_from_framebuffer(
> return 0;
> }
>
> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe(
> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe
> *pipe)
> +{
> + struct drm_plane *primary = pipe->crtc.primary;
> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe);
> +
> + if (primary && primary->state && primary->state->fb)
> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer(
> + fb_var, primary->state->fb,
> + modeset->fb_info->fix.smem_len);
> +
> + fb_var->xres_virtual = fb_var->xres;
> + fb_var->yres_virtual = fb_var->yres;
> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth;
This looks wrong to me: IMHO bits_per_pixel should be derived from
the fourcc format of the _new_ mode to be set...
> +
> + return 0;
> +}
> +
> /**
> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for
> * struct drm_simple_display_pipe_funcs.mode_valid
> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup(
> struct drm_crtc *crtc, const struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode)
> {
> + struct drm_simple_display_pipe *pipe =
> + container_of(crtc, struct drm_simple_display_pipe, crtc);
> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe);
> + struct fb_var_screeninfo fb_var;
> + int ret;
> +
> + if (!modeset->fb_info->fbops->fb_check_var)
> + return true;
> +
> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode);
> +
> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe(
> + &fb_var, &modeset->pipe);
> + if (ret)
> + return true;
> +
> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var,
> modeset->fb_info);
... hence this fails if the requested mode is valid with the new
fourcc format, but invalid with the old (but preferred) depth.
E.g. due to bandwidth limitations, a high-resolution mode is valid
with a low color depth, while a high color depth is limited to lower
resolutions.
Unfortunately we do not know the new fourcc format here, as both
drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed
the mode (from drm_mode_set.mode), but not the new format (from
drm_mode_set.fb->format).
Am I missing something? Is the new format available in some other way?
Thanks!
> + if (ret < 0)
> + return false;
> +
> + drm_mode_update_from_fb_var_screeninfo(adjusted_mode, &fb_var);
> +
> return true;
> }
> EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_mode_fixup);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 11:59 AM Arnd Bergmann wrote: > > It's CONFIG_ARM_AEABI, which is normally set everywhere. Without this > option, you the kernel is built for the old 'OABI' that forces all non-packed > struct members to be at least 16-bit aligned. Looks like forced word (32 bit) alignment to me. I wonder how many other structures that messes up, but I committed the EDID fix for now. This has presumably been broken for a long time, but maybe the affected targets don't typically use EDID and kernel modesetting, and only use some fixed display setup instead. Those structure definitions go back a _loong_ time (from a quick 'git blame' I see November 2008). But despite that, I did not mark my fix 'cc:stable' because I don't know if any of those machines affected by this bad arm ABI issue could possibly care. At least my tree hopefully now builds on them, with the BUILD_BUG_ON() that uncovered this. Linus
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 11:08:48AM -0700, Linus Torvalds wrote: > This smells like a compiler bug triggered by "there's a 16-bit member > field in that gtf2 structure, and despite it being packed and aligned > to 1, we somehow still align the size to 2". It's an age old thing, it's no compiler bug, and it's completely compliant with the C standards. Implementations are permitted by the C standard to pad structures and unions how they see fit - and some do if it makes sense for performance. The mistake is that people forget this detail, and they expect structs and unions to be laid out a certain way - because it doesn't matter to the same extent on x86. However, as older ARM CPUs could not do unaligned loads, ensuring that things were naturally aligned made complete sense, even if it meant that people who assume the world is x86 got tripped up - the only way around that would be to make every load very expensive. It's not "align to size of 2" in OABI, it tends to be align to a multiple of 4, because the underlying architecture is 32-bit. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")
On Sat, May 28, 2022 at 10:31 PM Linus Torvalds wrote: > On Sat, May 28, 2022 at 11:59 AM Arnd Bergmann wrote: > > > > It's CONFIG_ARM_AEABI, which is normally set everywhere. Without this > > option, you the kernel is built for the old 'OABI' that forces all > > non-packed > > struct members to be at least 16-bit aligned. > > Looks like forced word (32 bit) alignment to me. Ah, of course, I keep mixing it up with the odd structure alignment of m68k, which does the opposite and aligns struct members to no more than 16 bits. Arnd
[linux-next:master] BUILD REGRESSION d3fde8ff50ab265749704bd7fbcf70d35235421f
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: d3fde8ff50ab265749704bd7fbcf70d35235421f Add linux-next specific files for 20220527 Error/Warning reports: https://lore.kernel.org/linux-mm/202205031017.4twman3l-...@intel.com https://lore.kernel.org/linux-mm/202205041248.wgcwpcev-...@intel.com https://lore.kernel.org/linux-mm/202205150051.3rzuooag-...@intel.com https://lore.kernel.org/linux-mm/202205150117.sd6hzbvm-...@intel.com https://lore.kernel.org/linux-mm/202205211550.ifuenz5n-...@intel.com https://lore.kernel.org/linux-mm/202205281607.e8gokzmw-...@intel.com https://lore.kernel.org/lkml/202205100617.5uum3uet-...@intel.com https://lore.kernel.org/llvm/202205060132.uhqyux1l-...@intel.com https://lore.kernel.org/llvm/202205110148.mrgbbmtn-...@intel.com https://lore.kernel.org/llvm/202205141122.qihfguem-...@intel.com https://lore.kernel.org/llvm/202205280829.utnovd5s-...@intel.com Error/Warning: (recently discovered and may have been fixed) WARNING: modpost: vmlinux.o(.text.unlikely+0x11f90): Section mismatch in reference from the function find_next_bit() to the variable .init.rodata:__setup_str_initcall_blacklist drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:130:17: warning: 'DP_SINK_BRANCH_DEV_NAME_7580' defined but not used [-Wunused-const-variable=] drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1407:12: warning: stack frame size (1040) exceeds limit (1024) in 'smu_v13_0_7_get_power_profile_mode' [-Wframe-larger-than] drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:1986:6: warning: no previous prototype for function 'gfx_v11_0_rlc_stop' [-Wmissing-prototypes] drivers/gpu/drm/solomon/ssd130x-spi.c:154:35: warning: 'ssd130x_spi_table' defined but not used [-Wunused-const-variable=] drivers/video/fbdev/omap/hwa742.c:492:5: warning: no previous prototype for 'hwa742_update_window_async' [-Wmissing-prototypes] fs/buffer.c:2254:5: warning: stack frame size (2144) exceeds limit (1024) in 'block_read_full_folio' [-Wframe-larger-than] fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' [-Wframe-larger-than] kernel/trace/fgraph.c:37:12: warning: no previous prototype for 'ftrace_enable_ftrace_graph_caller' [-Wmissing-prototypes] kernel/trace/fgraph.c:46:12: warning: no previous prototype for 'ftrace_disable_ftrace_graph_caller' [-Wmissing-prototypes] llvm-objcopy: error: invalid output format: 'elf64-s390' Unverified Error/Warning (likely false positive, please contact us if interested): .__mulsi3.o.cmd: No such file or directory :27:6: error: expected assembly-time absolute expression Makefile:686: arch/h8300/Makefile: No such file or directory arch/Kconfig:10: can't open file "arch/h8300/Kconfig" arch/riscv/include/asm/pgtable-64.h:109:2: error: expected assembly-time absolute expression arch/riscv/kernel/cpufeature.c:292:6: warning: variable 'cpu_apply_feature' set but not used [-Wunused-but-set-variable] arch/riscv/purgatory/kexec-purgatory.c:1860:9: sparse: sparse: trying to concatenate 29720-character string (8191 bytes max) drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1133 amdgpu_discovery_reg_base_init() error: testing array offset 'adev->vcn.num_vcn_inst' after use. drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c:129:6: warning: no previous prototype for function 'amdgpu_ucode_print_imu_hdr' [-Wmissing-prototypes] drivers/gpu/drm/bridge/adv7511/adv7511.h:229:17: warning: 'ADV7511_REG_CEC_RX_FRAME_HDR' defined but not used [-Wunused-const-variable=] drivers/gpu/drm/bridge/adv7511/adv7511.h:235:17: warning: 'ADV7511_REG_CEC_RX_FRAME_LEN' defined but not used [-Wunused-const-variable=] drivers/iio/accel/bma400_core.c:1056:3: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign] drivers/infiniband/hw/hns/hns_roce_hw_v2.c:309:9: sparse: sparse: dubious: x & !y drivers/input/joystick/sensehat-joystick.c:102:2-9: line 102 is redundant because platform_get_irq() already prints an error drivers/input/misc/iqs7222.c:2418:9-34: WARNING: Threaded IRQ with no primary handler requested without IRQF_ONESHOT (unless it is nested IRQ) drivers/misc/cardreader/rts5261.c:406:13: warning: variable 'setting_reg2' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] drivers/pinctrl/stm32/pinctrl-stm32.c:294:24: warning: Value stored to 'pctl' during its initialization is never read [clang-analyzer-deadcode.DeadStores] drivers/rpmsg/rpmsg_core.c:607:3: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy] drivers/staging/rtl8723bs/hal/hal_btcoex.c:1182:30: warning: variable 'pHalData' set but not used [-Wunused-but-set-variable] drivers/staging/vt6655/card.c:758:16: sparse: sparse: cast to restricte
Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
On Sat, 28 May 2022 at 08:44, Paneer Selvam, Arunpravin wrote: > > [Public] > > Hi, > > After investigating quite some time on this issue, found freeze problem is > not with the amdgpu part of buddy allocator patch as the patch doesn’t throw > any issues when applied separately on top of the stable base of drm-next. > After digging more into this issue, the below patch seems to be the cause of > this problem, > > drm/ttm: rework bulk move handling v5 > https://cgit.freedesktop.org/drm/drm/commit/?id=fee2ede155423b0f7a559050a39750b98fe9db69 > > when this patch applied on top of the stable (working version) of drm-next > without buddy allocator patch, we can see multiple issues listed below, each > thrown randomly at every GravityMark run, 1. general protection fault at > ttm_lru_bulk_move_tail() 2. NULL pointer deference at > ttm_lru_bulk_move_tail() 3. NULL pointer deference at ttm_resource_init(). > > Regards, > Arun. Thanks for tracking it down, fee2ede155423b0f7a559050a39750b98fe9db69 isn't trivial to revert Hopefully Christian can figure it out
