Re: [PATCH v2 16/20] drm/msm/dp: Disable PSR for YUV420 streams

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm: endianness bug on DP_PSR_EN_CFG write using u32 pointer
--

commit d1403cc5c58c4528368941f08f1ea1ec332d1a49
Author: Xilin Wu <sophon@radxa.com>

drm/msm/dp: Disable PSR for YUV420 streams

This commit disables self-refresh for YUV420 streams because their
colorimetry VSC packets cannot share the automatic PSR packet generator.
It computes eligibility after common atomic checks and restores PSR
configuration accordingly.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 783cafb223748..82ef8e4055431 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
[ ... ]
> @@ -1923,15 +1923,20 @@ void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
[ ... ]
> -	cfg = DP_PSR_ENABLE;
> +	cfg = enable ? DP_PSR_ENABLE : 0;
>  	drm_dp_dpcd_write(ctrl->aux, DP_PSR_EN_CFG, &cfg, 1);

[Severity: Medium]
Could this silently write 0 and fail to enable PSR on big-endian platforms?

The variable cfg is defined as a u32 at the beginning of
msm_dp_ctrl_config_psr(). When its address is passed to drm_dp_dpcd_write()
with a size of 1 byte, little-endian systems will correctly read the least
significant byte containing the enable bit. 

However, on big-endian systems, taking the address of a u32 and reading 1
byte will read the most significant byte instead (which is 0x00).

Would using a u8 variable or drm_dp_dpcd_writeb() avoid this structural
mismatch and ensure the correct byte is written regardless of architecture
endianness?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=16

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic