[PATCH 1/2] drm/dp: add drm_dp_phy_name() for getting DP PHY name

[PATCH 1/2] drm/dp: add drm_dp_phy_name() for getting DP PHY name

[Jani Nikula]

Add a helper for getting the DP PHY name. In the interest of caller
simplicity and to avoid allocations and passing in of buffers, duplicate
the const strings to return. It's a minor penalty to pay for simplicity
in all the call sites.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 21 +++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 4d0d1e8e51fa..f1d03b5a4bab 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -197,6 +197,27 @@ void drm_dp_link_train_channel_eq_delay(const struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
 
+const char *drm_dp_phy_name(enum drm_dp_phy dp_phy)
+{
+	static const char * const phy_names[] = {
+		[DP_PHY_DPRX] = "DPRX",
+		[DP_PHY_LTTPR1] = "LTTPR 1",
+		[DP_PHY_LTTPR2] = "LTTPR 2",
+		[DP_PHY_LTTPR3] = "LTTPR 3",
+		[DP_PHY_LTTPR4] = "LTTPR 4",
+		[DP_PHY_LTTPR5] = "LTTPR 5",
+		[DP_PHY_LTTPR6] = "LTTPR 6",
+		[DP_PHY_LTTPR7] = "LTTPR 7",
+		[DP_PHY_LTTPR8] = "LTTPR 8",
+	};
+
+	if (dp_phy < 0 || dp_phy >= ARRAY_SIZE(phy_names))
+		return "<INVALID DP PHY>";
+
+	return phy_names[dp_phy];
+}
+EXPORT_SYMBOL(drm_dp_phy_name);
+
 void drm_dp_lttpr_link_train_clock_recovery_delay(void)
 {
 	usleep_range(100, 200);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index b52df4db3e8f..c873e6349b41 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -2115,6 +2115,8 @@ bool drm_dp_read_sink_count_cap(struct drm_connector *connector,
 				const struct drm_dp_desc *desc);
 int drm_dp_read_sink_count(struct drm_dp_aux *aux);
 
+const char *drm_dp_phy_name(enum drm_dp_phy dp_phy);
+
 int drm_dp_read_lttpr_common_caps(struct drm_dp_aux *aux,
 				  u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
 int drm_dp_read_lttpr_phy_caps(struct drm_dp_aux *aux,
-- 
2.30.2

[PATCH 2/2] drm/i915/dp: use drm_dp_phy_name() for logging

[Jani Nikula]

Drop the local intel_dp_phy_name() function, and replace with
drm_dp_phy_name(). This lets us drop a number of local buffers.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_dp_link_training.c | 24 ++++---------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index c052ce14894d..219568304c08 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -46,17 +46,6 @@ static void intel_dp_reset_lttpr_count(struct intel_dp *intel_dp)
 				    DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV] = 0;
 }
 
-static const char *intel_dp_phy_name(enum drm_dp_phy dp_phy,
-				     char *buf, size_t buf_size)
-{
-	if (dp_phy == DP_PHY_DPRX)
-		snprintf(buf, buf_size, "DPRX");
-	else
-		snprintf(buf, buf_size, "LTTPR %d", dp_phy - DP_PHY_LTTPR1 + 1);
-
-	return buf;
-}
-
 static u8 *intel_dp_lttpr_phy_caps(struct intel_dp *intel_dp,
 				   enum drm_dp_phy dp_phy)
 {
@@ -67,20 +56,17 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
 					 enum drm_dp_phy dp_phy)
 {
 	u8 *phy_caps = intel_dp_lttpr_phy_caps(intel_dp, dp_phy);
-	char phy_name[10];
-
-	intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name));
 
 	if (drm_dp_read_lttpr_phy_caps(&intel_dp->aux, dp_phy, phy_caps) < 0) {
 		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
 			    "failed to read the PHY caps for %s\n",
-			    phy_name);
+			    drm_dp_phy_name(dp_phy));
 		return;
 	}
 
 	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
 		    "%s PHY capabilities: %*ph\n",
-		    phy_name,
+		    drm_dp_phy_name(dp_phy),
 		    (int)sizeof(intel_dp->lttpr_phy_caps[0]),
 		    phy_caps);
 }
@@ -443,7 +429,6 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
 {
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	char phy_name[10];
 
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] lanes: %d, "
 		    "vswing levels: " TRAIN_SET_FMT ", "
