linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
@ 2020-01-13 14:14 Alex Riesen
  2020-03-13  8:21 ` Hans Verkuil
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Riesen @ 2020-01-13 14:14 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart,
	Rob Herring, Mark Rutland, devel, linux-media, linux-kernel,
	devicetree, linux-renesas-soc

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.24.1.508.g91d2dafee0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
  2020-01-13 14:14 [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio Alex Riesen
@ 2020-03-13  8:21 ` Hans Verkuil
  2020-03-13  8:31   ` Alex Riesen
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2020-03-13  8:21 UTC (permalink / raw)
  To: Alex Riesen, Kieran Bingham, Mauro Carvalho Chehab,
	Laurent Pinchart, Rob Herring, Mark Rutland, devel, linux-media,
	linux-kernel, devicetree, linux-renesas-soc

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

On 1/13/20 3:14 PM, Alex Riesen wrote:
> 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(-)
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
  2020-03-13  8:21 ` Hans Verkuil
@ 2020-03-13  8:31   ` Alex Riesen
  2020-03-13  8:37     ` Hans Verkuil
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Riesen @ 2020-03-13  8:31 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Kieran Bingham, Mauro Carvalho Chehab, Laurent Pinchart,
	Rob Herring, Mark Rutland, devel, linux-media, linux-kernel,
	devicetree, linux-renesas-soc

Hi Hans,

Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:
> 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

> On 1/13/20 3:14 PM, Alex Riesen wrote:
> > 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(-)
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
  2020-03-13  8:31   ` Alex Riesen
@ 2020-03-13  8:37     ` Hans Verkuil
  2020-03-13  8:54       ` Alex Riesen
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2020-03-13  8:37 UTC (permalink / raw)
  To: Alex Riesen, Kieran Bingham, Mauro Carvalho Chehab,
	Laurent Pinchart, Rob Herring, Mark Rutland, devel, linux-media,
	linux-kernel, devicetree, linux-renesas-soc

On 3/13/20 9:31 AM, Alex Riesen wrote:
> Hi Hans,
> 
> Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:
>> 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.

That sounds like what everyone else does as well.

> 
> 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

> 
> Regards,
> Alex
> 
>> On 1/13/20 3:14 PM, Alex Riesen wrote:
>>> 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(-)
>>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
  2020-03-13  8:37     ` Hans Verkuil
@ 2020-03-13  8:54       ` Alex Riesen
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Riesen @ 2020-03-13  8:54 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Kieran Bingham, Mauro Carvalho Chehab, Laurent Pinchart,
	Rob Herring, Mark Rutland, devel, linux-media, linux-kernel,
	devicetree, linux-renesas-soc

Hans Verkuil, Fri, Mar 13, 2020 09:37:18 +0100:
> On 3/13/20 9:31 AM, Alex Riesen wrote:
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio
@ 2020-01-13 14:19 Alex Riesen
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Riesen @ 2020-01-13 14:19 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart,
	Rob Herring, Mark Rutland, devel, linux-media, linux-kernel,
	devicetree, linux-renesas-soc

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.24.1.508.g91d2dafee0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-13  8:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13 14:14 [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio Alex Riesen
2020-03-13  8:21 ` Hans Verkuil
2020-03-13  8:31   ` Alex Riesen
2020-03-13  8:37     ` Hans Verkuil
2020-03-13  8:54       ` Alex Riesen
2020-01-13 14:19 Alex Riesen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).