| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4pituhltil4qy5p4yhupnfc2zppqdwe2eqp3h4v5wcf6esz7gy@kekccysqb3q3>
Date: Fri, 6 Dec 2024 11:09:36 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Stephen Boyd <swboyd@...omium.org>,
Chandan Uddaraju <chandanu@...eaurora.org>, Guenter Roeck <groeck@...omium.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Vara Reddy <quic_varar@...cinc.com>, Rob Clark <robdclark@...omium.org>,
Tanmay Shah <tanmay@...eaurora.org>, linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Jessica Zhang <quic_jesszhan@...cinc.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH 09/45] drm/msm/dp: allow dp_ctrl stream APIs to use any
panel passed to it
On Thu, Dec 05, 2024 at 08:31:40PM -0800, Abhinav Kumar wrote:
> Currently, the dp_ctrl stream APIs operate on their own dp_panel
> which is cached inside the dp_ctrl's private struct. However with MST,
> the cached panel represents the fixed link and not the sinks which
> are hotplugged. Allow the stream related APIs to work on the panel
> which is passed to them rather than the cached one. For SST cases,
> this shall continue to use the cached dp_panel.
"cached inside the dp_ctrl's private struct" usually means that dp_ctrl
stores a copy of the msm_dp_panel, not just a pointer to it, so the
commit message needs rephrasing.
Also if the stored pointer is (or will be) no longer applicable, can we
drop it completely and always pass the dp_panel by reference? Otherwise
it's hard to know when to pass a new one and when to use ctrl->panel
instead.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 37 ++++++++++++++++++++-----------------
> drivers/gpu/drm/msm/dp/dp_ctrl.h | 5 +++--
> drivers/gpu/drm/msm/dp/dp_display.c | 4 ++--
> 3 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 9e08996be0cb969cb96d9a3019c445ab4dfc92ef..0bed85b5c8e8133ffa8c74d5de22668905396d09 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -134,7 +134,8 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
> drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
> }
>
> -static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
> +static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl,
> + struct msm_dp_panel *msm_dp_panel)
> {
> u32 config = 0, tbd;
> const u8 *dpcd = ctrl->panel->dpcd;
> @@ -142,7 +143,7 @@ static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
> /* Default-> LSCLK DIV: 1/4 LCLK */
> config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
>
> - if (ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
> + if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
> config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
>
> /* Scrambler reset enable */
> @@ -150,7 +151,7 @@ static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
> config |= DP_CONFIGURATION_CTRL_ASSR;
>
> tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
> - ctrl->panel->msm_dp_mode.bpp);
> + msm_dp_panel->msm_dp_mode.bpp);
>
> config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
>
> @@ -173,20 +174,21 @@ static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
> msm_dp_catalog_ctrl_config_ctrl(ctrl->catalog, config);
> }
>
> -static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl)
> +static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl,
> + struct msm_dp_panel *msm_dp_panel)
> {
> u32 cc, tb;
>
> msm_dp_catalog_ctrl_lane_mapping(ctrl->catalog);
> msm_dp_catalog_setup_peripheral_flush(ctrl->catalog);
>
> - msm_dp_ctrl_config_ctrl(ctrl);
> + msm_dp_ctrl_config_ctrl(ctrl, msm_dp_panel);
>
> tb = msm_dp_link_get_test_bits_depth(ctrl->link,
> - ctrl->panel->msm_dp_mode.bpp);
> + msm_dp_panel->msm_dp_mode.bpp);
> cc = msm_dp_link_get_colorimetry_config(ctrl->link);
> msm_dp_catalog_ctrl_config_misc(ctrl->catalog, cc, tb);
> - msm_dp_panel_timing_cfg(ctrl->panel);
> + msm_dp_panel_timing_cfg(msm_dp_panel);
> }
>
> /*
> @@ -1279,7 +1281,7 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
> u8 assr;
> struct msm_dp_link_info link_info = {0};
>
> - msm_dp_ctrl_config_ctrl(ctrl);
> + msm_dp_ctrl_config_ctrl(ctrl, ctrl->panel);
>
> link_info.num_lanes = ctrl->link->link_params.num_lanes;
> link_info.rate = ctrl->link->link_params.rate;
> @@ -1696,7 +1698,8 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
> return success;
> }
>
> -static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl)
> +static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl,
> + struct msm_dp_panel *msm_dp_panel)
> {
> int ret;
> unsigned long pixel_rate;
> @@ -1720,7 +1723,7 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
> return ret;
> }
>
> - pixel_rate = ctrl->panel->msm_dp_mode.drm_mode.clock;
> + pixel_rate = msm_dp_panel->msm_dp_mode.drm_mode.clock;
> ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
> if (ret) {
> DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
> @@ -1758,7 +1761,7 @@ void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl)
>
> if (sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
> drm_dbg_dp(ctrl->drm_dev, "PHY_TEST_PATTERN request\n");
> - if (msm_dp_ctrl_process_phy_test_request(ctrl)) {
> + if (msm_dp_ctrl_process_phy_test_request(ctrl, ctrl->panel)) {
> DRM_ERROR("process phy_test_req failed\n");
> return;
> }
> @@ -1976,7 +1979,7 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_li
> return ret;
> }
>
> -int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
> +int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *msm_dp_panel)
> {
> int ret = 0;
> bool mainlink_ready = false;
> @@ -1989,9 +1992,9 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
>
> ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
>
> - pixel_rate = pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock;
> + pixel_rate = pixel_rate_orig = msm_dp_panel->msm_dp_mode.drm_mode.clock;
>
> - if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
> + if (msm_dp_ctrl->wide_bus_en || msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
> pixel_rate >>= 1;
>
> drm_dbg_dp(ctrl->drm_dev, "pixel_rate=%lu\n", pixel_rate);
> @@ -2019,12 +2022,12 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
> */
> reinit_completion(&ctrl->video_comp);
>
> - msm_dp_ctrl_configure_source_params(ctrl);
> + msm_dp_ctrl_configure_source_params(ctrl, msm_dp_panel);
>
> msm_dp_catalog_ctrl_config_msa(ctrl->catalog,
> ctrl->link->link_params.rate,
> pixel_rate_orig,
> - ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420);
> + msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420);
>
> msm_dp_ctrl_setup_tr_unit(ctrl);
>
> @@ -2042,7 +2045,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
> return ret;
> }
>
> -void msm_dp_ctrl_clear_vsc_sdp_pkt(struct msm_dp_ctrl *msm_dp_ctrl)
> +void msm_dp_ctrl_clear_vsc_sdp_pkt(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *dp_panel)
> {
> struct msm_dp_ctrl_private *ctrl;
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index 0f58b63c5c7c5aab43c0db2a697ba491959b79d2..547155ffa50fbe2f3a1f2c2e1ee17420daf0f3da 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -18,7 +18,7 @@ struct msm_dp_ctrl {
> struct phy;
>
> int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);
> -int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl);
> +int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *msm_dp_panel);
> int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *dp_ctrl, bool force_link_train);
> void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl);
> void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl);
> @@ -42,7 +42,8 @@ void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl);
> int msm_dp_ctrl_core_clk_enable(struct msm_dp_ctrl *msm_dp_ctrl);
> void msm_dp_ctrl_core_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl);
>
> -void msm_dp_ctrl_clear_vsc_sdp_pkt(struct msm_dp_ctrl *msm_dp_ctrl);
> +void msm_dp_ctrl_clear_vsc_sdp_pkt(struct msm_dp_ctrl *msm_dp_ctrl,
> + struct msm_dp_panel *msm_dp_panel);
> void msm_dp_ctrl_psm_config(struct msm_dp_ctrl *msm_dp_ctrl);
> void msm_dp_ctrl_reinit_phy(struct msm_dp_ctrl *msm_dp_ctrl);
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bbce8ca09ff70059458231982f002e1f22d2c3ab..c059f749c1f204deac9dfb0c56f537f5545d9acb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -852,7 +852,7 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp)
> return 0;
> }
>
> - rc = msm_dp_ctrl_on_stream(dp->ctrl);
> + rc = msm_dp_ctrl_on_stream(dp->ctrl, dp->panel);
> if (!rc)
> msm_dp_display->power_on = true;
>
> @@ -905,7 +905,7 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
> if (!msm_dp_display->power_on)
> return 0;
>
> - msm_dp_ctrl_clear_vsc_sdp_pkt(dp->ctrl);
> + msm_dp_ctrl_clear_vsc_sdp_pkt(dp->ctrl, dp->panel);
>
> /* dongle is still connected but sinks are disconnected */
> if (dp->link->sink_count == 0)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists