> -----Original Message-----
> From: Jani Nikula <[email protected]>
> Sent: Thursday, November 13, 2025 9:55 PM
> To: Deak, Imre <[email protected]>; Kandpal, Suraj
> <[email protected]>
> Cc: [email protected]; [email protected]; intel-
> [email protected]; Nautiyal, Ankit K <[email protected]>;
> Murthy, Arun R <[email protected]>
> Subject: Re: [PATCH] drm/display/dp_mst: Add protection against 0 vcpi
>
> On Thu, 13 Nov 2025, Imre Deak <[email protected]> wrote:
> > On Thu, Nov 13, 2025 at 10:09:19AM +0530, Suraj Kandpal wrote:
> >> When releasing a timeslot there is a slight chance we may end up with
> >> the wrong payload mask due to overflow if the delayed_destroy_work
> >> ends up coming into play after a DP 2.1 monitor gets disconnected
> >> which causes vcpi to become 0 then we try to make the payload =
> >> ~BIT(vcpi - 1) which is a negative shift.
> >>
> >> Signed-off-by: Suraj Kandpal <[email protected]>
> >> ---
> >> drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> index 64e5c176d5cc..3cf1eafcfcb5 100644
> >> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> @@ -4531,6 +4531,7 @@ int drm_dp_atomic_release_time_slots(struct
> drm_atomic_state *state,
> >> struct drm_dp_mst_atomic_payload *payload;
> >> struct drm_connector_state *old_conn_state, *new_conn_state;
> >> bool update_payload = true;
> >> + int bit;
> >>
> >> old_conn_state = drm_atomic_get_old_connector_state(state, port-
> >connector);
> >> if (!old_conn_state->crtc)
> >> @@ -4572,7 +4573,8 @@ int drm_dp_atomic_release_time_slots(struct
> drm_atomic_state *state,
> >> if (!payload->delete) {
> >> payload->pbn = 0;
> >> payload->delete = true;
> >> - topology_state->payload_mask &= ~BIT(payload->vcpi - 1);
> >> + bit = payload->vcpi ? payload->vcpi - 1 : 0;
> >> + topology_state->payload_mask &= ~BIT(bit);
> >
> > This looks wrong, clearing the bit for an unrelated payload.
>
> Agreed.
>
> The logs have, among other things,
>
> <7> [515.138211] xe 0000:03:00.0: [drm:intel_dp_sink_set_dsc_decompression
> [xe]] Failed to enable sink decompression state
>
> <7> [515.193484] xe 0000:03:00.0: [drm:drm_dp_add_payload_part1
> [drm_display_helper]] VCPI 0 for port ffff888126ce9000 not in topology, not
> creating a payload to remote
>
> <7> [515.194671] xe 0000:03:00.0: [drm:drm_dp_add_payload_part2
> [drm_display_helper]] Part 1 of payload creation for DP-5 failed, skipping
> part 2
>
> <7> [515.347331] xe 0000:03:00.0: [drm:drm_dp_remove_payload_part1
> [drm_display_helper]] Payload for VCPI 0 not in topology, not sending remove
>
> So it's no wonder the port's not in topology and everything fails. We
> obviously
> need to skip payload_mask updates when the VCPI is 0, but that's just a
> symptom of other stuff going wrong first. Perhaps we could do with some
> earlier error handling too?
>
Yes I agree the question is how high will the error handling needs to be added.
A lot of weird things going on here.
1st one is how is it finding a payload which we do not create while we call
destroy function
2nd how is VCPI with id 0 possible from what I see VCPI are 1 at least that's
what I gather from
drm_dp_mst_atomic_check_payload_alloc_limits.So what are we missing when we
create a payload?
Imre, Jani any idea still new to how payload creation work so am I missing
something.
Regards
Suraj Kandpal
> BR,
> Jani.
>
>
> >
> >> }
> >>
> >> return 0;
> >> --
> >> 2.34.1
> >>
>
> --
> Jani Nikula, Intel
