[Intel-gfx] [PATCH] drm/i915/audio: Track audio state per-transcoder
Swati Sharma
swati2.sharma at intel.com
Thu Feb 23 09:49:30 UTC 2023
Hi Ville,
Please add closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8222
On 23-Feb-23 1:48 PM, Shankar, Uma wrote:
>
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville Syrjala
>> Sent: Wednesday, February 22, 2023 8:45 PM
>> To: intel-gfx at lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH] drm/i915/audio: Track audio state per-transcoder
>>
>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>
>> 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 at intel.com>
>
>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> ---
>> 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
More information about the Intel-gfx
mailing list