Message ID | 20240309-bridge-hdmi-connector-v2-5-1380bea3ee70@linaro.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/msm: make use of the HDMI connector infrastructure | expand |
Hi, On Sat, Mar 09, 2024 at 12:31:32PM +0200, Dmitry Baryshkov wrote: > Setup the HDMI connector on the MSM HDMI outputs. Make use of > atomic_check hook and of the provided Infoframe infrastructure. > > Note: for now only AVI Infoframes are enabled. Audio Infoframes are > currenly handled separately. This will be fixed for the final version. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> I had a look at the driver, and it looks like mode_set and mode_valid could use the connector_state tmds_char_rate instead of pixclock and drm_connector_hdmi_compute_mode_clock respectively instead of calculating it by themselves. We can probably remove hdmi->pixclock entirely if we manage to pass the connector state to msm_hdmi_power_on. And that's unrelated to this series, but we can also remove hdmi->hdmi_mode for drm_display_info.is_hdmi. Maxime
On Mon, 11 Mar 2024 at 17:46, Maxime Ripard <mripard@kernel.org> wrote: > > Hi, > > On Sat, Mar 09, 2024 at 12:31:32PM +0200, Dmitry Baryshkov wrote: > > Setup the HDMI connector on the MSM HDMI outputs. Make use of > > atomic_check hook and of the provided Infoframe infrastructure. > > > > Note: for now only AVI Infoframes are enabled. Audio Infoframes are > > currenly handled separately. This will be fixed for the final version. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > I had a look at the driver, and it looks like mode_set and mode_valid > could use the connector_state tmds_char_rate instead of pixclock and > drm_connector_hdmi_compute_mode_clock respectively instead of > calculating it by themselves. Ack, I'll take a look.b > > We can probably remove hdmi->pixclock entirely if we manage to pass the > connector state to msm_hdmi_power_on. I'd like to defer this for a moment, I have a pending series moving MSM HDMI PHY drivers to generic PHY subsystem. However that patchset reworks the way the PHY is setup, so it doesn't make sense to rework msm_hdmi_power_on(). > > And that's unrelated to this series, but we can also remove > hdmi->hdmi_mode for drm_display_info.is_hdmi. Yes, that's the plan, once I rework the audio infoframe handling.
On Mon, Mar 11, 2024 at 05:55:36PM +0200, Dmitry Baryshkov wrote: > On Mon, 11 Mar 2024 at 17:46, Maxime Ripard <mripard@kernel.org> wrote: > > > > Hi, > > > > On Sat, Mar 09, 2024 at 12:31:32PM +0200, Dmitry Baryshkov wrote: > > > Setup the HDMI connector on the MSM HDMI outputs. Make use of > > > atomic_check hook and of the provided Infoframe infrastructure. > > > > > > Note: for now only AVI Infoframes are enabled. Audio Infoframes are > > > currenly handled separately. This will be fixed for the final version. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > I had a look at the driver, and it looks like mode_set and mode_valid > > could use the connector_state tmds_char_rate instead of pixclock and > > drm_connector_hdmi_compute_mode_clock respectively instead of > > calculating it by themselves. > > Ack, I'll take a look.b > > > > > We can probably remove hdmi->pixclock entirely if we manage to pass the > > connector state to msm_hdmi_power_on. > > I'd like to defer this for a moment, I have a pending series moving > MSM HDMI PHY drivers to generic PHY subsystem. However that patchset > reworks the way the PHY is setup, so it doesn't make sense to rework > msm_hdmi_power_on(). > > > > > And that's unrelated to this series, but we can also remove > > hdmi->hdmi_mode for drm_display_info.is_hdmi. > > Yes, that's the plan, once I rework the audio infoframe handling. Sure, if it makes more sense to defer it for now, then let's postpone it :) Maxime
On Mon, 11 Mar 2024 at 19:06, Maxime Ripard <mripard@kernel.org> wrote: > > On Mon, Mar 11, 2024 at 05:55:36PM +0200, Dmitry Baryshkov wrote: > > On Mon, 11 Mar 2024 at 17:46, Maxime Ripard <mripard@kernel.org> wrote: > > > > > > Hi, > > > > > > On Sat, Mar 09, 2024 at 12:31:32PM +0200, Dmitry Baryshkov wrote: > > > > Setup the HDMI connector on the MSM HDMI outputs. Make use of > > > > atomic_check hook and of the provided Infoframe infrastructure. > > > > > > > > Note: for now only AVI Infoframes are enabled. Audio Infoframes are > > > > currenly handled separately. This will be fixed for the final version. > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > I had a look at the driver, and it looks like mode_set and mode_valid > > > could use the connector_state tmds_char_rate instead of pixclock and > > > drm_connector_hdmi_compute_mode_clock respectively instead of > > > calculating it by themselves. > > > > Ack, I'll take a look.b > > > > > > > > We can probably remove hdmi->pixclock entirely if we manage to pass the > > > connector state to msm_hdmi_power_on. > > > > I'd like to defer this for a moment, I have a pending series moving > > MSM HDMI PHY drivers to generic PHY subsystem. However that patchset > > reworks the way the PHY is setup, so it doesn't make sense to rework > > msm_hdmi_power_on(). > > > > > > > > And that's unrelated to this series, but we can also remove > > > hdmi->hdmi_mode for drm_display_info.is_hdmi. > > > > Yes, that's the plan, once I rework the audio infoframe handling. > > Sure, if it makes more sense to defer it for now, then let's postpone it > :) I hope to fix this one for v3. Audio InfoFrame should be converted too.
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index d839c71091dc..26c847f42522 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -68,23 +68,15 @@ static void power_off(struct drm_bridge *bridge) #define AVI_IFRAME_LINE_NUMBER 1 -static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) +static int msm_hdmi_config_avi_infoframe(struct hdmi *hdmi, + const u8 *buffer, size_t len) { - struct drm_crtc *crtc = hdmi->encoder->crtc; - const struct drm_display_mode *mode = &crtc->state->adjusted_mode; - union hdmi_infoframe frame; - u8 buffer[HDMI_INFOFRAME_SIZE(AVI)]; u32 val; - int len; - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - hdmi->connector, mode); - - len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer)); - if (len < 0) { + if (len != HDMI_INFOFRAME_SIZE(AVI)) { DRM_DEV_ERROR(&hdmi->pdev->dev, "failed to configure avi infoframe\n"); - return; + return -EINVAL; } /* @@ -124,6 +116,55 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) val &= ~HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE__MASK; val |= HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE(AVI_IFRAME_LINE_NUMBER); hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val); + + return 0; +} + +static int msm_hdmi_bridge_clear_infoframe(struct drm_bridge *bridge, + enum hdmi_infoframe_type type) +{ + struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); + struct hdmi *hdmi = hdmi_bridge->hdmi; + u32 val; + + val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0); + + switch (type) { + case HDMI_INFOFRAME_TYPE_AVI: + val &= ~(HDMI_INFOFRAME_CTRL0_AVI_SEND | + HDMI_INFOFRAME_CTRL0_AVI_CONT); + break; + case HDMI_INFOFRAME_TYPE_AUDIO: + val &= ~(HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND | + HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT | + HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE | + HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE); + break; + default: + drm_dbg_driver(hdmi_bridge->base.dev, "Unsupported infoframe type %x\n", type); + } + + hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val); + + return 0; +} + +static int msm_hdmi_bridge_write_infoframe(struct drm_bridge *bridge, + enum hdmi_infoframe_type type, + const u8 *buffer, size_t len) +{ + struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); + struct hdmi *hdmi = hdmi_bridge->hdmi; + + msm_hdmi_bridge_clear_infoframe(bridge, type); + + switch (type) { + case HDMI_INFOFRAME_TYPE_AVI: + return msm_hdmi_config_avi_infoframe(hdmi, buffer, len); + default: + drm_dbg_driver(hdmi_bridge->base.dev, "Unsupported infoframe type %x\n", type); + return 0; + } } static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, @@ -132,6 +173,10 @@ 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_atomic_state *state = old_bridge_state->base.state; + struct drm_connector *connector = + drm_atomic_get_new_connector_for_encoder(state, encoder); DBG("power up"); @@ -139,12 +184,13 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, msm_hdmi_phy_resource_enable(phy); msm_hdmi_power_on(bridge); hdmi->power_on = true; - if (hdmi->hdmi_mode) { - msm_hdmi_config_avi_infoframe(hdmi); - msm_hdmi_audio_update(hdmi); - } } + drm_atomic_helper_connector_hdmi_update_infoframes(connector, state); + + if (hdmi->hdmi_mode) + msm_hdmi_audio_update(hdmi); + msm_hdmi_phy_powerup(phy, hdmi->pixclock); msm_hdmi_set_mode(hdmi, true); @@ -310,6 +356,8 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { .mode_valid = msm_hdmi_bridge_mode_valid, .edid_read = msm_hdmi_bridge_edid_read, .detect = msm_hdmi_bridge_detect, + .clear_infoframe = msm_hdmi_bridge_clear_infoframe, + .write_infoframe = msm_hdmi_bridge_write_infoframe, }; static void @@ -341,8 +389,11 @@ int msm_hdmi_bridge_init(struct hdmi *hdmi) bridge->funcs = &msm_hdmi_bridge_funcs; bridge->ddc = hdmi->i2c; bridge->type = DRM_MODE_CONNECTOR_HDMIA; + bridge->vendor = "Qualcomm"; + bridge->product = "Snapdragon"; bridge->ops = DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT | + DRM_BRIDGE_OP_HDMI | DRM_BRIDGE_OP_EDID; ret = devm_drm_bridge_add(hdmi->dev->dev, bridge);
Setup the HDMI connector on the MSM HDMI outputs. Make use of atomic_check hook and of the provided Infoframe infrastructure. Note: for now only AVI Infoframes are enabled. Audio Infoframes are currenly handled separately. This will be fixed for the final version. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 83 +++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 16 deletions(-)