| Message ID | 20230817125007.2681331-3-mitulkumar.ajitkumar.golani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Get optimal audio frequency and channels | expand |
On Thu, Aug 17, 2023 at 06:20:06PM +0530, Mitul Golani wrote: > Initialize the source audio capabilities in crtc_state > property by setting them to their maximum supported values, > including max_channel and max_frequency. This allows for the > calculation of audio source capabilities with respect to > the available mode bandwidth. These capabilities encompass > parameters such as supported frequency and channel configurations. > > --v1: > - Refactor max_channel and max_rate to this commit as it is being > initialised here > - Remove call for intel_audio_compute_eld to avoid any regression while > merge. instead call it in different commit when it is defined. > - Use int instead of unsigned int for max_channel and max_frequecy > - Update commit message and header > > --v2: > - Use signed instead of unsigned variables. > - Avoid using magic numbers and give them proper name. > > --v3: > - Move defines to intel_audio.c. > - use consistent naming convention for rate and channel. > - declare num_of_channel and aud_rate separately. > - Declare index value outside of for loop. > - Move Bandwidth calculation to intel_Audio.c as it is common for both > DP and HDMI. Also use static. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/i915/display/intel_audio.c | 38 +++++++++++++++++++ > .../drm/i915/display/intel_display_types.h | 6 +++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c > index e20ffc8e9654..79377e33a59b 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio.c > +++ b/drivers/gpu/drm/i915/display/intel_audio.c > @@ -64,6 +64,9 @@ > * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver. > */ > > +#define AUDIO_SAMPLE_CONTAINER_SIZE 32 > +#define MAX_CHANNEL_COUNT 8 > + > struct intel_audio_funcs { > void (*audio_codec_enable)(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > @@ -770,6 +773,39 @@ void intel_audio_sdp_split_update(struct intel_encoder *encoder, > crtc_state->sdp_split_enable ? AUD_ENABLE_SDP_SPLIT : 0); > } > > +static int calc_audio_bw(int channel_count, int rate) > +{ > + int bandwidth = channel_count * rate * AUDIO_SAMPLE_CONTAINER_SIZE; > + return bandwidth; > +} > + > +static void calc_audio_config_params(struct intel_crtc_state *pipe_config) > +{ > + struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode; > + int channel_count; > + int index, rate[] = { 192000, 176000, 96000, 88000, 48000, 44100, 32000 }; > + int audio_req_bandwidth, available_blank_bandwidth, vblank, hblank; > + > + hblank = adjusted_mode->htotal - adjusted_mode->hdisplay; > + vblank = adjusted_mode->vtotal - adjusted_mode->vdisplay; > + available_blank_bandwidth = hblank * vblank * > + drm_mode_vrefresh(adjusted_mode) * pipe_config->pipe_bpp; > + for (channel_count = MAX_CHANNEL_COUNT; channel_count > 0; channel_count--) { > + for (index = 0; index < ARRAY_SIZE(rate); index++) { > + audio_req_bandwidth = calc_audio_bw(channel_count, > + rate[index]); > + if (audio_req_bandwidth < available_blank_bandwidth) { > + pipe_config->audio.max_rate = rate[index]; > + pipe_config->audio.max_channel_count = channel_count; > + return; > + } > + } > + } > + > + pipe_config->audio.max_rate = 0; > + pipe_config->audio.max_channel_count = 0; > +} > + > bool intel_audio_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > @@ -791,6 +827,8 @@ bool intel_audio_compute_config(struct intel_encoder *encoder, > > crtc_state->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2; > > + calc_audio_config_params(crtc_state); > + > return true; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index ebd147180a6e..8815837a95a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1131,6 +1131,12 @@ struct intel_crtc_state { > > struct { > bool has_audio; > + > + /* Audio rate in Hz */ > + int max_rate; > + > + /* Number of audio channels */ > + int max_channel_count; From what I can see you only calculate these when computing the ELD, and immediately use them there and nowhere else. So I see no reason to bloat the crtc_state with this. > } audio; > > /* > -- > 2.25.1
Hi Ville, > -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: 17 August 2023 18:34 > To: Golani, Mitulkumar Ajitkumar > <mitulkumar.ajitkumar.golani@intel.com> > Cc: intel-gfx@lists.freedesktop.org; jyri.sarha@linux.intel.com > Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and > initialize HDMI audio capabilities > > On Thu, Aug 17, 2023 at 06:20:06PM +0530, Mitul Golani wrote: > > Initialize the source audio capabilities in crtc_state property by > > setting them to their maximum supported values, including max_channel > > and max_frequency. This allows for the calculation of audio source > > capabilities with respect to the available mode bandwidth. These > > capabilities encompass parameters such as supported frequency and > > channel configurations. > > > > --v1: > > - Refactor max_channel and max_rate to this commit as it is being > > initialised here > > - Remove call for intel_audio_compute_eld to avoid any regression > > while merge. instead call it in different commit when it is defined. > > - Use int instead of unsigned int for max_channel and max_frequecy > > - Update commit message and header > > > > --v2: > > - Use signed instead of unsigned variables. > > - Avoid using magic numbers and give them proper name. > > > > --v3: > > - Move defines to intel_audio.c. > > - use consistent naming convention for rate and channel. > > - declare num_of_channel and aud_rate separately. > > - Declare index value outside of for loop. > > - Move Bandwidth calculation to intel_Audio.c as it is common for both > > DP and HDMI. Also use static. > > > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_audio.c | 38 +++++++++++++++++++ > > .../drm/i915/display/intel_display_types.h | 6 +++ > > 2 files changed, 44 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c > > b/drivers/gpu/drm/i915/display/intel_audio.c > > index e20ffc8e9654..79377e33a59b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_audio.c > > +++ b/drivers/gpu/drm/i915/display/intel_audio.c > > @@ -64,6 +64,9 @@ > > * struct &i915_audio_component_audio_ops @audio_ops is called from > i915 driver. > > */ > > > > +#define AUDIO_SAMPLE_CONTAINER_SIZE 32 > > +#define MAX_CHANNEL_COUNT 8 > > + > > struct intel_audio_funcs { > > void (*audio_codec_enable)(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state, > @@ -770,6 +773,39 > > @@ void intel_audio_sdp_split_update(struct intel_encoder *encoder, > > crtc_state->sdp_split_enable ? > AUD_ENABLE_SDP_SPLIT : 0); } > > > > +static int calc_audio_bw(int channel_count, int rate) { > > + int bandwidth = channel_count * rate * > AUDIO_SAMPLE_CONTAINER_SIZE; > > + return bandwidth; > > +} > > + > > +static void calc_audio_config_params(struct intel_crtc_state > > +*pipe_config) { > > + struct drm_display_mode *adjusted_mode = &pipe_config- > >hw.adjusted_mode; > > + int channel_count; > > + int index, rate[] = { 192000, 176000, 96000, 88000, 48000, 44100, > 32000 }; > > + int audio_req_bandwidth, available_blank_bandwidth, vblank, > hblank; > > + > > + hblank = adjusted_mode->htotal - adjusted_mode->hdisplay; > > + vblank = adjusted_mode->vtotal - adjusted_mode->vdisplay; > > + available_blank_bandwidth = hblank * vblank * > > + drm_mode_vrefresh(adjusted_mode) * > pipe_config->pipe_bpp; > > + for (channel_count = MAX_CHANNEL_COUNT; channel_count > 0; > channel_count--) { > > + for (index = 0; index < ARRAY_SIZE(rate); index++) { > > + audio_req_bandwidth = > calc_audio_bw(channel_count, > > + rate[index]); > > + if (audio_req_bandwidth < > available_blank_bandwidth) { > > + pipe_config->audio.max_rate = rate[index]; > > + pipe_config->audio.max_channel_count = > channel_count; > > + return; > > + } > > + } > > + } > > + > > + pipe_config->audio.max_rate = 0; > > + pipe_config->audio.max_channel_count = 0; } > > + > > bool intel_audio_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *crtc_state, > > struct drm_connector_state *conn_state) > @@ -791,6 +827,8 @@ bool > > intel_audio_compute_config(struct intel_encoder *encoder, > > > > crtc_state->eld[6] = drm_av_sync_delay(connector, adjusted_mode) > / > > 2; > > > > + calc_audio_config_params(crtc_state); > > + > > return true; > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index ebd147180a6e..8815837a95a6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1131,6 +1131,12 @@ struct intel_crtc_state { > > > > struct { > > bool has_audio; > > + > > + /* Audio rate in Hz */ > > + int max_rate; > > + > > + /* Number of audio channels */ > > + int max_channel_count; > > From what I can see you only calculate these when computing the ELD, and > immediately use them there and nowhere else. So I see no reason to bloat > the crtc_state with this. Thanks for the inputs, As mentioned in the cover letter, this initial implementation is geared towards GLK scenarios with channel and rate constraints, serving as a strategic solution. Additionally, it proves advantageous for addressing future scenarios involving bandwidth limitations that may lead to exceedance. https://gitlab.freedesktop.org/drm/intel/-/issues/5368 Regards, Mitul > > > } audio; > > > > /* > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index e20ffc8e9654..79377e33a59b 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -64,6 +64,9 @@ * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver. */ +#define AUDIO_SAMPLE_CONTAINER_SIZE 32 +#define MAX_CHANNEL_COUNT 8 + struct intel_audio_funcs { void (*audio_codec_enable)(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, @@ -770,6 +773,39 @@ void intel_audio_sdp_split_update(struct intel_encoder *encoder, crtc_state->sdp_split_enable ? AUD_ENABLE_SDP_SPLIT : 0); } +static int calc_audio_bw(int channel_count, int rate) +{ + int bandwidth = channel_count * rate * AUDIO_SAMPLE_CONTAINER_SIZE; + return bandwidth; +} + +static void calc_audio_config_params(struct intel_crtc_state *pipe_config) +{ + struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode; + int channel_count; + int index, rate[] = { 192000, 176000, 96000, 88000, 48000, 44100, 32000 }; + int audio_req_bandwidth, available_blank_bandwidth, vblank, hblank; + + hblank = adjusted_mode->htotal - adjusted_mode->hdisplay; + vblank = adjusted_mode->vtotal - adjusted_mode->vdisplay; + available_blank_bandwidth = hblank * vblank * + drm_mode_vrefresh(adjusted_mode) * pipe_config->pipe_bpp; + for (channel_count = MAX_CHANNEL_COUNT; channel_count > 0; channel_count--) { + for (index = 0; index < ARRAY_SIZE(rate); index++) { + audio_req_bandwidth = calc_audio_bw(channel_count, + rate[index]); + if (audio_req_bandwidth < available_blank_bandwidth) { + pipe_config->audio.max_rate = rate[index]; + pipe_config->audio.max_channel_count = channel_count; + return; + } + } + } + + pipe_config->audio.max_rate = 0; + pipe_config->audio.max_channel_count = 0; +} + bool intel_audio_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state) @@ -791,6 +827,8 @@ bool intel_audio_compute_config(struct intel_encoder *encoder, crtc_state->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2; + calc_audio_config_params(crtc_state); + return true; } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index ebd147180a6e..8815837a95a6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1131,6 +1131,12 @@ struct intel_crtc_state { struct { bool has_audio; + + /* Audio rate in Hz */ + int max_rate; + + /* Number of audio channels */ + int max_channel_count; } audio; /*
Initialize the source audio capabilities in crtc_state property by setting them to their maximum supported values, including max_channel and max_frequency. This allows for the calculation of audio source capabilities with respect to the available mode bandwidth. These capabilities encompass parameters such as supported frequency and channel configurations. --v1: - Refactor max_channel and max_rate to this commit as it is being initialised here - Remove call for intel_audio_compute_eld to avoid any regression while merge. instead call it in different commit when it is defined. - Use int instead of unsigned int for max_channel and max_frequecy - Update commit message and header --v2: - Use signed instead of unsigned variables. - Avoid using magic numbers and give them proper name. --v3: - Move defines to intel_audio.c. - use consistent naming convention for rate and channel. - declare num_of_channel and aud_rate separately. - Declare index value outside of for loop. - Move Bandwidth calculation to intel_Audio.c as it is common for both DP and HDMI. Also use static. Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> --- drivers/gpu/drm/i915/display/intel_audio.c | 38 +++++++++++++++++++ .../drm/i915/display/intel_display_types.h | 6 +++ 2 files changed, 44 insertions(+)