Re: [Intel-gfx] [PATCH 4/5] drm/dp/mst: Provide defines for ACK vs. NAK reply type

[Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>]

On Fri, 2018-09-28 at 21:04 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make the code a bit easier to read by providing symbolic names
> for the reply_type (ACK vs. NAK). Also clean up some brace stuff
> while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 26 +++++++++++++----------
> ---
>  include/drm/drm_dp_helper.h           |  4 ++++
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index a0652fc166c6..c0f754364cc7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -567,7 +567,7 @@ static bool drm_dp_sideband_parse_reply(struct
> drm_dp_sideband_msg_rx *raw,
>  	msg->reply_type = (raw->msg[0] & 0x80) >> 7;
>  	msg->req_type = (raw->msg[0] & 0x7f);
>  
> -	if (msg->reply_type) {
> +	if (msg->reply_type == DP_REPLY_NAK) {
>  		memcpy(msg->u.nak.guid, &raw->msg[1], 16);
>  		msg->u.nak.reason = raw->msg[17];
>  		msg->u.nak.nak_data = raw->msg[18];
> @@ -1614,9 +1614,9 @@ static void drm_dp_send_link_address(struct
> drm_dp_mst_topology_mgr *mgr,
>  	if (ret > 0) {
>  		int i;
>  
> -		if (txmsg->reply.reply_type == 1)
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK) {
>  			DRM_DEBUG_KMS("link address nak received\n");
> -		else {
> +		} else {
>  			DRM_DEBUG_KMS("link address reply: %d\n",
> txmsg->reply.u.link_addr.nports);
>  			for (i = 0; i < txmsg-
> >reply.u.link_addr.nports; i++) {
>  				DRM_DEBUG_KMS("port %d: input %d, pdt:
> %d, pn: %d, dpcd_rev: %02x, mcs: %d, ddps: %d, ldps %d, sdp %d/%d\n",
> i,
> @@ -1665,9 +1665,9 @@ static int
> drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
>  
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
> -		if (txmsg->reply.reply_type == 1)
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK) {
>  			DRM_DEBUG_KMS("enum path resources nak
> received\n");
> -		else {
> +		} else {
>  			if (port->port_num != txmsg-
> >reply.u.path_resources.port_number)
>  				DRM_ERROR("got incorrect port in
> response\n");
>  			DRM_DEBUG_KMS("enum path resources %d: %d
> %d\n", txmsg->reply.u.path_resources.port_number, txmsg-
> >reply.u.path_resources.full_payload_bw_number,
> @@ -1755,9 +1755,9 @@ static int drm_dp_payload_send_msg(struct
> drm_dp_mst_topology_mgr *mgr,
>  
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
> -		if (txmsg->reply.reply_type == 1) {
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK)
>  			ret = -EINVAL;
> -		} else
> +		else
>  			ret = 0;
>  	}
>  	kfree(txmsg);
> @@ -1789,7 +1789,7 @@ int drm_dp_send_power_updown_phy(struct
> drm_dp_mst_topology_mgr *mgr,
>  
>  	ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
>  	if (ret > 0) {
> -		if (txmsg->reply.reply_type == 1)
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK)
>  			ret = -EINVAL;
>  		else
>  			ret = 0;
> @@ -2026,9 +2026,9 @@ static int drm_dp_send_dpcd_write(struct
> drm_dp_mst_topology_mgr *mgr,
>  
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
> -		if (txmsg->reply.reply_type == 1) {
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK)
>  			ret = -EINVAL;
> -		} else
> +		else
>  			ret = 0;
>  	}
>  	kfree(txmsg);
> @@ -2041,7 +2041,7 @@ static int drm_dp_encode_up_ack_reply(struct
> drm_dp_sideband_msg_tx *msg, u8 req
>  {
>  	struct drm_dp_sideband_msg_reply_body reply;
>  
> -	reply.reply_type = 0;
> +	reply.reply_type = DP_REPLY_ACK;
>  	reply.req_type = req_type;
>  	drm_dp_encode_sideband_reply(&reply, msg);
>  	return 0;
> @@ -2348,7 +2348,7 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
>  		}
>  
>  		drm_dp_sideband_parse_reply(&mgr->down_rep_recv,
> &txmsg->reply);
> -		if (txmsg->reply.reply_type == 1) {
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK) {
>  			DRM_DEBUG_KMS("Got NAK reply: req 0x%02x,
> reason 0x%02x, nak data 0x%02x\n", txmsg->reply.req_type, txmsg-
> >reply.u.nak.reason, txmsg->reply.u.nak.nak_data);
>  		}
>  
> @@ -3306,7 +3306,7 @@ static int drm_dp_mst_i2c_xfer(struct
> i2c_adapter *adapter, struct i2c_msg *msgs
>  	ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
>  	if (ret > 0) {
>  
> -		if (txmsg->reply.reply_type == 1) { /* got a NAK back
> */
> +		if (txmsg->reply.reply_type == DP_REPLY_NAK) {
>  			ret = -EREMOTEIO;
>  			goto out;
>  		}
> diff --git a/include/drm/drm_dp_helper.h
> b/include/drm/drm_dp_helper.h
> index 2a3843f248cf..2a0fd9d7066e 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -905,6 +905,10 @@
>  #define DP_AUX_HDCP_KSV_FIFO		0x6802C
>  #define DP_AUX_HDCP_AINFO		0x6803B
>  
> +/* DP 1.2 MST sideband reply types */
> +#define DP_REPLY_ACK		0x00
> +#define DP_REPLY_NAK		0x01
> +
bikeshed: How about calling these DP_SIDEBAND_ACK or DP_SIDEBAND_NAK to
differentiate from native AUX replies? And also move it right next to
the NAK reason definition.

Will leave it to you if you want to implement those bikesheds,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


>  /* DP 1.2 Sideband message defines */
>  /* peer device type - DP 1.2a Table 2-92 */
>  #define DP_PEER_DEVICE_NONE		0x0

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