| Message ID | 20240309-bridge-hdmi-connector-v2-0-1380bea3ee70@linaro.org |
|---|---|
| Headers | show |
| Series | drm/msm: make use of the HDMI connector infrastructure | expand |
Hi, On Sat, Mar 09, 2024 at 12:31:27PM +0200, Dmitry Baryshkov wrote: > This patchset sits on top Maxime's HDMI connector patchset ([1]). > > Currently this is an RFC exploring the interface between HDMI bridges > and HDMI connector code. This has been lightly verified on the Qualcomm > DB820c, which has native HDMI output. If this approach is considered to > be acceptable, I'll finish MSM HDMI bridge conversion (reworking the > Audio Infoframe code). Other bridges can follow the same approach (we > have lt9611 / lt9611uxc / adv7511 on Qualcomm hardware). > > [1] https://patchwork.freedesktop.org/series/122421/ > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Changes in v2: > - Dropped drm_connector_hdmi_setup(). Instead added > drm_connector_hdmi_init() to be used by drm_bridge_connector. > - Changed the drm_bridge_connector to act as a proxy for the HDMI > connector infrastructure. This removes most of the logic from > the bridge drivers. > - Link to v1: https://lore.kernel.org/r/20240308-bridge-hdmi-connector-v1-0-90b693550260@linaro.org Overall, aside from the small comments on individual patches, I think it's in good shape right now. Thanks! Maxime
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.
This patchset sits on top Maxime's HDMI connector patchset ([1]). Currently this is an RFC exploring the interface between HDMI bridges and HDMI connector code. This has been lightly verified on the Qualcomm DB820c, which has native HDMI output. If this approach is considered to be acceptable, I'll finish MSM HDMI bridge conversion (reworking the Audio Infoframe code). Other bridges can follow the same approach (we have lt9611 / lt9611uxc / adv7511 on Qualcomm hardware). [1] https://patchwork.freedesktop.org/series/122421/ Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes in v2: - Dropped drm_connector_hdmi_setup(). Instead added drm_connector_hdmi_init() to be used by drm_bridge_connector. - Changed the drm_bridge_connector to act as a proxy for the HDMI connector infrastructure. This removes most of the logic from the bridge drivers. - Link to v1: https://lore.kernel.org/r/20240308-bridge-hdmi-connector-v1-0-90b693550260@linaro.org --- Dmitry Baryshkov (5): drm/connector: hdmi: fix Infoframes generation drm/connector: hdmi: add drm_connector_hdmi_init drm/bridge-connector: implement glue code for HDMI connector drm/msm/hdmi: switch to atomic bridge callbacks drm/msm/hdmi: make use of the drm_connector_hdmi framework drivers/gpu/drm/drm_atomic_state_helper.c | 25 +++--- drivers/gpu/drm/drm_bridge_connector.c | 118 +++++++++++++++++++++++- drivers/gpu/drm/drm_connector.c | 143 +++++++++++++++++++++++------- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 96 +++++++++++++++----- include/drm/drm_bridge.h | 82 +++++++++++++++++ include/drm/drm_connector.h | 9 ++ 6 files changed, 401 insertions(+), 72 deletions(-) --- base-commit: b5b59b6c8b64e33de01434afd8f4297be175f62a change-id: 20240307-bridge-hdmi-connector-7e3754e661d0 Best regards,