[PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Imre Deak]

If the sink gets disconnected during receiving a multi-packet DP MST AUX
down-reply/up-request sideband message, the state keeping track of which
packets have been received already is not reset. This results in a failed
sanity check for the subsequent message packet received after a sink is
reconnected (due to the pending message not yet completed with an
end-of-message-transfer packet), indicated by the

"sideband msg set header failed"

error.

Fix the above by resetting the up/down message reception state after a
disconnect event.

Cc: Lyude Paul <lyude@redhat.com>
Cc: <stable@vger.kernel.org> # v3.17+
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 51a46689cda70..90819fff2c9ba 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
 		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
 		ret = 0;
 		mgr->payload_id_table_cleared = false;
+
+		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
+		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
 	}
 
 out_unlock:
-- 
2.37.1

[PATCH 2/3] drm/display/dp_mst: Fix down message handling after a packet reception error

[Imre Deak]

After an error during receiving a packet for a multi-packet DP MST
sideband message, the state tracking which packets have been received
already is not reset. This prevents the reception of subsequent down
messages (due to the pending message not yet completed with an
end-of-message-transfer packet).

Fix the above by resetting the reception state after a packet error.

Cc: Lyude Paul <lyude@redhat.com>
Cc: <stable@vger.kernel.org> # v3.17+
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
 1 file changed, 1 insertion(+), 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 90819fff2c9ba..01350510244f2 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3856,7 +3856,7 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr)
 	struct drm_dp_sideband_msg_rx *msg = &mgr->down_rep_recv;
 
 	if (!drm_dp_get_one_sb_msg(mgr, false, &mstb))
-		goto out;
+		goto out_clear_reply;
 
 	/* Multi-packet message transmission, don't clear the reply */
 	if (!msg->have_eomt)
-- 
2.37.1

[PATCH 3/3] drm/display/dp_mst: Fix payload addition on a disconnected sink

[Imre Deak]

If an MST stream is enabled on a disconnected sink, the payload for the
stream is not created and the MST manager's payload count/next start VC
slot is not updated. Since the payload's start VC slot may still contain
a valid value (!= -1) the subsequent disabling of such a stream could
cause an incorrect decrease of the payload count/next start VC slot in
drm_dp_remove_payload() and hence later payload additions will fail.

Fix the above by marking the payload as invalid in the above case, so
that it's skipped during payload removal. While at it add a debug print
for this case.

Cc: Lyude Paul <lyude@redhat.com>
Cc: <stable@vger.kernel.org> # v6.1+
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++++++-
 1 file changed, 6 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 01350510244f2..5861b0a6247bc 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3309,8 +3309,13 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
 	int ret;
 
 	port = drm_dp_mst_topology_get_port_validated(mgr, payload->port);
-	if (!port)
+	if (!port) {
+		drm_dbg_kms(mgr->dev,
+			    "VCPI %d for port %p not in topology, not creating a payload\n",
+			    payload->vcpi, payload->port);
+		payload->vc_start_slot = -1;
 		return 0;
+	}
 
 	if (mgr->payload_count == 0)
 		mgr->next_start_slot = mst_state->start_slot;
-- 
2.37.1

Re: [PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Lyude Paul]

For the whole series:

Reviewed-by: Lyude Paul <lyude@redhat.com>

Thanks!

On Wed, 2022-12-14 at 20:42 +0200, Imre Deak wrote:
> If the sink gets disconnected during receiving a multi-packet DP MST AUX
> down-reply/up-request sideband message, the state keeping track of which
> packets have been received already is not reset. This results in a failed
> sanity check for the subsequent message packet received after a sink is
> reconnected (due to the pending message not yet completed with an
> end-of-message-transfer packet), indicated by the
> 
> "sideband msg set header failed"
> 
> error.
> 
> Fix the above by resetting the up/down message reception state after a
> disconnect event.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: <stable@vger.kernel.org> # v3.17+
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 51a46689cda70..90819fff2c9ba 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
>  		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
>  		ret = 0;
>  		mgr->payload_id_table_cleared = false;
> +
> +		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
> +		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
>  	}
>  
>  out_unlock:

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Imre Deak]

On Wed, Dec 14, 2022 at 04:41:42PM -0500, Lyude Paul wrote:
> For the whole series:
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>

Thanks for the review, pushed it to drm-misc-next.

> Thanks!
> 
> On Wed, 2022-12-14 at 20:42 +0200, Imre Deak wrote:
> > If the sink gets disconnected during receiving a multi-packet DP MST AUX
> > down-reply/up-request sideband message, the state keeping track of which
> > packets have been received already is not reset. This results in a failed
> > sanity check for the subsequent message packet received after a sink is
> > reconnected (due to the pending message not yet completed with an
> > end-of-message-transfer packet), indicated by the
> > 
> > "sideband msg set header failed"
> > 
> > error.
> > 
> > Fix the above by resetting the up/down message reception state after a
> > disconnect event.
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: <stable@vger.kernel.org> # v3.17+
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > index 51a46689cda70..90819fff2c9ba 100644
> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > @@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
> >  		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
> >  		ret = 0;
> >  		mgr->payload_id_table_cleared = false;
> > +
> > +		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
> > +		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
> >  	}
> >  
> >  out_unlock:
> 
> -- 
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
> 

Re: [Intel-gfx] [PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Jani Nikula]

On Fri, 16 Dec 2022, Imre Deak <imre.deak@intel.com> wrote:
> On Wed, Dec 14, 2022 at 04:41:42PM -0500, Lyude Paul wrote:
>> For the whole series:
>> 
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>
> Thanks for the review, pushed it to drm-misc-next.

Hmm, with the drm-misc *not* cherry-picking patches from drm-misc-next
to drm-misc-fixes, these will only get backported to stable kernels
after they hit Linus' tree in the next (as opposed to current) merge
window after a full development cycle. Wonder if they should be
expedited.

BR,
Jani.

>
>> Thanks!
>> 
>> On Wed, 2022-12-14 at 20:42 +0200, Imre Deak wrote:
>> > If the sink gets disconnected during receiving a multi-packet DP MST AUX
>> > down-reply/up-request sideband message, the state keeping track of which
>> > packets have been received already is not reset. This results in a failed
>> > sanity check for the subsequent message packet received after a sink is
>> > reconnected (due to the pending message not yet completed with an
>> > end-of-message-transfer packet), indicated by the
>> > 
>> > "sideband msg set header failed"
>> > 
>> > error.
>> > 
>> > Fix the above by resetting the up/down message reception state after a
>> > disconnect event.
>> > 
>> > Cc: Lyude Paul <lyude@redhat.com>
>> > Cc: <stable@vger.kernel.org> # v3.17+
>> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> > ---
>> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> > 
>> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > index 51a46689cda70..90819fff2c9ba 100644
>> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > @@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
>> >  		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
>> >  		ret = 0;
>> >  		mgr->payload_id_table_cleared = false;
>> > +
>> > +		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
>> > +		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
>> >  	}
>> >  
>> >  out_unlock:
>> 
>> -- 
>> Cheers,
>>  Lyude Paul (she/her)
>>  Software Engineer at Red Hat
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Imre Deak]

On Fri, Dec 16, 2022 at 06:10:39PM +0200, Jani Nikula wrote:
> On Fri, 16 Dec 2022, Imre Deak <imre.deak@intel.com> wrote:
> > On Wed, Dec 14, 2022 at 04:41:42PM -0500, Lyude Paul wrote:
> >> For the whole series:
> >> 
> >> Reviewed-by: Lyude Paul <lyude@redhat.com>
> >
> > Thanks for the review, pushed it to drm-misc-next.
> 
> Hmm, with the drm-misc *not* cherry-picking patches from drm-misc-next
> to drm-misc-fixes, these will only get backported to stable kernels
> after they hit Linus' tree in the next (as opposed to current) merge
> window after a full development cycle. Wonder if they should be
> expedited.

Ok, it should've been pushed to -fixes then, will do that next time.
Yes, I think sending them already before the next merge window would be
good.

