mbox series

[v2,0/5] drm/bridge: sii902x: HDMI-audio support and some fixes

Message ID cover.1551303673.git.jsarha@ti.com
Headers show
Series drm/bridge: sii902x: HDMI-audio support and some fixes | expand

Message

Jyri Sarha Feb. 27, 2019, 9:54 p.m. UTC
Changes since first version:
- Moved reviewed patches to front:
  - drm/bridge: sii902x: add input_bus_flags
  - drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
  - drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
- Added a new fix:
  - drm/bridge: sii902x: Select I2C_MUX
- Applied some review suggestions to
  - drm/bridge: sii902x: Implement HDMI audio support
    - use clock-names property to name mclk
    - move comment describing added mutex to struct sii902x and improve it
    - cleanup sii902x_mute()
    - cleanup sii902x_select_mclk_div()
    - fix condition for checking ENABLE_BIT from i2s_fifo_routing in
      sii902x_audio_codec_init()

Still to do

- Agree on i2s wires to HDMI audio fifo routing in dts. 

  The current scheme is quite straight forward, but there is maybe
  there is even more straight forward solutions like:

  audio-fifo-enable = <1 1 1 1>;
  audio-i2s-pin-to-fifo = <0 1 2 3>;

  Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1
  to fifo 1, etc. I am not sure if the channel swap functionality
  should show in dts binding.

Jyri Sarha (4):
  drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
  drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
  drm/bridge: sii902x: Select I2C_MUX
  drm/bridge: sii902x: Implement HDMI audio support

Tomi Valkeinen (1):
  drm/bridge: sii902x: add input_bus_flags

 .../bindings/display/bridge/sii902x.txt       |  36 +-
 drivers/gpu/drm/bridge/Kconfig                |   1 +
 drivers/gpu/drm/bridge/sii902x.c              | 472 +++++++++++++++++-
 include/dt-bindings/sound/sii902x-audio.h     |  11 +
 4 files changed, 512 insertions(+), 8 deletions(-)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Comments

Laurent Pinchart March 4, 2019, 12:42 p.m. UTC | #1
Hi Jyri,

On Wed, Feb 27, 2019 at 11:54:18PM +0200, Jyri Sarha wrote:
> Changes since first version:

> - Moved reviewed patches to front:

>   - drm/bridge: sii902x: add input_bus_flags

>   - drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID

>   - drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz

> - Added a new fix:

>   - drm/bridge: sii902x: Select I2C_MUX

> - Applied some review suggestions to

>   - drm/bridge: sii902x: Implement HDMI audio support

>     - use clock-names property to name mclk

>     - move comment describing added mutex to struct sii902x and improve it

>     - cleanup sii902x_mute()

>     - cleanup sii902x_select_mclk_div()

>     - fix condition for checking ENABLE_BIT from i2s_fifo_routing in

>       sii902x_audio_codec_init()

> 

> Still to do

> 

> - Agree on i2s wires to HDMI audio fifo routing in dts. 

> 

>   The current scheme is quite straight forward, but there is maybe

>   there is even more straight forward solutions like:

> 

>   audio-fifo-enable = <1 1 1 1>;

>   audio-i2s-pin-to-fifo = <0 1 2 3>;

> 

>   Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1

>   to fifo 1, etc. I am not sure if the channel swap functionality

>   should show in dts binding.


Please forgive my lack of audio knowledge, but it this a system
description that should be encoded in DT, or a policy that should be
handled purely in software (either fully inside the kernel or with the
help of userspace) ?

> Jyri Sarha (4):

>   drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID

>   drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz

>   drm/bridge: sii902x: Select I2C_MUX

>   drm/bridge: sii902x: Implement HDMI audio support

> 

> Tomi Valkeinen (1):

>   drm/bridge: sii902x: add input_bus_flags

> 

>  .../bindings/display/bridge/sii902x.txt       |  36 +-

>  drivers/gpu/drm/bridge/Kconfig                |   1 +

>  drivers/gpu/drm/bridge/sii902x.c              | 472 +++++++++++++++++-

>  include/dt-bindings/sound/sii902x-audio.h     |  11 +

>  4 files changed, 512 insertions(+), 8 deletions(-)

>  create mode 100644 include/dt-bindings/sound/sii902x-audio.h


-- 
Regards,

Laurent Pinchart
Laurent Pinchart March 4, 2019, 4:10 p.m. UTC | #2
Hi Jyri,

On Mon, Mar 04, 2019 at 04:29:17PM +0200, Jyri Sarha wrote:
> On 04/03/2019 14:42, Laurent Pinchart wrote:

> > On Wed, Feb 27, 2019 at 11:54:18PM +0200, Jyri Sarha wrote:

> >> Changes since first version:

> >> - Moved reviewed patches to front:

> >>   - drm/bridge: sii902x: add input_bus_flags

> >>   - drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID

> >>   - drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz

> >> - Added a new fix:

> >>   - drm/bridge: sii902x: Select I2C_MUX

> >> - Applied some review suggestions to

> >>   - drm/bridge: sii902x: Implement HDMI audio support

> >>     - use clock-names property to name mclk

> >>     - move comment describing added mutex to struct sii902x and improve it

> >>     - cleanup sii902x_mute()

> >>     - cleanup sii902x_select_mclk_div()

> >>     - fix condition for checking ENABLE_BIT from i2s_fifo_routing in

> >>       sii902x_audio_codec_init()

> >>

> >> Still to do

> >>

> >> - Agree on i2s wires to HDMI audio fifo routing in dts. 

> >>

> >>   The current scheme is quite straight forward, but there is maybe

> >>   there is even more straight forward solutions like:

> >>

> >>   audio-fifo-enable = <1 1 1 1>;

> >>   audio-i2s-pin-to-fifo = <0 1 2 3>;

> >>

> >>   Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1

> >>   to fifo 1, etc. I am not sure if the channel swap functionality

> >>   should show in dts binding.

> > 

> > Please forgive my lack of audio knowledge, but it this a system

> > description that should be encoded in DT, or a policy that should be

> > handled purely in software (either fully inside the kernel or with the

> > help of userspace) ?

> 

> This property describes how many i2s wires are connected to sii902x and

> in what order, so I think it belongs to DTS.


That would belong to DT, yes. We have solved a similar problem with
CSI-2 receivers, where the number of data lanes and their mapping can
often be configured. See the definition of the data-lanes property in
Documentation/devicetree/bindings/media/video-interfaces.txt for more
information. How about using similar bindings ? It would also solve
Andrzej's concerns that you don't describe the audio connection in DT.

> One might of course wonder why anybody would put the i2s wires to any

> other order than 0 <-> 0, 1 <-> 1, 2 <-> 2, and 3 <-> 3, but then a

> again I've seen weirder board designs.


It can be useful to simplify signal routing on the board.

-- 
Regards,

Laurent Pinchart