RE: [PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr

["Lin, Wayne" <Wayne.Lin@amd.com>]

[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