On 13/01/2026 22:59, Nathan Chancellor wrote:
On Mon, Jan 12, 2026 at 04:37:46AM +0200, Dmitry Baryshkov wrote:
On Fri, Jan 09, 2026 at 10:06:29AM +0100, Konrad Dybcio wrote:
Dmitry, would it be beneficial to throw an actual error when the rate is
is mangled? i.e.
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index aa2303d0e148..4f710b8e6bc6 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2404,9 +2404,9 @@ static int msm_dp_ctrl_link_retrain(struct
msm_dp_ctrl_private *ctrl)
return msm_dp_ctrl_setup_main_link(ctrl, &training_step);
}
-static void msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
- u32 rate, u32 stream_rate_khz,
- bool is_ycbcr_420)
+static int msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
+ u32 rate, u32 stream_rate_khz,
+ bool is_ycbcr_420)
{
u32 pixel_m, pixel_n;
u32 mvid, nvid, pixel_div = 0, dispcc_input_rate;
@@ -2415,14 +2415,21 @@ static void msm_dp_ctrl_config_msa(struct
msm_dp_ctrl_private *ctrl,
u32 const link_rate_hbr3 = 810000;
unsigned long den, num;
- if (rate == link_rate_hbr3)
+ switch (rate) {
+ case link_rate_hbr3:
pixel_div = 6;
- else if (rate == 162000 || rate == 270000)
- pixel_div = 2;
- else if (rate == link_rate_hbr2)
+ break;
+ case link_rate_hbr2:
pixel_div = 4;
- else
+ break;
+ case 270000:
+ case 162000:
+ pixel_div = 2;
+ break;
+ default:
DRM_ERROR("Invalid pixel mux divider\n");
+ return -EINVAL;
Well... In the ideal world, we can't end up here, PHY's
configure_dp_clocks (or qcom_edp_set_vco_div()) will fail if the link
rate is is invalid here. I think, we should return an error here, but
there is no need to propagate it further.
See the discussion at
https://lore.kernel.org/dri-devel/[email protected]/
I interpret that as approving of the above hunk but omitting the hunk
that modifies msm_dp_ctrl_on_stream(). In that case, what is the point
of changing the return type of msm_dp_ctrl_config_msa()? Wouldn't the
below diff have the same exact effect as a smaller change? I don't mind
rolling this up as a v2.
Cheers,
Nathan
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index aa2303d0e148..d8ea73b89f7c 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2409,20 +2409,27 @@ static void msm_dp_ctrl_config_msa(struct
msm_dp_ctrl_private *ctrl,
bool is_ycbcr_420)
{
u32 pixel_m, pixel_n;
- u32 mvid, nvid, pixel_div = 0, dispcc_input_rate;
+ u32 mvid, nvid, pixel_div, dispcc_input_rate;
u32 const nvid_fixed = DP_LINK_CONSTANT_N_VALUE;
u32 const link_rate_hbr2 = 540000;
u32 const link_rate_hbr3 = 810000;
unsigned long den, num;
- if (rate == link_rate_hbr3)
+ switch (rate) {
+ case link_rate_hbr3:
pixel_div = 6;
- else if (rate == 162000 || rate == 270000)
- pixel_div = 2;
- else if (rate == link_rate_hbr2)
+ break;
+ case link_rate_hbr2:
pixel_div = 4;
- else
+ break;
+ case 162000:
+ case 270000:
+ pixel_div = 2;
+ break;
+ default:
DRM_ERROR("Invalid pixel mux divider\n");
+ return;
Please add a comment, stating that we can't actually reach it. LGTM
otherwise.
+ }
dispcc_input_rate = (rate * 10) / pixel_div;
--
With best wishes
Dmitry
