* [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request
@ 2021-02-22 4:00 Wayne Lin
2021-02-22 4:00 ` [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr Wayne Lin
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Wayne Lin @ 2021-02-22 4:00 UTC (permalink / raw)
To: dri-devel
Cc: lyude, stable, Nicholas.Kazlauskas, harry.wentland, jerry.zuo,
eryk.brol, qingqing.zhuo, Wayne Lin
While testing MST hotplug events on daisy chain monitors, find out
that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table
is not reset. Dig in deeper and find out two parts needed to be fixed.
1. Link_Count_Total & Link_Count_Remaining of Broadcast message are
incorrect. Should set lct=1 & lcr=6
2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast
request message. Should fix this.
Wayne Lin (2):
drm/dp_mst: Revise broadcast msg lct & lcr
drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-22 4:00 [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Wayne Lin @ 2021-02-22 4:00 ` Wayne Lin 2021-02-22 17:02 ` Ville Syrjälä 2021-02-22 4:00 ` [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast Wayne Lin 2021-02-24 18:06 ` [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Lyude Paul 2 siblings, 1 reply; 12+ messages in thread From: Wayne Lin @ 2021-02-22 4:00 UTC (permalink / raw) To: dri-devel Cc: lyude, stable, Nicholas.Kazlauskas, harry.wentland, jerry.zuo, eryk.brol, qingqing.zhuo, Wayne Lin [Why & How] According to DP spec, broadcast message LCT equals to 1 and LCR equals to 6. Current implementation is incorrect. Fix it. Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 17dbed0a9800..713ef3b42054 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, else hdr->broadcast = 0; hdr->path_msg = txmsg->path_msg; - hdr->lct = mstb->lct; - hdr->lcr = mstb->lct - 1; + if (hdr->broadcast) { + hdr->lct = 1; + hdr->lcr = 6; + } else { + hdr->lct = mstb->lct; + hdr->lcr = mstb->lct - 1; + } + if (mstb->lct > 1) memcpy(hdr->rad, mstb->rad, mstb->lct / 2); -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-22 4:00 ` [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr Wayne Lin @ 2021-02-22 17:02 ` Ville Syrjälä 2021-02-22 17:09 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2021-02-22 17:02 UTC (permalink / raw) To: Wayne Lin Cc: dri-devel, eryk.brol, qingqing.zhuo, stable, jerry.zuo, Nicholas.Kazlauskas On Mon, Feb 22, 2021 at 12:00:26PM +0800, Wayne Lin wrote: > [Why & How] > According to DP spec, broadcast message LCT equals to 1 and LCR equals > to 6. Current implementation is incorrect. Fix it. > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 17dbed0a9800..713ef3b42054 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, > else > hdr->broadcast = 0; > hdr->path_msg = txmsg->path_msg; > - hdr->lct = mstb->lct; > - hdr->lcr = mstb->lct - 1; > + if (hdr->broadcast) { > + hdr->lct = 1; > + hdr->lcr = 6; > + } else { > + hdr->lct = mstb->lct; > + hdr->lcr = mstb->lct - 1; > + } > + > if (mstb->lct > 1) > memcpy(hdr->rad, mstb->rad, mstb->lct / 2); We should also do something about RAD no? > > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-22 17:02 ` Ville Syrjälä @ 2021-02-22 17:09 ` Ville Syrjälä 2021-02-23 5:32 ` Lin, Wayne 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2021-02-22 17:09 UTC (permalink / raw) To: Wayne Lin Cc: eryk.brol, qingqing.zhuo, stable, jerry.zuo, dri-devel, Nicholas.Kazlauskas On Mon, Feb 22, 2021 at 07:02:03PM +0200, Ville Syrjälä wrote: > On Mon, Feb 22, 2021 at 12:00:26PM +0800, Wayne Lin wrote: > > [Why & How] > > According to DP spec, broadcast message LCT equals to 1 and LCR equals > > to 6. Current implementation is incorrect. Fix it. > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 17dbed0a9800..713ef3b42054 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, > > else > > hdr->broadcast = 0; > > hdr->path_msg = txmsg->path_msg; > > - hdr->lct = mstb->lct; > > - hdr->lcr = mstb->lct - 1; > > + if (hdr->broadcast) { > > + hdr->lct = 1; > > + hdr->lcr = 6; > > + } else { > > + hdr->lct = mstb->lct; > > + hdr->lcr = mstb->lct - 1; > > + } > > + > > if (mstb->lct > 1) > > memcpy(hdr->rad, mstb->rad, mstb->lct / 2); > > We should also do something about RAD no? Just skip the RAD stuff by s/mstb->lct/hdr->lct/ here I guess? -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-22 17:09 ` Ville Syrjälä @ 2021-02-23 5:32 ` Lin, Wayne 2021-02-23 13:26 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Lin, Wayne @ 2021-02-23 5:32 UTC (permalink / raw) To: Ville Syrjälä Cc: Brol, Eryk, Zhuo, Qingqing, stable@vger.kernel.org, Zuo, Jerry, dri-devel@lists.freedesktop.org, Kazlauskas, Nicholas [AMD Public Use] > -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Tuesday, February 23, 2021 1:09 AM > To: Lin, Wayne <Wayne.Lin@amd.com> > Cc: Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>; stable@vger.kernel.org; Zuo, Jerry > <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com> > Subject: Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr > > On Mon, Feb 22, 2021 at 07:02:03PM +0200, Ville Syrjälä wrote: > > On Mon, Feb 22, 2021 at 12:00:26PM +0800, Wayne Lin wrote: > > > [Why & How] > > > According to DP spec, broadcast message LCT equals to 1 and LCR > > > equals to 6. Current implementation is incorrect. Fix it. > > > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > > Cc: stable@vger.kernel.org > > > --- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 17dbed0a9800..713ef3b42054 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, > > > else > > > hdr->broadcast = 0; > > > hdr->path_msg = txmsg->path_msg; > > > -hdr->lct = mstb->lct; > > > -hdr->lcr = mstb->lct - 1; > > > +if (hdr->broadcast) { > > > +hdr->lct = 1; > > > +hdr->lcr = 6; > > > +} else { > > > +hdr->lct = mstb->lct; > > > +hdr->lcr = mstb->lct - 1; > > > +} > > > + > > > if (mstb->lct > 1) > > > memcpy(hdr->rad, mstb->rad, mstb->lct / 2); > > > > We should also do something about RAD no? > > Just skip the RAD stuff by s/mstb->lct/hdr->lct/ here I guess? Thanks Ville! Since LCT=1, broadcast message doesn't have a RAD and this is taken care while we're constructing the header in drm_dp_encode_sideband_msg_hdr(). In drm_dp_encode_sideband_msg_hdr(), we skip stuffing RAD if LCT=1. > > -- > Ville Syrjälä > Intel Regards, Wayne Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-23 5:32 ` Lin, Wayne @ 2021-02-23 13:26 ` Ville Syrjälä 2021-02-24 9:47 ` Lin, Wayne 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2021-02-23 13:26 UTC (permalink / raw) To: Lin, Wayne Cc: Brol, Eryk, Zhuo, Qingqing, stable@vger.kernel.org, Zuo, Jerry, dri-devel@lists.freedesktop.org, Kazlauskas, Nicholas On Tue, Feb 23, 2021 at 05:32:32AM +0000, Lin, Wayne wrote: > [AMD Public Use] > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Tuesday, February 23, 2021 1:09 AM > > To: Lin, Wayne <Wayne.Lin@amd.com> > > Cc: Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>; stable@vger.kernel.org; Zuo, Jerry > > <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com> > > Subject: Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr > > > > On Mon, Feb 22, 2021 at 07:02:03PM +0200, Ville Syrjälä wrote: > > > On Mon, Feb 22, 2021 at 12:00:26PM +0800, Wayne Lin wrote: > > > > [Why & How] > > > > According to DP spec, broadcast message LCT equals to 1 and LCR > > > > equals to 6. Current implementation is incorrect. Fix it. > > > > > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > > > Cc: stable@vger.kernel.org > > > > --- > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- > > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > index 17dbed0a9800..713ef3b42054 100644 > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, > > > > else > > > > hdr->broadcast = 0; > > > > hdr->path_msg = txmsg->path_msg; > > > > -hdr->lct = mstb->lct; > > > > -hdr->lcr = mstb->lct - 1; > > > > +if (hdr->broadcast) { > > > > +hdr->lct = 1; > > > > +hdr->lcr = 6; > > > > +} else { > > > > +hdr->lct = mstb->lct; > > > > +hdr->lcr = mstb->lct - 1; > > > > +} > > > > + > > > > if (mstb->lct > 1) > > > > memcpy(hdr->rad, mstb->rad, mstb->lct / 2); > > > > > > We should also do something about RAD no? > > > > Just skip the RAD stuff by s/mstb->lct/hdr->lct/ here I guess? > Thanks Ville! > Since LCT=1, broadcast message doesn't have a RAD and this is taken > care while we're constructing the header in drm_dp_encode_sideband_msg_hdr(). > In drm_dp_encode_sideband_msg_hdr(), we skip stuffing RAD if LCT=1. Ugh. How many levels of these do we really need... Either way I'd prefer the code be consistent so you don't have to sacrifice so many brain cells to understand what should be trivial details. -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr 2021-02-23 13:26 ` Ville Syrjälä @ 2021-02-24 9:47 ` Lin, Wayne 0 siblings, 0 replies; 12+ messages in thread From: Lin, Wayne @ 2021-02-24 9:47 UTC (permalink / raw) To: Ville Syrjälä Cc: Brol, Eryk, Zhuo, Qingqing, stable@vger.kernel.org, Zuo, Jerry, dri-devel@lists.freedesktop.org, Kazlauskas, Nicholas [AMD Public Use] > -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Tuesday, February 23, 2021 9:26 PM > To: Lin, Wayne <Wayne.Lin@amd.com> > Cc: Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>; stable@vger.kernel.org; Zuo, Jerry > <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com> > Subject: Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr > > On Tue, Feb 23, 2021 at 05:32:32AM +0000, Lin, Wayne wrote: > > [AMD Public Use] > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: Tuesday, February 23, 2021 1:09 AM > > > To: Lin, Wayne <Wayne.Lin@amd.com> > > > Cc: Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing > > > <Qingqing.Zhuo@amd.com>; stable@vger.kernel.org; Zuo, Jerry > > > <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org; Kazlauskas, > > > Nicholas <Nicholas.Kazlauskas@amd.com> > > > Subject: Re: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr > > > > > > On Mon, Feb 22, 2021 at 07:02:03PM +0200, Ville Syrjälä wrote: > > > > On Mon, Feb 22, 2021 at 12:00:26PM +0800, Wayne Lin wrote: > > > > > [Why & How] > > > > > According to DP spec, broadcast message LCT equals to 1 and LCR > > > > > equals to 6. Current implementation is incorrect. Fix it. > > > > > > > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > > > > Cc: stable@vger.kernel.org > > > > > --- > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++-- > > > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > index 17dbed0a9800..713ef3b42054 100644 > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > @@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct > > > > > drm_dp_sideband_msg_hdr *hdr, else hdr->broadcast = 0; > > > > > hdr->path_msg = txmsg->path_msg; > > > > > -hdr->lct = mstb->lct; > > > > > -hdr->lcr = mstb->lct - 1; > > > > > +if (hdr->broadcast) { > > > > > +hdr->lct = 1; > > > > > +hdr->lcr = 6; > > > > > +} else { > > > > > +hdr->lct = mstb->lct; > > > > > +hdr->lcr = mstb->lct - 1; > > > > > +} > > > > > + > > > > > if (mstb->lct > 1) > > > > > memcpy(hdr->rad, mstb->rad, mstb->lct / 2); > > > > > > > > We should also do something about RAD no? > > > > > > Just skip the RAD stuff by s/mstb->lct/hdr->lct/ here I guess? > > Thanks Ville! > > Since LCT=1, broadcast message doesn't have a RAD and this is taken > > care while we're constructing the header in drm_dp_encode_sideband_msg_hdr(). > > In drm_dp_encode_sideband_msg_hdr(), we skip stuffing RAD if LCT=1. > > Ugh. How many levels of these do we really need... > Either way I'd prefer the code be consistent so you don't have to sacrifice so many brain cells to understand what should be trivial > details. Hi Ville, Ya I know.. Currently it goes few levels to encapsulate the final mst packet. From my understanding, this function is trying to prepare needed data and the actual mst packet header is constructed in drm_dp_encode_sideband_msg_hdr(). However, I will push another version by your suggestion. Thanks for your time! > > -- > Ville Syrjälä > Intel Regards, Wayne Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast 2021-02-22 4:00 [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Wayne Lin 2021-02-22 4:00 ` [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr Wayne Lin @ 2021-02-22 4:00 ` Wayne Lin 2021-02-22 17:00 ` Ville Syrjälä 2021-02-24 18:06 ` [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Lyude Paul 2 siblings, 1 reply; 12+ messages in thread From: Wayne Lin @ 2021-02-22 4:00 UTC (permalink / raw) To: dri-devel Cc: lyude, stable, Nicholas.Kazlauskas, harry.wentland, jerry.zuo, eryk.brol, qingqing.zhuo, Wayne Lin [Why & How] According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request message and current implementation is incorrect. Fix it. Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 713ef3b42054..6d73559046e5 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg) req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; drm_dp_encode_sideband_req(&req, msg); + msg->path_msg = true; } static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, req_type = txmsg->msg[0] & 0x7f; if (req_type == DP_CONNECTION_STATUS_NOTIFY || - req_type == DP_RESOURCE_STATUS_NOTIFY) + req_type == DP_RESOURCE_STATUS_NOTIFY || + req_type == DP_CLEAR_PAYLOAD_ID_TABLE) hdr->broadcast = 1; else hdr->broadcast = 0; -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast 2021-02-22 4:00 ` [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast Wayne Lin @ 2021-02-22 17:00 ` Ville Syrjälä 2021-02-23 5:32 ` Lin, Wayne 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2021-02-22 17:00 UTC (permalink / raw) To: Wayne Lin Cc: dri-devel, eryk.brol, qingqing.zhuo, stable, jerry.zuo, Nicholas.Kazlauskas, Dhinakaran Pandiyan On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote: > [Why & How] > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request > message and current implementation is incorrect. Fix it. > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 713ef3b42054..6d73559046e5 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg) > > req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; > drm_dp_encode_sideband_req(&req, msg); > + msg->path_msg = true; > } > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, > @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, > > req_type = txmsg->msg[0] & 0x7f; > if (req_type == DP_CONNECTION_STATUS_NOTIFY || > - req_type == DP_RESOURCE_STATUS_NOTIFY) > + req_type == DP_RESOURCE_STATUS_NOTIFY || > + req_type == DP_CLEAR_PAYLOAD_ID_TABLE) > hdr->broadcast = 1; Looks correct. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path requests, but apparently forget to mark them as broadcast messages. > else > hdr->broadcast = 0; > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast 2021-02-22 17:00 ` Ville Syrjälä @ 2021-02-23 5:32 ` Lin, Wayne 2021-02-23 13:21 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Lin, Wayne @ 2021-02-23 5:32 UTC (permalink / raw) To: Ville Syrjälä Cc: dri-devel@lists.freedesktop.org, Brol, Eryk, Zhuo, Qingqing, stable@vger.kernel.org, Zuo, Jerry, Kazlauskas, Nicholas, Dhinakaran Pandiyan [AMD Public Use] > -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Tuesday, February 23, 2021 1:00 AM > To: Lin, Wayne <Wayne.Lin@amd.com> > Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>; > stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran > Pandiyan <dhinakaran.pandiyan@intel.com> > Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast > > On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote: > > [Why & How] > > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast > > request message and current implementation is incorrect. Fix it. > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 713ef3b42054..6d73559046e5 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct > > drm_dp_sideband_msg_tx *msg) > > > > req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; > > drm_dp_encode_sideband_req(&req, msg); > > +msg->path_msg = true; > > } > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx > > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct > > drm_dp_sideband_msg_hdr *hdr, > > > > req_type = txmsg->msg[0] & 0x7f; > > if (req_type == DP_CONNECTION_STATUS_NOTIFY || > > -req_type == DP_RESOURCE_STATUS_NOTIFY) > > +req_type == DP_RESOURCE_STATUS_NOTIFY || > > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE) > > hdr->broadcast = 1; > > Looks correct. > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path > requests, but apparently forget to mark them as broadcast messages. Hi Ville, I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here. Appreciate for your time! > > > else > > hdr->broadcast = 0; > > -- > > 2.17.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist > > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7C > > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884 > > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb > > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% > > 7C1000&sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am > > p;reserved=0 > > -- > Ville Syrjälä > Intel Regards, Wayne Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast 2021-02-23 5:32 ` Lin, Wayne @ 2021-02-23 13:21 ` Ville Syrjälä 0 siblings, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2021-02-23 13:21 UTC (permalink / raw) To: Lin, Wayne Cc: dri-devel@lists.freedesktop.org, Brol, Eryk, Zhuo, Qingqing, stable@vger.kernel.org, Zuo, Jerry, Kazlauskas, Nicholas, Dhinakaran Pandiyan On Tue, Feb 23, 2021 at 05:32:36AM +0000, Lin, Wayne wrote: > [AMD Public Use] > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Tuesday, February 23, 2021 1:00 AM > > To: Lin, Wayne <Wayne.Lin@amd.com> > > Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>; > > stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran > > Pandiyan <dhinakaran.pandiyan@intel.com> > > Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast > > > > On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote: > > > [Why & How] > > > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast > > > request message and current implementation is incorrect. Fix it. > > > > > > Signed-off-by: Wayne Lin <Wayne.Lin@amd.com> > > > Cc: stable@vger.kernel.org > > > --- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 713ef3b42054..6d73559046e5 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct > > > drm_dp_sideband_msg_tx *msg) > > > > > > req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; > > > drm_dp_encode_sideband_req(&req, msg); > > > +msg->path_msg = true; > > > } > > > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx > > > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct > > > drm_dp_sideband_msg_hdr *hdr, > > > > > > req_type = txmsg->msg[0] & 0x7f; > > > if (req_type == DP_CONNECTION_STATUS_NOTIFY || > > > -req_type == DP_RESOURCE_STATUS_NOTIFY) > > > +req_type == DP_RESOURCE_STATUS_NOTIFY || > > > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE) > > > hdr->broadcast = 1; > > > > Looks correct. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path > > requests, but apparently forget to mark them as broadcast messages. > Hi Ville, > I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here. Doh. Yeah, you're correct. Not sure what section I was reading earlier when I came to that conclusion. > Appreciate for your time! > > > > > else > > > hdr->broadcast = 0; > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist > > > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7C > > > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884 > > > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb > > > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% > > > 7C1000&sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am > > > p;reserved=0 > > > > -- > > Ville Syrjälä > > Intel > Regards, > Wayne Lin -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request 2021-02-22 4:00 [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Wayne Lin 2021-02-22 4:00 ` [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr Wayne Lin 2021-02-22 4:00 ` [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast Wayne Lin @ 2021-02-24 18:06 ` Lyude Paul 2 siblings, 0 replies; 12+ messages in thread From: Lyude Paul @ 2021-02-24 18:06 UTC (permalink / raw) To: Wayne Lin, dri-devel Cc: stable, Nicholas.Kazlauskas, harry.wentland, jerry.zuo, eryk.brol, qingqing.zhuo ooh, nice catches! For the whole series this is: Reviewed-by: Lyude Paul <lyude@redhat.com> I will go ahead and push these to drm-misc-next On Mon, 2021-02-22 at 12:00 +0800, Wayne Lin wrote: > While testing MST hotplug events on daisy chain monitors, find out > that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table > is not reset. Dig in deeper and find out two parts needed to be fixed. > > 1. Link_Count_Total & Link_Count_Remaining of Broadcast message are > incorrect. Should set lct=1 & lcr=6 > 2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast > request message. Should fix this. > > Wayne Lin (2): > drm/dp_mst: Revise broadcast msg lct & lcr > drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast > > drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > -- > 2.17.1 > -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite! ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-02-24 18:07 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-22 4:00 [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Wayne Lin 2021-02-22 4:00 ` [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr Wayne Lin 2021-02-22 17:02 ` Ville Syrjälä 2021-02-22 17:09 ` Ville Syrjälä 2021-02-23 5:32 ` Lin, Wayne 2021-02-23 13:26 ` Ville Syrjälä 2021-02-24 9:47 ` Lin, Wayne 2021-02-22 4:00 ` [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast Wayne Lin 2021-02-22 17:00 ` Ville Syrjälä 2021-02-23 5:32 ` Lin, Wayne 2021-02-23 13:21 ` Ville Syrjälä 2021-02-24 18:06 ` [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request Lyude Paul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox