[PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers

[PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers

[Stephen Boyd]

This driver open-codes a few of the DPCD register reads when it can be
simplified by using the helpers instead. This series reworks the MSM DP
driver to use the DPCD helpers and removes some dead code along the way.
There's the potential for even more code reduction around the test
registers, but I haven't tried to do that yet.

Stephen Boyd (7):
  drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
  drm/msm/dp: Use drm_dp_read_sink_count() helper
  drm/msm/dp: Remove dead code related to downstream cap info
  drm/msm/dp: Remove aux_cfg_update_done and related code
  drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
  drm/msm/dp: Inline dp_link_parse_sink_count()
  drm/msm/dp: Remove dp_display_is_ds_bridge()

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>

 drivers/gpu/drm/msm/dp/dp_display.c |   9 +--
 drivers/gpu/drm/msm/dp/dp_link.c    |  38 +---------
 drivers/gpu/drm/msm/dp/dp_panel.c   | 105 +++++-----------------------
 drivers/gpu/drm/msm/dp/dp_panel.h   |  10 +--
 4 files changed, 22 insertions(+), 140 deletions(-)


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
-- 
https://chromeos.dev

[PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()

[Stephen Boyd]

This function duplicates the common function drm_dp_read_dpcd_caps().
The array of DPCD registers filled in is one size larger than the
function takes, but from what I can tell that extra byte was never used.
Resize the array and use the common function to reduce the code here.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 42 ++++---------------------------
 drivers/gpu/drm/msm/dp/dp_panel.h |  4 +--
 2 files changed, 6 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 42d52510ffd4..09d4f6c38ef8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -48,47 +48,15 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
 	ssize_t rlen;
 	struct dp_panel_private *panel;
 	struct dp_link_info *link_info;
-	u8 *dpcd, major = 0, minor = 0, temp;
-	u32 offset = DP_DPCD_REV;
+	u8 *dpcd, major, minor;
 
+	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
 	dpcd = dp_panel->dpcd;
+	rc = drm_dp_read_dpcd_caps(panel->aux, dpcd);
+	if (rc)
+		return rc;
 
-	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
 	link_info = &dp_panel->link_info;
-
-	rlen = drm_dp_dpcd_read(panel->aux, offset,
-			dpcd, (DP_RECEIVER_CAP_SIZE + 1));
-	if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
-		DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
-		if (rlen == -ETIMEDOUT)
-			rc = rlen;
-		else
-			rc = -EINVAL;
-
-		goto end;
-	}
-
-	temp = dpcd[DP_TRAINING_AUX_RD_INTERVAL];
-
-	/* check for EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT */
-	if (temp & BIT(7)) {
-		drm_dbg_dp(panel->drm_dev,
-				"using EXTENDED_RECEIVER_CAPABILITY_FIELD\n");
-		offset = DPRX_EXTENDED_DPCD_FIELD;
-	}
-
-	rlen = drm_dp_dpcd_read(panel->aux, offset,
-		dpcd, (DP_RECEIVER_CAP_SIZE + 1));
-	if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
-		DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
-		if (rlen == -ETIMEDOUT)
-			rc = rlen;
-		else
-			rc = -EINVAL;
-
-		goto end;
-	}
-
 	link_info->revision = dpcd[DP_DPCD_REV];
 	major = (link_info->revision >> 4) & 0x0f;
 	minor = link_info->revision & 0x0f;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index ed1030e17e1b..6d733480a62d 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -13,8 +13,6 @@
 
 struct edid;
 
-#define DPRX_EXTENDED_DPCD_FIELD	0x2200
-
 #define DP_DOWNSTREAM_PORTS		4
 #define DP_DOWNSTREAM_CAP_SIZE		4
 
@@ -40,7 +38,7 @@ struct dp_panel_psr {
 
 struct dp_panel {
 	/* dpcd raw data */
-	u8 dpcd[DP_RECEIVER_CAP_SIZE + 1];
+	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
 	u32 ds_port_cnt;
 	u32 dfp_present;
-- 
https://chromeos.dev

[PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper

[Stephen Boyd]

Use the common function drm_dp_read_sink_count() instead of open-coding
it. This shrinks the kernel text a tiny bit.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 09d4f6c38ef8..a0523b18b9e9 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -147,8 +147,8 @@ static int dp_panel_update_modes(struct drm_connector *connector,
 int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
 	struct drm_connector *connector)
 {
-	int rc = 0, bw_code;
-	int rlen, count;
+	int rc, bw_code;
+	int count;
 	struct dp_panel_private *panel;
 
 	if (!dp_panel || !connector) {
@@ -174,16 +174,11 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
 	}
 
 	if (dp_panel->dfp_present) {
-		rlen = drm_dp_dpcd_read(panel->aux, DP_SINK_COUNT,
-				&count, 1);
-		if (rlen == 1) {
-			count = DP_GET_SINK_COUNT(count);
-			if (!count) {
-				DRM_ERROR("no downstream ports connected\n");
-				panel->link->sink_count = 0;
-				rc = -ENOTCONN;
-				goto end;
-			}
+		count = drm_dp_read_sink_count(panel->aux);
+		if (!count) {
+			DRM_ERROR("no downstream ports connected\n");
+			panel->link->sink_count = 0;
+			return -ENOTCONN;
 		}
 	}
 
-- 
https://chromeos.dev

[PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info

[Stephen Boyd]

We read the downstream port count and capability info but never use it
anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code
from this driver. Fold the check for 'dfp_present' into a call to
drm_dp_is_branch() at the one place it is used to get rid of any member
storage related to downstream ports.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++----------------------
 drivers/gpu/drm/msm/dp/dp_panel.h |  6 ------
 2 files changed, 3 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index a0523b18b9e9..9fb4e963fefb 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -43,9 +43,7 @@ static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
 
 static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
 {
-	int rc = 0;
-	size_t len;
-	ssize_t rlen;
+	int rc;
 	struct dp_panel_private *panel;
 	struct dp_link_info *link_info;
 	u8 *dpcd, major, minor;
@@ -79,25 +77,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
 	if (drm_dp_enhanced_frame_cap(dpcd))
 		link_info->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
 
-	dp_panel->dfp_present = dpcd[DP_DOWNSTREAMPORT_PRESENT];
-	dp_panel->dfp_present &= DP_DWN_STRM_PORT_PRESENT;
-
-	if (dp_panel->dfp_present && (dpcd[DP_DPCD_REV] > 0x10)) {
-		dp_panel->ds_port_cnt = dpcd[DP_DOWN_STREAM_PORT_COUNT];
-		dp_panel->ds_port_cnt &= DP_PORT_COUNT_MASK;
-		len = DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE;
-
-		rlen = drm_dp_dpcd_read(panel->aux,
-			DP_DOWNSTREAM_PORT_0, dp_panel->ds_cap_info, len);
-		if (rlen < len) {
-			DRM_ERROR("ds port status failed, rlen=%zd\n", rlen);
-			rc = -EINVAL;
-			goto end;
-		}
-	}
-
 	dp_panel_read_psr_cap(panel);
-end:
+
 	return rc;
 }
 
@@ -173,7 +154,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
 		return -EINVAL;
 	}
 
-	if (dp_panel->dfp_present) {
+	if (drm_dp_is_branch(dp_panel->dpcd)) {
 		count = drm_dp_read_sink_count(panel->aux);
 		if (!count) {
 			DRM_ERROR("no downstream ports connected\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 6d733480a62d..3cb1f8dcfd3b 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -13,9 +13,6 @@
 
 struct edid;
 
-#define DP_DOWNSTREAM_PORTS		4
-#define DP_DOWNSTREAM_CAP_SIZE		4
-
 struct dp_display_mode {
 	struct drm_display_mode drm_mode;
 	u32 capabilities;
@@ -39,9 +36,6 @@ struct dp_panel_psr {
 struct dp_panel {
 	/* dpcd raw data */
 	u8 dpcd[DP_RECEIVER_CAP_SIZE];
-	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
-	u32 ds_port_cnt;
-	u32 dfp_present;
 
 	struct dp_link_info link_info;
 	struct drm_dp_desc desc;
-- 
https://chromeos.dev

[PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code

[Stephen Boyd]

The member 'aux_cfg_update_done' is always false. This is dead code that
never runs. Remove it.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 9fb4e963fefb..0893522ae158 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -17,7 +17,6 @@ struct dp_panel_private {
 	struct dp_link *link;
 	struct dp_catalog *catalog;
 	bool panel_on;
-	bool aux_cfg_update_done;
 };
 
 static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
@@ -177,19 +176,6 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
 		}
 	}
 
-	if (panel->aux_cfg_update_done) {
-		drm_dbg_dp(panel->drm_dev,
-				"read DPCD with updated AUX config\n");
-		rc = dp_panel_read_dpcd(dp_panel);
-		bw_code = drm_dp_link_rate_to_bw_code(dp_panel->link_info.rate);
-		if (rc || !is_link_rate_valid(bw_code) ||
-			!is_lane_count_valid(dp_panel->link_info.num_lanes)
-			|| (bw_code > dp_panel->max_bw_code)) {
-			DRM_ERROR("read dpcd failed %d\n", rc);
-			return rc;
-		}
-		panel->aux_cfg_update_done = false;
-	}
 end:
 	return rc;
 }
@@ -434,7 +420,6 @@ struct dp_panel *dp_panel_get(struct dp_panel_in *in)
 
 	dp_panel = &panel->dp_panel;
 	dp_panel->max_bw_code = DP_LINK_BW_8_1;
-	panel->aux_cfg_update_done = false;
 
 	return dp_panel;
 }
-- 
https://chromeos.dev

[PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()

[Stephen Boyd]

These are open-coded versions of common functions. Replace them with the
common code to improve readability.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 0893522ae158..97ba41593820 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -58,8 +58,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
 	major = (link_info->revision >> 4) & 0x0f;
 	minor = link_info->revision & 0x0f;
 
-	link_info->rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
-	link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
+	link_info->rate = drm_dp_max_link_rate(dpcd);
+	link_info->num_lanes = drm_dp_max_lane_count(dpcd);
 
 	/* Limit data lanes from data-lanes of endpoint property of dtsi */
 	if (link_info->num_lanes > dp_panel->max_dp_lanes)
-- 
https://chromeos.dev

[PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()

[Stephen Boyd]

The function dp_link_parse_sink_count() is really just
drm_dp_read_sink_count(). It debug prints out the bit for content
protection (DP_SINK_CP_READY), but that is not useful beyond debug
because 'link->dp_link.sink_count' is overwritten to only contain the
sink_count in this same function. Just use drm_dp_read_sink_count() in
the one place this function is called to simplify.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_link.c | 38 +++-----------------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 42427129acea..94a37914a47f 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -712,49 +712,17 @@ static int dp_link_parse_request(struct dp_link_private *link)
 	return ret;
 }
 
-/**
- * dp_link_parse_sink_count() - parses the sink count
- * @dp_link: pointer to link module data
- *
- * Parses the DPCD to check if there is an update to the sink count
- * (Byte 0x200), and whether all the sink devices connected have Content
- * Protection enabled.
- */
-static int dp_link_parse_sink_count(struct dp_link *dp_link)
-{
-	ssize_t rlen;
-	bool cp_ready;
-
-	struct dp_link_private *link = container_of(dp_link,
-			struct dp_link_private, dp_link);
-
-	rlen = drm_dp_dpcd_readb(link->aux, DP_SINK_COUNT,
-				 &link->dp_link.sink_count);
-	if (rlen < 0) {
-		DRM_ERROR("sink count read failed. rlen=%zd\n", rlen);
-		return rlen;
-	}
-
-	cp_ready = link->dp_link.sink_count & DP_SINK_CP_READY;
-
-	link->dp_link.sink_count =
-		DP_GET_SINK_COUNT(link->dp_link.sink_count);
-
-	drm_dbg_dp(link->drm_dev, "sink_count = 0x%x, cp_ready = 0x%x\n",
-				link->dp_link.sink_count, cp_ready);
-	return 0;
-}
-
 static int dp_link_parse_sink_status_field(struct dp_link_private *link)
 {
-	int len = 0;
+	int len;
 
 	link->prev_sink_count = link->dp_link.sink_count;
-	len = dp_link_parse_sink_count(&link->dp_link);
+	len = drm_dp_read_sink_count(link->aux);
 	if (len < 0) {
 		DRM_ERROR("DP parse sink count failed\n");
 		return len;
 	}
+	link->dp_link.sink_count = len;
 
 	len = drm_dp_dpcd_read_link_status(link->aux,
 		link->link_status);
-- 
https://chromeos.dev

[PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()

[Stephen Boyd]

This function is simply drm_dp_is_branch() so use that instead of
open-coding it.

Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 76f13954015b..96bbf6fec2f1 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
 	.unbind = dp_display_unbind,
 };
 
-static bool dp_display_is_ds_bridge(struct dp_panel *panel)
-{
-	return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
-		DP_DWN_STRM_PORT_PRESENT);
-}
-
 static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
 {
 	drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
 			dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
 		dp->link->sink_count);
-	return dp_display_is_ds_bridge(dp->panel) &&
-		(dp->link->sink_count == 0);
+	return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
 }
 
 static void dp_display_send_hpd_event(struct msm_dp *dp_display)
-- 
https://chromeos.dev

Re: [PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> This function duplicates the common function drm_dp_read_dpcd_caps().
> The array of DPCD registers filled in is one size larger than the
> function takes, but from what I can tell that extra byte was never used.
> Resize the array and use the common function to reduce the code here.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 42 ++++---------------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h |  4 +--
>   2 files changed, 6 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 42d52510ffd4..09d4f6c38ef8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -48,47 +48,15 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   	ssize_t rlen;
>   	struct dp_panel_private *panel;
>   	struct dp_link_info *link_info;
> -	u8 *dpcd, major = 0, minor = 0, temp;
> -	u32 offset = DP_DPCD_REV;
> +	u8 *dpcd, major, minor;
>   
> +	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>   	dpcd = dp_panel->dpcd;
> +	rc = drm_dp_read_dpcd_caps(panel->aux, dpcd);
> +	if (rc)
> +		return rc;
>   
> -	panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>   	link_info = &dp_panel->link_info;
> -
> -	rlen = drm_dp_dpcd_read(panel->aux, offset,
> -			dpcd, (DP_RECEIVER_CAP_SIZE + 1));
> -	if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
> -		DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
> -		if (rlen == -ETIMEDOUT)
> -			rc = rlen;
> -		else
> -			rc = -EINVAL;
> -
> -		goto end;
> -	}
> -
> -	temp = dpcd[DP_TRAINING_AUX_RD_INTERVAL];
> -
> -	/* check for EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT */
> -	if (temp & BIT(7)) {
> -		drm_dbg_dp(panel->drm_dev,
> -				"using EXTENDED_RECEIVER_CAPABILITY_FIELD\n");
> -		offset = DPRX_EXTENDED_DPCD_FIELD;
> -	}
> -
> -	rlen = drm_dp_dpcd_read(panel->aux, offset,
> -		dpcd, (DP_RECEIVER_CAP_SIZE + 1));
> -	if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
> -		DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
> -		if (rlen == -ETIMEDOUT)
> -			rc = rlen;
> -		else
> -			rc = -EINVAL;
> -
> -		goto end;
> -	}
> -
>   	link_info->revision = dpcd[DP_DPCD_REV];
>   	major = (link_info->revision >> 4) & 0x0f;
>   	minor = link_info->revision & 0x0f;
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index ed1030e17e1b..6d733480a62d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -13,8 +13,6 @@
>   
>   struct edid;
>   
> -#define DPRX_EXTENDED_DPCD_FIELD	0x2200
> -
>   #define DP_DOWNSTREAM_PORTS		4
>   #define DP_DOWNSTREAM_CAP_SIZE		4
>   
> @@ -40,7 +38,7 @@ struct dp_panel_psr {
>   
>   struct dp_panel {
>   	/* dpcd raw data */
> -	u8 dpcd[DP_RECEIVER_CAP_SIZE + 1];
> +	u8 dpcd[DP_RECEIVER_CAP_SIZE];
>   	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
>   	u32 ds_port_cnt;
>   	u32 dfp_present;

Re: [PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> Use the common function drm_dp_read_sink_count() instead of open-coding
> it. This shrinks the kernel text a tiny bit.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 09d4f6c38ef8..a0523b18b9e9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -147,8 +147,8 @@ static int dp_panel_update_modes(struct drm_connector *connector,
>   int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   	struct drm_connector *connector)
>   {
> -	int rc = 0, bw_code;
> -	int rlen, count;
> +	int rc, bw_code;
> +	int count;
>   	struct dp_panel_private *panel;
>   
>   	if (!dp_panel || !connector) {
> @@ -174,16 +174,11 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   	}
>   
>   	if (dp_panel->dfp_present) {
> -		rlen = drm_dp_dpcd_read(panel->aux, DP_SINK_COUNT,
> -				&count, 1);
> -		if (rlen == 1) {
> -			count = DP_GET_SINK_COUNT(count);
> -			if (!count) {
> -				DRM_ERROR("no downstream ports connected\n");
> -				panel->link->sink_count = 0;
> -				rc = -ENOTCONN;
> -				goto end;
> -			}
> +		count = drm_dp_read_sink_count(panel->aux);
> +		if (!count) {
> +			DRM_ERROR("no downstream ports connected\n");
> +			panel->link->sink_count = 0;
> +			return -ENOTCONN;
>   		}
>   	}
>   

Re: [PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> We read the downstream port count and capability info but never use it
> anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code
> from this driver. Fold the check for 'dfp_present' into a call to
> drm_dp_is_branch() at the one place it is used to get rid of any member
> storage related to downstream ports.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++----------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h |  6 ------
>   2 files changed, 3 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index a0523b18b9e9..9fb4e963fefb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -43,9 +43,7 @@ static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
>   
>   static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   {
> -	int rc = 0;
> -	size_t len;
> -	ssize_t rlen;
> +	int rc;
>   	struct dp_panel_private *panel;
>   	struct dp_link_info *link_info;
>   	u8 *dpcd, major, minor;
> @@ -79,25 +77,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   	if (drm_dp_enhanced_frame_cap(dpcd))
>   		link_info->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
>   
> -	dp_panel->dfp_present = dpcd[DP_DOWNSTREAMPORT_PRESENT];
> -	dp_panel->dfp_present &= DP_DWN_STRM_PORT_PRESENT;
> -
> -	if (dp_panel->dfp_present && (dpcd[DP_DPCD_REV] > 0x10)) {
> -		dp_panel->ds_port_cnt = dpcd[DP_DOWN_STREAM_PORT_COUNT];
> -		dp_panel->ds_port_cnt &= DP_PORT_COUNT_MASK;
> -		len = DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE;
> -
> -		rlen = drm_dp_dpcd_read(panel->aux,
> -			DP_DOWNSTREAM_PORT_0, dp_panel->ds_cap_info, len);
> -		if (rlen < len) {
> -			DRM_ERROR("ds port status failed, rlen=%zd\n", rlen);
> -			rc = -EINVAL;
> -			goto end;
> -		}
> -	}
> -
>   	dp_panel_read_psr_cap(panel);
> -end:
> +
>   	return rc;
>   }
>   
> @@ -173,7 +154,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   		return -EINVAL;
>   	}
>   
> -	if (dp_panel->dfp_present) {
> +	if (drm_dp_is_branch(dp_panel->dpcd)) {
>   		count = drm_dp_read_sink_count(panel->aux);
>   		if (!count) {
>   			DRM_ERROR("no downstream ports connected\n");
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index 6d733480a62d..3cb1f8dcfd3b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -13,9 +13,6 @@
>   
>   struct edid;
>   
> -#define DP_DOWNSTREAM_PORTS		4
> -#define DP_DOWNSTREAM_CAP_SIZE		4
> -
>   struct dp_display_mode {
>   	struct drm_display_mode drm_mode;
>   	u32 capabilities;
> @@ -39,9 +36,6 @@ struct dp_panel_psr {
>   struct dp_panel {
>   	/* dpcd raw data */
>   	u8 dpcd[DP_RECEIVER_CAP_SIZE];
> -	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
> -	u32 ds_port_cnt;
> -	u32 dfp_present;
>   
>   	struct dp_link_info link_info;
>   	struct drm_dp_desc desc;

Re: [PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> We read the downstream port count and capability info but never use it
> anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code
> from this driver. Fold the check for 'dfp_present' into a call to
> drm_dp_is_branch() at the one place it is used to get rid of any member
> storage related to downstream ports.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++----------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h |  6 ------
>   2 files changed, 3 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index a0523b18b9e9..9fb4e963fefb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -43,9 +43,7 @@ static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
>   
>   static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   {
> -	int rc = 0;
> -	size_t len;
> -	ssize_t rlen;
> +	int rc;
>   	struct dp_panel_private *panel;
>   	struct dp_link_info *link_info;
>   	u8 *dpcd, major, minor;
> @@ -79,25 +77,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   	if (drm_dp_enhanced_frame_cap(dpcd))
>   		link_info->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
>   
> -	dp_panel->dfp_present = dpcd[DP_DOWNSTREAMPORT_PRESENT];
> -	dp_panel->dfp_present &= DP_DWN_STRM_PORT_PRESENT;
> -
> -	if (dp_panel->dfp_present && (dpcd[DP_DPCD_REV] > 0x10)) {
> -		dp_panel->ds_port_cnt = dpcd[DP_DOWN_STREAM_PORT_COUNT];
> -		dp_panel->ds_port_cnt &= DP_PORT_COUNT_MASK;
> -		len = DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE;
> -
> -		rlen = drm_dp_dpcd_read(panel->aux,
> -			DP_DOWNSTREAM_PORT_0, dp_panel->ds_cap_info, len);
> -		if (rlen < len) {
> -			DRM_ERROR("ds port status failed, rlen=%zd\n", rlen);
> -			rc = -EINVAL;
> -			goto end;
> -		}
> -	}
> -
>   	dp_panel_read_psr_cap(panel);
> -end:
> +
>   	return rc;
>   }
>   
> @@ -173,7 +154,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   		return -EINVAL;
>   	}
>   
> -	if (dp_panel->dfp_present) {
> +	if (drm_dp_is_branch(dp_panel->dpcd)) {
>   		count = drm_dp_read_sink_count(panel->aux);
>   		if (!count) {
>   			DRM_ERROR("no downstream ports connected\n");
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index 6d733480a62d..3cb1f8dcfd3b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -13,9 +13,6 @@
>   
>   struct edid;
>   
> -#define DP_DOWNSTREAM_PORTS		4
> -#define DP_DOWNSTREAM_CAP_SIZE		4
> -
>   struct dp_display_mode {
>   	struct drm_display_mode drm_mode;
>   	u32 capabilities;
> @@ -39,9 +36,6 @@ struct dp_panel_psr {
>   struct dp_panel {
>   	/* dpcd raw data */
>   	u8 dpcd[DP_RECEIVER_CAP_SIZE];
> -	u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
> -	u32 ds_port_cnt;
> -	u32 dfp_present;
>   
>   	struct dp_link_info link_info;
>   	struct drm_dp_desc desc;

Re: [PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> The member 'aux_cfg_update_done' is always false. This is dead code that
> never runs. Remove it.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 15 ---------------
>   1 file changed, 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 9fb4e963fefb..0893522ae158 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -17,7 +17,6 @@ struct dp_panel_private {
>   	struct dp_link *link;
>   	struct dp_catalog *catalog;
>   	bool panel_on;
> -	bool aux_cfg_update_done;
>   };
>   
>   static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
> @@ -177,19 +176,6 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
>   		}
>   	}
>   
> -	if (panel->aux_cfg_update_done) {
> -		drm_dbg_dp(panel->drm_dev,
> -				"read DPCD with updated AUX config\n");
> -		rc = dp_panel_read_dpcd(dp_panel);
> -		bw_code = drm_dp_link_rate_to_bw_code(dp_panel->link_info.rate);
> -		if (rc || !is_link_rate_valid(bw_code) ||
> -			!is_lane_count_valid(dp_panel->link_info.num_lanes)
> -			|| (bw_code > dp_panel->max_bw_code)) {
> -			DRM_ERROR("read dpcd failed %d\n", rc);
> -			return rc;
> -		}
> -		panel->aux_cfg_update_done = false;
> -	}
>   end:
>   	return rc;
>   }
> @@ -434,7 +420,6 @@ struct dp_panel *dp_panel_get(struct dp_panel_in *in)
>   
>   	dp_panel = &panel->dp_panel;
>   	dp_panel->max_bw_code = DP_LINK_BW_8_1;
> -	panel->aux_cfg_update_done = false;
>   
>   	return dp_panel;
>   }

Re: [PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> These are open-coded versions of common functions. Replace them with the
> common code to improve readability.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 0893522ae158..97ba41593820 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -58,8 +58,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   	major = (link_info->revision >> 4) & 0x0f;
>   	minor = link_info->revision & 0x0f;
>   
> -	link_info->rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
> -	link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
> +	link_info->rate = drm_dp_max_link_rate(dpcd);
> +	link_info->num_lanes = drm_dp_max_lane_count(dpcd);
>   
>   	/* Limit data lanes from data-lanes of endpoint property of dtsi */
>   	if (link_info->num_lanes > dp_panel->max_dp_lanes)

Re: [PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> The function dp_link_parse_sink_count() is really just
> drm_dp_read_sink_count(). It debug prints out the bit for content
> protection (DP_SINK_CP_READY), but that is not useful beyond debug
> because 'link->dp_link.sink_count' is overwritten to only contain the
> sink_count in this same function. Just use drm_dp_read_sink_count() in
> the one place this function is called to simplify.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_link.c | 38 +++-----------------------------
>   1 file changed, 3 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
> index 42427129acea..94a37914a47f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_link.c
> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
> @@ -712,49 +712,17 @@ static int dp_link_parse_request(struct dp_link_private *link)
>   	return ret;
>   }
>   
> -/**
> - * dp_link_parse_sink_count() - parses the sink count
> - * @dp_link: pointer to link module data
> - *
> - * Parses the DPCD to check if there is an update to the sink count
> - * (Byte 0x200), and whether all the sink devices connected have Content
> - * Protection enabled.
> - */
> -static int dp_link_parse_sink_count(struct dp_link *dp_link)
> -{
> -	ssize_t rlen;
> -	bool cp_ready;
> -
> -	struct dp_link_private *link = container_of(dp_link,
> -			struct dp_link_private, dp_link);
> -
> -	rlen = drm_dp_dpcd_readb(link->aux, DP_SINK_COUNT,
> -				 &link->dp_link.sink_count);
> -	if (rlen < 0) {
> -		DRM_ERROR("sink count read failed. rlen=%zd\n", rlen);
> -		return rlen;
> -	}
> -
> -	cp_ready = link->dp_link.sink_count & DP_SINK_CP_READY;
> -
> -	link->dp_link.sink_count =
> -		DP_GET_SINK_COUNT(link->dp_link.sink_count);
> -
> -	drm_dbg_dp(link->drm_dev, "sink_count = 0x%x, cp_ready = 0x%x\n",
> -				link->dp_link.sink_count, cp_ready);
> -	return 0;
> -}
> -
>   static int dp_link_parse_sink_status_field(struct dp_link_private *link)
>   {
> -	int len = 0;
> +	int len;
>   
>   	link->prev_sink_count = link->dp_link.sink_count;
> -	len = dp_link_parse_sink_count(&link->dp_link);
> +	len = drm_dp_read_sink_count(link->aux);
>   	if (len < 0) {
>   		DRM_ERROR("DP parse sink count failed\n");
>   		return len;
>   	}
> +	link->dp_link.sink_count = len;
>   
>   	len = drm_dp_dpcd_read_link_status(link->aux,
>   		link->link_status);

Re: [PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()

[Kuogee Hsieh]

On 8/29/2023 11:47 AM, Stephen Boyd wrote:
> This function is simply drm_dp_is_branch() so use that instead of
> open-coding it.
>
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 76f13954015b..96bbf6fec2f1 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
>   	.unbind = dp_display_unbind,
>   };
>   
> -static bool dp_display_is_ds_bridge(struct dp_panel *panel)
> -{
> -	return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> -		DP_DWN_STRM_PORT_PRESENT);
> -}
> -
>   static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
>   {
>   	drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
>   			dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
>   		dp->link->sink_count);
> -	return dp_display_is_ds_bridge(dp->panel) &&
> -		(dp->link->sink_count == 0);
> +	return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
>   }
>   
>   static void dp_display_send_hpd_event(struct msm_dp *dp_display)

Re: [PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> This function duplicates the common function drm_dp_read_dpcd_caps().
> The array of DPCD registers filled in is one size larger than the
> function takes, but from what I can tell that extra byte was never used.
> Resize the array and use the common function to reduce the code here.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 42 ++++---------------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h |  4 +--
>   2 files changed, 6 insertions(+), 40 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> Use the common function drm_dp_read_sink_count() instead of open-coding
> it. This shrinks the kernel text a tiny bit.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> We read the downstream port count and capability info but never use it
> anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code
> from this driver. Fold the check for 'dfp_present' into a call to
> drm_dp_is_branch() at the one place it is used to get rid of any member
> storage related to downstream ports.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++----------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h |  6 ------
>   2 files changed, 3 insertions(+), 28 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> The member 'aux_cfg_update_done' is always false. This is dead code that
> never runs. Remove it.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 15 ---------------
>   1 file changed, 15 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> These are open-coded versions of common functions. Replace them with the
> common code to improve readability.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> The function dp_link_parse_sink_count() is really just
> drm_dp_read_sink_count(). It debug prints out the bit for content
> protection (DP_SINK_CP_READY), but that is not useful beyond debug
> because 'link->dp_link.sink_count' is overwritten to only contain the
> sink_count in this same function. Just use drm_dp_read_sink_count() in
> the one place this function is called to simplify.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_link.c | 38 +++-----------------------------
>   1 file changed, 3 insertions(+), 35 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> This function is simply drm_dp_is_branch() so use that instead of
> open-coding it.
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 76f13954015b..96bbf6fec2f1 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
>   	.unbind = dp_display_unbind,
>   };
>   
> -static bool dp_display_is_ds_bridge(struct dp_panel *panel)
> -{
> -	return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> -		DP_DWN_STRM_PORT_PRESENT);
> -}
> -
>   static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)

Nit: you might as well inline this function

>   {
>   	drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
>   			dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
>   		dp->link->sink_count);
> -	return dp_display_is_ds_bridge(dp->panel) &&
> -		(dp->link->sink_count == 0);
> +	return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
>   }
>   
>   static void dp_display_send_hpd_event(struct msm_dp *dp_display)

-- 
With best wishes
Dmitry

Re: [PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers

[Dmitry Baryshkov]

On 29/08/2023 21:47, Stephen Boyd wrote:
> This driver open-codes a few of the DPCD register reads when it can be
> simplified by using the helpers instead. This series reworks the MSM DP
> driver to use the DPCD helpers and removes some dead code along the way.
> There's the potential for even more code reduction around the test
> registers, but I haven't tried to do that yet.

For the whole series:

Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Using drm_dp_get_phy_test_pattern() / drm_dp_set_phy_test_pattern() 
would be definitely a benefit, especially since the latter one has 
support for DP >= 1.2, while msm DP code doesn't.

> 
> Stephen Boyd (7):
>    drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
>    drm/msm/dp: Use drm_dp_read_sink_count() helper
>    drm/msm/dp: Remove dead code related to downstream cap info
>    drm/msm/dp: Remove aux_cfg_update_done and related code
>    drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
>    drm/msm/dp: Inline dp_link_parse_sink_count()
>    drm/msm/dp: Remove dp_display_is_ds_bridge()
> 
> Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> 
>   drivers/gpu/drm/msm/dp/dp_display.c |   9 +--
>   drivers/gpu/drm/msm/dp/dp_link.c    |  38 +---------
>   drivers/gpu/drm/msm/dp/dp_panel.c   | 105 +++++-----------------------
>   drivers/gpu/drm/msm/dp/dp_panel.h   |  10 +--
>   4 files changed, 22 insertions(+), 140 deletions(-)
> 
> 
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c

-- 
With best wishes
Dmitry

Re: [PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()

[Stephen Boyd]

Quoting Dmitry Baryshkov (2023-09-03 15:40:49)
> On 29/08/2023 21:47, Stephen Boyd wrote:
> > This function is simply drm_dp_is_branch() so use that instead of
> > open-coding it.
> >
> > Cc: Vinod Polimera <quic_vpolimer@quicinc.com>
> > Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_display.c | 9 +--------
> >   1 file changed, 1 insertion(+), 8 deletions(-)
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Thanks.

>
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > index 76f13954015b..96bbf6fec2f1 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
> >       .unbind = dp_display_unbind,
> >   };
> >
> > -static bool dp_display_is_ds_bridge(struct dp_panel *panel)
> > -{
> > -     return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > -             DP_DWN_STRM_PORT_PRESENT);
> > -}
> > -
> >   static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
>
> Nit: you might as well inline this function

Ok. I'll send a followup to this series with a patch for that. I found
that with an Apple dongle it always prints out a message to the kernel
log when I have HDMI disconnected that there isn't a sink connected,
which is annoying.

So at least two more patches are incoming.

Re: [PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 11:47:25 -0700, Stephen Boyd wrote:
> This driver open-codes a few of the DPCD register reads when it can be
> simplified by using the helpers instead. This series reworks the MSM DP
> driver to use the DPCD helpers and removes some dead code along the way.
> There's the potential for even more code reduction around the test
> registers, but I haven't tried to do that yet.
> 
> Stephen Boyd (7):
>   drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
>   drm/msm/dp: Use drm_dp_read_sink_count() helper
>   drm/msm/dp: Remove dead code related to downstream cap info
>   drm/msm/dp: Remove aux_cfg_update_done and related code
>   drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
>   drm/msm/dp: Inline dp_link_parse_sink_count()
>   drm/msm/dp: Remove dp_display_is_ds_bridge()
> 
> [...]

Applied, thanks!

[1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
      https://gitlab.freedesktop.org/lumag/msm/-/commit/f906b95755f7
[2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper
      https://gitlab.freedesktop.org/lumag/msm/-/commit/284a245d8bdc
[3/7] drm/msm/dp: Remove dead code related to downstream cap info
      https://gitlab.freedesktop.org/lumag/msm/-/commit/8bddc2d12e9c
[4/7] drm/msm/dp: Remove aux_cfg_update_done and related code
      https://gitlab.freedesktop.org/lumag/msm/-/commit/62ebb19fb32d
[5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
      https://gitlab.freedesktop.org/lumag/msm/-/commit/a9905b469931
[6/7] drm/msm/dp: Inline dp_link_parse_sink_count()
      https://gitlab.freedesktop.org/lumag/msm/-/commit/d89ce4cdb7a6
[7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()
      https://gitlab.freedesktop.org/lumag/msm/-/commit/b41c5ca70684

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>