On 10/01/15 19:50, Arnaud Pouliquen wrote:
Version 2: This version integrates missing features upgraded to be aligned when possible with patch set: [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x
There are still some details I would like to change if we decide to go the drm audio bridge way. But before all that, I would like to ask, why should we go forward with your approach? Is there anything that can be done with your approach, but can not be done with mine?
Don't take me wrong, I do not see anything fundamentally wrong with your approach. I would just like hear some justification why we should abandon my approach - that I've been working on for some time - and go forward with yours.
Here is couple of benefits I can name in my approach: Video side agnostic implementation The ASoC side does not need to know anything about video side implementation. There is no real exposure ASoC side internals in video side either. Even fbdev driver, or some other non DRM video driver, could use my implementation. - HDMI encoder driver implementations that do not use DRM bridge abstraction do not need add an extra DRM object just to get the audio working.
Short comings I see in the current HDMI audio bridge approach:
In its current from the DRM audio bridge abstraction pretends to be a generic audio abstraction for DRM devices, but the implementation is quite specific to external HDMI encoders with spdif and/or i2s interface. There is a lot of HDMI video devices that provide the digital audio interface (ASoC DAI) directly and there is no need for anything but dummy codec implementation (if following ASoC paradigm). Before going forward I think we should at least consider how this abstraction would serve those devices.
Also, I am not entirely happy how the drm_audio_bridge_funcs are used at the moment. The do not map too well to ASoC DAI callbacks and I do not see too much point in creating a completely new audio-callback abstraction, that is sligtly incompatible with ALSA, and then translating alsa callbacks to these new callbacks. I think the callbacks should map more or less directly ALSA callbacks.
Best regards, Jyri
It does not support the abort callback, the mute and syclk ops. This could be added in V3 or separate patch if justified.
[RFC 1/5]video: hdmi: add help function for N and CTS
- Replace real frequency value by enum in switch case
Notice that this patch is independant from the rest of the implentation [RFC 2/5]drm: add helper functions to add audio capabilities for bridge
- extend audio bridge ops
[RFC 3/5]ASoC: codec: hdmi drm codec driver aligned when possible with hdmi-codec.c implementation . ELD management for alsa contraints . Support of SPDIF and I2S mode . hwparam ond set_fmt functions implemented [RFC 4/5]drm: sti: connect audio driver => Implementation for STI platform [RFC 5/5]DT: sti: add audio HDMI dai link in audio card => Devicetree example
Version 1:
This patch set is a tentative to implement a generic code for the HDMI audio. Main concept are aligned with solution proposeded for TI platform.
- ASoC codec driver registered by DRM driver
- ASOC driver is a generic driver.
- compatible with simple card
Difference is that i propose a DRM generic interface based on bridge structure. Advantage is that all data exchanges are done through the DRM API.
I think also that some helper functions could been used for N and CTS parameters calculation, as suggested by Russell King in a previous mail.
I full aware that some features (like ELD and info frame) are partially or not implemented in my patches. This patch set is more a skeleton than a full implementation... I just post it to suggest a possible DRM API.
Arnaud Pouliquen (5): video: hdmi: add helper function for N and CTS drm: add helper function to add audio capabilities for bridge ASoC: codec: hdmi drm codec driver drm: sti: connect audio driver DT: sti: add audio HDMI dai link in audio card
arch/arm/boot/dts/stih410.dtsi | 6 +- arch/arm/boot/dts/stihxxx-b2120.dtsi | 21 ++ drivers/gpu/drm/drm_bridge.c | 137 +++++++++++++- drivers/gpu/drm/sti/sti_hdmi.c | 184 ++++++++++++++++-- drivers/gpu/drm/sti/sti_hdmi.h | 3 + drivers/video/hdmi.c | 147 ++++++++++++++ include/drm/drm_crtc.h | 62 ++++++ include/drm/drm_modes.h | 12 ++ include/linux/hdmi.h | 12 ++ include/sound/hdmi_drm.h | 15 ++ sound/soc/codecs/Kconfig | 8 +- sound/soc/codecs/Makefile | 2 + sound/soc/codecs/hdmi_drm.c | 358 +++++++++++++++++++++++++++++++++++ 13 files changed, 951 insertions(+), 16 deletions(-) create mode 100644 include/sound/hdmi_drm.h create mode 100644 sound/soc/codecs/hdmi_drm.c