Messages in this thread |  | | Date | Thu, 22 May 2025 09:38:34 +0200 | | From | neil.armstrong@linaro ... | | Subject | Re: [PATCH v4 1/5] drm/msm/hdmi: switch to generic PHY subsystem |
| |
On 20/05/2025 22:44, Dmitry Baryshkov wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Change the MSM HDMI driver to use generic PHY subsystem. Moving PHY > drivers allows better code sharing with the rest of the PHY system. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/gpu/drm/msm/Makefile | 7 - > drivers/gpu/drm/msm/hdmi/hdmi.c | 58 +- > drivers/gpu/drm/msm/hdmi/hdmi.h | 80 +-- > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 32 +- > drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 225 ------- > drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 51 -- > drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c | 765 ---------------------- > drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 769 ----------------------- > drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 141 ----- > drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 44 -- > drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c | 458 -------------- > drivers/phy/qualcomm/Kconfig | 21 + > drivers/phy/qualcomm/Makefile | 14 + > drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c | 71 +++ > drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c | 441 +++++++++++++ > drivers/phy/qualcomm/phy-qcom-hdmi-45nm.c | 186 ++++++ > drivers/phy/qualcomm/phy-qcom-hdmi-preqmp.c | 212 +++++++ > drivers/phy/qualcomm/phy-qcom-hdmi-preqmp.h | 81 +++ > drivers/phy/qualcomm/phy-qcom-qmp-hdmi-base.c | 185 ++++++ > drivers/phy/qualcomm/phy-qcom-qmp-hdmi-msm8996.c | 442 +++++++++++++ > drivers/phy/qualcomm/phy-qcom-qmp-hdmi-msm8998.c | 495 +++++++++++++++ > drivers/phy/qualcomm/phy-qcom-qmp-hdmi.h | 77 +++ > 22 files changed, 2259 insertions(+), 2596 deletions(-) >
<snip>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > index 53a7ce8cc7bc7b6278eae2cbc42c3fda8d697f6d..1a00c26c1b40fc81623c9fb22ba25f448c27bffb 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > @@ -5,6 +5,7 @@ > */ > > #include <linux/delay.h> > +#include <linux/phy/phy.h> > #include <drm/drm_bridge_connector.h> > #include <drm/drm_edid.h> > #include <drm/display/drm_hdmi_helper.h> > @@ -286,11 +287,12 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > { > struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); > struct hdmi *hdmi = hdmi_bridge->hdmi; > - struct hdmi_phy *phy = hdmi->phy; > struct drm_encoder *encoder = bridge->encoder; > struct drm_connector *connector; > struct drm_connector_state *conn_state; > struct drm_crtc_state *crtc_state; > + union phy_configure_opts phy_opts; > + int ret; > > DBG("power up"); > > @@ -304,7 +306,7 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > > mutex_lock(&hdmi->state_mutex); > if (!hdmi->power_on) { > - msm_hdmi_phy_resource_enable(phy); > + phy_init(hdmi->phy); > msm_hdmi_power_on(bridge); > hdmi->power_on = true; > } > @@ -315,7 +317,23 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > > drm_atomic_helper_connector_hdmi_update_infoframes(connector, state); > > - msm_hdmi_phy_powerup(phy, hdmi->pixclock); > + phy_opts.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate; > + phy_opts.hdmi.bpc = 8; > + phy_configure(hdmi->phy, &phy_opts); > + > + ret = phy_power_on(hdmi->phy); > + if (WARN_ON(ret)) > + return; > + > + if (hdmi->extp_clk) { > + ret = clk_set_rate(hdmi->extp_clk, hdmi->pixclock); > + if (ret) > + DRM_DEV_ERROR(bridge->dev->dev, "failed to set extp clk rate: %d\n", ret); > + > + ret = clk_prepare_enable(hdmi->extp_clk); > + if (ret) > + DRM_DEV_ERROR(bridge->dev->dev, "failed to enable extp clk: %d\n", ret); > + }
Why do you again set the extp_clk since it's already set & enabled in msm_hdmi_power_on() ?
Perhaps I missed a change but it's like that on next-20250521
> > msm_hdmi_set_mode(hdmi, true); > > @@ -328,7 +346,6 @@ static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, > { > struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); > struct hdmi *hdmi = hdmi_bridge->hdmi; > - struct hdmi_phy *phy = hdmi->phy; > > if (hdmi->hdcp_ctrl) > msm_hdmi_hdcp_off(hdmi->hdcp_ctrl); > @@ -339,14 +356,17 @@ static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, > mutex_lock(&hdmi->state_mutex); > msm_hdmi_set_mode(hdmi, hdmi->hpd_enabled); > > - msm_hdmi_phy_powerdown(phy); > + if (hdmi->extp_clk) > + clk_disable_unprepare(hdmi->extp_clk); > + > + phy_power_off(hdmi->phy); > > if (hdmi->power_on) { > power_off(bridge); > hdmi->power_on = false; > if (hdmi->connector->display_info.is_hdmi) > msm_hdmi_audio_update(hdmi); > - msm_hdmi_phy_resource_disable(phy); > + phy_exit(hdmi->phy); > } > mutex_unlock(&hdmi->state_mutex); > } <snip>
Otherwise it looks fine even there's a lot to digest and hard to figure out the exact changes done to the PHY drivers.
Neil
|  |