On Mon, Jun 29, 2026 at 10:14:24PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <[email protected]>
>
> Add support for additional pixel register blocks (p1, p2, p3) to enable
> 4‑stream MST pixel clocks. Introduce the helper functions msm_dp_read_pn
> and msm_dp_write_pn for pixel register programming. All pixel clocks
> share the same register layout but use different base addresses.
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Yongxing Mou <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 40 +++++++++++++-----
> drivers/gpu/drm/msm/dp/dp_panel.c | 82
> ++++++++++++++++++-------------------
> drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
> 3 files changed, 71 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9cd243411e44..74f481a18164 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -85,8 +85,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;
> };
> @@ -564,7 +564,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[0]);
> if (IS_ERR(dp->panel)) {
> rc = PTR_ERR(dp->panel);
> DRM_ERROR("failed to initialize panel, rc = %d\n", rc);
> @@ -850,8 +850,14 @@ 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");
> + msm_disp_snapshot_add_block(disp_state, msm_dp_display->pixel_len,
> + msm_dp_display->pixel_base[1], "dp_p1");
> + msm_disp_snapshot_add_block(disp_state, msm_dp_display->pixel_len,
> + msm_dp_display->pixel_base[2], "dp_p2");
> + msm_disp_snapshot_add_block(disp_state, msm_dp_display->pixel_len,
> + msm_dp_display->pixel_base[3], "dp_p3");
It should be:
for int i = 0; i < DP_STREAM_MAX; i++)
Also, you've just added a NULL pointer exception in the crash handler.
Check for the address being non-zero before adding it to the snapshots.
> }
>
> void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
> @@ -1131,6 +1137,7 @@ static void __iomem *msm_dp_ioremap(struct
> platform_device *pdev, int idx, size_
> 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))
> @@ -1160,8 +1167,8 @@ 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;
> + display->pixel_base[0] = display->ahb_base +
> DP_DEFAULT_P0_OFFSET;
> + display->pixel_len = DP_DEFAULT_P0_SIZE;
>
> return 0;
> }
> @@ -1172,10 +1179,21 @@ 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 < DP_STREAM_MAX; 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]);
> + display->pixel_base[i] = NULL;
> + break;
Here we should differentiate between the address being not present in
DT (which should be ignored) and any other errors.
> + }
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 745ee6976897..238920c45261 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -25,7 +25,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;
> bool panel_on;
> };
>
> @@ -44,24 +44,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,
> - u32 offset, u32 data)
> +static inline void msm_dp_write_pn(struct msm_dp_panel_private *panel,
> + u32 offset, u32 data)
> {
> /*
> * 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 + offset);
> }
>
> -static inline u32 msm_dp_read_p0(struct msm_dp_panel_private *panel,
> - u32 offset)
> +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()
Hmm, so the comment talks about writel(_relaxed), but the code is readl.
Is the comment wrong? Or is it not applcable and we should be using
readl() here?
> */
> - return readl_relaxed(panel->p0_base + offset);
> + return readl_relaxed(panel->pixel_base + offset);
> }
>
--
With best wishes
Dmitry
