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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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