Re: [Intel-gfx] [PATCH v2 1/1] drm/dp_mst: Use kHz as link rate units when settig source max link caps at init

2021-05-13 Thread Lyude Paul
Reviewed-by: Lyude Paul 

Will let this sit on the list for a few days to see if anyone's got any
objections and then I'll go ahead and push it

On Wed, 2021-05-12 at 17:00 -0400, Nikola Cornij wrote:
> [why]
> Link rate in kHz is what is eventually required to calculate the link
> bandwidth, which makes kHz a more generic unit. This should also make
> forward-compatibility with new DP standards easier.
> 
> [how]
> - Replace 'link rate DPCD code' with 'link rate in kHz' when used with
> drm_dp_mst_topology_mgr_init()
> - Add/remove related DPCD code conversion from/to kHz where applicable
> 
> Signed-off-by: Nikola Cornij 
> Acked-by: Jani Nikula 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 4 ++--
>  drivers/gpu/drm/drm_dp_mst_topology.c | 8 
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   | 4 ++--
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   | 5 +++--
>  drivers/gpu/drm/radeon/radeon_dp_mst.c    | 2 +-
>  include/drm/drm_dp_mst_helper.h   | 8 
>  6 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 4a0c24ce5f7d..f78dd021f591 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -458,8 +458,8 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> &aconnector->dm_dp_aux.aux,
> 16,
> 4,
> -   (u8)max_link_enc_cap.lane_count,
> -   (u8)max_link_enc_cap.link_rate,
> +   max_link_enc_cap.lane_count,
> +   drm_dp_bw_code_to_link_rate(max_link_enc_cap.link_rate),
> aconnector->connector_id);
>  
> drm_connector_attach_dp_subconnector_property(&aconnector->base);
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 54604633e65c..32b7f8983b94 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3722,9 +3722,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> drm_dp_mst_topology_mgr *mgr, bool ms
> }
>  
> lane_count = min_t(int, mgr->dpcd[2] & DP_MAX_LANE_COUNT_MASK,
> mgr->max_lane_count);
> -   link_rate = min_t(int, mgr->dpcd[1], mgr->max_link_rate);
> +   link_rate = min_t(int, drm_dp_bw_code_to_link_rate(mgr-
> >dpcd[1]), mgr->max_link_rate);
> mgr->pbn_div = drm_dp_get_vc_payload_bw(mgr,
> -
>    
> drm_dp_bw_code_to_link_r
> ate(link_rate),
> +   link_rate,
> lane_count);
> if (mgr->pbn_div == 0) {
> ret = -EINVAL;
> @@ -5454,7 +5454,7 @@ EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
>   * @max_dpcd_transaction_bytes: hw specific DPCD transaction limit
>   * @max_payloads: maximum number of payloads this GPU can source
>   * @max_lane_count: maximum number of lanes this GPU supports
> - * @max_link_rate: maximum link rate this GPU supports, units as in DPCD
> + * @max_link_rate: maximum link rate per lane this GPU supports in kHz
>   * @conn_base_id: the connector object ID the MST device is connected to.
>   *
>   * Return 0 for success, or negative error code on failure
> @@ -5462,7 +5462,7 @@ EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
>  int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
>  struct drm_device *dev, struct drm_dp_aux
> *aux,
>  int max_dpcd_transaction_bytes, int
> max_payloads,
> -    u8 max_lane_count, u8 max_link_rate,
> +    int max_lane_count, int max_link_rate,
>  int conn_base_id)
>  {
> struct drm_dp_mst_topology_state *mst_state;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index f608c0cb98f4..26f65445bc8a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -960,8 +960,8 @@ intel_dp_mst_encoder_init(struct intel_digital_port
> *dig_port, int conn_base_id)
> intel_dp_create_fake_mst_encoders(dig_port);
> ret = drm_dp_mst_topology_mgr_init(&intel_dp->mst_mgr, &i915->drm,
>    &intel_dp->aux, 16, 3,
> -  (u8)dig_port->max_lanes,
> - 
> drm_dp_link_rate_to_bw_code(max_source_rate),
> +  dig_port->max_lanes,
> + 

[Intel-gfx] [PATCH v2 1/1] drm/dp_mst: Use kHz as link rate units when settig source max link caps at init

2021-05-12 Thread Nikola Cornij
[why]
Link rate in kHz is what is eventually required to calculate the link
bandwidth, which makes kHz a more generic unit. This should also make
forward-compatibility with new DP standards easier.

[how]
- Replace 'link rate DPCD code' with 'link rate in kHz' when used with
drm_dp_mst_topology_mgr_init()
- Add/remove related DPCD code conversion from/to kHz where applicable

Signed-off-by: Nikola Cornij 
Acked-by: Jani Nikula 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 4 ++--
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 
 drivers/gpu/drm/i915/display/intel_dp_mst.c   | 4 ++--
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 5 +++--
 drivers/gpu/drm/radeon/radeon_dp_mst.c| 2 +-
 include/drm/drm_dp_mst_helper.h   | 8 
 6 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 4a0c24ce5f7d..f78dd021f591 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -458,8 +458,8 @@ void amdgpu_dm_initialize_dp_connector(struct 
amdgpu_display_manager *dm,
&aconnector->dm_dp_aux.aux,
16,
4,
-   (u8)max_link_enc_cap.lane_count,
-   (u8)max_link_enc_cap.link_rate,
+   max_link_enc_cap.lane_count,
+   drm_dp_bw_code_to_link_rate(max_link_enc_cap.link_rate),
aconnector->connector_id);
 
drm_connector_attach_dp_subconnector_property(&aconnector->base);
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 54604633e65c..32b7f8983b94 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3722,9 +3722,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
drm_dp_mst_topology_mgr *mgr, bool ms
}
 
lane_count = min_t(int, mgr->dpcd[2] & DP_MAX_LANE_COUNT_MASK, 
mgr->max_lane_count);
-   link_rate = min_t(int, mgr->dpcd[1], mgr->max_link_rate);
+   link_rate = min_t(int, 
drm_dp_bw_code_to_link_rate(mgr->dpcd[1]), mgr->max_link_rate);
mgr->pbn_div = drm_dp_get_vc_payload_bw(mgr,
-   
drm_dp_bw_code_to_link_rate(link_rate),
+   link_rate,
lane_count);
if (mgr->pbn_div == 0) {
ret = -EINVAL;
@@ -5454,7 +5454,7 @@ EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
  * @max_dpcd_transaction_bytes: hw specific DPCD transaction limit
  * @max_payloads: maximum number of payloads this GPU can source
  * @max_lane_count: maximum number of lanes this GPU supports
- * @max_link_rate: maximum link rate this GPU supports, units as in DPCD
+ * @max_link_rate: maximum link rate per lane this GPU supports in kHz
  * @conn_base_id: the connector object ID the MST device is connected to.
  *
  * Return 0 for success, or negative error code on failure
@@ -5462,7 +5462,7 @@ EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
 int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 struct drm_device *dev, struct drm_dp_aux *aux,
 int max_dpcd_transaction_bytes, int 
max_payloads,
-u8 max_lane_count, u8 max_link_rate,
+int max_lane_count, int max_link_rate,
 int conn_base_id)
 {
struct drm_dp_mst_topology_state *mst_state;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index f608c0cb98f4..26f65445bc8a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -960,8 +960,8 @@ intel_dp_mst_encoder_init(struct intel_digital_port 
*dig_port, int conn_base_id)
intel_dp_create_fake_mst_encoders(dig_port);
ret = drm_dp_mst_topology_mgr_init(&intel_dp->mst_mgr, &i915->drm,
   &intel_dp->aux, 16, 3,
-  (u8)dig_port->max_lanes,
-  
drm_dp_link_rate_to_bw_code(max_source_rate),
+  dig_port->max_lanes,
+  max_source_rate,
   conn_base_id);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index c46d0374b6e6..f949767698fc 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1617,8 +1