On Mon, Jun 09, 2025 at 08:21:20PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <quic_abhin...@quicinc.com>
>
> In preparation of DP MST where link caps are read for the
> immediate downstream device and the edid is read through
EDID, not edid. Please review all your patches for up/down case.
> sideband messaging, split the msm_dp_panel_read_sink_caps() into
> two parts which read the link parameters and the edid parts
> respectively. Also drop the panel drm_edid cached as we actually
> don't need it.
Also => separate change.
>
> Signed-off-by: Abhinav Kumar <quic_abhin...@quicinc.com>
> Signed-off-by: Yongxing Mou <quic_yong...@quicinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++----
> drivers/gpu/drm/msm/dp/dp_panel.c | 55
> ++++++++++++++++++++-----------------
> drivers/gpu/drm/msm/dp/dp_panel.h | 6 ++--
> 3 files changed, 40 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index
> 6f05a939ce9e648e9601597155999b6f85adfcff..4a9b65647cdef1ed6c3bb851f93df0db8be977af
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -389,7 +389,11 @@ static int msm_dp_display_process_hpd_high(struct
> msm_dp_display_private *dp)
>
> dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
>
> - rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
> + rc = msm_dp_panel_read_link_caps(dp->panel);
> + if (rc)
> + goto end;
> +
> + rc = msm_dp_panel_read_edid(dp->panel, connector);
> if (rc)
> goto end;
>
> @@ -720,7 +724,6 @@ static int msm_dp_irq_hpd_handle(struct
> msm_dp_display_private *dp, u32 data)
> static void msm_dp_display_deinit_sub_modules(struct msm_dp_display_private
> *dp)
> {
> msm_dp_audio_put(dp->audio);
> - msm_dp_panel_put(dp->panel);
> msm_dp_aux_put(dp->aux);
> }
>
> @@ -783,7 +786,7 @@ static int msm_dp_init_sub_modules(struct
> msm_dp_display_private *dp)
> rc = PTR_ERR(dp->ctrl);
> DRM_ERROR("failed to initialize ctrl, rc = %d\n", rc);
> dp->ctrl = NULL;
> - goto error_ctrl;
> + goto error_link;
> }
>
> dp->audio = msm_dp_audio_get(dp->msm_dp_display.pdev, dp->catalog);
> @@ -791,13 +794,11 @@ static int msm_dp_init_sub_modules(struct
> msm_dp_display_private *dp)
> rc = PTR_ERR(dp->audio);
> pr_err("failed to initialize audio, rc = %d\n", rc);
> dp->audio = NULL;
> - goto error_ctrl;
> + goto error_link;
> }
>
> return rc;
>
> -error_ctrl:
> - msm_dp_panel_put(dp->panel);
> error_link:
> msm_dp_aux_put(dp->aux);
> error:
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index
> 4e8ab75c771b1e3a2d62f75e9993e1062118482b..d9041e235104a74b3cc50ff2e307eae0c4301ef3
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -118,14 +118,13 @@ static u32 msm_dp_panel_get_supported_bpp(struct
> msm_dp_panel *msm_dp_panel,
> return min_supported_bpp;
> }
>
> -int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
> - struct drm_connector *connector)
> +int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel)
> {
> int rc, bw_code;
> int count;
> struct msm_dp_panel_private *panel;
>
> - if (!msm_dp_panel || !connector) {
> + if (!msm_dp_panel) {
> DRM_ERROR("invalid input\n");
> return -EINVAL;
> }
> @@ -160,26 +159,29 @@ int msm_dp_panel_read_sink_caps(struct msm_dp_panel
> *msm_dp_panel,
>
> rc = drm_dp_read_downstream_info(panel->aux, msm_dp_panel->dpcd,
> msm_dp_panel->downstream_ports);
> - if (rc)
> - return rc;
> + return rc;
> +}
>
> - drm_edid_free(msm_dp_panel->drm_edid);
> +int msm_dp_panel_read_edid(struct msm_dp_panel *msm_dp_panel, struct
> drm_connector *connector)
> +{
> + struct msm_dp_panel_private *panel;
> + const struct drm_edid *drm_edid;
> +
> + panel = container_of(msm_dp_panel, struct msm_dp_panel_private,
> msm_dp_panel);
>
> - msm_dp_panel->drm_edid = drm_edid_read_ddc(connector, &panel->aux->ddc);
> + drm_edid = drm_edid_read_ddc(connector, &panel->aux->ddc);
>
> - drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
> + drm_edid_connector_update(connector, drm_edid);
>
> - if (!msm_dp_panel->drm_edid) {
> + if (!drm_edid) {
> DRM_ERROR("panel edid read failed\n");
> /* check edid read fail is due to unplug */
> if (!msm_dp_catalog_link_is_connected(panel->catalog)) {
> - rc = -ETIMEDOUT;
> - goto end;
> + return -ETIMEDOUT;
> }
> }
>
> -end:
> - return rc;
> + return 0;
> }
>
> u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
> @@ -208,15 +210,20 @@ u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel
> *msm_dp_panel,
> int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
> struct drm_connector *connector)
> {
> + struct msm_dp_panel_private *panel;
> + const struct drm_edid *drm_edid;
> +
> if (!msm_dp_panel) {
> DRM_ERROR("invalid input\n");
> return -EINVAL;
> }
>
> - if (msm_dp_panel->drm_edid)
> - return drm_edid_connector_add_modes(connector);
> + panel = container_of(msm_dp_panel, struct msm_dp_panel_private,
> msm_dp_panel);
> +
> + drm_edid = drm_edid_read_ddc(connector, &panel->aux->ddc);
> + drm_edid_connector_update(connector, drm_edid);
If EDID has been read and processed after HPD high event, why do we need
to re-read it again? Are we expecting that EDID will change?
>
> - return 0;
> + return drm_edid_connector_add_modes(connector);
> }
>
> static u8 msm_dp_panel_get_edid_checksum(const struct edid *edid)
> @@ -229,6 +236,7 @@ static u8 msm_dp_panel_get_edid_checksum(const struct
> edid *edid)
> void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel)
> {
> struct msm_dp_panel_private *panel;
> + const struct drm_edid *drm_edid;
>
> if (!msm_dp_panel) {
> DRM_ERROR("invalid input\n");
> @@ -238,8 +246,13 @@ void msm_dp_panel_handle_sink_request(struct
> msm_dp_panel *msm_dp_panel)
> panel = container_of(msm_dp_panel, struct msm_dp_panel_private,
> msm_dp_panel);
>
> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
> + drm_edid = drm_edid_read_ddc(msm_dp_panel->connector,
> &panel->aux->ddc);
And again....
> +
> + if (!drm_edid)
> + return;
> +
> /* FIXME: get rid of drm_edid_raw() */
> - const struct edid *edid = drm_edid_raw(msm_dp_panel->drm_edid);
> + const struct edid *edid = drm_edid_raw(drm_edid);
> u8 checksum;
>
> if (edid)
> @@ -515,11 +528,3 @@ struct msm_dp_panel *msm_dp_panel_get(struct device
> *dev, struct drm_dp_aux *aux
>
> return msm_dp_panel;
> }
> -
> -void msm_dp_panel_put(struct msm_dp_panel *msm_dp_panel)
> -{
> - if (!msm_dp_panel)
> - return;
> -
> - drm_edid_free(msm_dp_panel->drm_edid);
> -}
Too many changes to be stuffed under the hood of "Also perform foo"
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
> b/drivers/gpu/drm/msm/dp/dp_panel.h
> index
> 4906f4f09f2451cfed3c1007f38b4db7dfdb1d90..7f139478e1012d5b8f1f745f0de5fc3943745428
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -32,7 +32,6 @@ struct msm_dp_panel {
> u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>
> struct msm_dp_link_info link_info;
> - const struct drm_edid *drm_edid;
> struct drm_connector *connector;
> struct msm_dp_display_mode msm_dp_mode;
> struct msm_dp_panel_psr psr_cap;
> @@ -51,7 +50,9 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel
> *msm_dp_panel);
> int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
> struct drm_connector *connector);
> u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel, u32
> mode_max_bpp,
> - u32 mode_pclk_khz);
> + u32 mode_pclk_khz);
> +int msm_dp_panel_read_link_caps(struct msm_dp_panel *dp_panel);
> +int msm_dp_panel_read_edid(struct msm_dp_panel *dp_panel, struct
> drm_connector *connector);
> int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
> struct drm_connector *connector);
> void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel);
> @@ -86,5 +87,4 @@ static inline bool is_lane_count_valid(u32 lane_count)
>
> struct msm_dp_panel *msm_dp_panel_get(struct device *dev, struct drm_dp_aux
> *aux,
> struct msm_dp_link *link, struct msm_dp_catalog
> *catalog);
> -void msm_dp_panel_put(struct msm_dp_panel *msm_dp_panel);
> #endif /* _DP_PANEL_H_ */
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
