[Intel-gfx] [RFC 2/3] drm/i915/display: Configure and initialize HDMI audio capabilities

[Golani, Mitulkumar Ajitkumar]

Hi Kai,

> -----Original Message-----
> From: Kai Vehmanen <kai.vehmanen at linux.intel.com>
> Sent: 21 June 2023 22:36
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> Cc: Kai Vehmanen <kai.vehmanen at linux.intel.com>; Golani, Mitulkumar
> Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>; intel-
> gfx at lists.freedesktop.org; jyri.sarha at linux.intel.com
> Subject: RE: [Intel-gfx] [RFC 2/3] drm/i915/display: Configure and initialize
> HDMI audio capabilities
> 
> Hey,
> 
> On Tue, 20 Jun 2023, Borah, Chaitanya Kumar wrote:
> 
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > > Of Kai Vehmanen On Fri, 9 Jun 2023, Mitul Golani wrote:
> [...]
> > > This will hit a problem if we have a case where bandwidth is not
> > > enough for 5.1 at 192kHz, but it is enough for 2ch 192kHz audio.
> > > This approach forces us to give preference to either channel acount or
> sampling rate.
> > >
> > > What if we just store the 'max audio samples per second' into pipe config:
> > >
> > >  - have "int max_audio_samples_per_second;" in pipe_config
> > >  - pipe_config->audio.max_audio_samples_per_second =
> > > available_blank_bandwidth / 32;
> > >
> > > Then when filtering SADs, the invidial channels+rate combination of
> > > each SAD is compared to the max_audio_samples_per_second and based
> > > on that, the SAD is either filter or passed on. What do you think?
> [...]>
> > This has been one my concern as well and we have thought about a
> > similar approach as you suggest. One disadvantage of this approach
> > that I can see, would be that if there are hardware limitations on
> > channels (as was in GLK) or frequencies independently. It will be
> > become tricky with this approach. However, one can argue that these
> > limitations arise from bandwidth itself.
> [...]
> > Regarding the bits per sample, Is using 32bit for all audio formats
> > fair or should we take into account different audio formats and their
> > bit depth?
> 
> hmm, I see the point. This is indeed trickier than it first seems. The 32bit is a
> good worst-case bound, but in practise actual bandwidth needed will be less.
> And problem is, we don't really know which bit depth the application will
> choose, so again we need to limit based on the highest value in SAD.
> 
> And then you have the problem that this calculation assumes LPCM
> encoding.
> If the audio is compressed, e.g. a 8ch DTS stream, the bandwidth calculation
> needs to be done differently (see
> linux/sound/pci/hda/hda_eld.c:hdmi_update_short_audio_desc()):
> 
> So I think there are (at least) two ways to go about this:
>   1) reduce the scope and make the channel/rate limit more
>      limited, and only cover cases (like) GLK where a specific limitation
>      is known -> max values not set for other platforms
> 
>   2) go for more generic description and expose the raw
>      bandwidth (in bits per second) available for audio
> 	-> then SAD filtering can be done based on raw bandwidth
> 	-> can be done only to LPCM at first, extended to compressed
> 	formats later
> 	-> still the problem that code needs to prioritze
> 	   between channels/srate/bitdepth when filtering

I tried to prioritise the logic with help of rate first. But in that I found some issues,
it is picking lowest channel with highest rate every time. 

I think apart from current params, adding max_audio_samples_per_second,
can solve both the issue with following case,

Let's say, with the obtained audio_req_bandwidth, if I encounter the following combination:
pipe_config->audio.max_channel = 8 and pipe_config->audio.max_frequency = "X" value.
Now, let's assume my sink supports only 7 channels. In this case, with current implementation,
pruning will be bypassed, and there is a possibility that the sink-supported channel multiplied
by the sink-supported rate exceeding the available blank bandwidth, but pruning didn't occur.

For this situation, I am considering adding an "else" part in intel_audio_compute_eld.
This "else" part would check if (sad_to_channels(sad) < crtc_state->audio.max_channel), for example, in the GLK case also. 
In that case, the channel would be fixed. 

So, if Channel * audio sample container size * (iterating from Max rate to Min rate) is less than
max_audio_samples_per_second, I believe we can eliminate the above mentioned situation.
If the sink's supported channel is lower than pipe_config->audio.max_channel, we can get another
chance to adjust the rate based on the sink's maximum capability.

now pruning will happen in 2 cases,

1. If pipe_config->audio.max_channel < sink's supported channels then prune as per obtained combination from,
intel_hdmi_audio_compute_config.
2. If pipe_config->audio.max_channel >  sink's supported channels, but sink's supported channel * sink supported max rate * audio sample container size
exceeds the max_audio_samples_per_second then prune with sink's supported channel and rate (which satisfy bandwidth condition. range: in between  Max to min).

Please give your opinion.

Thanks
Mitul
 

> 
> Br, Kai