diff mbox series

[v3,09/10] drm/bridge: allow limiting I2S formats

Message ID 20250126-drm-hdmi-connector-cec-v3-9-5b5b2d4956da@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/display: generic HDMI CEC helpers | expand

Commit Message

Dmitry Baryshkov Jan. 26, 2025, 1:29 p.m. UTC
By default HDMI codec registers all formats supported on the I2S bus.
Allow bridges (and connectors) to limit the list of the PCM formats
supported by the HDMI codec.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/display/drm_bridge_connector.c  | 1 +
 drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 2 ++
 drivers/gpu/drm/vc4/vc4_hdmi.c                  | 2 +-
 include/drm/display/drm_hdmi_audio_helper.h     | 1 +
 include/drm/drm_bridge.h                        | 5 +++++
 5 files changed, 10 insertions(+), 1 deletion(-)

Comments

Maxime Ripard Jan. 28, 2025, 4:11 p.m. UTC | #1
On Sun, Jan 26, 2025 at 03:29:14PM +0200, Dmitry Baryshkov wrote:
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index b55e80a57758e8b652eac0cd01cb245a04e221f5..d16af5fe90cb48f6671e798d9dee61a359c9233f 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -920,6 +920,11 @@ struct drm_bridge {
>  	 */
>  	int hdmi_audio_max_i2s_playback_channels;
>  
> +	/**
> +	 * @hdmi_audio_i2s_formats: supported I2S formats, optional
> +	 */
> +	u64 hdmi_audio_i2s_formats;
> +

We should document what the default value is if it's optional.

Once fixed:
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime
Dmitry Baryshkov Jan. 28, 2025, 11:45 p.m. UTC | #2
On Tue, Jan 28, 2025 at 05:11:16PM +0100, Maxime Ripard wrote:
> On Sun, Jan 26, 2025 at 03:29:14PM +0200, Dmitry Baryshkov wrote:
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index b55e80a57758e8b652eac0cd01cb245a04e221f5..d16af5fe90cb48f6671e798d9dee61a359c9233f 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -920,6 +920,11 @@ struct drm_bridge {
> >  	 */
> >  	int hdmi_audio_max_i2s_playback_channels;
> >  
> > +	/**
> > +	 * @hdmi_audio_i2s_formats: supported I2S formats, optional
> > +	 */
> > +	u64 hdmi_audio_i2s_formats;
> > +
> 
> We should document what the default value is if it's optional.

"The default is to allow all formats supported by the corresponding
I2S bus driver." Does that sound fine from your POV?

> 
> Once fixed:
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> 
> Maxime
Maxime Ripard Feb. 5, 2025, 2:29 p.m. UTC | #3
On Wed, Jan 29, 2025 at 01:45:56AM +0200, Dmitry Baryshkov wrote:
> On Tue, Jan 28, 2025 at 05:11:16PM +0100, Maxime Ripard wrote:
> > On Sun, Jan 26, 2025 at 03:29:14PM +0200, Dmitry Baryshkov wrote:
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index b55e80a57758e8b652eac0cd01cb245a04e221f5..d16af5fe90cb48f6671e798d9dee61a359c9233f 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -920,6 +920,11 @@ struct drm_bridge {
> > >  	 */
> > >  	int hdmi_audio_max_i2s_playback_channels;
> > >  
> > > +	/**
> > > +	 * @hdmi_audio_i2s_formats: supported I2S formats, optional
> > > +	 */
> > > +	u64 hdmi_audio_i2s_formats;
> > > +
> > 
> > We should document what the default value is if it's optional.
> 
> "The default is to allow all formats supported by the corresponding
> I2S bus driver." Does that sound fine from your POV?

Yep, sounds good.

Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 5b77fd59d79abddd419e611a7868b001857ccb37..28055bc2e7069d738bbe76b16c3bbde06f2d6e4e 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -698,6 +698,7 @@  struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 							    bridge->hdmi_dev,
 							    &drm_bridge_connector_hdmi_audio_funcs,
 							    bridge->hdmi_audio_max_i2s_playback_channels,
+							    bridge->hdmi_audio_i2s_formats,
 							    bridge->hdmi_audio_spdif_playback,
 							    bridge->hdmi_audio_dai_port);
 			if (ret)
diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
index 05afc9f0bdd6b6f00d74223a9d8875e6d16aea5f..589b0bd6c21366b83bd4d1131e89c71644ebc401 100644
--- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
@@ -154,6 +154,7 @@  int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 				  struct device *hdmi_codec_dev,
 				  const struct drm_connector_hdmi_audio_funcs *funcs,
 				  unsigned int max_i2s_playback_channels,
+				  u64 i2s_formats,
 				  bool spdif_playback,
 				  int dai_port)
 {
@@ -161,6 +162,7 @@  int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 		.ops = &drm_connector_hdmi_audio_ops,
 		.max_i2s_channels = max_i2s_playback_channels,
 		.i2s = !!max_i2s_playback_channels,
+		.i2s_formats = i2s_formats,
 		.spdif = spdif_playback,
 		.no_i2s_capture = true,
 		.no_spdif_capture = true,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1108983c44858382cb9f09b686956903645ebe0a..fcaba4a64a33b2267b22960772c2977b4109c67f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -582,7 +582,7 @@  static int vc4_hdmi_connector_init(struct drm_device *dev,
 
 	ret = drm_connector_hdmi_audio_init(connector, dev->dev,
 					    &vc4_hdmi_audio_funcs,
-					    8, false, -1);
+					    8, 0, false, -1);
 	if (ret)
 		return ret;
 
diff --git a/include/drm/display/drm_hdmi_audio_helper.h b/include/drm/display/drm_hdmi_audio_helper.h
index c9a6faef4109f20ba79b610a9d5e8d5980efe2d1..44d910bdc72dd2fdbbe7ada65b67080d4a41e88b 100644
--- a/include/drm/display/drm_hdmi_audio_helper.h
+++ b/include/drm/display/drm_hdmi_audio_helper.h
@@ -14,6 +14,7 @@  int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 				  struct device *hdmi_codec_dev,
 				  const struct drm_connector_hdmi_audio_funcs *funcs,
 				  unsigned int max_i2s_playback_channels,
+				  u64 i2s_formats,
 				  bool spdif_playback,
 				  int sound_dai_port);
 void drm_connector_hdmi_audio_plugged_notify(struct drm_connector *connector,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index b55e80a57758e8b652eac0cd01cb245a04e221f5..d16af5fe90cb48f6671e798d9dee61a359c9233f 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -920,6 +920,11 @@  struct drm_bridge {
 	 */
 	int hdmi_audio_max_i2s_playback_channels;
 
+	/**
+	 * @hdmi_audio_i2s_formats: supported I2S formats, optional
+	 */
+	u64 hdmi_audio_i2s_formats;
+
 	/**
 	 * @hdmi_audio_spdif_playback: set if HDMI codec has S/PDIF playback port
 	 */