| Message ID | 20250209-dp-hdmi-audio-v2-1-16db6ebf22ff@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: reuse DRM HDMI Audio helpers for DisplayPort bridges | expand |
Hi Dmitry, Thank you for the patch. On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote: > DRM HDMI Codec framework is useful not only for the HDMI bridges, but > also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort > define in order to distinguish DP bridges. Create HDMI codec device > automatically for DP bridges which have declared audio support. > > Note, unlike HDMI devices, which already have a framework to handle HPD > notifications in a standard way, DP drivers don't (yet?) have such a > framework. As such it is necessary to manually call > drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully > can be lifted later on, if/when DRM framework gets better DisplayPort > ports support in the core layer. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------ > include/drm/drm_bridge.h | 14 +++++- > 2 files changed, 65 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c > index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644 > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c > @@ -98,6 +98,13 @@ struct drm_bridge_connector { > * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI). > */ > struct drm_bridge *bridge_hdmi; > + /** > + * @bridge_dp: > + * > + * The bridge in the chain that implements necessary support for the > + * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort). > + */ > + struct drm_bridge *bridge_dp; > }; > > #define to_drm_bridge_connector(x) \ > @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud > .mute_stream = drm_bridge_connector_audio_mute_stream, > }; > > +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector, > + struct drm_bridge *bridge) > +{ > + if (!bridge->hdmi_audio_max_i2s_playback_channels && > + !bridge->hdmi_audio_spdif_playback) > + return 0; > + > + if (!bridge->funcs->hdmi_audio_prepare || > + !bridge->funcs->hdmi_audio_shutdown) > + return -EINVAL; > + > + return drm_connector_hdmi_audio_init(connector, > + bridge->hdmi_audio_dev, > + &drm_bridge_connector_hdmi_audio_funcs, > + bridge->hdmi_audio_max_i2s_playback_channels, > + bridge->hdmi_audio_spdif_playback, > + bridge->hdmi_audio_dai_port); > +} > + > /* ----------------------------------------------------------------------------- > * Bridge Connector Initialisation > */ > @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > if (bridge->ops & DRM_BRIDGE_OP_HDMI) { > if (bridge_connector->bridge_hdmi) > return ERR_PTR(-EBUSY); > + if (bridge_connector->bridge_dp) > + return ERR_PTR(-EINVAL); > if (!bridge->funcs->hdmi_write_infoframe || > !bridge->funcs->hdmi_clear_infoframe) > return ERR_PTR(-EINVAL); > @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > max_bpc = bridge->max_bpc; > } > > + if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) { > + if (bridge_connector->bridge_dp) > + return ERR_PTR(-EBUSY); > + if (bridge_connector->bridge_hdmi) > + return ERR_PTR(-EINVAL); > + > + bridge_connector->bridge_dp = bridge; > + > + } > + > if (!drm_bridge_get_next_bridge(bridge)) > connector_type = bridge->type; > > @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > if (ret) > return ERR_PTR(ret); > > - if (bridge->hdmi_audio_max_i2s_playback_channels || > - bridge->hdmi_audio_spdif_playback) { > - if (!bridge->funcs->hdmi_audio_prepare || > - !bridge->funcs->hdmi_audio_shutdown) > - return ERR_PTR(-EINVAL); > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > + if (ret) > + return ERR_PTR(ret); > + } else if (bridge_connector->bridge_dp) { > + bridge = bridge_connector->bridge_dp; > > - ret = drm_connector_hdmi_audio_init(connector, > - bridge->hdmi_audio_dev, > - &drm_bridge_connector_hdmi_audio_funcs, > - bridge->hdmi_audio_max_i2s_playback_channels, > - bridge->hdmi_audio_spdif_playback, > - bridge->hdmi_audio_dai_port); > - if (ret) > - return ERR_PTR(ret); > - } > + ret = drmm_connector_init(drm, connector, > + &drm_bridge_connector_funcs, > + connector_type, ddc); > + if (ret) > + return ERR_PTR(ret); > + > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > + if (ret) > + return ERR_PTR(ret); > } else { > ret = drmm_connector_init(drm, connector, > &drm_bridge_connector_funcs, > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -811,9 +811,21 @@ enum drm_bridge_ops { > * > * Note: currently there can be at most one bridge in a chain that sets > * this bit. This is to simplify corresponding glue code in connector > - * drivers. > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > + * chain is also not allowed. > */ > DRM_BRIDGE_OP_HDMI = BIT(4), > + /** > + * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector > + * operations. Currently this is limited to the optional HDMI codec > + * support. > + * > + * Note: currently there can be at most one bridge in a chain that sets > + * this bit. This is to simplify corresponding glue code in connector > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > + * chain is also not allowed. > + */ > + DRM_BRIDGE_OP_DisplayPort = BIT(5), The OP bits are not supposed to describe tbe type of bridge, but the operations it implements. I see quite a bit of duplication between HDMI and DisplayPort in this patch. Can we have a single bit named after the feature that you want to support ? The bridge_hdmi and bridge_dp fields should also be merged into a single one. > }; > > /** >
On Fri, Feb 14, 2025 at 01:57:45AM +0200, Laurent Pinchart wrote: > Hi Dmitry, > > Thank you for the patch. > > On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote: > > DRM HDMI Codec framework is useful not only for the HDMI bridges, but > > also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort > > define in order to distinguish DP bridges. Create HDMI codec device > > automatically for DP bridges which have declared audio support. > > > > Note, unlike HDMI devices, which already have a framework to handle HPD > > notifications in a standard way, DP drivers don't (yet?) have such a > > framework. As such it is necessary to manually call > > drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully > > can be lifted later on, if/when DRM framework gets better DisplayPort > > ports support in the core layer. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------ > > include/drm/drm_bridge.h | 14 +++++- > > 2 files changed, 65 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c > > index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644 > > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c > > @@ -98,6 +98,13 @@ struct drm_bridge_connector { > > * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI). > > */ > > struct drm_bridge *bridge_hdmi; > > + /** > > + * @bridge_dp: > > + * > > + * The bridge in the chain that implements necessary support for the > > + * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort). > > + */ > > + struct drm_bridge *bridge_dp; > > }; > > > > #define to_drm_bridge_connector(x) \ > > @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud > > .mute_stream = drm_bridge_connector_audio_mute_stream, > > }; > > > > +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector, > > + struct drm_bridge *bridge) > > +{ > > + if (!bridge->hdmi_audio_max_i2s_playback_channels && > > + !bridge->hdmi_audio_spdif_playback) > > + return 0; > > + > > + if (!bridge->funcs->hdmi_audio_prepare || > > + !bridge->funcs->hdmi_audio_shutdown) > > + return -EINVAL; > > + > > + return drm_connector_hdmi_audio_init(connector, > > + bridge->hdmi_audio_dev, > > + &drm_bridge_connector_hdmi_audio_funcs, > > + bridge->hdmi_audio_max_i2s_playback_channels, > > + bridge->hdmi_audio_spdif_playback, > > + bridge->hdmi_audio_dai_port); > > +} > > + > > /* ----------------------------------------------------------------------------- > > * Bridge Connector Initialisation > > */ > > @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > if (bridge->ops & DRM_BRIDGE_OP_HDMI) { > > if (bridge_connector->bridge_hdmi) > > return ERR_PTR(-EBUSY); > > + if (bridge_connector->bridge_dp) > > + return ERR_PTR(-EINVAL); > > if (!bridge->funcs->hdmi_write_infoframe || > > !bridge->funcs->hdmi_clear_infoframe) > > return ERR_PTR(-EINVAL); > > @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > max_bpc = bridge->max_bpc; > > } > > > > + if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) { > > + if (bridge_connector->bridge_dp) > > + return ERR_PTR(-EBUSY); > > + if (bridge_connector->bridge_hdmi) > > + return ERR_PTR(-EINVAL); > > + > > + bridge_connector->bridge_dp = bridge; > > + > > + } > > + > > if (!drm_bridge_get_next_bridge(bridge)) > > connector_type = bridge->type; > > > > @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > if (ret) > > return ERR_PTR(ret); > > > > - if (bridge->hdmi_audio_max_i2s_playback_channels || > > - bridge->hdmi_audio_spdif_playback) { > > - if (!bridge->funcs->hdmi_audio_prepare || > > - !bridge->funcs->hdmi_audio_shutdown) > > - return ERR_PTR(-EINVAL); > > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > > + if (ret) > > + return ERR_PTR(ret); > > + } else if (bridge_connector->bridge_dp) { > > + bridge = bridge_connector->bridge_dp; > > > > - ret = drm_connector_hdmi_audio_init(connector, > > - bridge->hdmi_audio_dev, > > - &drm_bridge_connector_hdmi_audio_funcs, > > - bridge->hdmi_audio_max_i2s_playback_channels, > > - bridge->hdmi_audio_spdif_playback, > > - bridge->hdmi_audio_dai_port); > > - if (ret) > > - return ERR_PTR(ret); > > - } > > + ret = drmm_connector_init(drm, connector, > > + &drm_bridge_connector_funcs, > > + connector_type, ddc); > > + if (ret) > > + return ERR_PTR(ret); > > + > > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > > + if (ret) > > + return ERR_PTR(ret); > > } else { > > ret = drmm_connector_init(drm, connector, > > &drm_bridge_connector_funcs, > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644 > > --- a/include/drm/drm_bridge.h > > +++ b/include/drm/drm_bridge.h > > @@ -811,9 +811,21 @@ enum drm_bridge_ops { > > * > > * Note: currently there can be at most one bridge in a chain that sets > > * this bit. This is to simplify corresponding glue code in connector > > - * drivers. > > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > > + * chain is also not allowed. > > */ > > DRM_BRIDGE_OP_HDMI = BIT(4), > > + /** > > + * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector > > + * operations. Currently this is limited to the optional HDMI codec > > + * support. > > + * > > + * Note: currently there can be at most one bridge in a chain that sets > > + * this bit. This is to simplify corresponding glue code in connector > > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > > + * chain is also not allowed. > > + */ > > + DRM_BRIDGE_OP_DisplayPort = BIT(5), > > The OP bits are not supposed to describe tbe type of bridge, but the > operations it implements. I see quite a bit of duplication between HDMI > and DisplayPort in this patch. Can we have a single bit named after the > feature that you want to support ? The bridge_hdmi and bridge_dp fields > should also be merged into a single one. In this case these ops actually describe the set of ops implemented by the bridge. DRM_BRIDGE_OP_HDMI implements hdmi_tmds_char_rate_valid(), hdmi_write_infoframe(), hdmi_clear_infoframe() and hdmi_audio_*() callbacks. It is impossible to just use HDMI helpers for any DRM_MODE_CONNECTOR_HDMIA bridge as they lack required callbacks. At the same time it is perfectly legic to have a DRM_MODE_CONNECTOR_HDMIA bridge which doesn't set DRM_BRIDGE_OP_HDMI: this bridge chain will not use HDMI Connector / HDMI State helpers, but it should be fine otherwise. DRM_BRIDGE_OP_DisplayPort bridges currently implement hdmi_audio_*(), but I have long-term plans for that set of ops. It is not quite possible to merge bridge_hdmi and bridge_dp fields: for bridges, which implement DRM_BRIDGE_OP_HDMI, drm_bridge_connector call various drm_atomic_helper_connector_hdmi_*() functions. For DisplayPort there is no corresponding functionality (yet).
On Fri, Feb 14, 2025 at 02:32:56AM +0200, Dmitry Baryshkov wrote: > On Fri, Feb 14, 2025 at 01:57:45AM +0200, Laurent Pinchart wrote: > > Hi Dmitry, > > > > Thank you for the patch. > > > > On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote: > > > DRM HDMI Codec framework is useful not only for the HDMI bridges, but > > > also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort > > > define in order to distinguish DP bridges. Create HDMI codec device > > > automatically for DP bridges which have declared audio support. > > > > > > Note, unlike HDMI devices, which already have a framework to handle HPD > > > notifications in a standard way, DP drivers don't (yet?) have such a > > > framework. As such it is necessary to manually call > > > drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully > > > can be lifted later on, if/when DRM framework gets better DisplayPort > > > ports support in the core layer. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------ > > > include/drm/drm_bridge.h | 14 +++++- > > > 2 files changed, 65 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c > > > index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644 > > > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > > > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c > > > @@ -98,6 +98,13 @@ struct drm_bridge_connector { > > > * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI). > > > */ > > > struct drm_bridge *bridge_hdmi; > > > + /** > > > + * @bridge_dp: > > > + * > > > + * The bridge in the chain that implements necessary support for the > > > + * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort). > > > + */ > > > + struct drm_bridge *bridge_dp; > > > }; > > > > > > #define to_drm_bridge_connector(x) \ > > > @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud > > > .mute_stream = drm_bridge_connector_audio_mute_stream, > > > }; > > > > > > +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector, > > > + struct drm_bridge *bridge) > > > +{ > > > + if (!bridge->hdmi_audio_max_i2s_playback_channels && > > > + !bridge->hdmi_audio_spdif_playback) > > > + return 0; > > > + > > > + if (!bridge->funcs->hdmi_audio_prepare || > > > + !bridge->funcs->hdmi_audio_shutdown) > > > + return -EINVAL; > > > + > > > + return drm_connector_hdmi_audio_init(connector, > > > + bridge->hdmi_audio_dev, > > > + &drm_bridge_connector_hdmi_audio_funcs, > > > + bridge->hdmi_audio_max_i2s_playback_channels, > > > + bridge->hdmi_audio_spdif_playback, > > > + bridge->hdmi_audio_dai_port); > > > +} > > > + > > > /* ----------------------------------------------------------------------------- > > > * Bridge Connector Initialisation > > > */ > > > @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > if (bridge->ops & DRM_BRIDGE_OP_HDMI) { > > > if (bridge_connector->bridge_hdmi) > > > return ERR_PTR(-EBUSY); > > > + if (bridge_connector->bridge_dp) > > > + return ERR_PTR(-EINVAL); > > > if (!bridge->funcs->hdmi_write_infoframe || > > > !bridge->funcs->hdmi_clear_infoframe) > > > return ERR_PTR(-EINVAL); > > > @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > max_bpc = bridge->max_bpc; > > > } > > > > > > + if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) { > > > + if (bridge_connector->bridge_dp) > > > + return ERR_PTR(-EBUSY); > > > + if (bridge_connector->bridge_hdmi) > > > + return ERR_PTR(-EINVAL); > > > + > > > + bridge_connector->bridge_dp = bridge; > > > + > > > + } > > > + > > > if (!drm_bridge_get_next_bridge(bridge)) > > > connector_type = bridge->type; > > > > > > @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > if (ret) > > > return ERR_PTR(ret); > > > > > > - if (bridge->hdmi_audio_max_i2s_playback_channels || > > > - bridge->hdmi_audio_spdif_playback) { > > > - if (!bridge->funcs->hdmi_audio_prepare || > > > - !bridge->funcs->hdmi_audio_shutdown) > > > - return ERR_PTR(-EINVAL); > > > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > > > + if (ret) > > > + return ERR_PTR(ret); > > > + } else if (bridge_connector->bridge_dp) { > > > + bridge = bridge_connector->bridge_dp; > > > > > > - ret = drm_connector_hdmi_audio_init(connector, > > > - bridge->hdmi_audio_dev, > > > - &drm_bridge_connector_hdmi_audio_funcs, > > > - bridge->hdmi_audio_max_i2s_playback_channels, > > > - bridge->hdmi_audio_spdif_playback, > > > - bridge->hdmi_audio_dai_port); > > > - if (ret) > > > - return ERR_PTR(ret); > > > - } > > > + ret = drmm_connector_init(drm, connector, > > > + &drm_bridge_connector_funcs, > > > + connector_type, ddc); > > > + if (ret) > > > + return ERR_PTR(ret); > > > + > > > + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); > > > + if (ret) > > > + return ERR_PTR(ret); > > > } else { > > > ret = drmm_connector_init(drm, connector, > > > &drm_bridge_connector_funcs, > > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > > index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644 > > > --- a/include/drm/drm_bridge.h > > > +++ b/include/drm/drm_bridge.h > > > @@ -811,9 +811,21 @@ enum drm_bridge_ops { > > > * > > > * Note: currently there can be at most one bridge in a chain that sets > > > * this bit. This is to simplify corresponding glue code in connector > > > - * drivers. > > > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > > > + * chain is also not allowed. > > > */ > > > DRM_BRIDGE_OP_HDMI = BIT(4), > > > + /** > > > + * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector > > > + * operations. Currently this is limited to the optional HDMI codec > > > + * support. > > > + * > > > + * Note: currently there can be at most one bridge in a chain that sets > > > + * this bit. This is to simplify corresponding glue code in connector > > > + * drivers. Having both HDMI and DisplayPort bridges in the same bridge > > > + * chain is also not allowed. > > > + */ > > > + DRM_BRIDGE_OP_DisplayPort = BIT(5), > > > > The OP bits are not supposed to describe tbe type of bridge, but the > > operations it implements. I see quite a bit of duplication between HDMI > > and DisplayPort in this patch. Can we have a single bit named after the > > feature that you want to support ? The bridge_hdmi and bridge_dp fields > > should also be merged into a single one. > > In this case these ops actually describe the set of ops implemented by > the bridge. DRM_BRIDGE_OP_HDMI implements hdmi_tmds_char_rate_valid(), > hdmi_write_infoframe(), hdmi_clear_infoframe() Those are required for HDMI bridges (using the common infrastructure). > and hdmi_audio_*() callbacks. But those are certainly not required. If the OP enum is meant to list what a bridge implements, then we need to have a different enum for video, audio, and CEC. Maxime
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644 --- a/drivers/gpu/drm/display/drm_bridge_connector.c +++ b/drivers/gpu/drm/display/drm_bridge_connector.c @@ -98,6 +98,13 @@ struct drm_bridge_connector { * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI). */ struct drm_bridge *bridge_hdmi; + /** + * @bridge_dp: + * + * The bridge in the chain that implements necessary support for the + * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort). + */ + struct drm_bridge *bridge_dp; }; #define to_drm_bridge_connector(x) \ @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud .mute_stream = drm_bridge_connector_audio_mute_stream, }; +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector, + struct drm_bridge *bridge) +{ + if (!bridge->hdmi_audio_max_i2s_playback_channels && + !bridge->hdmi_audio_spdif_playback) + return 0; + + if (!bridge->funcs->hdmi_audio_prepare || + !bridge->funcs->hdmi_audio_shutdown) + return -EINVAL; + + return drm_connector_hdmi_audio_init(connector, + bridge->hdmi_audio_dev, + &drm_bridge_connector_hdmi_audio_funcs, + bridge->hdmi_audio_max_i2s_playback_channels, + bridge->hdmi_audio_spdif_playback, + bridge->hdmi_audio_dai_port); +} + /* ----------------------------------------------------------------------------- * Bridge Connector Initialisation */ @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (bridge->ops & DRM_BRIDGE_OP_HDMI) { if (bridge_connector->bridge_hdmi) return ERR_PTR(-EBUSY); + if (bridge_connector->bridge_dp) + return ERR_PTR(-EINVAL); if (!bridge->funcs->hdmi_write_infoframe || !bridge->funcs->hdmi_clear_infoframe) return ERR_PTR(-EINVAL); @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, max_bpc = bridge->max_bpc; } + if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) { + if (bridge_connector->bridge_dp) + return ERR_PTR(-EBUSY); + if (bridge_connector->bridge_hdmi) + return ERR_PTR(-EINVAL); + + bridge_connector->bridge_dp = bridge; + + } + if (!drm_bridge_get_next_bridge(bridge)) connector_type = bridge->type; @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (ret) return ERR_PTR(ret); - if (bridge->hdmi_audio_max_i2s_playback_channels || - bridge->hdmi_audio_spdif_playback) { - if (!bridge->funcs->hdmi_audio_prepare || - !bridge->funcs->hdmi_audio_shutdown) - return ERR_PTR(-EINVAL); + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); + if (ret) + return ERR_PTR(ret); + } else if (bridge_connector->bridge_dp) { + bridge = bridge_connector->bridge_dp; - ret = drm_connector_hdmi_audio_init(connector, - bridge->hdmi_audio_dev, - &drm_bridge_connector_hdmi_audio_funcs, - bridge->hdmi_audio_max_i2s_playback_channels, - bridge->hdmi_audio_spdif_playback, - bridge->hdmi_audio_dai_port); - if (ret) - return ERR_PTR(ret); - } + ret = drmm_connector_init(drm, connector, + &drm_bridge_connector_funcs, + connector_type, ddc); + if (ret) + return ERR_PTR(ret); + + ret = drm_bridge_connector_hdmi_audio_init(connector, bridge); + if (ret) + return ERR_PTR(ret); } else { ret = drmm_connector_init(drm, connector, &drm_bridge_connector_funcs, diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -811,9 +811,21 @@ enum drm_bridge_ops { * * Note: currently there can be at most one bridge in a chain that sets * this bit. This is to simplify corresponding glue code in connector - * drivers. + * drivers. Having both HDMI and DisplayPort bridges in the same bridge + * chain is also not allowed. */ DRM_BRIDGE_OP_HDMI = BIT(4), + /** + * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector + * operations. Currently this is limited to the optional HDMI codec + * support. + * + * Note: currently there can be at most one bridge in a chain that sets + * this bit. This is to simplify corresponding glue code in connector + * drivers. Having both HDMI and DisplayPort bridges in the same bridge + * chain is also not allowed. + */ + DRM_BRIDGE_OP_DisplayPort = BIT(5), }; /**
DRM HDMI Codec framework is useful not only for the HDMI bridges, but also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort define in order to distinguish DP bridges. Create HDMI codec device automatically for DP bridges which have declared audio support. Note, unlike HDMI devices, which already have a framework to handle HPD notifications in a standard way, DP drivers don't (yet?) have such a framework. As such it is necessary to manually call drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully can be lifted later on, if/when DRM framework gets better DisplayPort ports support in the core layer. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------ include/drm/drm_bridge.h | 14 +++++- 2 files changed, 65 insertions(+), 15 deletions(-)