On Mon, Oct 12, 2026 at 05:08:53PM +0800, Yongxing Mou wrote: > > > On 10/1/2026 6:34 PM, Dmitry Baryshkov wrote: > > 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. > > > Thanks for pointing this out. The issue is valid, and we will fix it in the > next revision. >>
Please respond to other Sashiko comments too. You got a review. It's a nice practice to respond to them (unless you agree to all of them and are going to fix all of the findings in the next revision). -- With best wishes Dmitry
