On Tue, Sep 29, 2026 at 04:49:59AM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] drm/msm/dp: bitwise OR of STATUS5 and STATUS7 without masking in > ISR > -- > > commit 987123505b6cfcc4664c72feae6b070ad91b79a8 > Author: Abhinav Kumar <[email protected]> > > drm/msm/dp: add support for sending VCPF packets in DP controller > > This commit adds support for sending VC Payload Fill (VCPF) sequences in the > MSM DP controller when stream symbols are absent. It introduces interrupt > handling for streams 0-3 using the STATUS5 and STATUS7 registers to signal > VCPF completion. > > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > > index 9a0c9737e6d00..929e0f94f8ddb 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > > [ ... ] > > > @@ -2801,6 +2877,16 @@ irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl > > *msm_dp_ctrl, > > ret = IRQ_HANDLED; > > } > > > > + isr = msm_dp_ctrl_get_mst_vcpf_interrupt_0_1(ctrl); > > + if (ctrl->num_pixel_clks > DP_STREAM_2) > > + isr |= msm_dp_ctrl_get_mst_vcpf_interrupt_2_3(ctrl); > > + > > + if (isr & (DP_INTR_MST_DP0_VCPF_SENT | DP_INTR_MST_DP1_VCPF_SENT | > > + DP_INTR_MST_DP2_VCPF_SENT | DP_INTR_MST_DP3_VCPF_SENT)) { > > [Severity: Medium] > Does this bitwise OR of two different status registers cause false positives? > > The msm_dp_ctrl_get_mst_vcpf_interrupt_*() functions return the unmasked > register values. Since the values from REG_DP_INTR_STATUS5 and > REG_DP_INTR_STATUS7 are ORed together here, bits from one register could > overlap with the VCPF bits checked for the other. > > For example, if an unrelated interrupt sets bit 18 in REG_DP_INTR_STATUS5, > and it gets ORed into `isr`, the condition will see bit 18 set and assume > it's DP_INTR_MST_DP2_VCPF_SENT (which is bit 18 of REG_DP_INTR_STATUS7). > > This could cause a spurious completion of ctrl->idle_comp during a VCPF push, > allowing the driver to proceed while the hardware is still executing the > pattern. > > Should the return values of msm_dp_ctrl_get_mst_vcpf_interrupt_*() be masked > against their respective VCPF bits before returning, or should they be checked > independently instead of ORing them together?
Please respond to this comment. > > > + complete(&ctrl->idle_comp); > > + ret = IRQ_HANDLED; > > + } > > + > > /* DP aux isr */ > > isr = msm_dp_ctrl_get_aux_interrupt(ctrl); > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=12 -- With best wishes Dmitry
