[PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request

[PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request

[Wayne Lin]

While testing MST hotplug events on daisy chain monitors, find out
that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table
is not reset. Dig in deeper and find out two parts needed to be fixed.

1. Link_Count_Total & Link_Count_Remaining of Broadcast message are
incorrect. Should set lct=1 & lcr=6
2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast
request message. Should fix this.

Wayne Lin (2):
  drm/dp_mst: Revise broadcast msg lct & lcr
  drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

 drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--
2.17.1

[PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request

[Wayne Lin]

While testing MST hotplug events on daisy chain monitors, find out
that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table
is not reset. Dig in deeper and find out two parts needed to be fixed.

1. Link_Count_Total & Link_Count_Remaining of Broadcast message are
incorrect. Should set lct=1 & lcr=6
2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast
request message. Should fix this.

Wayne Lin (2):
  drm/dp_mst: Revise broadcast msg lct & lcr
  drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

 drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Wayne Lin]

[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);
 
-- 
2.17.1

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

[Wayne Lin]

[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);
 
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Wayne Lin]

[Why & How]
According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request
message and 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 713ef3b42054..6d73559046e5 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg)
 
 	req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
 	drm_dp_encode_sideband_req(&req, msg);
+	msg->path_msg = true;
 }
 
 static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg,
@@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr,
 
 	req_type = txmsg->msg[0] & 0x7f;
 	if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
-		req_type == DP_RESOURCE_STATUS_NOTIFY)
+		req_type == DP_RESOURCE_STATUS_NOTIFY ||
+		req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
 		hdr->broadcast = 1;
 	else
 		hdr->broadcast = 0;
-- 
2.17.1

[PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Wayne Lin]

[Why & How]
According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request
message and 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 713ef3b42054..6d73559046e5 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg)
 
 	req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
 	drm_dp_encode_sideband_req(&req, msg);
+	msg->path_msg = true;
 }
 
 static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg,
@@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr,
 
 	req_type = txmsg->msg[0] & 0x7f;
 	if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
-		req_type == DP_RESOURCE_STATUS_NOTIFY)
+		req_type == DP_RESOURCE_STATUS_NOTIFY ||
+		req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
 		hdr->broadcast = 1;
 	else
 		hdr->broadcast = 0;
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Ville Syrjälä]

On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> [Why & How]
> According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request
> message and 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 713ef3b42054..6d73559046e5 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg)
>  
>  	req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
>  	drm_dp_encode_sideband_req(&req, msg);
> +	msg->path_msg = true;
>  }
>  
>  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg,
> @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr,
>  
>  	req_type = txmsg->msg[0] & 0x7f;
>  	if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> -		req_type == DP_RESOURCE_STATUS_NOTIFY)
> +		req_type == DP_RESOURCE_STATUS_NOTIFY ||
> +		req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
>  		hdr->broadcast = 1;

Looks correct.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY
here as well. We do try to send them as path requests, but apparently
forget to mark them as broadcast messages.

>  	else
>  		hdr->broadcast = 0;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Ville Syrjälä]

On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> [Why & How]
> According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request
> message and 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 713ef3b42054..6d73559046e5 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg)
>  
>  	req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
>  	drm_dp_encode_sideband_req(&req, msg);
> +	msg->path_msg = true;
>  }
>  
>  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg,
> @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr,
>  
>  	req_type = txmsg->msg[0] & 0x7f;
>  	if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> -		req_type == DP_RESOURCE_STATUS_NOTIFY)
> +		req_type == DP_RESOURCE_STATUS_NOTIFY ||
> +		req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
>  		hdr->broadcast = 1;

Looks correct.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY
here as well. We do try to send them as path requests, but apparently
forget to mark them as broadcast messages.

>  	else
>  		hdr->broadcast = 0;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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?

-- 
Ville Syrjälä
Intel

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

[Ville Syrjälä]

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?

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lin, Wayne]