@@ -452,7 +437,7 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
 		    crtc_state->lane_count,
 		    TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
 		    TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set),
-		    intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)));
+		    drm_dp_phy_name(dp_phy));
 
 	if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
 		encoder->set_signal_levels(encoder, crtc_state);
@@ -849,7 +834,6 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
 			enum drm_dp_phy dp_phy)
 {
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
-	char phy_name[10];
 	bool ret = false;
 
 	if (!intel_dp_link_training_clock_recovery(intel_dp, crtc_state, dp_phy))
@@ -867,7 +851,7 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
 		    intel_connector->base.name,
 		    ret ? "passed" : "failed",
 		    crtc_state->port_clock, crtc_state->lane_count,
-		    intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)));
+		    drm_dp_phy_name(dp_phy));
 
 	return ret;
 }
-- 
2.30.2

Re: [PATCH 1/2] drm/dp: add drm_dp_phy_name() for getting DP PHY name

[Ville Syrjälä]

On Tue, Oct 05, 2021 at 11:10:52AM +0300, Jani Nikula wrote:
> Add a helper for getting the DP PHY name. In the interest of caller
> simplicity and to avoid allocations and passing in of buffers, duplicate
> the const strings to return. It's a minor penalty to pay for simplicity
> in all the call sites.

Yeah, the on stack extra buffer is getting a bit annoying, especially
with the calls multiplying like rabbits in my recent patches.

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

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 21 +++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |  2 ++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 4d0d1e8e51fa..f1d03b5a4bab 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -197,6 +197,27 @@ void drm_dp_link_train_channel_eq_delay(const struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
>  
> +const char *drm_dp_phy_name(enum drm_dp_phy dp_phy)
> +{
> +	static const char * const phy_names[] = {
> +		[DP_PHY_DPRX] = "DPRX",
> +		[DP_PHY_LTTPR1] = "LTTPR 1",
> +		[DP_PHY_LTTPR2] = "LTTPR 2",
> +		[DP_PHY_LTTPR3] = "LTTPR 3",
> +		[DP_PHY_LTTPR4] = "LTTPR 4",
> +		[DP_PHY_LTTPR5] = "LTTPR 5",
> +		[DP_PHY_LTTPR6] = "LTTPR 6",
> +		[DP_PHY_LTTPR7] = "LTTPR 7",
> +		[DP_PHY_LTTPR8] = "LTTPR 8",
> +	};
> +
> +	if (dp_phy < 0 || dp_phy >= ARRAY_SIZE(phy_names))
> +		return "<INVALID DP PHY>";
> +
> +	return phy_names[dp_phy];
> +}
> +EXPORT_SYMBOL(drm_dp_phy_name);
> +
>  void drm_dp_lttpr_link_train_clock_recovery_delay(void)
>  {
>  	usleep_range(100, 200);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b52df4db3e8f..c873e6349b41 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -2115,6 +2115,8 @@ bool drm_dp_read_sink_count_cap(struct drm_connector *connector,
>  				const struct drm_dp_desc *desc);
>  int drm_dp_read_sink_count(struct drm_dp_aux *aux);
>  
> +const char *drm_dp_phy_name(enum drm_dp_phy dp_phy);
> +
>  int drm_dp_read_lttpr_common_caps(struct drm_dp_aux *aux,
>  				  u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
>  int drm_dp_read_lttpr_phy_caps(struct drm_dp_aux *aux,
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/2] drm/dp: add drm_dp_phy_name() for getting DP PHY name

[Jani Nikula]

On Tue, 05 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Oct 05, 2021 at 11:10:52AM +0300, Jani Nikula wrote:
>> Add a helper for getting the DP PHY name. In the interest of caller
>> simplicity and to avoid allocations and passing in of buffers, duplicate
>> the const strings to return. It's a minor penalty to pay for simplicity
>> in all the call sites.
>
> Yeah, the on stack extra buffer is getting a bit annoying, especially
> with the calls multiplying like rabbits in my recent patches.
>
> Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks.

I don't want to block your patches with this, so I can wait and rebase.


BR,
Jani.


>
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 21 +++++++++++++++++++++
>>  include/drm/drm_dp_helper.h     |  2 ++
>>  2 files changed, 23 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 4d0d1e8e51fa..f1d03b5a4bab 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -197,6 +197,27 @@ void drm_dp_link_train_channel_eq_delay(const struct drm_dp_aux *aux,
>>  }
>>  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
>>  
>> +const char *drm_dp_phy_name(enum drm_dp_phy dp_phy)
>> +{
>> +	static const char * const phy_names[] = {
>> +		[DP_PHY_DPRX] = "DPRX",
>> +		[DP_PHY_LTTPR1] = "LTTPR 1",
>> +		[DP_PHY_LTTPR2] = "LTTPR 2",
>> +		[DP_PHY_LTTPR3] = "LTTPR 3",
>> +		[DP_PHY_LTTPR4] = "LTTPR 4",
>> +		[DP_PHY_LTTPR5] = "LTTPR 5",
>> +		[DP_PHY_LTTPR6] = "LTTPR 6",
>> +		[DP_PHY_LTTPR7] = "LTTPR 7",
>> +		[DP_PHY_LTTPR8] = "LTTPR 8",
>> +	};
>> +
>> +	if (dp_phy < 0 || dp_phy >= ARRAY_SIZE(phy_names))
>> +		return "<INVALID DP PHY>";
>> +
>> +	return phy_names[dp_phy];
>> +}
>> +EXPORT_SYMBOL(drm_dp_phy_name);
>> +
>>  void drm_dp_lttpr_link_train_clock_recovery_delay(void)
>>  {
>>  	usleep_range(100, 200);
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index b52df4db3e8f..c873e6349b41 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -2115,6 +2115,8 @@ bool drm_dp_read_sink_count_cap(struct drm_connector *connector,
>>  				const struct drm_dp_desc *desc);
>>  int drm_dp_read_sink_count(struct drm_dp_aux *aux);
>>  
>> +const char *drm_dp_phy_name(enum drm_dp_phy dp_phy);
>> +
>>  int drm_dp_read_lttpr_common_caps(struct drm_dp_aux *aux,
>>  				  u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
>>  int drm_dp_read_lttpr_phy_caps(struct drm_dp_aux *aux,
>> -- 
>> 2.30.2

-- 
Jani Nikula, Intel Open Source Graphics Center

[PATCH 1/2] drm/dp: add drm_dp_phy_name() for getting DP PHY name

[Jani Nikula]

Add a helper for getting the DP PHY name. In the interest of caller
simplicity and to avoid allocations and passing in of buffers, duplicate
the const strings to return. It's a minor penalty to pay for simplicity
in all the call sites.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 21 +++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ada0a1ff262d..2c36fad88781 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -314,6 +314,27 @@ void drm_dp_link_train_channel_eq_delay(const struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
 
+const char *drm_dp_phy_name(enum drm_dp_phy dp_phy)
+{
+	static const char * const phy_names[] = {
+		[DP_PHY_DPRX] = "DPRX",
+		[DP_PHY_LTTPR1] = "LTTPR 1",
+		[DP_PHY_LTTPR2] = "LTTPR 2",
+		[DP_PHY_LTTPR3] = "LTTPR 3",
+		[DP_PHY_LTTPR4] = "LTTPR 4",
+		[DP_PHY_LTTPR5] = "LTTPR 5",
+		[DP_PHY_LTTPR6] = "LTTPR 6",
+		[DP_PHY_LTTPR7] = "LTTPR 7",
+		[DP_PHY_LTTPR8] = "LTTPR 8",
+	};
+
+	if (dp_phy < 0 || dp_phy >= ARRAY_SIZE(phy_names))
+		return "<INVALID DP PHY>";
+
+	return phy_names[dp_phy];
+}
+EXPORT_SYMBOL(drm_dp_phy_name);
+
 void drm_dp_lttpr_link_train_clock_recovery_delay(void)
 {
 	usleep_range(100, 200);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index afdf7f4183f9..39a249d99a51 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -2132,6 +2132,8 @@ bool drm_dp_read_sink_count_cap(struct drm_connector *connector,
 				const struct drm_dp_desc *desc);
 int drm_dp_read_sink_count(struct drm_dp_aux *aux);
 
+const char *drm_dp_phy_name(enum drm_dp_phy dp_phy);
+
 int drm_dp_read_lttpr_common_caps(struct drm_dp_aux *aux,
 				  u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
 int drm_dp_read_lttpr_phy_caps(struct drm_dp_aux *aux,
-- 
2.30.2