drm/msm/dp: Simplify DPCD related code with helpers

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

Posted by Stephen Boyd 2 years, 3 months ago

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

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

Posted by Dmitry Baryshkov 2 years, 3 months ago

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 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()

Posted by Kuogee Hsieh 2 years, 3 months ago

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;

[PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
[PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper
[PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info
[PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code
[PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
[PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()
[PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()