Hi Kai, > -----Original Message----- > From: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > Sent: 21 June 2023 22:36 > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@xxxxxxxxx> > Cc: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx>; Golani, Mitulkumar > Ajitkumar <mitulkumar.ajitkumar.golani@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx; jyri.sarha@xxxxxxxxxxxxxxx > Subject: RE: [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@xxxxxxxxxxxxxxxxxxxxx> 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