Re: [PATCH 2/3] drm/msm/dp: Remove pixel_rate from struct dp_ctrl
From: Dmitry Baryshkov
Date: Wed Jun 22 2022 - 13:59:05 EST
On 22/06/2022 18:22, Kuogee Hsieh wrote:
On 6/22/2022 12:24 AM, Dmitry Baryshkov wrote:
On 22/06/2022 05:59, Stephen Boyd wrote:
Quoting Dmitry Baryshkov (2022-06-17 16:07:58)
On 17/06/2022 23:47, Stephen Boyd wrote:
This struct member is stored to in the function that calls the
function
which uses it. That's possible with a function argument instead of
storing to a struct member. Pass the pixel_rate as an argument instead
to simplify the code. Note that dp_ctrl_link_maintenance() was storing
the pixel_rate but never using it so we just remove the assignment
from
there.
Cc: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 57
++++++++++++++++----------------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 -
2 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index bd445e683cfc..e114521af2e9 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1336,7 +1336,7 @@ static void dp_ctrl_set_clock_rate(struct
dp_ctrl_private *ctrl,
name, rate);
}
-static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private
*ctrl)
+static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private
*ctrl, unsigned long pixel_rate)
I think we can read pixel_rate here rather than getting it as an
argument. We'd need to move handling (DP_TEST_LINK_PHY_TEST_PATTERN &&
!ctrl->panel->dp_mode.drm_mode.clock) case here from dp_ctrl_on_link().
This is also called from dp_ctrl_on_stream() and
dp_ctrl_reinitialize_mainlink(). In the dp_ctrl_on_stream() case we may
divide the pixel_rate by 2 with widebus. We could move the
dp_ctrl_on_link() code here, but then we also need to move widebus, and
then I'm not sure which pixel rate to use.
It looks like the test code doesn't care about widebus? And similarly,
we may run the pixel clk faster until we get a modeset and then divide
it for widebus.
Good question. I'll let Kuogee or somebody else from Qualcomm to
comment on test code vs widebus vs pixel rate, as I don't know these
details.
I'm not sure if we should halve the pixel clock in
dp_ctrl_on_stream_phy_test_report() or not if the widebus is supported.
From the current code I'd assume that we have to do this. Let's raise
this question in the corresponding patch discussion.
yes, phy test does not care pixel clock rate.
So, is it 'does not care' or 'set to mode clock'?
In other words, can we unify both functions by always accounting for the
wide_bus_en value?
--
With best wishes
Dmitry