On Thu, Feb 23, 2023 at 03:19:30PM +0530, Swati Sharma wrote: > Hi Ville, > > Please add closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8222 Added, and pushed. Thanks for the review Uma. > > On 23-Feb-23 1:48 PM, Shankar, Uma wrote: > > > > > >> -----Original Message----- > >> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville Syrjala > >> Sent: Wednesday, February 22, 2023 8:45 PM > >> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> Subject: [PATCH] drm/i915/audio: Track audio state per-transcoder > >> > >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> > >> The audio logic lives in the transcoder rather than the pipe, so start tracking it like > >> that. > >> > >> This is only really important for bigjoiner cases where tracking by pipe doesn't work > >> at all since intel_audio_codec_{enable,disable}() > >> won't even be called for the slave pipe. This means the state checker won't find the > >> ELD for the slave pipe and gets upset. > >> The PD->has_audio readout does currently work since that gets read out from the > >> same transcoder for both pipes. > >> > >> For other cases this doesn't actually matter since it's only the normal pipe > >> transcoders that are audio capable, whereas the more special transcoders (EDP/DSI) > >> are not. > > > > Change Looks Good to me. > > Reviewed-by: Uma Shankar <uma.shankar@xxxxxxxxx> > > > >> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/display/intel_audio.c | 86 +++++++++---------- > >> .../gpu/drm/i915/display/intel_display_core.h | 2 +- > >> .../gpu/drm/i915/display/intel_lpe_audio.c | 6 +- > >> .../gpu/drm/i915/display/intel_lpe_audio.h | 4 +- > >> 4 files changed, 48 insertions(+), 50 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c > >> b/drivers/gpu/drm/i915/display/intel_audio.c > >> index a9335c856644..65151f5dcb15 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_audio.c > >> +++ b/drivers/gpu/drm/i915/display/intel_audio.c > >> @@ -581,8 +581,7 @@ static void enable_audio_dsc_wa(struct intel_encoder > >> *encoder, > >> const struct intel_crtc_state *crtc_state) { > >> struct drm_i915_private *i915 = to_i915(encoder->base.dev); > >> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> - enum pipe pipe = crtc->pipe; > >> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >> unsigned int hblank_early_prog, samples_room; > >> unsigned int val; > >> > >> @@ -592,32 +591,32 @@ static void enable_audio_dsc_wa(struct intel_encoder > >> *encoder, > >> val = intel_de_read(i915, AUD_CONFIG_BE); > >> > >> if (DISPLAY_VER(i915) == 11) > >> - val |= HBLANK_EARLY_ENABLE_ICL(pipe); > >> + val |= HBLANK_EARLY_ENABLE_ICL(cpu_transcoder); > >> else if (DISPLAY_VER(i915) >= 12) > >> - val |= HBLANK_EARLY_ENABLE_TGL(pipe); > >> + val |= HBLANK_EARLY_ENABLE_TGL(cpu_transcoder); > >> > >> if (crtc_state->dsc.compression_enable && > >> crtc_state->hw.adjusted_mode.hdisplay >= 3840 && > >> crtc_state->hw.adjusted_mode.vdisplay >= 2160) { > >> /* Get hblank early enable value required */ > >> - val &= ~HBLANK_START_COUNT_MASK(pipe); > >> + val &= ~HBLANK_START_COUNT_MASK(cpu_transcoder); > >> hblank_early_prog = calc_hblank_early_prog(encoder, crtc_state); > >> if (hblank_early_prog < 32) > >> - val |= HBLANK_START_COUNT(pipe, > >> HBLANK_START_COUNT_32); > >> + val |= HBLANK_START_COUNT(cpu_transcoder, > >> HBLANK_START_COUNT_32); > >> else if (hblank_early_prog < 64) > >> - val |= HBLANK_START_COUNT(pipe, > >> HBLANK_START_COUNT_64); > >> + val |= HBLANK_START_COUNT(cpu_transcoder, > >> HBLANK_START_COUNT_64); > >> else if (hblank_early_prog < 96) > >> - val |= HBLANK_START_COUNT(pipe, > >> HBLANK_START_COUNT_96); > >> + val |= HBLANK_START_COUNT(cpu_transcoder, > >> HBLANK_START_COUNT_96); > >> else > >> - val |= HBLANK_START_COUNT(pipe, > >> HBLANK_START_COUNT_128); > >> + val |= HBLANK_START_COUNT(cpu_transcoder, > >> HBLANK_START_COUNT_128); > >> > >> /* Get samples room value required */ > >> - val &= ~NUMBER_SAMPLES_PER_LINE_MASK(pipe); > >> + val &= ~NUMBER_SAMPLES_PER_LINE_MASK(cpu_transcoder); > >> samples_room = calc_samples_room(crtc_state); > >> if (samples_room < 3) > >> - val |= NUMBER_SAMPLES_PER_LINE(pipe, samples_room); > >> + val |= NUMBER_SAMPLES_PER_LINE(cpu_transcoder, > >> samples_room); > >> else /* Program 0 i.e "All Samples available in buffer" */ > >> - val |= NUMBER_SAMPLES_PER_LINE(pipe, 0x0); > >> + val |= NUMBER_SAMPLES_PER_LINE(cpu_transcoder, 0x0); > >> } > >> > >> intel_de_write(i915, AUD_CONFIG_BE, val); @@ -812,9 +811,9 @@ void > >> intel_audio_codec_enable(struct intel_encoder *encoder, > >> struct i915_audio_component *acomp = i915->display.audio.component; > >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> struct intel_connector *connector = to_intel_connector(conn_state- > >>> connector); > >> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >> struct intel_audio_state *audio_state; > >> enum port port = encoder->port; > >> - enum pipe pipe = crtc->pipe; > >> > >> if (!crtc_state->has_audio) > >> return; > >> @@ -832,7 +831,7 @@ void intel_audio_codec_enable(struct intel_encoder > >> *encoder, > >> > >> mutex_lock(&i915->display.audio.mutex); > >> > >> - audio_state = &i915->display.audio.state[pipe]; > >> + audio_state = &i915->display.audio.state[cpu_transcoder]; > >> > >> audio_state->encoder = encoder; > >> BUILD_BUG_ON(sizeof(audio_state->eld) != sizeof(crtc_state->eld)); @@ - > >> 842,14 +841,14 @@ void intel_audio_codec_enable(struct intel_encoder *encoder, > >> > >> if (acomp && acomp->base.audio_ops && > >> acomp->base.audio_ops->pin_eld_notify) { > >> - /* audio drivers expect pipe = -1 to indicate Non-MST cases */ > >> + /* audio drivers expect cpu_transcoder = -1 to indicate Non-MST > >> cases > >> +*/ > >> if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > >> - pipe = -1; > >> + cpu_transcoder = -1; > >> acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops- > >>> audio_ptr, > >> - (int)port, (int)pipe); > >> + (int)port, (int)cpu_transcoder); > >> } > >> > >> - intel_lpe_audio_notify(i915, pipe, port, crtc_state->eld, > >> + intel_lpe_audio_notify(i915, cpu_transcoder, port, crtc_state->eld, > >> crtc_state->port_clock, > >> intel_crtc_has_dp_encoder(crtc_state)); > >> } > >> @@ -871,9 +870,9 @@ void intel_audio_codec_disable(struct intel_encoder > >> *encoder, > >> struct i915_audio_component *acomp = i915->display.audio.component; > >> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > >> struct intel_connector *connector = to_intel_connector(old_conn_state- > >>> connector); > >> + enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; > >> struct intel_audio_state *audio_state; > >> enum port port = encoder->port; > >> - enum pipe pipe = crtc->pipe; > >> > >> if (!old_crtc_state->has_audio) > >> return; > >> @@ -890,7 +889,7 @@ void intel_audio_codec_disable(struct intel_encoder > >> *encoder, > >> > >> mutex_lock(&i915->display.audio.mutex); > >> > >> - audio_state = &i915->display.audio.state[pipe]; > >> + audio_state = &i915->display.audio.state[cpu_transcoder]; > >> > >> audio_state->encoder = NULL; > >> memset(audio_state->eld, 0, sizeof(audio_state->eld)); @@ -899,27 > >> +898,26 @@ void intel_audio_codec_disable(struct intel_encoder *encoder, > >> > >> if (acomp && acomp->base.audio_ops && > >> acomp->base.audio_ops->pin_eld_notify) { > >> - /* audio drivers expect pipe = -1 to indicate Non-MST cases */ > >> + /* audio drivers expect cpu_transcoder = -1 to indicate Non-MST > >> cases > >> +*/ > >> if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) > >> - pipe = -1; > >> + cpu_transcoder = -1; > >> acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops- > >>> audio_ptr, > >> - (int)port, (int)pipe); > >> + (int)port, (int)cpu_transcoder); > >> } > >> > >> - intel_lpe_audio_notify(i915, pipe, port, NULL, 0, false); > >> + intel_lpe_audio_notify(i915, cpu_transcoder, port, NULL, 0, false); > >> } > >> > >> static void intel_acomp_get_config(struct intel_encoder *encoder, > >> struct intel_crtc_state *crtc_state) { > >> struct drm_i915_private *i915 = to_i915(encoder->base.dev); > >> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >> struct intel_audio_state *audio_state; > >> - enum pipe pipe = crtc->pipe; > >> > >> mutex_lock(&i915->display.audio.mutex); > >> > >> - audio_state = &i915->display.audio.state[pipe]; > >> + audio_state = &i915->display.audio.state[cpu_transcoder]; > >> > >> if (audio_state->encoder) > >> memcpy(crtc_state->eld, audio_state->eld, sizeof(audio_state- > >>> eld)); @@ -1147,27 +1145,27 @@ static int > >> i915_audio_component_get_cdclk_freq(struct device *kdev) } > >> > >> /* > >> - * get the intel audio state according to the parameter port and pipe > >> - * MST & (pipe >= 0): return the audio.state[pipe].encoder], > >> + * get the intel audio state according to the parameter port and > >> + cpu_transcoder > >> + * MST & (cpu_transcoder >= 0): return the > >> + audio.state[cpu_transcoder].encoder], > >> * when port is matched > >> - * MST & (pipe < 0): this is invalid > >> - * Non-MST & (pipe >= 0): only pipe = 0 (the first device entry) > >> + * MST & (cpu_transcoder < 0): this is invalid > >> + * Non-MST & (cpu_transcoder >= 0): only cpu_transcoder = 0 (the first > >> + device entry) > >> * will get the right intel_encoder with port matched > >> - * Non-MST & (pipe < 0): get the right intel_encoder with port matched > >> + * Non-MST & (cpu_transcoder < 0): get the right intel_encoder with > >> + port matched > >> */ > >> static struct intel_audio_state *find_audio_state(struct drm_i915_private *i915, > >> - int port, int pipe) > >> + int port, int cpu_transcoder) > >> { > >> /* MST */ > >> - if (pipe >= 0) { > >> + if (cpu_transcoder >= 0) { > >> struct intel_audio_state *audio_state; > >> struct intel_encoder *encoder; > >> > >> if (drm_WARN_ON(&i915->drm, > >> - pipe >= ARRAY_SIZE(i915->display.audio.state))) > >> + cpu_transcoder >= ARRAY_SIZE(i915- > >>> display.audio.state))) > >> return NULL; > >> > >> - audio_state = &i915->display.audio.state[pipe]; > >> + audio_state = &i915->display.audio.state[cpu_transcoder]; > >> encoder = audio_state->encoder; > >> > >> if (encoder && encoder->port == port && @@ -1176,14 +1174,14 > >> @@ static struct intel_audio_state *find_audio_state(struct drm_i915_private > >> *i915, > >> } > >> > >> /* Non-MST */ > >> - if (pipe > 0) > >> + if (cpu_transcoder > 0) > >> return NULL; > >> > >> - for_each_pipe(i915, pipe) { > >> + for_each_cpu_transcoder(i915, cpu_transcoder) { > >> struct intel_audio_state *audio_state; > >> struct intel_encoder *encoder; > >> > >> - audio_state = &i915->display.audio.state[pipe]; > >> + audio_state = &i915->display.audio.state[cpu_transcoder]; > >> encoder = audio_state->encoder; > >> > >> if (encoder && encoder->port == port && @@ -1195,7 +1193,7 @@ > >> static struct intel_audio_state *find_audio_state(struct drm_i915_private *i915, } > >> > >> static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, > >> - int pipe, int rate) > >> + int cpu_transcoder, int rate) > >> { > >> struct drm_i915_private *i915 = kdev_to_i915(kdev); > >> struct i915_audio_component *acomp = i915->display.audio.component; > >> @@ -1211,7 +1209,7 @@ static int i915_audio_component_sync_audio_rate(struct > >> device *kdev, int port, > >> cookie = i915_audio_component_get_power(kdev); > >> mutex_lock(&i915->display.audio.mutex); > >> > >> - audio_state = find_audio_state(i915, port, pipe); > >> + audio_state = find_audio_state(i915, port, cpu_transcoder); > >> if (!audio_state) { > >> drm_dbg_kms(&i915->drm, "Not valid for port %c\n", > >> port_name(port)); > >> err = -ENODEV; > >> @@ -1223,7 +1221,7 @@ static int i915_audio_component_sync_audio_rate(struct > >> device *kdev, int port, > >> /* FIXME stop using the legacy crtc pointer */ > >> crtc = to_intel_crtc(encoder->base.crtc); > >> > >> - /* port must be valid now, otherwise the pipe will be invalid */ > >> + /* port must be valid now, otherwise the cpu_transcoder will be > >> +invalid */ > >> acomp->aud_sample_rate[port] = rate; > >> > >> /* FIXME get rid of the crtc->config stuff */ @@ -1236,7 +1234,7 @@ static > >> int i915_audio_component_sync_audio_rate(struct device *kdev, int port, } > >> > >> static int i915_audio_component_get_eld(struct device *kdev, int port, > >> - int pipe, bool *enabled, > >> + int cpu_transcoder, bool *enabled, > >> unsigned char *buf, int max_bytes) { > >> struct drm_i915_private *i915 = kdev_to_i915(kdev); @@ -1245,7 +1243,7 > >> @@ static int i915_audio_component_get_eld(struct device *kdev, int port, > >> > >> mutex_lock(&i915->display.audio.mutex); > >> > >> - audio_state = find_audio_state(i915, port, pipe); > >> + audio_state = find_audio_state(i915, port, cpu_transcoder); > >> if (!audio_state) { > >> drm_dbg_kms(&i915->drm, "Not valid for port %c\n", > >> port_name(port)); > >> mutex_unlock(&i915->display.audio.mutex); > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h > >> b/drivers/gpu/drm/i915/display/intel_display_core.h > >> index b870f7f47f2b..631a7b17899e 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_core.h > >> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h > >> @@ -103,7 +103,7 @@ struct intel_audio { > >> u32 freq_cntrl; > >> > >> /* current audio state for the audio component hooks */ > >> - struct intel_audio_state state[I915_MAX_PIPES]; > >> + struct intel_audio_state state[I915_MAX_TRANSCODERS]; > >> > >> /* necessary resource sharing with HDMI LPE audio driver. */ > >> struct { > >> diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c > >> b/drivers/gpu/drm/i915/display/intel_lpe_audio.c > >> index 8aaaef4d7856..20c8581f4868 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c > >> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c > >> @@ -315,7 +315,7 @@ void intel_lpe_audio_teardown(struct drm_i915_private > >> *dev_priv) > >> * intel_lpe_audio_notify() - notify lpe audio event > >> * audio driver and i915 > >> * @dev_priv: the i915 drm device private data > >> - * @pipe: pipe > >> + * @cpt_transocer: CPU transcoder > >> * @port: port > >> * @eld : ELD data > >> * @ls_clock: Link symbol clock in kHz > >> @@ -324,7 +324,7 @@ void intel_lpe_audio_teardown(struct drm_i915_private > >> *dev_priv) > >> * Notify lpe audio driver of eld change. > >> */ > >> void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, > >> - enum pipe pipe, enum port port, > >> + enum transcoder cpu_transcoder, enum port port, > >> const void *eld, int ls_clock, bool dp_output) { > >> unsigned long irqflags; > >> @@ -344,7 +344,7 @@ void intel_lpe_audio_notify(struct drm_i915_private > >> *dev_priv, > >> > >> if (eld != NULL) { > >> memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); > >> - ppdata->pipe = pipe; > >> + ppdata->pipe = cpu_transcoder; > >> ppdata->ls_clock = ls_clock; > >> ppdata->dp_output = dp_output; > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.h > >> b/drivers/gpu/drm/i915/display/intel_lpe_audio.h > >> index f848c5038714..0beecac267ae 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.h > >> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.h > >> @@ -8,15 +8,15 @@ > >> > >> #include <linux/types.h> > >> > >> -enum pipe; > >> enum port; > >> +enum transcoder; > >> struct drm_i915_private; > >> > >> int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void > >> intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void > >> intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void > >> intel_lpe_audio_notify(struct drm_i915_private *dev_priv, > >> - enum pipe pipe, enum port port, > >> + enum transcoder cpu_transcoder, enum port port, > >> const void *eld, int ls_clock, bool dp_output); > >> > >> #endif /* __INTEL_LPE_AUDIO_H__ */ > >> -- > >> 2.39.2 > > > > -- > ~Swati Sharma -- Ville Syrjälä Intel
