[PATCH] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ

[Lin, Wayne]

[AMD Official Use Only - Internal Distribution Only]

> ________________________________________
> From: Wentland, Harry <Harry.Wentland at amd.com>
> Sent: Monday, December 30, 2019 23:26
> To: Lin, Wayne; dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org
> Cc: lyude at redhat.com; Zuo, Jerry; Kazlauskas, Nicholas; Wentland, Harry
> Subject: Re: [PATCH] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ
>
> On 2019-12-30 2:05 a.m., Wayne Lin wrote:
> > [Why]
> > According to DP spec, it should shift left 4 digits for NO_STOP_BIT
> > in REMOTE_I2C_READ message. Not 5 digits.
> >
> > [How]
> > Correct the shifting value of NO_STOP_BIT for DP_REMOTE_I2C_READ case in
> > drm_dp_encode_sideband_req().
> >
> > Signed-off-by: Wayne Lin <Wayne.Lin at amd.com>
>
> Good catch. Looks like this has been there since the beginning of MST in
> the kernel. How did you find this? Did this cause bad EDID reads or some
> other problem? If so the commit message should probably mention it.
>
> Harry
Thanks for your time.

I found this while I was trying to debug MST issues by using AUX monitor.
So far, I don't observe problems that relate to this under my test cases and environment.
However, this bit might affect the I2C signal generated by I2C master, I will mention more
in the commit message.

Thanks.
>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 1d1bfa49ca2b..0557e225ff67 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -393,7 +393,7 @@ drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req,
> >                       memcpy(&buf[idx], req->u.i2c_read.transactions[i].bytes, req->u.i2c_read.transactions[i].num_bytes);
> >                       idx += req->u.i2c_read.transactions[i].num_bytes;
> >
> > -                     buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 5;
> > +                     buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 4;
> >                       buf[idx] |= (req->u.i2c_read.transactions[i].i2c_transaction_delay & 0xf);
> >                       idx++;
> >               }
> >
--
Wayne Lin