| Message ID | 20210222040027.23505-2-Wayne.Lin@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Set CLEAR_PAYLOAD_ID_TABLE as broadcast request | expand |
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
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?
[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
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.
[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
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);
[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(-)