Skip to contentSkip to conversation

media: i2c: adv748x: add support for HDMI audio

PATCH In review Alex Riesen 2 participants 5 messages Jan 13, 2020 – Mar 13, 2020 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

cover media: i2c: adv748x: add support for HDMI audio

This adds minimal support for controlling the audio output I2S port available on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio signal from the decoded HDMI stream.

An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of this code.

Alex Riesen (8):
media: adv748x: add a device-specific wrapper for register block read
media: adv748x: add audio mute control and output selection ioctls
media: adv748x: add log_status ioctl
media: adv748x: reserve space for the audio (I2S) port in the driver
structures
media: adv748x: add an ASoC DAI definition to the driver
media: adv748x: reduce amount of code for bitwise modification of
device registers
dt-bindings: adv748x: add information about serial audio interface

    (I2S/TDM)

arm64: dts: renesas: salvator: add a connection from adv748x codec
(HDMI input) to the R-Car SoC

 .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
 .../dts/renesas/r8a7795-es1-salvator-x.dts    |  24 +-
 .../boot/dts/renesas/salvator-common.dtsi     |  35 +-
 drivers/media/i2c/adv748x/adv748x-core.c      |  54 +++
 drivers/media/i2c/adv748x/adv748x-hdmi.c      | 355 ++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x.h           |  53 ++-
 6 files changed, 523 insertions(+), 11 deletions(-)
Signature · 1 line
2.24.1.508.g91d2dafee0
#1 lore raw

Hi Alex,

Again, sorry for the late reply.

Patch 2/8 has its own comment since that approach won't work.

As a general note for this series: it might be better to have two patch series: one for patches 1 and 3-6 (not sure whether 5 can be included or not), and one where the public API changes (i.e. new V4L2 audio controls) are added. The first can probably be merged fairly quickly, the second will likely require more iterations since public API patches always take much longer before they are mature.

Regards,

Hans

Quoted 29 lines · Alex Riesen

This adds minimal support for controlling the audio output I2S port available on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio signal from the decoded HDMI stream.

An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of this code.

Alex Riesen (8):
media: adv748x: add a device-specific wrapper for register block read
media: adv748x: add audio mute control and output selection ioctls
media: adv748x: add log_status ioctl
media: adv748x: reserve space for the audio (I2S) port in the driver
structures
media: adv748x: add an ASoC DAI definition to the driver
media: adv748x: reduce amount of code for bitwise modification of
device registers
dt-bindings: adv748x: add information about serial audio interface

    (I2S/TDM)

arm64: dts: renesas: salvator: add a connection from adv748x codec
(HDMI input) to the R-Car SoC

 .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
 .../dts/renesas/r8a7795-es1-salvator-x.dts    |  24 +-
 .../boot/dts/renesas/salvator-common.dtsi     |  35 +-
 drivers/media/i2c/adv748x/adv748x-core.c      |  54 +++
 drivers/media/i2c/adv748x/adv748x-hdmi.c      | 355 ++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x.h           |  53 ++-
 6 files changed, 523 insertions(+), 11 deletions(-)
#2 lore raw

Hi Hans,

Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:

Quoted 6 lines

As a general note for this series: it might be better to have two patch series: one for patches 1 and 3-6 (not sure whether 5 can be included or not), and one where the public API changes (i.e. new V4L2 audio controls) are added. The first can probably be merged fairly quickly, the second will likely require more iterations since public API patches always take much longer before they are mature.

I see. After the discussion started, I started to have suspicions of my own regarding the V4L2 ioctls. Except for log-status, which is a practical diagnostics feature (even supported by v4l2-ctl), I'm thinking about dropping them altogether in favor of audio soc DAI implementation.
The DAI implementation does all we ever needed from the device. Besides, selecting a I2S protocol variant from user space (I2S vs I2S/TDM) never felt right.

Shall I submit the log-status separately?

Regards,
Alex

Quoted 30 lines · Alex Riesen
Quoted 29 lines

This adds minimal support for controlling the audio output I2S port available on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio signal from the decoded HDMI stream.

An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of this code.

Alex Riesen (8):
1. media: adv748x: add a device-specific wrapper for register block read
2. media: adv748x: add audio mute control and output selection ioctls
3. media: adv748x: add log_status ioctl
4. media: adv748x: reserve space for the audio (I2S) port in the driver structures
5. media: adv748x: add an ASoC DAI definition to the driver
6. media: adv748x: reduce amount of code for bitwise modification of device registers
7. dt-bindings: adv748x: add information about serial audio interface

    (I2S/TDM)

8. arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

 .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
 .../dts/renesas/r8a7795-es1-salvator-x.dts    |  24 +-
 .../boot/dts/renesas/salvator-common.dtsi     |  35 +-
 drivers/media/i2c/adv748x/adv748x-core.c      |  54 +++
 drivers/media/i2c/adv748x/adv748x-hdmi.c      | 355 ++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x.h           |  53 ++-
 6 files changed, 523 insertions(+), 11 deletions(-)
#3 lore raw
Quoted 10 lines · Alex Riesen

Hi Hans,

Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:

Quoted 6 lines

As a general note for this series: it might be better to have two patch series: one for patches 1 and 3-6 (not sure whether 5 can be included or not), and one where the public API changes (i.e. new V4L2 audio controls) are added. The first can probably be merged fairly quickly, the second will likely require more iterations since public API patches always take much longer before they are mature.

Quoted 7 lines

I see. After the discussion started, I started to have suspicions of my own regarding the V4L2 ioctls. Except for log-status, which is a practical diagnostics feature (even supported by v4l2-ctl), I'm thinking about dropping them altogether in favor of audio soc DAI implementation.
The DAI implementation does all we ever needed from the device. Besides, selecting a I2S protocol variant from user space (I2S vs I2S/TDM) never felt right.

That sounds like what everyone else does as well.

Quoted 2 lines

Shall I submit the log-status separately?

Yes please. In my experience, log status is a very nice and very useful feature.

If you have other sensible cleanups, then feel free to add those as well.

Regards,

Hans

Quoted 34 lines

Regards,
Alex

Quoted 30 lines · Alex Riesen
Quoted 29 lines

This adds minimal support for controlling the audio output I2S port available on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio signal from the decoded HDMI stream.

An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of this code.

Alex Riesen (8):
1. media: adv748x: add a device-specific wrapper for register block read
2. media: adv748x: add audio mute control and output selection ioctls
3. media: adv748x: add log_status ioctl
4. media: adv748x: reserve space for the audio (I2S) port in the driver structures
5. media: adv748x: add an ASoC DAI definition to the driver
6. media: adv748x: reduce amount of code for bitwise modification of device registers
7. dt-bindings: adv748x: add information about serial audio interface

    (I2S/TDM)

8. arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

 .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
 .../dts/renesas/r8a7795-es1-salvator-x.dts    |  24 +-
 .../boot/dts/renesas/salvator-common.dtsi     |  35 +-
 drivers/media/i2c/adv748x/adv748x-core.c      |  54 +++
 drivers/media/i2c/adv748x/adv748x-hdmi.c      | 355 ++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x.h           |  53 ++-
 6 files changed, 523 insertions(+), 11 deletions(-)
#4 lore raw

Hans Verkuil, Fri, Mar 13, 2020 09:37:18 +0100:

Quoted 6 lines · Alex Riesen
Quoted 1 line

Shall I submit the log-status separately?

Yes please. In my experience, log status is a very nice and very useful feature.

If you have other sensible cleanups, then feel free to add those as well.

Noted. I shall send it after the DAI series: less inter-series dependencies this way (the log-status needs a new routine used by the DAI code).

Regards,
Alex