Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: Alex Riesen <alexander.riesen@cetitec.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	devel@driverdev.osuosl.org, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls
Date: Fri, 13 Mar 2020 11:52:03 +0100	[thread overview]
Message-ID: <c260defe-bb3c-0dc2-476f-5cf8ff27b915@xs4all.nl> (raw)
In-Reply-To: <20200313102600.GF3832@pflmari>

On 3/13/20 11:26 AM, Alex Riesen wrote:
> Hi Hans,
> 
> Hans Verkuil, Fri, Mar 13, 2020 09:16:11 +0100:
>> On 1/13/20 3:15 PM, Alex Riesen wrote:
>>> This change implements audio-related V4L2 ioctls for the HDMI subdevice.
>>
>> This is really where things go wrong. These V4L2 audio ioctls are meant for
>> old PCI TV tuner devices where the audio was implemented as audio jack outputs
>> that are typically looped back to audio inputs on a (PCI) soundcard. And when
>> these ioctls were designed ALSA didn't even exist.
> 
> I see. That was before my time :)
> 
>> Generally an hdmi driver will configure the i2s audio automatically, which is
>> typically connected to the SoC and controlled by the ALSA driver of the SoC,
>> but there may well be missing features (audio never got a lot of attention in
>> hdmi receivers). So what I would like to know is: what features are missing?
> 
> Well, the audio is missing. The current adv748x driver does not export the
> audio features of the device at all. There is no code to enable the I2S audio
> output and it is disabled (all clock and the data lines) by default.

Sorry, I was vague in my question. Obviously that needs to be added, but besides
adding the low-level i2s support I was wondering if there are additional things
that need to be exposed to userspace in order for audio to fully work.

> 
> But, by now it seems to be clear that implementation of ALSA SoC DAI
> interfaces is the way to support the audio.
> 
> And I am already slowly working on it.
> 
>> Anything missing can likely be resolved by adding HDMI audio specific V4L2 controls,
>> which would be the right approach for this.
>>
>> So I would expect to see a proposal for V4L2_CID_DV_RX_AUDIO_ controls to be
>> added here:
>>
>> https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-dv.html
> 
> This seems to be an explicitly "digital video" control class. And it has no
> control option for mute. Or did you mean a similarly structured new class for
> "digital audio"?

There are no DV_ audio controls at all today. So any new audio controls would be
added to the DV class. But if there is nothing that needs to be exposed, then
nothing needs to be added :-)

Regards,

	Hans

> 
> This feels like an overkill for this particular driver...
> 
> Regards,
> Alex
> 