diff mbox series

[RFC,1/4] drm/i915/hdmi: Add audio config related params in crtc_state

Message ID 20230609084504.1929424-2-mitulkumar.ajitkumar.golani@intel.com (mailing list archive)
State New, archived
Headers show
Series Get optimal audio frequency and channels | expand

Commit Message

Mitul Golani June 9, 2023, 8:45 a.m. UTC
Add source audio-related config params in crtc_state.
These params can be supported frequency, supported channel,
and audio support, which can be further computed based on
source capabilities.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Jani Nikula June 9, 2023, 9:36 a.m. UTC | #1
On Fri, 09 Jun 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add source audio-related config params in crtc_state.
> These params can be supported frequency, supported channel,
> and audio support, which can be further computed based on
> source capabilities.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 731f2ec04d5c..873a60f3f870 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1129,9 +1129,15 @@ struct intel_crtc_state {
>  	/* Whether we should send NULL infoframes. Required for audio. */
>  	bool has_hdmi_sink;
>  
> -	/* Audio enabled on this pipe. Only valid if either has_hdmi_sink or
> -	 * has_dp_encoder is set. */
> -	bool has_audio;
> +	struct {
> +		bool has_audio;
> +
> +		/* Audio rate in Hz */
> +		unsigned int max_frequency;
> +
> +		/* Number of audio channels */
> +		unsigned int max_channel;
> +	} audio_config;

This breaks the build. Every commit should build on its own.

audio_config is too verbose. Please just use "audio".

Please don't add the new members in this commit, just first add the
substruct and make the updates. Then add new members in a separate
commit.

BR,
Jani.

>  
>  	/*
>  	 * Enable dithering, used when the selected pipe bpp doesn't match the
Jani Nikula June 9, 2023, 9:38 a.m. UTC | #2
On Fri, 09 Jun 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add source audio-related config params in crtc_state.
> These params can be supported frequency, supported channel,
> and audio support, which can be further computed based on
> source capabilities.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 731f2ec04d5c..873a60f3f870 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1129,9 +1129,15 @@ struct intel_crtc_state {
>  	/* Whether we should send NULL infoframes. Required for audio. */
>  	bool has_hdmi_sink;
>  
> -	/* Audio enabled on this pipe. Only valid if either has_hdmi_sink or
> -	 * has_dp_encoder is set. */
> -	bool has_audio;
> +	struct {
> +		bool has_audio;
> +
> +		/* Audio rate in Hz */
> +		unsigned int max_frequency;
> +
> +		/* Number of audio channels */
> +		unsigned int max_channel;

Please just use int, not unsigned int, for both of these.

BR,
Jani.

> +	} audio_config;
>  
>  	/*
>  	 * Enable dithering, used when the selected pipe bpp doesn't match the
Mitul Golani June 9, 2023, 5:42 p.m. UTC | #3
Hi @Jani Nikula

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 09 June 2023 15:08
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [RFC 1/4] drm/i915/hdmi: Add audio config related
> params in crtc_state
> 
> On Fri, 09 Jun 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> wrote:
> > Add source audio-related config params in crtc_state.
> > These params can be supported frequency, supported channel, and audio
> > support, which can be further computed based on source capabilities.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_types.h | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 731f2ec04d5c..873a60f3f870 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1129,9 +1129,15 @@ struct intel_crtc_state {
> >  	/* Whether we should send NULL infoframes. Required for audio. */
> >  	bool has_hdmi_sink;
> >
> > -	/* Audio enabled on this pipe. Only valid if either has_hdmi_sink or
> > -	 * has_dp_encoder is set. */
> > -	bool has_audio;
> > +	struct {
> > +		bool has_audio;
> > +
> > +		/* Audio rate in Hz */
> > +		unsigned int max_frequency;
> > +
> > +		/* Number of audio channels */
> > +		unsigned int max_channel;
> 
> Please just use int, not unsigned int, for both of these.

Thanks. Updated changes to new fix version.

> 
> BR,
> Jani.
> 
> > +	} audio_config;
> >
> >  	/*
> >  	 * Enable dithering, used when the selected pipe bpp doesn't match
> > the
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
Mitul Golani June 9, 2023, 5:44 p.m. UTC | #4
Hi @Jani Nikula

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 09 June 2023 15:06
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [RFC 1/4] drm/i915/hdmi: Add audio config related
> params in crtc_state
> 
> On Fri, 09 Jun 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> wrote:
> > Add source audio-related config params in crtc_state.
> > These params can be supported frequency, supported channel, and audio
> > support, which can be further computed based on source capabilities.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_types.h | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 731f2ec04d5c..873a60f3f870 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1129,9 +1129,15 @@ struct intel_crtc_state {
> >  	/* Whether we should send NULL infoframes. Required for audio. */
> >  	bool has_hdmi_sink;
> >
> > -	/* Audio enabled on this pipe. Only valid if either has_hdmi_sink or
> > -	 * has_dp_encoder is set. */
> > -	bool has_audio;
> > +	struct {
> > +		bool has_audio;
> > +
> > +		/* Audio rate in Hz */
> > +		unsigned int max_frequency;
> > +
> > +		/* Number of audio channels */
> > +		unsigned int max_channel;
> > +	} audio_config;
> 
> This breaks the build. Every commit should build on its own.
> 
> audio_config is too verbose. Please just use "audio".
> 
> Please don't add the new members in this commit, just first add the
> substruct and make the updates. Then add new members in a separate
> commit.
> 
> BR,
> Jani.

Thanks. Updated changes to new fix version. Refactored has_audio and other audio params to separate commit where it is used.

Regards,
Mitul

> 
> >
> >  	/*
> >  	 * Enable dithering, used when the selected pipe bpp doesn't match
> > the
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 731f2ec04d5c..873a60f3f870 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1129,9 +1129,15 @@  struct intel_crtc_state {
 	/* Whether we should send NULL infoframes. Required for audio. */
 	bool has_hdmi_sink;
 
-	/* Audio enabled on this pipe. Only valid if either has_hdmi_sink or
-	 * has_dp_encoder is set. */
-	bool has_audio;
+	struct {
+		bool has_audio;
+
+		/* Audio rate in Hz */
+		unsigned int max_frequency;
+
+		/* Number of audio channels */
+		unsigned int max_channel;
+	} audio_config;
 
 	/*
 	 * Enable dithering, used when the selected pipe bpp doesn't match the