Re: [PATCH v2 20/20] drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*()

[Robert Foss]

Hey Lyude,

This patch looks good, but I have one question below. With it
addressed, feel free to add my r-b.

Reviewed-by: Robert Foss 

>
> -static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> +static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr 
> *mgr,
> + struct drm_dp_sideband_msg_rx *raw,
>   struct drm_dp_sideband_msg_req_body 
> *msg)
>  {
> memset(msg, 0, sizeof(*msg));
> @@ -1117,12 +1125,12 @@ static bool drm_dp_sideband_parse_req(struct 
> drm_dp_sideband_msg_rx *raw,
>
> switch (msg->req_type) {
> case DP_CONNECTION_STATUS_NOTIFY:
> -   return drm_dp_sideband_parse_connection_status_notify(raw, 
> msg);
> +   return drm_dp_sideband_parse_connection_status_notify(mgr, 
> raw, msg);
> case DP_RESOURCE_STATUS_NOTIFY:
> -   return drm_dp_sideband_parse_resource_status_notify(raw, msg);
> +   return drm_dp_sideband_parse_resource_status_notify(mgr, raw, 
> msg);
> default:
> -   DRM_ERROR("Got unknown request 0x%02x (%s)\n", msg->req_type,
> - drm_dp_mst_req_type_str(msg->req_type));
> +   drm_err(mgr->dev, "Got unknown request 0x%02x (%s)\n",
> +   msg->req_type, 
> drm_dp_mst_req_type_str(msg->req_type));
> return false;
> }
>  }
>

.. snip ..

> @@ -4118,12 +4121,12 @@ static int drm_dp_mst_handle_up_req(struct 
> drm_dp_mst_topology_mgr *mgr)
>
> INIT_LIST_HEAD(_req->next);
>
> -   drm_dp_sideband_parse_req(>up_req_recv, _req->msg);
> +   drm_dp_sideband_parse_req(mgr, >up_req_recv, _req->msg);

drm_dp_sideband_parse_req() is only called here, and the function
arguments could probably stand to have `>up_req_recv` removed
(here and in the func. declaration) since the same data structure is
accessible through the `mgr` pointer inside of
drm_dp_sideband_parse_req(). I guess this is a matter of taste, so
feel free to do what you want with this.

>
> if (up_req->msg.req_type != DP_CONNECTION_STATUS_NOTIFY &&
> up_req->msg.req_type != DP_RESOURCE_STATUS_NOTIFY) {
> -   DRM_DEBUG_KMS("Received unknown up req type, ignoring: %x\n",
> - up_req->msg.req_type);
> +   drm_dbg_kms(mgr->dev, "Received unknown up req type, 
> ignoring: %x\n",
> +   up_req->msg.req_type);
> kfree(up_req);
> goto out;
> }

[PATCH v2 20/20] drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*()

[Lyude Paul]

And finally, convert all of the code in drm_dp_mst_topology.c over to using
drm_err() and drm_dbg*(). Note that this refactor would have been a lot
more complicated to have tried writing a coccinelle script for, so this
whole thing was done by hand.

v2:
* Fix line-wrapping in drm_dp_mst_atomic_check_mstb_bw_limit()

Signed-off-by: Lyude Paul 
Cc: Robert Foss 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 368 +-
 1 file changed, 187 insertions(+), 181 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 74c420f5f204..b8c3a99a4119 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -286,7 +286,8 @@ static void drm_dp_encode_sideband_msg_hdr(struct 
drm_dp_sideband_msg_hdr *hdr,
*len = idx;
 }
 
-static bool drm_dp_decode_sideband_msg_hdr(struct drm_dp_sideband_msg_hdr *hdr,
+static bool drm_dp_decode_sideband_msg_hdr(const struct 
drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_sideband_msg_hdr *hdr,
   u8 *buf, int buflen, u8 *hdrlen)
 {
u8 crc4;
@@ -303,7 +304,7 @@ static bool drm_dp_decode_sideband_msg_hdr(struct 
drm_dp_sideband_msg_hdr *hdr,
crc4 = drm_dp_msg_header_crc4(buf, (len * 2) - 1);
 
if ((crc4 & 0xf) != (buf[len - 1] & 0xf)) {
-   DRM_DEBUG_KMS("crc4 mismatch 0x%x 0x%x\n", crc4, buf[len - 1]);
+   drm_dbg_kms(mgr->dev, "crc4 mismatch 0x%x 0x%x\n", crc4, 
buf[len - 1]);
return false;
}
 
@@ -789,7 +790,8 @@ static bool drm_dp_sideband_append_payload(struct 
drm_dp_sideband_msg_rx *msg,
return true;
 }
 
-static bool drm_dp_sideband_parse_link_address(struct drm_dp_sideband_msg_rx 
*raw,
+static bool drm_dp_sideband_parse_link_address(const struct 
drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_sideband_msg_rx 
*raw,
   struct 
drm_dp_sideband_msg_reply_body *repmsg)
 {
int idx = 1;
@@ -1014,7 +1016,8 @@ drm_dp_sideband_parse_query_stream_enc_status(
return true;
 }
 
-static bool drm_dp_sideband_parse_reply(struct drm_dp_sideband_msg_rx *raw,
+static bool drm_dp_sideband_parse_reply(const struct drm_dp_mst_topology_mgr 
*mgr,
+   struct drm_dp_sideband_msg_rx *raw,
struct drm_dp_sideband_msg_reply_body 
*msg)
 {
memset(msg, 0, sizeof(*msg));
@@ -1030,7 +1033,7 @@ static bool drm_dp_sideband_parse_reply(struct 
drm_dp_sideband_msg_rx *raw,
 
switch (msg->req_type) {
case DP_LINK_ADDRESS:
-   return drm_dp_sideband_parse_link_address(raw, msg);
+   return drm_dp_sideband_parse_link_address(mgr, raw, msg);
case DP_QUERY_PAYLOAD:
return drm_dp_sideband_parse_query_payload_ack(raw, msg);
case DP_REMOTE_DPCD_READ:
@@ -1053,14 +1056,16 @@ static bool drm_dp_sideband_parse_reply(struct 
drm_dp_sideband_msg_rx *raw,
case DP_QUERY_STREAM_ENC_STATUS:
return drm_dp_sideband_parse_query_stream_enc_status(raw, msg);
default:
-   DRM_ERROR("Got unknown reply 0x%02x (%s)\n", msg->req_type,
- drm_dp_mst_req_type_str(msg->req_type));
+   drm_err(mgr->dev, "Got unknown reply 0x%02x (%s)\n",
+   msg->req_type, drm_dp_mst_req_type_str(msg->req_type));
return false;
}
 }
 
-static bool drm_dp_sideband_parse_connection_status_notify(struct 
drm_dp_sideband_msg_rx *raw,
-  struct 
drm_dp_sideband_msg_req_body *msg)
+static bool
+drm_dp_sideband_parse_connection_status_notify(const struct 
drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_sideband_msg_rx 
*raw,
+  struct 
drm_dp_sideband_msg_req_body *msg)
 {
int idx = 1;
 
@@ -1082,12 +1087,14 @@ static bool 
drm_dp_sideband_parse_connection_status_notify(struct drm_dp_sideban
idx++;
return true;
 fail_len:
-   DRM_DEBUG_KMS("connection status reply parse length fail %d %d\n", idx, 
raw->curlen);
+   drm_dbg_kms(mgr->dev, "connection status reply parse length fail %d 
%d\n",
+   idx, raw->curlen);
return false;
 }
 
-static bool drm_dp_sideband_parse_resource_status_notify(struct 
drm_dp_sideband_msg_rx *raw,
-  struct 
drm_dp_sideband_msg_req_body *msg)
+static bool drm_dp_sideband_parse_resource_status_notify(const struct 
drm_dp_mst_topology_mgr *mgr,
+struct 
drm_dp_sideband_msg_rx *raw,
+struct 
drm_dp_sideband_msg_req_body *msg)
 {