diff mbox series

[(repost),3/5] drm_dp_mst_topology: fix broken drm_dp_sideband_parse_remote_dpcd_read()

Message ID 20180817141122.9541-4-hverkuil@xs4all.nl (mailing list archive)
State New, archived
Headers show
Series drm/nouveau+amdgpu: add DP CEC-Tunneling-over-AUX (repost) | expand

Commit Message

Hans Verkuil Aug. 17, 2018, 2:11 p.m. UTC
From: Hans Verkuil <hans.verkuil@cisco.com>

When parsing the reply of a DP_REMOTE_DPCD_READ DPCD command the
result is wrong due to a missing idx increment.

This was never noticed since DP_REMOTE_DPCD_READ is currently not
used, but if you enable it, then it is all wrong.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Lyude Paul Aug. 20, 2018, 6:59 p.m. UTC | #1
Reviewed-by: Lyude Paul <lyude@redhat.com>

We really need to add support for using this into the MST helpers. A good way to
test this would probably be to hook up an aux device to the DP AUX adapters we
create for each MST topology

On Fri, 2018-08-17 at 16:11 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> When parsing the reply of a DP_REMOTE_DPCD_READ DPCD command the
> result is wrong due to a missing idx increment.
> 
> This was never noticed since DP_REMOTE_DPCD_READ is currently not
> used, but if you enable it, then it is all wrong.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 7780567aa669..5ff1d79b86c4 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -439,6 +439,7 @@ static bool drm_dp_sideband_parse_remote_dpcd_read(struct
> drm_dp_sideband_msg_rx
>  	if (idx > raw->curlen)
>  		goto fail_len;
>  	repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx];
> +	idx++;
>  	if (idx > raw->curlen)
>  		goto fail_len;
>
Hans Verkuil Aug. 20, 2018, 8:43 p.m. UTC | #2
On 08/20/2018 08:59 PM, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 
> We really need to add support for using this into the MST helpers. A good way to
> test this would probably be to hook up an aux device to the DP AUX adapters we
> create for each MST topology

If you are interested, I have code for that in my MST test branch:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=cec-nv-amd-mst

It's the "drm_dp_mst_topology: use correct AUX channel" patch.

I don't have plans to post this patch since CEC for MST isn't working
(still trying to figure out why not), but you are free to pick it up
if you want.

Regards,

	Hans

> 
> On Fri, 2018-08-17 at 16:11 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> When parsing the reply of a DP_REMOTE_DPCD_READ DPCD command the
>> result is wrong due to a missing idx increment.
>>
>> This was never noticed since DP_REMOTE_DPCD_READ is currently not
>> used, but if you enable it, then it is all wrong.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/gpu/drm/drm_dp_mst_topology.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index 7780567aa669..5ff1d79b86c4 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -439,6 +439,7 @@ static bool drm_dp_sideband_parse_remote_dpcd_read(struct
>> drm_dp_sideband_msg_rx
>>  	if (idx > raw->curlen)
>>  		goto fail_len;
>>  	repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx];
>> +	idx++;
>>  	if (idx > raw->curlen)
>>  		goto fail_len;
>>  
>
Lyude Paul Aug. 20, 2018, 8:49 p.m. UTC | #3
On Mon, 2018-08-20 at 22:43 +0200, Hans Verkuil wrote:
> On 08/20/2018 08:59 PM, Lyude Paul wrote:
> > Reviewed-by: Lyude Paul <lyude@redhat.com>
> > 
> > We really need to add support for using this into the MST helpers. A good
> > way to
> > test this would probably be to hook up an aux device to the DP AUX adapters
> > we
> > create for each MST topology
> 
> If you are interested, I have code for that in my MST test branch:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=cec-nv-amd-mst
> 
> It's the "drm_dp_mst_topology: use correct AUX channel" patch.
> 
> I don't have plans to post this patch since CEC for MST isn't working
> (still trying to figure out why not), but you are free to pick it up
> if you want.
Maybe someday but don't count on it yet! I've got a lot of stuff on my plate atm
:)

> 
> Regards,
> 
> 	Hans
> 
> > 
> > On Fri, 2018-08-17 at 16:11 +0200, Hans Verkuil wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > 
> > > When parsing the reply of a DP_REMOTE_DPCD_READ DPCD command the
> > > result is wrong due to a missing idx increment.
> > > 
> > > This was never noticed since DP_REMOTE_DPCD_READ is currently not
> > > used, but if you enable it, then it is all wrong.
> > > 
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 7780567aa669..5ff1d79b86c4 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -439,6 +439,7 @@ static bool
> > > drm_dp_sideband_parse_remote_dpcd_read(struct
> > > drm_dp_sideband_msg_rx
> > >  	if (idx > raw->curlen)
> > >  		goto fail_len;
> > >  	repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx];
> > > +	idx++;
> > >  	if (idx > raw->curlen)
> > >  		goto fail_len;
> > >  
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7780567aa669..5ff1d79b86c4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -439,6 +439,7 @@  static bool drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx
 	if (idx > raw->curlen)
 		goto fail_len;
 	repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx];
+	idx++;
 	if (idx > raw->curlen)
 		goto fail_len;