[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

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

[Lin, Wayne]

[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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Lin, Wayne]

[AMD Public Use]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, February 23, 2021 1:00 AM
> To: Lin, Wayne <Wayne.Lin@amd.com>
> Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>;
> stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran
> Pandiyan <dhinakaran.pandiyan@intel.com>
> Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
>
> On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> > [Why & How]
> > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast
> > request message and 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 | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 713ef3b42054..6d73559046e5 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct
> > drm_dp_sideband_msg_tx *msg)
> >
> >  req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
> >  drm_dp_encode_sideband_req(&req, msg);
> > +msg->path_msg = true;
> >  }
> >
> >  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct
> > drm_dp_sideband_msg_hdr *hdr,
> >
> >  req_type = txmsg->msg[0] & 0x7f;
> >  if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> > -req_type == DP_RESOURCE_STATUS_NOTIFY)
> > +req_type == DP_RESOURCE_STATUS_NOTIFY ||
> > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
> >  hdr->broadcast = 1;
>
> Looks correct.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path
> requests, but apparently forget to mark them as broadcast messages.
Hi Ville,
I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here.
Appreciate for your time!
>
> >  else
> >  hdr->broadcast = 0;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7C
> > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884
> > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb
> > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > 7C1000&amp;sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am
> > p;reserved=0
>
> --
> Ville Syrjälä
> Intel
Regards,
Wayne Lin

RE: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Lin, Wayne]

[AMD Public Use]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, February 23, 2021 1:00 AM
> To: Lin, Wayne <Wayne.Lin@amd.com>
> Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>;
> stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran
> Pandiyan <dhinakaran.pandiyan@intel.com>
> Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
>
> On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> > [Why & How]
> > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast
> > request message and 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 | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 713ef3b42054..6d73559046e5 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct
> > drm_dp_sideband_msg_tx *msg)
> >
> >  req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
> >  drm_dp_encode_sideband_req(&req, msg);
> > +msg->path_msg = true;
> >  }
> >
> >  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct
> > drm_dp_sideband_msg_hdr *hdr,
> >
> >  req_type = txmsg->msg[0] & 0x7f;
> >  if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> > -req_type == DP_RESOURCE_STATUS_NOTIFY)
> > +req_type == DP_RESOURCE_STATUS_NOTIFY ||
> > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
> >  hdr->broadcast = 1;
>
> Looks correct.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path
> requests, but apparently forget to mark them as broadcast messages.
Hi Ville,
I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here.
Appreciate for your time!
>
> >  else
> >  hdr->broadcast = 0;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7C
> > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884
> > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb
> > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > 7C1000&amp;sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am
> > p;reserved=0
>
> --
> Ville Syrjälä
> Intel
Regards,
Wayne Lin
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Ville Syrjälä]

On Tue, Feb 23, 2021 at 05:32:36AM +0000, Lin, Wayne wrote:
> [AMD Public Use]
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, February 23, 2021 1:00 AM
> > To: Lin, Wayne <Wayne.Lin@amd.com>
> > Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>;
> > stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran
> > Pandiyan <dhinakaran.pandiyan@intel.com>
> > Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
> >
> > On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> > > [Why & How]
> > > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast
> > > request message and 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 | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 713ef3b42054..6d73559046e5 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct
> > > drm_dp_sideband_msg_tx *msg)
> > >
> > >  req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
> > >  drm_dp_encode_sideband_req(&req, msg);
> > > +msg->path_msg = true;
> > >  }
> > >
> > >  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct
> > > drm_dp_sideband_msg_hdr *hdr,
> > >
> > >  req_type = txmsg->msg[0] & 0x7f;
> > >  if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> > > -req_type == DP_RESOURCE_STATUS_NOTIFY)
> > > +req_type == DP_RESOURCE_STATUS_NOTIFY ||
> > > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
> > >  hdr->broadcast = 1;
> >
> > Looks correct.
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path
> > requests, but apparently forget to mark them as broadcast messages.
> Hi Ville,
> I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here.

Doh. Yeah, you're correct. Not sure what section I was reading earlier
when I came to that conclusion.

> Appreciate for your time!
> >
> > >  else
> > >  hdr->broadcast = 0;
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7C
> > > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884
> > > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb
> > > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > > 7C1000&amp;sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am
> > > p;reserved=0
> >
> > --
> > Ville Syrjälä
> > Intel
> Regards,
> Wayne Lin

-- 
Ville Syrjälä
Intel

Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast

[Ville Syrjälä]

