| Message ID | 20250124-bridge-hdmi-connector-v6-2-1592632327f7@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: make use of the HDMI connector infrastructure | expand |
On 1/24/2025 1:47 PM, Dmitry Baryshkov wrote: > The mode_set callback is deprecated, it doesn't get the > drm_bridge_state, just mode-related argumetns. Also Abhinav pointed out > that HDMI timings should be programmed after setting up HDMI PHY and > PLL. Rework the code to program HDMI timings at the end of > atomic_pre_enable(). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > index d839c71091dcdc3b020fcbba8d698d58ee7fc749..d5ab1f74c0e6f47dc59872c016104e9a84d85e9e 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c > @@ -126,15 +126,26 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) > hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val); > } > > +static void msm_hdmi_bridge_atomic_set_timings(struct hdmi *hdmi, > + const struct drm_display_mode *mode); > static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > struct drm_bridge_state *old_bridge_state) > { > + struct drm_atomic_state *state = old_bridge_state->base.state; > 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; > > DBG("power up"); > > + connector = drm_atomic_get_new_connector_for_encoder(state, encoder); > + conn_state = drm_atomic_get_new_connector_state(state, connector); > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > + > if (!hdmi->power_on) { > msm_hdmi_phy_resource_enable(phy); > msm_hdmi_power_on(bridge); > @@ -151,6 +162,8 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > > if (hdmi->hdcp_ctrl) > msm_hdmi_hdcp_on(hdmi->hdcp_ctrl); > + > + msm_hdmi_bridge_atomic_set_timings(hdmi, &crtc_state->adjusted_mode); > } > This addresses my comment about setting up the HDMI timing registers before setting up the timing engine registers. But prior to this change, mode_set was doing the same thing as msm_hdmi_bridge_atomic_set_timings() which means msm_hdmi_bridge_atomic_set_timings() should be called at the beginning of pre_enable()? The controller is enabled in msm_hdmi_set_mode(). So this should be done before that. > static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, > @@ -177,17 +190,12 @@ static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, > } > } > > -static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge, > - const struct drm_display_mode *mode, > - const struct drm_display_mode *adjusted_mode) > +static void msm_hdmi_bridge_atomic_set_timings(struct hdmi *hdmi, > + const struct drm_display_mode *mode) > { > - struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); > - struct hdmi *hdmi = hdmi_bridge->hdmi; > int hstart, hend, vstart, vend; > uint32_t frame_ctrl; > > - mode = adjusted_mode; > - > hdmi->pixclock = mode->clock * 1000; > > hstart = mode->htotal - mode->hsync_start; > @@ -306,7 +314,6 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { > .atomic_reset = drm_atomic_helper_bridge_reset, > .atomic_pre_enable = msm_hdmi_bridge_atomic_pre_enable, > .atomic_post_disable = msm_hdmi_bridge_atomic_post_disable, > - .mode_set = msm_hdmi_bridge_mode_set, > .mode_valid = msm_hdmi_bridge_mode_valid, > .edid_read = msm_hdmi_bridge_edid_read, > .detect = msm_hdmi_bridge_detect, >
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index d839c71091dcdc3b020fcbba8d698d58ee7fc749..d5ab1f74c0e6f47dc59872c016104e9a84d85e9e 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -126,15 +126,26 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val); } +static void msm_hdmi_bridge_atomic_set_timings(struct hdmi *hdmi, + const struct drm_display_mode *mode); static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, struct drm_bridge_state *old_bridge_state) { + struct drm_atomic_state *state = old_bridge_state->base.state; 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; DBG("power up"); + connector = drm_atomic_get_new_connector_for_encoder(state, encoder); + conn_state = drm_atomic_get_new_connector_state(state, connector); + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); + if (!hdmi->power_on) { msm_hdmi_phy_resource_enable(phy); msm_hdmi_power_on(bridge); @@ -151,6 +162,8 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, if (hdmi->hdcp_ctrl) msm_hdmi_hdcp_on(hdmi->hdcp_ctrl); + + msm_hdmi_bridge_atomic_set_timings(hdmi, &crtc_state->adjusted_mode); } static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, @@ -177,17 +190,12 @@ static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge, } } -static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge, - const struct drm_display_mode *mode, - const struct drm_display_mode *adjusted_mode) +static void msm_hdmi_bridge_atomic_set_timings(struct hdmi *hdmi, + const struct drm_display_mode *mode) { - struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); - struct hdmi *hdmi = hdmi_bridge->hdmi; int hstart, hend, vstart, vend; uint32_t frame_ctrl; - mode = adjusted_mode; - hdmi->pixclock = mode->clock * 1000; hstart = mode->htotal - mode->hsync_start; @@ -306,7 +314,6 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_pre_enable = msm_hdmi_bridge_atomic_pre_enable, .atomic_post_disable = msm_hdmi_bridge_atomic_post_disable, - .mode_set = msm_hdmi_bridge_mode_set, .mode_valid = msm_hdmi_bridge_mode_valid, .edid_read = msm_hdmi_bridge_edid_read, .detect = msm_hdmi_bridge_detect,
The mode_set callback is deprecated, it doesn't get the drm_bridge_state, just mode-related argumetns. Also Abhinav pointed out that HDMI timings should be programmed after setting up HDMI PHY and PLL. Rework the code to program HDMI timings at the end of atomic_pre_enable(). Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)