[PATCH] drm/dp_mst: Fix drm RAD print

[Imre Deak]

On Wed, Nov 27, 2024 at 05:49:38PM +0800, Wayne Lin wrote:
> [Why]
> The RAD of sideband message printed today is incorrect.
> For RAD stored within MST branch
> - If MST branch LCT is 1, it's RAD array is untouched and remained as 0.
> - If MST branch LCT is larger than 1, usd nibble to store the up facing
>   port number in cascaded sequence as illustrated below:
> 
>   u8 RAD[0] = (LCT_2_UFP << 4) | LCT_3_UFP
>      RAD[1] = (LCT_4_UFP << 4) | LCT_5_UFP
>      ...
> 
> In drm_dp_mst_rad_to_str(), it wrongly to use BIT_MASK(4) to fetch the port
> number of one nibble.
> 
> [How]
> Adjust the code by:
> - RAD array items are valuable only for LCT >= 1.
> - Use 0xF as the mask to replace BIT_MASK(4)
> 
> Fixes: 2f015ec6eab6 ("drm/dp_mst: Add sideband down request tracing + selftests")
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Harry Wentland <hwentlan at amd.com>
> Cc: Lyude Paul <lyude at redhat.com>
> Signed-off-by: Wayne Lin <Wayne.Lin at amd.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index f7c6b60629c2..152c60f1e80f 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -179,13 +179,13 @@ static int
>  drm_dp_mst_rad_to_str(const u8 rad[8], u8 lct, char *out, size_t len)
>  {
>  	int i;
> -	u8 unpacked_rad[16];
> +	u8 unpacked_rad[16] = {0};

The usual way is arr[16] = {};

>  
> -	for (i = 0; i < lct; i++) {
> +	for (i = 1; i < lct; i++) {
>  		if (i % 2)
> -			unpacked_rad[i] = rad[i / 2] >> 4;
> +			unpacked_rad[i] = rad[(i - 1) / 2] >> 4;
>  		else
> -			unpacked_rad[i] = rad[i / 2] & BIT_MASK(4);
> +			unpacked_rad[i] = rad[(i - 1) / 2] & 0xF;
>  	}

So unpacked_rad[0] will be always zero, which represents
drm_dp_mst_topology_mgr::mst_primary, but not actually stored in
drm_dp_mst_branch::rad[]. In each element of rad[] the nibbles are
ordered the most signifcant 4 bits first and the least significant 4
bits second. This could be documented at drm_dp_mst_branch::rad[] imo.

The fix looks ok to me:
Acked-by: Imre Deak <imre.deak at intel.com>

drm_dp_get_mst_branch_device() needs the same logic as above extracting
one element of RAD at a given index, so a helper for this could be added
and used in both places.

>  
>  	/* TODO: Eventually add something to printk so we can format the rad
> -- 
> 2.37.3
>