On Tue, Feb 23, 2021 at 05:32:36AM +0000, Lin, Wayne wrote:
> [AMD Public Use]
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, February 23, 2021 1:00 AM
> > To: Lin, Wayne <Wayne.Lin@amd.com>
> > Cc: dri-devel@lists.freedesktop.org; Brol, Eryk <Eryk.Brol@amd.com>; Zhuo, Qingqing <Qingqing.Zhuo@amd.com>;
> > stable@vger.kernel.org; Zuo, Jerry <Jerry.Zuo@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Dhinakaran
> > Pandiyan <dhinakaran.pandiyan@intel.com>
> > Subject: Re: [PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
> >
> > On Mon, Feb 22, 2021 at 12:00:27PM +0800, Wayne Lin wrote:
> > > [Why & How]
> > > According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast
> > > request message and 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 | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 713ef3b42054..6d73559046e5 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct
> > > drm_dp_sideband_msg_tx *msg)
> > >
> > >  req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE;
> > >  drm_dp_encode_sideband_req(&req, msg);
> > > +msg->path_msg = true;
> > >  }
> > >
> > >  static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct
> > > drm_dp_sideband_msg_hdr *hdr,
> > >
> > >  req_type = txmsg->msg[0] & 0x7f;
> > >  if (req_type == DP_CONNECTION_STATUS_NOTIFY ||
> > > -req_type == DP_RESOURCE_STATUS_NOTIFY)
> > > +req_type == DP_RESOURCE_STATUS_NOTIFY ||
> > > +req_type == DP_CLEAR_PAYLOAD_ID_TABLE)
> > >  hdr->broadcast = 1;
> >
> > Looks correct.
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Hmm. Looks like we're missing DP_POWER_DOWN_PHY and DP_POWER_UP_PHY here as well. We do try to send them as path
> > requests, but apparently forget to mark them as broadcast messages.
> Hi Ville,
> I also look up the spec but DP_POWER_DOWN_PHY & DP_POWER_UP_PHY seems to be defined as path or node request only. Not broadcast message. Please correct me if I'm wrong here.

Doh. Yeah, you're correct. Not sure what section I was reading earlier
when I came to that conclusion.

> Appreciate for your time!
> >
> > >  else
> > >  hdr->broadcast = 0;
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7C
> > > Wayne.Lin%40amd.com%7C372bbed7b5354ca05f5608d8d753533a%7C3dd8961fe4884
> > > e608e11a82d994e183d%7C0%7C0%7C637496100180287539%7CUnknown%7CTWFpbGZsb
> > > 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > > 7C1000&amp;sdata=2uhm9Nf31hfhf%2FbmwfqYW7b6ay9swWb8oS10Uc%2FVFRQ%3D&am
> > > p;reserved=0
> >
> > --
> > Ville Syrjälä
> > Intel
> Regards,
> Wayne Lin

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lin, Wayne]

[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

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

[Lin, Wayne]

[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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request

[Lyude Paul]

ooh, nice catches! For the whole series this is:

Reviewed-by: Lyude Paul <lyude@redhat.com>

I will go ahead and push these to drm-misc-next

On Mon, 2021-02-22 at 12:00 +0800, Wayne Lin wrote:
> While testing MST hotplug events on daisy chain monitors, find out
> that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table
> is not reset. Dig in deeper and find out two parts needed to be fixed.
> 
> 1. Link_Count_Total & Link_Count_Remaining of Broadcast message are
> incorrect. Should set lct=1 & lcr=6
> 2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast
> request message. Should fix this.
> 
> Wayne Lin (2):
>   drm/dp_mst: Revise broadcast msg lct & lcr
>   drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
> 
>  drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> --
> 2.17.1
> 

-- 
Sincerely,
   Lyude Paul (she/her)
   Software Engineer at Red Hat
   
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!

Re: [PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request

[Lyude Paul]

ooh, nice catches! For the whole series this is:

Reviewed-by: Lyude Paul <lyude@redhat.com>

I will go ahead and push these to drm-misc-next

On Mon, 2021-02-22 at 12:00 +0800, Wayne Lin wrote:
> While testing MST hotplug events on daisy chain monitors, find out
> that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table
> is not reset. Dig in deeper and find out two parts needed to be fixed.
> 
> 1. Link_Count_Total & Link_Count_Remaining of Broadcast message are
> incorrect. Should set lct=1 & lcr=6
> 2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast
> request message. Should fix this.
> 
> Wayne Lin (2):
>   drm/dp_mst: Revise broadcast msg lct & lcr
>   drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
> 
>  drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> --
> 2.17.1
> 

-- 
Sincerely,
   Lyude Paul (she/her)
   Software Engineer at Red Hat
   
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel