* [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels
@ 2023-08-17 12:50 Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add has_audio to separate audio parameter in crtc_state Mitul Golani
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Mitul Golani @ 2023-08-17 12:50 UTC (permalink / raw)
To: intel-gfx; +Cc: jyri.sarha
Currently we do not check if there is enough bandwidth for
audio, and what channels and freq it can really support.
Also sometimes there can be HW constraints e.g. GLK where audio
channels supported are only 2.
https://patchwork.freedesktop.org/series/107647/
Obtain the optimal audio rate and channel based on available display
timing constraints.
This can be achieved by:
- Retrieve the supported channel and rate information from SADs
- Adding audio-related config parameters in the CRTC state, such
as audio support, rate, and channel.
- Initializing the audio config parameters with the maximum supported
rate and channel by the audio source.
- Computing the SADs based on the audio source's capabilities.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Mitul Golani (3):
drm/i915: Add has_audio to separate audio parameter in crtc_state
drm/i915/display: Configure and initialize HDMI audio capabilities
drm/i915/display: Add wrapper to Compute SAD
drivers/gpu/drm/i915/display/g4x_dp.c | 4 +-
drivers/gpu/drm/i915/display/g4x_hdmi.c | 16 +-
drivers/gpu/drm/i915/display/intel_audio.c | 149 +++++++++++++++++-
drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +-
.../drm/i915/display/intel_crtc_state_dump.c | 4 +-
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/display/intel_display.c | 4 +-
.../drm/i915/display/intel_display_types.h | 12 +-
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 10 +-
12 files changed, 181 insertions(+), 32 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] [PATCH 1/3] drm/i915: Add has_audio to separate audio parameter in crtc_state
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
@ 2023-08-17 12:50 ` Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities Mitul Golani
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Mitul Golani @ 2023-08-17 12:50 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, jyri.sarha
To enhance the relationship between the has_audio and the source
audio parameter, create a separate crtc_state audio property and
add the has_audio parameter into it. Additionally, update the
access of the has_audio parameter from the crtc_state pointer as
it is wrapped under the audio. These modifications establish
a more cohesive structure and improve the accessibility and
organization of the audio-related parameters within the codebase.
--v1:
- add audio instead of audio_config in crtc_state
- add only has_audio then update related parameter access
- refactor other member to different commit where it is being used
- update commit message and header
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/g4x_dp.c | 4 ++--
drivers/gpu/drm/i915/display/g4x_hdmi.c | 16 ++++++++--------
drivers/gpu/drm/i915/display/intel_audio.c | 6 +++---
drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +++---
.../gpu/drm/i915/display/intel_crtc_state_dump.c | 4 ++--
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
.../gpu/drm/i915/display/intel_display_types.h | 6 +++---
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 10 +++++-----
12 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 4c7187f7913e..59e66cdc4553 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -345,7 +345,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
tmp = intel_de_read(dev_priv, intel_dp->output_reg);
- pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
+ pipe_config->audio.has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
u32 trans_dp = intel_de_read(dev_priv,
@@ -625,7 +625,7 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
* fail when the power sequencer is freshly used for this port.
*/
intel_dp->DP |= DP_PORT_EN;
- if (crtc_state->has_audio)
+ if (crtc_state->audio.has_audio)
intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
intel_de_write(dev_priv, intel_dp->output_reg, intel_dp->DP);
diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index 634b14116d9d..8a3b50a59ea8 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -178,7 +178,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
pipe_config->has_infoframe = true;
if (tmp & HDMI_AUDIO_ENABLE)
- pipe_config->has_audio = true;
+ pipe_config->audio.has_audio = true;
if (!HAS_PCH_SPLIT(dev_priv) &&
tmp & HDMI_COLOR_RANGE_16_235)
@@ -224,7 +224,7 @@ static void g4x_hdmi_enable_port(struct intel_encoder *encoder,
temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
temp |= SDVO_ENABLE;
- if (pipe_config->has_audio)
+ if (pipe_config->audio.has_audio)
temp |= HDMI_AUDIO_ENABLE;
intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
@@ -240,7 +240,7 @@ static void g4x_enable_hdmi(struct intel_atomic_state *state,
g4x_hdmi_enable_port(encoder, pipe_config);
- drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
+ drm_WARN_ON(&dev_priv->drm, pipe_config->audio.has_audio &&
!pipe_config->has_hdmi_sink);
intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
@@ -258,7 +258,7 @@ static void ibx_enable_hdmi(struct intel_atomic_state *state,
temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
temp |= SDVO_ENABLE;
- if (pipe_config->has_audio)
+ if (pipe_config->audio.has_audio)
temp |= HDMI_AUDIO_ENABLE;
/*
@@ -293,7 +293,7 @@ static void ibx_enable_hdmi(struct intel_atomic_state *state,
intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
}
- drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
+ drm_WARN_ON(&dev_priv->drm, pipe_config->audio.has_audio &&
!pipe_config->has_hdmi_sink);
intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
@@ -313,7 +313,7 @@ static void cpt_enable_hdmi(struct intel_atomic_state *state,
temp = intel_de_read(dev_priv, intel_hdmi->hdmi_reg);
temp |= SDVO_ENABLE;
- if (pipe_config->has_audio)
+ if (pipe_config->audio.has_audio)
temp |= HDMI_AUDIO_ENABLE;
/*
@@ -348,7 +348,7 @@ static void cpt_enable_hdmi(struct intel_atomic_state *state,
TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE, 0);
}
- drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
+ drm_WARN_ON(&dev_priv->drm, pipe_config->audio.has_audio &&
!pipe_config->has_hdmi_sink);
intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
@@ -360,7 +360,7 @@ static void vlv_enable_hdmi(struct intel_atomic_state *state,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
+ drm_WARN_ON(&dev_priv->drm, pipe_config->audio.has_audio &&
!pipe_config->has_hdmi_sink);
intel_audio_codec_enable(encoder, pipe_config, conn_state);
}
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 3d9c9b4f27f8..e20ffc8e9654 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -815,7 +815,7 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
struct intel_audio_state *audio_state;
enum port port = encoder->port;
- if (!crtc_state->has_audio)
+ if (!crtc_state->audio.has_audio)
return;
drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s] Enable audio codec on [CRTC:%d:%s], %u bytes ELD\n",
@@ -874,7 +874,7 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
struct intel_audio_state *audio_state;
enum port port = encoder->port;
- if (!old_crtc_state->has_audio)
+ if (!old_crtc_state->audio.has_audio)
return;
drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s] Disable audio codec on [CRTC:%d:%s]\n",
@@ -930,7 +930,7 @@ void intel_audio_codec_get_config(struct intel_encoder *encoder,
{
struct drm_i915_private *i915 = to_i915(encoder->base.dev);
- if (!crtc_state->has_audio)
+ if (!crtc_state->audio.has_audio)
return;
if (i915->display.funcs.audio)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 2fb030b1ff1d..fe67366c866c 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2554,7 +2554,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
* restriction for GLK is 316.8 MHz.
*/
if (intel_crtc_has_dp_encoder(crtc_state) &&
- crtc_state->has_audio &&
+ crtc_state->audio.has_audio &&
crtc_state->port_clock >= 540000 &&
crtc_state->lane_count == 4) {
if (DISPLAY_VER(dev_priv) == 10) {
@@ -2570,7 +2570,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
* According to BSpec, "The CD clock frequency must be at least twice
* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
*/
- if (crtc_state->has_audio && DISPLAY_VER(dev_priv) >= 9)
+ if (crtc_state->audio.has_audio && DISPLAY_VER(dev_priv) >= 9)
min_cdclk = max(2 * 96000, min_cdclk);
/*
@@ -2581,7 +2581,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
* 162 | 200 or higher"
*/
if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
- intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio)
+ intel_crtc_has_dp_encoder(crtc_state) && crtc_state->audio.has_audio)
min_cdclk = max(crtc_state->port_clock, min_cdclk);
/*
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 8d4640d0fd34..b7d1be42bf0f 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -265,7 +265,7 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
drm_dbg_kms(&i915->drm,
"audio: %i, infoframes: %i, infoframes enabled: 0x%x\n",
- pipe_config->has_audio, pipe_config->has_infoframe,
+ pipe_config->audio.has_audio, pipe_config->has_infoframe,
pipe_config->infoframes.enable);
if (pipe_config->infoframes.enable &
@@ -291,7 +291,7 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
intel_hdmi_infoframe_enable(DP_SDP_VSC))
intel_dump_dp_vsc_sdp(i915, &pipe_config->infoframes.vsc);
- if (pipe_config->has_audio)
+ if (pipe_config->audio.has_audio)
intel_dump_buffer(i915, "ELD: ", pipe_config->eld,
drm_eld_size(pipe_config->eld));
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 84bbf854337a..36d843fe2fd5 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3784,7 +3784,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
intel_ddi_mso_get_config(encoder, pipe_config);
- pipe_config->has_audio =
+ pipe_config->audio.has_audio =
intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
if (encoder->type == INTEL_OUTPUT_EDP)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 763ab569d8f3..b7d453a05eab 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1857,7 +1857,7 @@ static void get_crtc_power_domains(struct intel_crtc_state *crtc_state,
set_bit(intel_encoder->power_domain, mask->bits);
}
- if (HAS_DDI(dev_priv) && crtc_state->has_audio)
+ if (HAS_DDI(dev_priv) && crtc_state->audio.has_audio)
set_bit(POWER_DOMAIN_AUDIO_MMIO, mask->bits);
if (crtc_state->shared_dpll)
@@ -5257,7 +5257,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_BOOL(has_infoframe);
PIPE_CONF_CHECK_BOOL(fec_enable);
- PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
+ PIPE_CONF_CHECK_BOOL_INCOMPLETE(audio.has_audio);
PIPE_CONF_CHECK_BUFFER(eld, MAX_ELD_BYTES);
PIPE_CONF_CHECK_X(gmch_pfit.control);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 731f2ec04d5c..ebd147180a6e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1129,9 +1129,9 @@ 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;
/*
* Enable dithering, used when the selected pipe bpp doesn't match the
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 964bf0551bdc..4fd0298918ac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2264,7 +2264,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && encoder->port != PORT_A)
pipe_config->has_pch_encoder = true;
- pipe_config->has_audio =
+ pipe_config->audio.has_audio =
intel_dp_has_audio(encoder, conn_state) &&
intel_audio_compute_config(encoder, pipe_config, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index e3f176a093d2..f6e1bf3d9e25 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -322,7 +322,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
pipe_config->has_pch_encoder = false;
- pipe_config->has_audio =
+ pipe_config->audio.has_audio =
intel_dp_mst_has_audio(conn_state) &&
intel_audio_compute_config(encoder, pipe_config, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9442bf43550e..2ff232d9f68a 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2302,7 +2302,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
pipe_config->pixel_multiplier = 2;
- pipe_config->has_audio =
+ pipe_config->audio.has_audio =
intel_hdmi_has_audio(encoder, pipe_config, conn_state) &&
intel_audio_compute_config(encoder, pipe_config, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 7d25a64698e2..73321b54cf60 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -1191,7 +1191,7 @@ static void intel_sdvo_get_eld(struct intel_sdvo *intel_sdvo,
ssize_t len;
u8 val;
- if (!crtc_state->has_audio)
+ if (!crtc_state->audio.has_audio)
return;
if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT, &val, 1))
@@ -1406,7 +1406,7 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
pipe_config->has_hdmi_sink = intel_has_hdmi_sink(intel_sdvo_connector, conn_state);
- pipe_config->has_audio =
+ pipe_config->audio.has_audio =
intel_sdvo_has_audio(encoder, pipe_config, conn_state) &&
intel_audio_compute_config(encoder, pipe_config, conn_state);
@@ -1760,7 +1760,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
&val, 1)) {
if (val & SDVO_AUDIO_PRESENCE_DETECT)
- pipe_config->has_audio = true;
+ pipe_config->audio.has_audio = true;
}
if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
@@ -1805,7 +1805,7 @@ static void intel_disable_sdvo(struct intel_atomic_state *state,
struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
u32 temp;
- if (old_crtc_state->has_audio)
+ if (old_crtc_state->audio.has_audio)
intel_sdvo_disable_audio(intel_sdvo);
intel_sdvo_set_active_outputs(intel_sdvo, 0);
@@ -1898,7 +1898,7 @@ static void intel_enable_sdvo(struct intel_atomic_state *state,
DRM_MODE_DPMS_ON);
intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
- if (pipe_config->has_audio)
+ if (pipe_config->audio.has_audio)
intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add has_audio to separate audio parameter in crtc_state Mitul Golani
@ 2023-08-17 12:50 ` Mitul Golani
2023-08-17 13:04 ` Ville Syrjälä
2023-08-17 12:50 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD Mitul Golani
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Mitul Golani @ 2023-08-17 12:50 UTC (permalink / raw)
To: intel-gfx; +Cc: jyri.sarha
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;
} audio;
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add has_audio to separate audio parameter in crtc_state Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities Mitul Golani
@ 2023-08-17 12:50 ` Mitul Golani
2023-08-17 15:24 ` kernel test robot
2023-08-18 8:38 ` Jani Nikula
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Get optimal audio frequency and channels (rev9) Patchwork
` (4 subsequent siblings)
7 siblings, 2 replies; 14+ messages in thread
From: Mitul Golani @ 2023-08-17 12:50 UTC (permalink / raw)
To: intel-gfx; +Cc: jyri.sarha
Compute SADs that takes into account the supported rate
and channel based on the capabilities of the audio source.
This wrapper function should encapsulate the logic for
determining the supported rate and channel and should
return a set of SADs that are compatible with the source.
--v1:
- call intel_audio_compute_eld in this commit as it is
defined here
--v2:
- Handle case when max frequency is less than 32k.
- remove drm prefix.
- name change for parse_sad to eld_to_sad.
--v3:
- Use signed int wherever required.
- add debug trace when channel is limited.
--v4:
- remove inline from eld_to_sad.
- declare index outside of for loop with int type.
- Correct mask value calculation.
- remove drm_err, instead just return if eld parsing failed.
- remove unncessary typecast
- reduce indentation while parsing sad
- use intel_audio_compute_eld as static and call bandwidth
calculation just before that.
--v9:
- Handling the case when, sink supported channel is less
than max supported. In that case, rate needs to be calibrate
in accordance with available bandwidth.
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
drivers/gpu/drm/i915/display/intel_audio.c | 107 ++++++++++++++++++++-
1 file changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 79377e33a59b..c90ac2608eef 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -806,6 +806,111 @@ static void calc_audio_config_params(struct intel_crtc_state *pipe_config)
pipe_config->audio.max_channel_count = 0;
}
+static void calibrate_audio_config_params(struct intel_crtc_state *pipe_config, int channel)
+{
+ 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 (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;
+}
+
+static int sad_to_channels(const u8 *sad)
+{
+ return 1 + (sad[0] & 0x7);
+}
+
+static u8 *eld_to_sad(u8 *eld)
+{
+ int ver, mnl;
+
+ ver = (eld[DRM_ELD_VER] & DRM_ELD_VER_MASK) >> DRM_ELD_VER_SHIFT;
+ if (ver != 2 && ver != 31)
+ return NULL;
+
+ mnl = drm_eld_mnl(eld);
+ if (mnl > 16)
+ return NULL;
+
+ return eld + DRM_ELD_CEA_SAD(mnl, 0);
+}
+
+static int get_supported_freq_mask(struct intel_crtc_state *crtc_state)
+{
+ int rate[] = { 32000, 44100, 48000, 88000, 96000, 176000, 192000 };
+ int mask = 0, index;
+
+ for (index = 0; index < ARRAY_SIZE(rate); index++) {
+ if (rate[index] > crtc_state->audio.max_rate)
+ break;
+
+ mask |= 1 << index;
+
+ if (crtc_state->audio.max_rate != rate[index])
+ continue;
+ }
+
+ return mask;
+}
+
+static void intel_audio_compute_eld(struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ u8 *eld, *sad;
+ int index, mask = 0;
+
+ eld = crtc_state->eld;
+ if (!eld)
+ return;
+
+ sad = eld_to_sad(eld);
+ if (!sad)
+ return;
+
+ calc_audio_config_params(crtc_state);
+
+ mask = get_supported_freq_mask(crtc_state);
+ for (index = 0; index < drm_eld_sad_count(eld); index++, sad += 3) {
+ /*
+ * Respect source restricitions. Limit capabilities to a subset that is
+ * supported both by the source and the sink.
+ */
+ if (sad_to_channels(sad) >= crtc_state->audio.max_channel_count) {
+ sad[0] &= ~0x7;
+ sad[0] |= crtc_state->audio.max_channel_count - 1;
+ drm_dbg_kms(&i915->drm, "Channel count is limited to %d\n",
+ crtc_state->audio.max_channel_count - 1);
+ } else {
+ /*
+ * calibrate rate when, sink supported channel
+ * count is slight less than max supported
+ * channel count.
+ */
+ calibrate_audio_config_params(crtc_state, sad_to_channels(sad));
+ mask = get_supported_freq_mask(crtc_state);
+ }
+
+ sad[1] &= mask;
+ }
+}
+
bool intel_audio_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
@@ -827,7 +932,7 @@ 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);
+ intel_audio_compute_eld(crtc_state);
return true;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities
2023-08-17 12:50 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities Mitul Golani
@ 2023-08-17 13:04 ` Ville Syrjälä
2023-08-17 13:50 ` Golani, Mitulkumar Ajitkumar
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2023-08-17 13:04 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx, jyri.sarha
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
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities
2023-08-17 13:04 ` Ville Syrjälä
@ 2023-08-17 13:50 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 14+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-08-17 13:50 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx@lists.freedesktop.org, jyri.sarha@linux.intel.com
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Get optimal audio frequency and channels (rev9)
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
` (2 preceding siblings ...)
2023-08-17 12:50 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD Mitul Golani
@ 2023-08-17 14:46 ` Patchwork
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-08-17 14:46 UTC (permalink / raw)
To: Golani, Mitulkumar Ajitkumar; +Cc: intel-gfx
== Series Details ==
Series: Get optimal audio frequency and channels (rev9)
URL : https://patchwork.freedesktop.org/series/119121/
State : warning
== Summary ==
Error: dim checkpatch failed
/home/kbuild2/linux/maintainer-tools/dim: line 50: /home/kbuild2/.dimrc: No such file or directory
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Get optimal audio frequency and channels (rev9)
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
` (3 preceding siblings ...)
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Get optimal audio frequency and channels (rev9) Patchwork
@ 2023-08-17 14:46 ` Patchwork
2023-08-17 15:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-08-17 14:46 UTC (permalink / raw)
To: Golani, Mitulkumar Ajitkumar; +Cc: intel-gfx
== Series Details ==
Series: Get optimal audio frequency and channels (rev9)
URL : https://patchwork.freedesktop.org/series/119121/
State : warning
== Summary ==
Error: dim sparse failed
/home/kbuild/linux/maintainer-tools/dim: line 50: /home/kbuild/.dimrc: No such file or directory
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for Get optimal audio frequency and channels (rev9)
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
` (4 preceding siblings ...)
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-08-17 15:03 ` Patchwork
2023-08-18 15:24 ` [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Kai Vehmanen
2023-08-22 6:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Get optimal audio frequency and channels (rev9) Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-08-17 15:03 UTC (permalink / raw)
To: Golani, Mitulkumar Ajitkumar; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7870 bytes --]
== Series Details ==
Series: Get optimal audio frequency and channels (rev9)
URL : https://patchwork.freedesktop.org/series/119121/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13530 -> Patchwork_119121v9
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_119121v9 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_119121v9, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/index.html
Participating hosts (39 -> 39)
------------------------------
Additional (1): fi-kbl-soraka
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_119121v9:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@objects:
- fi-ilk-650: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/fi-ilk-650/igt@i915_selftest@live@objects.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-ilk-650/igt@i915_selftest@live@objects.html
Known issues
------------
Here are the changes found in Patchwork_119121v9 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][5] ([i915#4613]) +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@gem_lmem_swapping@verify-random.html
* igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][6] ([i915#6621])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][7] ([i915#1886] / [i915#7913])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-adlm-1: NOTRUN -> [INCOMPLETE][8] ([i915#7443])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-adlm-1/igt@i915_suspend@basic-s3-without-i915.html
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#6645])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-kbl-soraka: NOTRUN -> [SKIP][10] ([fdo#109271]) +8 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_psr@primary_mmap_gtt:
- bat-rplp-1: NOTRUN -> [ABORT][11] ([i915#8442] / [i915#8469] / [i915#8668])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html
* igt@prime_vgem@basic-fence-mmap:
- bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#3708] / [i915#4077]) +1 similar issue
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@prime_vgem@basic-fence-mmap.html
* igt@prime_vgem@basic-fence-read:
- bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#3708]) +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@prime_vgem@basic-fence-read.html
#### Possible fixes ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- bat-mtlp-8: [ABORT][14] ([i915#7977] / [i915#8668]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_selftest@live@gt_lrc:
- bat-adlm-1: [INCOMPLETE][16] ([i915#4983]) -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-adlm-1/igt@i915_selftest@live@gt_lrc.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-adlm-1/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@migrate:
- bat-dg2-11: [DMESG-WARN][18] ([i915#7699]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-dg2-11/igt@i915_selftest@live@migrate.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-dg2-11/igt@i915_selftest@live@migrate.html
#### Warnings ####
* igt@kms_psr@sprite_plane_onoff:
- bat-rplp-1: [ABORT][20] ([i915#8442] / [i915#8668] / [i915#8712]) -> [SKIP][21] ([i915#1072])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[Intel XE#486]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/486
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7977]: https://gitlab.freedesktop.org/drm/intel/issues/7977
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
[i915#8469]: https://gitlab.freedesktop.org/drm/intel/issues/8469
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
[i915#8712]: https://gitlab.freedesktop.org/drm/intel/issues/8712
[i915#8879]: https://gitlab.freedesktop.org/drm/intel/issues/8879
Build changes
-------------
* Linux: CI_DRM_13530 -> Patchwork_119121v9
CI-20190529: 20190529
CI_DRM_13530: d97cc063202bec7acb2e45909f45ebdecb44ff41 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7442: f6eea5a02525d62d10596408d1175c83d6f4038e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_119121v9: d97cc063202bec7acb2e45909f45ebdecb44ff41 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
b4eae9c6b086 drm/i915/display: Add wrapper to Compute SAD
c93e25e4cc51 drm/i915/display: Configure and initialize HDMI audio capabilities
14706fa40af7 drm/i915: Add has_audio to separate audio parameter in crtc_state
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/index.html
[-- Attachment #2: Type: text/html, Size: 9144 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD
2023-08-17 12:50 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD Mitul Golani
@ 2023-08-17 15:24 ` kernel test robot
2023-08-18 8:38 ` Jani Nikula
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-08-17 15:24 UTC (permalink / raw)
To: Mitul Golani, intel-gfx; +Cc: jyri.sarha, llvm, oe-kbuild-all
Hi Mitul,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-i915-Add-has_audio-to-separate-audio-parameter-in-crtc_state/20230817-205556
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/20230817125007.2681331-4-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD
config: x86_64-buildonly-randconfig-r001-20230817 (https://download.01.org/0day-ci/archive/20230817/202308172319.rTgU3hnc-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230817/202308172319.rTgU3hnc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308172319.rTgU3hnc-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_audio.c:822:39: warning: variable 'channel_count' is uninitialized when used here [-Wuninitialized]
audio_req_bandwidth = calc_audio_bw(channel_count,
^~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_audio.c:812:19: note: initialize the variable 'channel_count' to silence this warning
int channel_count;
^
= 0
1 warning generated.
vim +/channel_count +822 drivers/gpu/drm/i915/display/intel_audio.c
808
809 static void calibrate_audio_config_params(struct intel_crtc_state *pipe_config, int channel)
810 {
811 struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
812 int channel_count;
813 int index, rate[] = { 192000, 176000, 96000, 88000, 48000, 44100, 32000 };
814 int audio_req_bandwidth, available_blank_bandwidth, vblank, hblank;
815
816 hblank = adjusted_mode->htotal - adjusted_mode->hdisplay;
817 vblank = adjusted_mode->vtotal - adjusted_mode->vdisplay;
818 available_blank_bandwidth = hblank * vblank *
819 drm_mode_vrefresh(adjusted_mode) * pipe_config->pipe_bpp;
820
821 for (index = 0; index < ARRAY_SIZE(rate); index++) {
> 822 audio_req_bandwidth = calc_audio_bw(channel_count,
823 rate[index]);
824 if (audio_req_bandwidth < available_blank_bandwidth) {
825 pipe_config->audio.max_rate = rate[index];
826 pipe_config->audio.max_channel_count = channel_count;
827 return;
828 }
829 }
830
831 pipe_config->audio.max_rate = 0;
832 pipe_config->audio.max_channel_count = 0;
833 }
834
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD
2023-08-17 12:50 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD Mitul Golani
2023-08-17 15:24 ` kernel test robot
@ 2023-08-18 8:38 ` Jani Nikula
2023-08-21 16:10 ` Golani, Mitulkumar Ajitkumar
1 sibling, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2023-08-18 8:38 UTC (permalink / raw)
To: Mitul Golani, intel-gfx; +Cc: jyri.sarha
On Thu, 17 Aug 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Compute SADs that takes into account the supported rate
> and channel based on the capabilities of the audio source.
> This wrapper function should encapsulate the logic for
> determining the supported rate and channel and should
> return a set of SADs that are compatible with the source.
>
> --v1:
> - call intel_audio_compute_eld in this commit as it is
> defined here
>
> --v2:
> - Handle case when max frequency is less than 32k.
> - remove drm prefix.
> - name change for parse_sad to eld_to_sad.
>
> --v3:
> - Use signed int wherever required.
> - add debug trace when channel is limited.
>
> --v4:
> - remove inline from eld_to_sad.
> - declare index outside of for loop with int type.
> - Correct mask value calculation.
> - remove drm_err, instead just return if eld parsing failed.
> - remove unncessary typecast
> - reduce indentation while parsing sad
> - use intel_audio_compute_eld as static and call bandwidth
> calculation just before that.
>
> --v9:
> - Handling the case when, sink supported channel is less
> than max supported. In that case, rate needs to be calibrate
> in accordance with available bandwidth.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_audio.c | 107 ++++++++++++++++++++-
> 1 file changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 79377e33a59b..c90ac2608eef 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -806,6 +806,111 @@ static void calc_audio_config_params(struct intel_crtc_state *pipe_config)
> pipe_config->audio.max_channel_count = 0;
> }
>
This was added in previous patch:
+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;
+}
And this here:
> +static void calibrate_audio_config_params(struct intel_crtc_state *pipe_config, int channel)
> +{
> + 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 (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;
> +}
This kind of stuff needs to be deduplicated.
> +
> +static int sad_to_channels(const u8 *sad)
> +{
> + return 1 + (sad[0] & 0x7);
> +}
> +
> +static u8 *eld_to_sad(u8 *eld)
> +{
> + int ver, mnl;
> +
> + ver = (eld[DRM_ELD_VER] & DRM_ELD_VER_MASK) >> DRM_ELD_VER_SHIFT;
> + if (ver != 2 && ver != 31)
> + return NULL;
> +
> + mnl = drm_eld_mnl(eld);
> + if (mnl > 16)
> + return NULL;
> +
> + return eld + DRM_ELD_CEA_SAD(mnl, 0);
> +}
I'm still not happy with the copy-paste here. The parsing details should
be hidden in drm_edid.c and that should be the Single Point of Truth how
to deal with this.
> +
> +static int get_supported_freq_mask(struct intel_crtc_state *crtc_state)
> +{
> + int rate[] = { 32000, 44100, 48000, 88000, 96000, 176000, 192000 };
This is like the third copy of the same array already... and the values
are also present in the existing dp/hdmi aud arrays in intel_audio.c.
And those have 88200 instead of 88000, so which one is correct?
> + int mask = 0, index;
> +
> + for (index = 0; index < ARRAY_SIZE(rate); index++) {
> + if (rate[index] > crtc_state->audio.max_rate)
> + break;
> +
> + mask |= 1 << index;
> +
> + if (crtc_state->audio.max_rate != rate[index])
> + continue;
> + }
> +
> + return mask;
> +}
> +
> +static void intel_audio_compute_eld(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> + u8 *eld, *sad;
> + int index, mask = 0;
> +
> + eld = crtc_state->eld;
> + if (!eld)
> + return;
> +
> + sad = eld_to_sad(eld);
> + if (!sad)
> + return;
> +
> + calc_audio_config_params(crtc_state);
> +
> + mask = get_supported_freq_mask(crtc_state);
> + for (index = 0; index < drm_eld_sad_count(eld); index++, sad += 3) {
> + /*
> + * Respect source restricitions. Limit capabilities to a subset that is
> + * supported both by the source and the sink.
> + */
> + if (sad_to_channels(sad) >= crtc_state->audio.max_channel_count) {
> + sad[0] &= ~0x7;
Also not happy with magic values here.
> + sad[0] |= crtc_state->audio.max_channel_count - 1;
> + drm_dbg_kms(&i915->drm, "Channel count is limited to %d\n",
> + crtc_state->audio.max_channel_count - 1);
> + } else {
> + /*
> + * calibrate rate when, sink supported channel
> + * count is slight less than max supported
> + * channel count.
> + */
> + calibrate_audio_config_params(crtc_state, sad_to_channels(sad));
> + mask = get_supported_freq_mask(crtc_state);
> + }
> +
> + sad[1] &= mask;
> + }
> +}
> +
> bool intel_audio_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> @@ -827,7 +932,7 @@ 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);
> + intel_audio_compute_eld(crtc_state);
>
> return true;
> }
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
` (5 preceding siblings ...)
2023-08-17 15:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2023-08-18 15:24 ` Kai Vehmanen
2023-08-22 6:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Get optimal audio frequency and channels (rev9) Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Kai Vehmanen @ 2023-08-18 15:24 UTC (permalink / raw)
To: Mitul Golani; +Cc: intel-gfx, jyri.sarha
Hi,
On Thu, 17 Aug 2023, Mitul Golani wrote:
> Currently we do not check if there is enough bandwidth for
> audio, and what channels and freq it can really support.
> Also sometimes there can be HW constraints e.g. GLK where audio
> channels supported are only 2.
[...]
> Mitul Golani (3):
> drm/i915: Add has_audio to separate audio parameter in crtc_state
> drm/i915/display: Configure and initialize HDMI audio capabilities
> drm/i915/display: Add wrapper to Compute SAD
thanks, looks good now. The adjusted logic to balance between
channels and rates seems to hit a fair balance. For the series:
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Br, Kai
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD
2023-08-18 8:38 ` Jani Nikula
@ 2023-08-21 16:10 ` Golani, Mitulkumar Ajitkumar
0 siblings, 0 replies; 14+ messages in thread
From: Golani, Mitulkumar Ajitkumar @ 2023-08-21 16:10 UTC (permalink / raw)
To: Jani Nikula, intel-gfx@lists.freedesktop.org; +Cc: jyri.sarha@linux.intel.com
Hi @Jani Nikula
Thanks for reviewing patch series.
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 18 August 2023 14:08
> To: Golani, Mitulkumar Ajitkumar
> <mitulkumar.ajitkumar.golani@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: jyri.sarha@linux.intel.com
> Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to
> Compute SAD
>
> On Thu, 17 Aug 2023, Mitul Golani
> <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Compute SADs that takes into account the supported rate and channel
> > based on the capabilities of the audio source.
> > This wrapper function should encapsulate the logic for determining the
> > supported rate and channel and should return a set of SADs that are
> > compatible with the source.
> >
> > --v1:
> > - call intel_audio_compute_eld in this commit as it is defined here
> >
> > --v2:
> > - Handle case when max frequency is less than 32k.
> > - remove drm prefix.
> > - name change for parse_sad to eld_to_sad.
> >
> > --v3:
> > - Use signed int wherever required.
> > - add debug trace when channel is limited.
> >
> > --v4:
> > - remove inline from eld_to_sad.
> > - declare index outside of for loop with int type.
> > - Correct mask value calculation.
> > - remove drm_err, instead just return if eld parsing failed.
> > - remove unncessary typecast
> > - reduce indentation while parsing sad
> > - use intel_audio_compute_eld as static and call bandwidth calculation
> > just before that.
> >
> > --v9:
> > - Handling the case when, sink supported channel is less than max
> > supported. In that case, rate needs to be calibrate in accordance with
> > available bandwidth.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_audio.c | 107
> > ++++++++++++++++++++-
> > 1 file changed, 106 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> > b/drivers/gpu/drm/i915/display/intel_audio.c
> > index 79377e33a59b..c90ac2608eef 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > @@ -806,6 +806,111 @@ static void calc_audio_config_params(struct
> intel_crtc_state *pipe_config)
> > pipe_config->audio.max_channel_count = 0; }
> >
>
> This was added in previous patch:
>
> +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;
> +}
>
> And this here:
Thanks I have removed duplicated code for calibration and calculation,
and merged them to one to avoid redundancy.
>
> > +static void calibrate_audio_config_params(struct intel_crtc_state
> > +*pipe_config, int channel) {
> > + 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 (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; }
>
> This kind of stuff needs to be deduplicated.
>
> > +
> > +static int sad_to_channels(const u8 *sad) {
> > + return 1 + (sad[0] & 0x7);
> > +}
> > +
> > +static u8 *eld_to_sad(u8 *eld)
> > +{
> > + int ver, mnl;
> > +
> > + ver = (eld[DRM_ELD_VER] & DRM_ELD_VER_MASK) >>
> DRM_ELD_VER_SHIFT;
> > + if (ver != 2 && ver != 31)
> > + return NULL;
> > +
> > + mnl = drm_eld_mnl(eld);
> > + if (mnl > 16)
> > + return NULL;
> > +
> > + return eld + DRM_ELD_CEA_SAD(mnl, 0); }
>
> I'm still not happy with the copy-paste here. The parsing details should be
> hidden in drm_edid.c and that should be the Single Point of Truth how to
> deal with this.
>
Thanks, I have added wrapper function adhering to
const correctness while accessing SAD information. Please suggest
if any other approach we can try.
> > +
> > +static int get_supported_freq_mask(struct intel_crtc_state
> > +*crtc_state) {
> > + int rate[] = { 32000, 44100, 48000, 88000, 96000, 176000, 192000 };
>
> This is like the third copy of the same array already... and the values are
> also present in the existing dp/hdmi aud arrays in intel_audio.c.
>
> And those have 88200 instead of 88000, so which one is correct?
This was mistake while validating previous patches. I have corrected
the same with reference to HDMI spec.
>
> > + int mask = 0, index;
> > +
> > + for (index = 0; index < ARRAY_SIZE(rate); index++) {
> > + if (rate[index] > crtc_state->audio.max_rate)
> > + break;
> > +
> > + mask |= 1 << index;
> > +
> > + if (crtc_state->audio.max_rate != rate[index])
> > + continue;
> > + }
> > +
> > + return mask;
> > +}
> > +
> > +static void intel_audio_compute_eld(struct intel_crtc_state
> > +*crtc_state) {
> > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> > + u8 *eld, *sad;
> > + int index, mask = 0;
> > +
> > + eld = crtc_state->eld;
> > + if (!eld)
> > + return;
> > +
> > + sad = eld_to_sad(eld);
> > + if (!sad)
> > + return;
> > +
> > + calc_audio_config_params(crtc_state);
> > +
> > + mask = get_supported_freq_mask(crtc_state);
> > + for (index = 0; index < drm_eld_sad_count(eld); index++, sad += 3) {
> > + /*
> > + * Respect source restricitions. Limit capabilities to a subset
> that is
> > + * supported both by the source and the sink.
> > + */
> > + if (sad_to_channels(sad) >= crtc_state-
> >audio.max_channel_count) {
> > + sad[0] &= ~0x7;
>
> Also not happy with magic values here.
Thanks, defined the magic value.
Regards,
Mitul
>
> > + sad[0] |= crtc_state->audio.max_channel_count - 1;
> > + drm_dbg_kms(&i915->drm, "Channel count is
> limited to %d\n",
> > + crtc_state->audio.max_channel_count - 1);
> > + } else {
> > + /*
> > + * calibrate rate when, sink supported channel
> > + * count is slight less than max supported
> > + * channel count.
> > + */
> > + calibrate_audio_config_params(crtc_state,
> sad_to_channels(sad));
> > + mask = get_supported_freq_mask(crtc_state);
> > + }
> > +
> > + sad[1] &= mask;
> > + }
> > +}
> > +
> > bool intel_audio_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *crtc_state,
> > struct drm_connector_state *conn_state)
> @@ -827,7 +932,7 @@ 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);
> > + intel_audio_compute_eld(crtc_state);
> >
> > return true;
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Get optimal audio frequency and channels (rev9)
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
` (6 preceding siblings ...)
2023-08-18 15:24 ` [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Kai Vehmanen
@ 2023-08-22 6:38 ` Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2023-08-22 6:38 UTC (permalink / raw)
To: Golani, Mitulkumar Ajitkumar; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7463 bytes --]
== Series Details ==
Series: Get optimal audio frequency and channels (rev9)
URL : https://patchwork.freedesktop.org/series/119121/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13530 -> Patchwork_119121v9
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/index.html
Participating hosts (39 -> 39)
------------------------------
Additional (1): fi-kbl-soraka
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_119121v9 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613]) +3 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#4613]) +3 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@gem_lmem_swapping@verify-random.html
* igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][4] ([i915#6621])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][5] ([i915#1886] / [i915#7913])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@objects:
- fi-ilk-650: [PASS][6] -> [INCOMPLETE][7] ([i915#9167])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/fi-ilk-650/igt@i915_selftest@live@objects.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-ilk-650/igt@i915_selftest@live@objects.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-adlm-1: NOTRUN -> [INCOMPLETE][8] ([i915#7443])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-adlm-1/igt@i915_suspend@basic-s3-without-i915.html
- bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#6645])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-kbl-soraka: NOTRUN -> [SKIP][10] ([fdo#109271]) +8 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_psr@primary_mmap_gtt:
- bat-rplp-1: NOTRUN -> [ABORT][11] ([i915#8442] / [i915#8469] / [i915#8668])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html
* igt@prime_vgem@basic-fence-mmap:
- bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#3708] / [i915#4077]) +1 similar issue
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@prime_vgem@basic-fence-mmap.html
* igt@prime_vgem@basic-fence-read:
- bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#3708]) +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@prime_vgem@basic-fence-read.html
#### Possible fixes ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- bat-mtlp-8: [ABORT][14] ([i915#7977] / [i915#8668]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_selftest@live@gt_lrc:
- bat-adlm-1: [INCOMPLETE][16] ([i915#4983]) -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-adlm-1/igt@i915_selftest@live@gt_lrc.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-adlm-1/igt@i915_selftest@live@gt_lrc.html
* igt@i915_selftest@live@migrate:
- bat-dg2-11: [DMESG-WARN][18] ([i915#7699]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-dg2-11/igt@i915_selftest@live@migrate.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-dg2-11/igt@i915_selftest@live@migrate.html
#### Warnings ####
* igt@kms_psr@sprite_plane_onoff:
- bat-rplp-1: [ABORT][20] ([i915#8442] / [i915#8668] / [i915#8712]) -> [SKIP][21] ([i915#1072])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13530/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[Intel XE#486]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/486
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7977]: https://gitlab.freedesktop.org/drm/intel/issues/7977
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
[i915#8469]: https://gitlab.freedesktop.org/drm/intel/issues/8469
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
[i915#8712]: https://gitlab.freedesktop.org/drm/intel/issues/8712
[i915#8879]: https://gitlab.freedesktop.org/drm/intel/issues/8879
[i915#9167]: https://gitlab.freedesktop.org/drm/intel/issues/9167
Build changes
-------------
* Linux: CI_DRM_13530 -> Patchwork_119121v9
CI-20190529: 20190529
CI_DRM_13530: d97cc063202bec7acb2e45909f45ebdecb44ff41 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7442: f6eea5a02525d62d10596408d1175c83d6f4038e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_119121v9: d97cc063202bec7acb2e45909f45ebdecb44ff41 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
b4eae9c6b086 drm/i915/display: Add wrapper to Compute SAD
c93e25e4cc51 drm/i915/display: Configure and initialize HDMI audio capabilities
14706fa40af7 drm/i915: Add has_audio to separate audio parameter in crtc_state
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119121v9/index.html
[-- Attachment #2: Type: text/html, Size: 8725 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-08-22 6:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-17 12:50 [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add has_audio to separate audio parameter in crtc_state Mitul Golani
2023-08-17 12:50 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities Mitul Golani
2023-08-17 13:04 ` Ville Syrjälä
2023-08-17 13:50 ` Golani, Mitulkumar Ajitkumar
2023-08-17 12:50 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Add wrapper to Compute SAD Mitul Golani
2023-08-17 15:24 ` kernel test robot
2023-08-18 8:38 ` Jani Nikula
2023-08-21 16:10 ` Golani, Mitulkumar Ajitkumar
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Get optimal audio frequency and channels (rev9) Patchwork
2023-08-17 14:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-17 15:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-08-18 15:24 ` [Intel-gfx] [PATCH 0/3] Get optimal audio frequency and channels Kai Vehmanen
2023-08-22 6:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Get optimal audio frequency and channels (rev9) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox