| 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: <34he7xawyuq5z4iiyq4y4ehkjhfalx2vxhtejgyxly4zgyqma7@4uqoas4sz3nl>
Date: Mon, 25 Aug 2025 20:59:46 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Yongxing Mou <yongxing.mou@....qualcomm.com>
Cc: Rob Clark <robin.clark@....qualcomm.com>,
Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>,
Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Abhinav Kumar <quic_abhinavk@...cinc.com>
Subject: Re: [PATCH v3 14/38] drm/msm/dp: Add support for programming
p1/p2/p3 register blocks
On Mon, Aug 25, 2025 at 10:16:00PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <quic_abhinavk@...cinc.com>
>
> QCS8300 supports 4-stream MST. This patch adds support for the additional
> pixel register blocks (p1, p2, p3), enabling multi-stream configurations.
>
> To reduce code duplication, introduce helper functions msm_dp_read_pn and
> msm_dp_write_pn. All pixel clocks (PCLKs) share the same register layout,
> but use different base addresses.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
> Signed-off-by: Yongxing Mou <yongxing.mou@....qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 39 +++++++++++++--------
> drivers/gpu/drm/msm/dp/dp_panel.c | 68 ++++++++++++++++++-------------------
> 2 files changed, 59 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 3422f18bdec71a99407edfe943d31957d0e8847a..935a0c57a928b15a1e9a6f1fab2576b7b09acb8e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -84,8 +84,8 @@ struct msm_dp_display_private {
> void __iomem *link_base;
> size_t link_len;
>
> - void __iomem *p0_base;
> - size_t p0_len;
> + void __iomem *pixel_base[DP_STREAM_MAX];
> + size_t pixel_len;
>
> int max_stream;
> };
> @@ -619,7 +619,7 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
> goto error_link;
> }
>
> - dp->panel = msm_dp_panel_get(dev, dp->aux, dp->link, dp->link_base, dp->p0_base);
> + dp->panel = msm_dp_panel_get(dev, dp->aux, dp->link, dp->link_base, dp->pixel_base);
Why do we need to pass pixel base here? Shouldn't it be pixel_base[P0]?
> if (IS_ERR(dp->panel)) {
> rc = PTR_ERR(dp->panel);
> DRM_ERROR("failed to initialize panel, rc = %d\n", rc);
> @@ -937,8 +937,8 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp)
> msm_dp_display->aux_base, "dp_aux");
> msm_disp_snapshot_add_block(disp_state, msm_dp_display->link_len,
> msm_dp_display->link_base, "dp_link");
> - msm_disp_snapshot_add_block(disp_state, msm_dp_display->p0_len,
> - msm_dp_display->p0_base, "dp_p0");
> + msm_disp_snapshot_add_block(disp_state, msm_dp_display->pixel_len,
> + msm_dp_display->pixel_base[0], "dp_p0");
This should add all blocks used on this platform.
> }
>
> void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
> @@ -1181,12 +1181,13 @@ static void __iomem *msm_dp_ioremap(struct platform_device *pdev, int idx, size_
> #define DP_DEFAULT_AUX_SIZE 0x0200
> #define DP_DEFAULT_LINK_OFFSET 0x0400
> #define DP_DEFAULT_LINK_SIZE 0x0C00
> -#define DP_DEFAULT_P0_OFFSET 0x1000
> -#define DP_DEFAULT_P0_SIZE 0x0400
> +#define DP_DEFAULT_PIXEL_OFFSET 0x1000
> +#define DP_DEFAULT_PIXEL_SIZE 0x0400
No need to touch this. It's only required for legacy bindings.
>
> static int msm_dp_display_get_io(struct msm_dp_display_private *display)
> {
> struct platform_device *pdev = display->msm_dp_display.pdev;
> + int i;
>
> display->ahb_base = msm_dp_ioremap(pdev, 0, &display->ahb_len);
> if (IS_ERR(display->ahb_base))
> @@ -1206,7 +1207,7 @@ static int msm_dp_display_get_io(struct msm_dp_display_private *display)
> * reg is specified, so fill in the sub-region offsets and
> * lengths based on this single region.
> */
> - if (display->ahb_len < DP_DEFAULT_P0_OFFSET + DP_DEFAULT_P0_SIZE) {
> + if (display->ahb_len < DP_DEFAULT_PIXEL_OFFSET + DP_DEFAULT_PIXEL_SIZE) {
> DRM_ERROR("legacy memory region not large enough\n");
> return -EINVAL;
> }
> @@ -1216,8 +1217,10 @@ static int msm_dp_display_get_io(struct msm_dp_display_private *display)
> display->aux_len = DP_DEFAULT_AUX_SIZE;
> display->link_base = display->ahb_base + DP_DEFAULT_LINK_OFFSET;
> display->link_len = DP_DEFAULT_LINK_SIZE;
> - display->p0_base = display->ahb_base + DP_DEFAULT_P0_OFFSET;
> - display->p0_len = DP_DEFAULT_P0_SIZE;
> + for (i = DP_STREAM_0; i < display->max_stream; i++)
> + display->pixel_base[i] = display->ahb_base +
> + (i+1) * DP_DEFAULT_PIXEL_OFFSET;
> + display->pixel_len = DP_DEFAULT_PIXEL_SIZE;
>
> return 0;
> }
> @@ -1228,10 +1231,18 @@ static int msm_dp_display_get_io(struct msm_dp_display_private *display)
> return PTR_ERR(display->link_base);
> }
>
> - display->p0_base = msm_dp_ioremap(pdev, 3, &display->p0_len);
> - if (IS_ERR(display->p0_base)) {
> - DRM_ERROR("unable to remap p0 region: %pe\n", display->p0_base);
> - return PTR_ERR(display->p0_base);
> + display->pixel_base[0] = msm_dp_ioremap(pdev, 3, &display->pixel_len);
> + if (IS_ERR(display->pixel_base[0])) {
> + DRM_ERROR("unable to remap p0 region: %pe\n", display->pixel_base[0]);
> + return PTR_ERR(display->pixel_base[0]);
> + }
> +
> + for (i = DP_STREAM_1; i < display->max_stream; i++) {
> + /* pixels clk reg index start from 3*/
> + display->pixel_base[i] = msm_dp_ioremap(pdev, i + 3, &display->pixel_len);
> + if (IS_ERR(display->pixel_base[i]))
> + DRM_DEBUG_DP("unable to remap p%d region: %pe\n", i,
> + display->pixel_base[i]);
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index eae125972934bb2fb3b716dc47ae71cd0421bd1a..e8c1cf0c7dab7217b8bfe7ecd586af33d7547ca9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -26,7 +26,7 @@ struct msm_dp_panel_private {
> struct drm_dp_aux *aux;
> struct msm_dp_link *link;
> void __iomem *link_base;
> - void __iomem *p0_base;
> + void __iomem *pixel_base[DP_STREAM_MAX];
> bool panel_on;
> };
>
> @@ -45,24 +45,24 @@ static inline void msm_dp_write_link(struct msm_dp_panel_private *panel,
> writel(data, panel->link_base + offset);
> }
>
> -static inline void msm_dp_write_p0(struct msm_dp_panel_private *panel,
> +static inline void msm_dp_write_pn(struct msm_dp_panel_private *panel,
> u32 offset, u32 data)
Is it really multiplexed on the panel level? I'd assume that each panel
is connected to only one stream instance... If that's not the case, such
details must be explained in the commit message.
> {
> /*
> * To make sure interface reg writes happens before any other operation,
> * this function uses writel() instread of writel_relaxed()
> */
> - writel(data, panel->p0_base + offset);
> + writel(data, panel->pixel_base[panel->msm_dp_panel.stream_id] + offset);
> }
>
> -static inline u32 msm_dp_read_p0(struct msm_dp_panel_private *panel,
> +static inline u32 msm_dp_read_pn(struct msm_dp_panel_private *panel,
> u32 offset)
> {
> /*
> * To make sure interface reg writes happens before any other operation,
> * this function uses writel() instread of writel_relaxed()
> */
> - return readl_relaxed(panel->p0_base + offset);
> + return readl_relaxed(panel->pixel_base[panel->msm_dp_panel.stream_id] + offset);
> }
>
> static void msm_dp_panel_read_psr_cap(struct msm_dp_panel_private *panel)
> @@ -297,33 +297,33 @@ static void msm_dp_panel_tpg_enable(struct msm_dp_panel *msm_dp_panel,
> display_hctl = (hsync_end_x << 16) | hsync_start_x;
>
>
> - msm_dp_write_p0(panel, MMSS_DP_INTF_HSYNC_CTL, hsync_ctl);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_VSYNC_PERIOD_F0, vsync_period *
> + msm_dp_write_pn(panel, MMSS_DP_INTF_HSYNC_CTL, hsync_ctl);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_VSYNC_PERIOD_F0, vsync_period *
> hsync_period);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_VSYNC_PULSE_WIDTH_F0, v_sync_width *
> + msm_dp_write_pn(panel, MMSS_DP_INTF_VSYNC_PULSE_WIDTH_F0, v_sync_width *
> hsync_period);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_VSYNC_PERIOD_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_VSYNC_PULSE_WIDTH_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_DISPLAY_HCTL, display_hctl);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_ACTIVE_HCTL, 0);
> - msm_dp_write_p0(panel, MMSS_INTF_DISPLAY_V_START_F0, display_v_start);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_DISPLAY_V_END_F0, display_v_end);
> - msm_dp_write_p0(panel, MMSS_INTF_DISPLAY_V_START_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_DISPLAY_V_END_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_ACTIVE_V_START_F0, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_ACTIVE_V_END_F0, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_ACTIVE_V_START_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_ACTIVE_V_END_F1, 0);
> - msm_dp_write_p0(panel, MMSS_DP_INTF_POLARITY_CTL, 0);
> -
> - msm_dp_write_p0(panel, MMSS_DP_TPG_MAIN_CONTROL,
> + msm_dp_write_pn(panel, MMSS_DP_INTF_VSYNC_PERIOD_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_VSYNC_PULSE_WIDTH_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_DISPLAY_HCTL, display_hctl);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_ACTIVE_HCTL, 0);
> + msm_dp_write_pn(panel, MMSS_INTF_DISPLAY_V_START_F0, display_v_start);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_DISPLAY_V_END_F0, display_v_end);
> + msm_dp_write_pn(panel, MMSS_INTF_DISPLAY_V_START_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_DISPLAY_V_END_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_ACTIVE_V_START_F0, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_ACTIVE_V_END_F0, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_ACTIVE_V_START_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_ACTIVE_V_END_F1, 0);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_POLARITY_CTL, 0);
> +
> + msm_dp_write_pn(panel, MMSS_DP_TPG_MAIN_CONTROL,
> DP_TPG_CHECKERED_RECT_PATTERN);
> - msm_dp_write_p0(panel, MMSS_DP_TPG_VIDEO_CONFIG,
> + msm_dp_write_pn(panel, MMSS_DP_TPG_VIDEO_CONFIG,
> DP_TPG_VIDEO_CONFIG_BPP_8BIT |
> DP_TPG_VIDEO_CONFIG_RGB);
> - msm_dp_write_p0(panel, MMSS_DP_BIST_ENABLE,
> + msm_dp_write_pn(panel, MMSS_DP_BIST_ENABLE,
> DP_BIST_ENABLE_DPBIST_EN);
> - msm_dp_write_p0(panel, MMSS_DP_TIMING_ENGINE_EN,
> + msm_dp_write_pn(panel, MMSS_DP_TIMING_ENGINE_EN,
> DP_TIMING_ENGINE_EN_EN);
> drm_dbg_dp(panel->drm_dev, "%s: enabled tpg\n", __func__);
> }
> @@ -333,9 +333,9 @@ static void msm_dp_panel_tpg_disable(struct msm_dp_panel *msm_dp_panel)
> struct msm_dp_panel_private *panel =
> container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
>
> - msm_dp_write_p0(panel, MMSS_DP_TPG_MAIN_CONTROL, 0x0);
> - msm_dp_write_p0(panel, MMSS_DP_BIST_ENABLE, 0x0);
> - msm_dp_write_p0(panel, MMSS_DP_TIMING_ENGINE_EN, 0x0);
> + msm_dp_write_pn(panel, MMSS_DP_TPG_MAIN_CONTROL, 0x0);
> + msm_dp_write_pn(panel, MMSS_DP_BIST_ENABLE, 0x0);
> + msm_dp_write_pn(panel, MMSS_DP_TIMING_ENGINE_EN, 0x0);
> }
>
> void msm_dp_panel_tpg_config(struct msm_dp_panel *msm_dp_panel, bool enable)
> @@ -369,7 +369,7 @@ void msm_dp_panel_clear_dsc_dto(struct msm_dp_panel *msm_dp_panel)
> struct msm_dp_panel_private *panel =
> container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
>
> - msm_dp_write_p0(panel, MMSS_DP_DSC_DTO, 0x0);
> + msm_dp_write_pn(panel, MMSS_DP_DSC_DTO, 0x0);
> }
>
> static void msm_dp_panel_send_vsc_sdp(struct msm_dp_panel_private *panel, struct dp_sdp *vsc_sdp)
> @@ -559,7 +559,7 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
> msm_dp_write_link(panel, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY, width_blanking);
> msm_dp_write_link(panel, REG_DP_ACTIVE_HOR_VER, msm_dp_active);
>
> - reg = msm_dp_read_p0(panel, MMSS_DP_INTF_CONFIG);
> + reg = msm_dp_read_pn(panel, MMSS_DP_INTF_CONFIG);
> if (wide_bus_en)
> reg |= DP_INTF_CONFIG_DATABUS_WIDEN;
> else
> @@ -567,7 +567,7 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
>
> drm_dbg_dp(panel->drm_dev, "wide_bus_en=%d reg=%#x\n", wide_bus_en, reg);
>
> - msm_dp_write_p0(panel, MMSS_DP_INTF_CONFIG, reg);
> + msm_dp_write_pn(panel, MMSS_DP_INTF_CONFIG, reg);
>
> if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
> msm_dp_panel_setup_vsc_sdp_yuv_420(msm_dp_panel);
> @@ -673,7 +673,7 @@ static int msm_dp_panel_parse_dt(struct msm_dp_panel *msm_dp_panel)
> struct msm_dp_panel *msm_dp_panel_get(struct device *dev, struct drm_dp_aux *aux,
> struct msm_dp_link *link,
> void __iomem *link_base,
> - void __iomem *p0_base)
> + void __iomem *pixel_base[])
> {
> struct msm_dp_panel_private *panel;
> struct msm_dp_panel *msm_dp_panel;
> @@ -692,7 +692,7 @@ struct msm_dp_panel *msm_dp_panel_get(struct device *dev, struct drm_dp_aux *aux
> panel->aux = aux;
> panel->link = link;
> panel->link_base = link_base;
> - panel->p0_base = p0_base;
> + memcpy(panel->pixel_base, pixel_base, sizeof(panel->pixel_base));
>
> msm_dp_panel = &panel->msm_dp_panel;
> msm_dp_panel->max_bw_code = DP_LINK_BW_8_1;
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists