On Thu, 23 Feb 2023 at 12:57, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote: > > > > On 23.02.2023 10:57, Dmitry Baryshkov wrote: > > The rewritten dpu_hw_ctl_setup_blendstage() can lightly smash the stack > > when setting the SSPP_NONE pipe. However it was unnoticed until the > > kernel was tested under AOSP (with some kind of stack protection/check). > > > > This fixes the following backtrace: > > > > Unexpected kernel BRK exception at EL1 > > Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP > > Hardware name: Thundercomm Dragonboard 845c (DT) > > pstate: a0400005 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > pc : dpu_hw_ctl_setup_blendstage+0x26c/0x278 [msm] > > lr : _dpu_crtc_blend_setup+0x4b4/0x5a0 [msm] > > sp : ffffffc00bdcb720 > > x29: ffffffc00bdcb720 x28: ffffff8085debac0 x27: 0000000000000002 > > x26: ffffffd74af18320 x25: ffffff8083af75a0 x24: ffffffc00bdcb878 > > x23: 0000000000000001 x22: 0000000000000000 x21: ffffff8085a70000 > > x20: ffffff8083012dc0 x19: 0000000000000001 x18: 0000000000000000 > > x17: 000000040044ffff x16: 045000f4b5593519 x15: 0000000000000000 > > x14: 000000000000000b x13: 0000000000000001 x12: 0000000000000000 > > x11: 0000000000000001 x10: ffffffc00bdcb764 x9 : ffffffd74af06a08 > > x8 : 0000000000000001 x7 : 0000000000000001 x6 : 0000000000000000 > > x5 : ffffffc00bdcb878 x4 : 0000000000000002 x3 : ffffffffffffffff > > x2 : ffffffc00bdcb878 x1 : 0000000000000000 x0 : 0000000000000002 > > Call trace: > > dpu_hw_ctl_setup_blendstage+0x26c/0x278 [msm] > > _dpu_crtc_blend_setup+0x4b4/0x5a0 [msm] > > dpu_crtc_atomic_begin+0xd8/0x22c [msm] > > drm_atomic_helper_commit_planes+0x80/0x208 [drm_kms_helper] > > msm_atomic_commit_tail+0x134/0x6f0 [msm] > > commit_tail+0xa4/0x1a4 [drm_kms_helper] > > drm_atomic_helper_commit+0x170/0x184 [drm_kms_helper] > > drm_atomic_commit+0xac/0xe8 > > drm_mode_atomic_ioctl+0xbf0/0xdac > > drm_ioctl_kernel+0xc4/0x178 > > drm_ioctl+0x2c8/0x608 > > __arm64_sys_ioctl+0xa8/0xec > > invoke_syscall+0x44/0x104 > > el0_svc_common.constprop.0+0x44/0xec > > do_el0_svc+0x38/0x98 > > el0_svc+0x2c/0xb4 > > el0t_64_sync_handler+0xb8/0xbc > > el0t_64_sync+0x1a0/0x1a4 > > Code: 52800016 52800017 52800018 17ffffc7 (d4207d00) > > > > Fixes: 4488f71f6373 ("drm/msm/dpu: simplify blend configuration") > > Reported-by: Amit Pundir <amit.pundir@xxxxxxxxxx> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > > index b88a2f3724e6..6c53ea560ffa 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > > @@ -446,7 +446,9 @@ static void dpu_hw_ctl_setup_blendstage(struct dpu_hw_ctl *ctx, > > * CTL_LAYER has 3-bit field (and extra bits in EXT register), > > * all EXT registers has 4-bit fields. > > */ > > - if (cfg->idx == 0) { > > + if (cfg->idx == -1) { > if (cfg->idx == ctl_blend_config[SSPP_NONE][0].idx)? Why? -1 is simpler and more obvious (and doesn't bind it only to SSPP_NONE). > > Konrad > > + continue; > > + } else if (cfg->idx == 0) { > > mixercfg[0] |= mix << cfg->shift; > > mixercfg[1] |= ext << cfg->ext_shift; > > } else { > > -- With best wishes Dmitry