> 
> BR,
> Jani.
> 
> >
> >> Thanks!
> >> 
> >> On Wed, 2022-12-14 at 20:42 +0200, Imre Deak wrote:
> >> > If the sink gets disconnected during receiving a multi-packet DP MST AUX
> >> > down-reply/up-request sideband message, the state keeping track of which
> >> > packets have been received already is not reset. This results in a failed
> >> > sanity check for the subsequent message packet received after a sink is
> >> > reconnected (due to the pending message not yet completed with an
> >> > end-of-message-transfer packet), indicated by the
> >> > 
> >> > "sideband msg set header failed"
> >> > 
> >> > error.
> >> > 
> >> > Fix the above by resetting the up/down message reception state after a
> >> > disconnect event.
> >> > 
> >> > Cc: Lyude Paul <lyude@redhat.com>
> >> > Cc: <stable@vger.kernel.org> # v3.17+
> >> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
> >> >  1 file changed, 3 insertions(+)
> >> > 
> >> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> > index 51a46689cda70..90819fff2c9ba 100644
> >> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> >> > @@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
> >> >  		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
> >> >  		ret = 0;
> >> >  		mgr->payload_id_table_cleared = false;
> >> > +
> >> > +		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
> >> > +		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
> >> >  	}
> >> >  
> >> >  out_unlock:
> >> 
> >> -- 
> >> Cheers,
> >>  Lyude Paul (she/her)
> >>  Software Engineer at Red Hat
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/3] drm/display/dp_mst: Fix down/up message handling after sink disconnect

[Jani Nikula]

On Fri, 16 Dec 2022, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, Dec 16, 2022 at 06:10:39PM +0200, Jani Nikula wrote:
>> On Fri, 16 Dec 2022, Imre Deak <imre.deak@intel.com> wrote:
>> > On Wed, Dec 14, 2022 at 04:41:42PM -0500, Lyude Paul wrote:
>> >> For the whole series:
>> >> 
>> >> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> >
>> > Thanks for the review, pushed it to drm-misc-next.
>> 
>> Hmm, with the drm-misc *not* cherry-picking patches from drm-misc-next
>> to drm-misc-fixes, these will only get backported to stable kernels
>> after they hit Linus' tree in the next (as opposed to current) merge
>> window after a full development cycle. Wonder if they should be
>> expedited.
>
> Ok, it should've been pushed to -fixes then, will do that next time.
> Yes, I think sending them already before the next merge window would be
> good.

Cc: drm-misc maintainers, I think this is for you to figure out.

BR,
Jani.


>
>> 
>> BR,
>> Jani.
>> 
>> >
>> >> Thanks!
>> >> 
>> >> On Wed, 2022-12-14 at 20:42 +0200, Imre Deak wrote:
>> >> > If the sink gets disconnected during receiving a multi-packet DP MST AUX
>> >> > down-reply/up-request sideband message, the state keeping track of which
>> >> > packets have been received already is not reset. This results in a failed
>> >> > sanity check for the subsequent message packet received after a sink is
>> >> > reconnected (due to the pending message not yet completed with an
>> >> > end-of-message-transfer packet), indicated by the
>> >> > 
>> >> > "sideband msg set header failed"
>> >> > 
>> >> > error.
>> >> > 
>> >> > Fix the above by resetting the up/down message reception state after a
>> >> > disconnect event.
>> >> > 
>> >> > Cc: Lyude Paul <lyude@redhat.com>
>> >> > Cc: <stable@vger.kernel.org> # v3.17+
>> >> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++
>> >> >  1 file changed, 3 insertions(+)
>> >> > 
>> >> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> >> > index 51a46689cda70..90819fff2c9ba 100644
>> >> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> >> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> >> > @@ -3641,6 +3641,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
>> >> >  		drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
>> >> >  		ret = 0;
>> >> >  		mgr->payload_id_table_cleared = false;
>> >> > +
>> >> > +		memset(&mgr->down_rep_recv, 0, sizeof(mgr->down_rep_recv));
>> >> > +		memset(&mgr->up_req_recv, 0, sizeof(mgr->up_req_recv));
>> >> >  	}
>> >> >  
>> >> >  out_unlock:
>> >> 
>> >> -- 
>> >> Cheers,
>> >>  Lyude Paul (she/her)
>> >>  Software Engineer at Red Hat
>> >> 
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center