* [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards
@ 2016-03-28 14:36 Jose Abreu
2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Jose Abreu @ 2016-03-28 14:36 UTC (permalink / raw)
To: linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: airlied-cv59FeDIM0c, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
broonie-DgEjT+Ai2ygdnm+yROfE0A, perex-/Fr2/VpizcU,
tiwai-IBi9RG/b67k,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
lars-Qo5EllUWu/uELgA04lAiVw, ville.syrjala-VuQAYsv1563Yd54FQh9/CA,
nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
alexander.deucher-5C7GfCeVMHo, Maruthi.Bayyavarapu-5C7GfCeVMHo,
buyitian-Re5JQEeQqe8AvxtiuMwx3w, tixy-QSEj5FYQhm4dnm+yROfE0A,
yitian.bu-sf4bEIuTEZ8/jFmSBShxMA,
Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w, Jose Abreu
ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.
This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.
Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)
Jose Abreu (3):
drm/i2c/adv7511: Add audio support
ASoC: dwc: Add I2S HDMI audio support
arc: axs10x: Add support for Designware I2S on DT
arch/arc/boot/dts/axs10x_mb.dtsi | 49 +-
drivers/gpu/drm/i2c/Kconfig | 11 +
drivers/gpu/drm/i2c/Makefile | 2 +
drivers/gpu/drm/i2c/adv7511.c | 1024 -----------------------------------
drivers/gpu/drm/i2c/adv7511.h | 41 ++
drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++
include/sound/soc-dai.h | 1 +
sound/soc/dwc/Kconfig | 1 +
sound/soc/dwc/designware_i2s.c | 385 ++++++++++++-
10 files changed, 1788 insertions(+), 1041 deletions(-)
delete mode 100644 drivers/gpu/drm/i2c/adv7511.c
create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c
create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-28 14:36 [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards Jose Abreu @ 2016-03-28 14:36 ` Jose Abreu 2016-03-29 8:05 ` Archit Taneja ` (3 more replies) [not found] ` <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 2016-03-29 17:00 ` [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards Mark Brown 2 siblings, 4 replies; 23+ messages in thread From: Jose Abreu @ 2016-03-28 14:36 UTC (permalink / raw) To: linux-snps-arc, linux-kernel, dri-devel, alsa-devel, devicetree Cc: airlied, lgirdwood, broonie, perex, tiwai, laurent.pinchart+renesas, wsa+renesas, lars, ville.syrjala, nariman, alexander.deucher, Maruthi.Bayyavarapu, buyitian, tixy, yitian.bu, Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, Vineet.Gupta1, CARLOS.PALMINHA, Jose Abreu This patch adds audio support for the ADV7511 HDMI transmitter using ALSA SoC. The code was ported from Analog Devices linux tree from commit 1770c4a1e32b ("Merge remote-tracking branch 'xilinx/master' into xcomm_zynq"), which is available at: - https://github.com/analogdevicesinc/linux/ The main core file was renamed from adv7511.c to adv7511_core.c so that audio and video compile into a single adv7511.ko module and to keep up with Analog Devices kernel tree. The audio can be disabled using menu-config so it is possible to use only video mode. The HDMI mode is automatically started at boot and the audio (when enabled) registers as a codec into ALSA. SPDIF DAI format was also added to ASoC as it is required by adv7511 audio. Signed-off-by: Jose Abreu <joabreu@synopsys.com> --- No changes v1 -> v2. drivers/gpu/drm/i2c/Kconfig | 11 + drivers/gpu/drm/i2c/Makefile | 2 + drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------------- drivers/gpu/drm/i2c/adv7511.h | 41 ++ drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++ include/sound/soc-dai.h | 1 + 7 files changed, 1370 insertions(+), 1024 deletions(-) delete mode 100644 drivers/gpu/drm/i2c/adv7511.c create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig index 22c7ed6..baed409 100644 --- a/drivers/gpu/drm/i2c/Kconfig +++ b/drivers/gpu/drm/i2c/Kconfig @@ -7,6 +7,17 @@ config DRM_I2C_ADV7511 help Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. +config DRM_I2C_ADV7511_AUDIO + bool "ADV7511 audio" + depends on DRM_I2C_ADV7511 && SND_SOC + default y + help + This adds support for audio on the ADV7511(W) and ADV7513 HDMI + encoders. + + By selecting this option the ADV7511 will register a codec interface + into ALSA. + config DRM_I2C_CH7006 tristate "Chrontel ch7006 TV encoder" default m if DRM_NOUVEAU diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile index 2c72eb5..7fae13b9 100644 --- a/drivers/gpu/drm/i2c/Makefile +++ b/drivers/gpu/drm/i2c/Makefile @@ -1,5 +1,7 @@ ccflags-y := -Iinclude/drm +adv7511-y := adv7511_core.o +adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o ch7006-y := ch7006_drv.o ch7006_mode.o diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c deleted file mode 100644 index a02112b..0000000 --- a/drivers/gpu/drm/i2c/adv7511.c +++ /dev/null @@ -1,1024 +0,0 @@ -/* - * Analog Devices ADV7511 HDMI transmitter driver - * - * Copyright 2012 Analog Devices Inc. - * - * Licensed under the GPL-2. - */ - -#include <linux/device.h> -#include <linux/gpio/consumer.h> -#include <linux/i2c.h> -#include <linux/module.h> -#include <linux/regmap.h> -#include <linux/slab.h> - -#include <drm/drmP.h> -#include <drm/drm_crtc_helper.h> -#include <drm/drm_edid.h> -#include <drm/drm_encoder_slave.h> - -#include "adv7511.h" - -struct adv7511 { - struct i2c_client *i2c_main; - struct i2c_client *i2c_edid; - - struct regmap *regmap; - struct regmap *packet_memory_regmap; - enum drm_connector_status status; - bool powered; - - unsigned int f_tmds; - - unsigned int current_edid_segment; - uint8_t edid_buf[256]; - bool edid_read; - - wait_queue_head_t wq; - struct drm_encoder *encoder; - - bool embedded_sync; - enum adv7511_sync_polarity vsync_polarity; - enum adv7511_sync_polarity hsync_polarity; - bool rgb; - - struct edid *edid; - - struct gpio_desc *gpio_pd; -}; - -static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder) -{ - return to_encoder_slave(encoder)->slave_priv; -} - -/* ADI recommended values for proper operation. */ -static const struct reg_sequence adv7511_fixed_registers[] = { - { 0x98, 0x03 }, - { 0x9a, 0xe0 }, - { 0x9c, 0x30 }, - { 0x9d, 0x61 }, - { 0xa2, 0xa4 }, - { 0xa3, 0xa4 }, - { 0xe0, 0xd0 }, - { 0xf9, 0x00 }, - { 0x55, 0x02 }, -}; - -/* ----------------------------------------------------------------------------- - * Register access - */ - -static const uint8_t adv7511_register_defaults[] = { - 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 00 */ - 0x00, 0x00, 0x01, 0x0e, 0xbc, 0x18, 0x01, 0x13, - 0x25, 0x37, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 10 */ - 0x46, 0x62, 0x04, 0xa8, 0x00, 0x00, 0x1c, 0x84, - 0x1c, 0xbf, 0x04, 0xa8, 0x1e, 0x70, 0x02, 0x1e, /* 20 */ - 0x00, 0x00, 0x04, 0xa8, 0x08, 0x12, 0x1b, 0xac, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 */ - 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0xb0, - 0x00, 0x50, 0x90, 0x7e, 0x79, 0x70, 0x00, 0x00, /* 40 */ - 0x00, 0xa8, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x02, 0x0d, 0x00, 0x00, 0x00, 0x00, /* 50 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 60 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 70 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 80 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0xc0, 0x00, 0x00, 0x00, /* 90 */ - 0x0b, 0x02, 0x00, 0x18, 0x5a, 0x60, 0x00, 0x00, - 0x00, 0x00, 0x80, 0x80, 0x08, 0x04, 0x00, 0x00, /* a0 */ - 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x40, 0x14, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* b0 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* c0 */ - 0x00, 0x03, 0x00, 0x00, 0x02, 0x00, 0x01, 0x04, - 0x30, 0xff, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00, /* d0 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x01, - 0x80, 0x75, 0x00, 0x00, 0x60, 0x00, 0x00, 0x00, /* e0 */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x75, 0x11, 0x00, /* f0 */ - 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, -}; - -static bool adv7511_register_volatile(struct device *dev, unsigned int reg) -{ - switch (reg) { - case ADV7511_REG_CHIP_REVISION: - case ADV7511_REG_SPDIF_FREQ: - case ADV7511_REG_CTS_AUTOMATIC1: - case ADV7511_REG_CTS_AUTOMATIC2: - case ADV7511_REG_VIC_DETECTED: - case ADV7511_REG_VIC_SEND: - case ADV7511_REG_AUX_VIC_DETECTED: - case ADV7511_REG_STATUS: - case ADV7511_REG_GC(1): - case ADV7511_REG_INT(0): - case ADV7511_REG_INT(1): - case ADV7511_REG_PLL_STATUS: - case ADV7511_REG_AN(0): - case ADV7511_REG_AN(1): - case ADV7511_REG_AN(2): - case ADV7511_REG_AN(3): - case ADV7511_REG_AN(4): - case ADV7511_REG_AN(5): - case ADV7511_REG_AN(6): - case ADV7511_REG_AN(7): - case ADV7511_REG_HDCP_STATUS: - case ADV7511_REG_BCAPS: - case ADV7511_REG_BKSV(0): - case ADV7511_REG_BKSV(1): - case ADV7511_REG_BKSV(2): - case ADV7511_REG_BKSV(3): - case ADV7511_REG_BKSV(4): - case ADV7511_REG_DDC_STATUS: - case ADV7511_REG_EDID_READ_CTRL: - case ADV7511_REG_BSTATUS(0): - case ADV7511_REG_BSTATUS(1): - case ADV7511_REG_CHIP_ID_HIGH: - case ADV7511_REG_CHIP_ID_LOW: - return true; - } - - return false; -} - -static const struct regmap_config adv7511_regmap_config = { - .reg_bits = 8, - .val_bits = 8, - - .max_register = 0xff, - .cache_type = REGCACHE_RBTREE, - .reg_defaults_raw = adv7511_register_defaults, - .num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults), - - .volatile_reg = adv7511_register_volatile, -}; - -/* ----------------------------------------------------------------------------- - * Hardware configuration - */ - -static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable, - const uint16_t *coeff, - unsigned int scaling_factor) -{ - unsigned int i; - - regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), - ADV7511_CSC_UPDATE_MODE, ADV7511_CSC_UPDATE_MODE); - - if (enable) { - for (i = 0; i < 12; ++i) { - regmap_update_bits(adv7511->regmap, - ADV7511_REG_CSC_UPPER(i), - 0x1f, coeff[i] >> 8); - regmap_write(adv7511->regmap, - ADV7511_REG_CSC_LOWER(i), - coeff[i] & 0xff); - } - } - - if (enable) - regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), - 0xe0, 0x80 | (scaling_factor << 5)); - else - regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), - 0x80, 0x00); - - regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), - ADV7511_CSC_UPDATE_MODE, 0); -} - -static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet) -{ - if (packet & 0xff) - regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, - packet, 0xff); - - if (packet & 0xff00) { - packet >>= 8; - regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, - packet, 0xff); - } - - return 0; -} - -static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet) -{ - if (packet & 0xff) - regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, - packet, 0x00); - - if (packet & 0xff00) { - packet >>= 8; - regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, - packet, 0x00); - } - - return 0; -} - -/* Coefficients for adv7511 color space conversion */ -static const uint16_t adv7511_csc_ycbcr_to_rgb[] = { - 0x0734, 0x04ad, 0x0000, 0x1c1b, - 0x1ddc, 0x04ad, 0x1f24, 0x0135, - 0x0000, 0x04ad, 0x087c, 0x1b77, -}; - -static void adv7511_set_config_csc(struct adv7511 *adv7511, - struct drm_connector *connector, - bool rgb) -{ - struct adv7511_video_config config; - bool output_format_422, output_format_ycbcr; - unsigned int mode; - uint8_t infoframe[17]; - - if (adv7511->edid) - config.hdmi_mode = drm_detect_hdmi_monitor(adv7511->edid); - else - config.hdmi_mode = false; - - hdmi_avi_infoframe_init(&config.avi_infoframe); - - config.avi_infoframe.scan_mode = HDMI_SCAN_MODE_UNDERSCAN; - - if (rgb) { - config.csc_enable = false; - config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB; - } else { - config.csc_scaling_factor = ADV7511_CSC_SCALING_4; - config.csc_coefficents = adv7511_csc_ycbcr_to_rgb; - - if ((connector->display_info.color_formats & - DRM_COLOR_FORMAT_YCRCB422) && - config.hdmi_mode) { - config.csc_enable = false; - config.avi_infoframe.colorspace = - HDMI_COLORSPACE_YUV422; - } else { - config.csc_enable = true; - config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB; - } - } - - if (config.hdmi_mode) { - mode = ADV7511_HDMI_CFG_MODE_HDMI; - - switch (config.avi_infoframe.colorspace) { - case HDMI_COLORSPACE_YUV444: - output_format_422 = false; - output_format_ycbcr = true; - break; - case HDMI_COLORSPACE_YUV422: - output_format_422 = true; - output_format_ycbcr = true; - break; - default: - output_format_422 = false; - output_format_ycbcr = false; - break; - } - } else { - mode = ADV7511_HDMI_CFG_MODE_DVI; - output_format_422 = false; - output_format_ycbcr = false; - } - - adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); - - adv7511_set_colormap(adv7511, config.csc_enable, - config.csc_coefficents, - config.csc_scaling_factor); - - regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x81, - (output_format_422 << 7) | output_format_ycbcr); - - regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, - ADV7511_HDMI_CFG_MODE_MASK, mode); - - hdmi_avi_infoframe_pack(&config.avi_infoframe, infoframe, - sizeof(infoframe)); - - /* The AVI infoframe id is not configurable */ - regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION, - infoframe + 1, sizeof(infoframe) - 1); - - adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); -} - -static void adv7511_set_link_config(struct adv7511 *adv7511, - const struct adv7511_link_config *config) -{ - /* - * The input style values documented in the datasheet don't match the - * hardware register field values :-( - */ - static const unsigned int input_styles[4] = { 0, 2, 1, 3 }; - - unsigned int clock_delay; - unsigned int color_depth; - unsigned int input_id; - - clock_delay = (config->clock_delay + 1200) / 400; - color_depth = config->input_color_depth == 8 ? 3 - : (config->input_color_depth == 10 ? 1 : 2); - - /* TODO Support input ID 6 */ - if (config->input_colorspace != HDMI_COLORSPACE_YUV422) - input_id = config->input_clock == ADV7511_INPUT_CLOCK_DDR - ? 5 : 0; - else if (config->input_clock == ADV7511_INPUT_CLOCK_DDR) - input_id = config->embedded_sync ? 8 : 7; - else if (config->input_clock == ADV7511_INPUT_CLOCK_2X) - input_id = config->embedded_sync ? 4 : 3; - else - input_id = config->embedded_sync ? 2 : 1; - - regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, 0xf, - input_id); - regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x7e, - (color_depth << 4) | - (input_styles[config->input_style] << 2)); - regmap_write(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG2, - config->input_justification << 3); - regmap_write(adv7511->regmap, ADV7511_REG_TIMING_GEN_SEQ, - config->sync_pulse << 2); - - regmap_write(adv7511->regmap, 0xba, clock_delay << 5); - - adv7511->embedded_sync = config->embedded_sync; - adv7511->hsync_polarity = config->hsync_polarity; - adv7511->vsync_polarity = config->vsync_polarity; - adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB; -} - -static void adv7511_power_on(struct adv7511 *adv7511) -{ - adv7511->current_edid_segment = -1; - - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, 0); - if (adv7511->i2c_main->irq) { - /* - * Documentation says the INT_ENABLE registers are reset in - * POWER_DOWN mode. My 7511w preserved the bits, however. - * Still, let's be safe and stick to the documentation. - */ - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), - ADV7511_INT0_EDID_READY); - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), - ADV7511_INT1_DDC_ERROR); - } - - /* - * Per spec it is allowed to pulse the HPD signal to indicate that the - * EDID information has changed. Some monitors do this when they wakeup - * from standby or are enabled. When the HPD goes low the adv7511 is - * reset and the outputs are disabled which might cause the monitor to - * go to standby again. To avoid this we ignore the HPD pin for the - * first few seconds after enabling the output. - */ - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, - ADV7511_REG_POWER2_HPD_SRC_MASK, - ADV7511_REG_POWER2_HPD_SRC_NONE); - - /* - * Most of the registers are reset during power down or when HPD is low. - */ - regcache_sync(adv7511->regmap); - - adv7511->powered = true; -} - -static void adv7511_power_off(struct adv7511 *adv7511) -{ - /* TODO: setup additional power down modes */ - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, - ADV7511_POWER_POWER_DOWN); - regcache_mark_dirty(adv7511->regmap); - - adv7511->powered = false; -} - -/* ----------------------------------------------------------------------------- - * Interrupt and hotplug detection - */ - -static bool adv7511_hpd(struct adv7511 *adv7511) -{ - unsigned int irq0; - int ret; - - ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); - if (ret < 0) - return false; - - if (irq0 & ADV7511_INT0_HPD) { - regmap_write(adv7511->regmap, ADV7511_REG_INT(0), - ADV7511_INT0_HPD); - return true; - } - - return false; -} - -static int adv7511_irq_process(struct adv7511 *adv7511) -{ - unsigned int irq0, irq1; - int ret; - - ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); - if (ret < 0) - return ret; - - ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); - if (ret < 0) - return ret; - - regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); - regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); - - if (irq0 & ADV7511_INT0_HPD && adv7511->encoder) - drm_helper_hpd_irq_event(adv7511->encoder->dev); - - if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { - adv7511->edid_read = true; - - if (adv7511->i2c_main->irq) - wake_up_all(&adv7511->wq); - } - - return 0; -} - -static irqreturn_t adv7511_irq_handler(int irq, void *devid) -{ - struct adv7511 *adv7511 = devid; - int ret; - - ret = adv7511_irq_process(adv7511); - return ret < 0 ? IRQ_NONE : IRQ_HANDLED; -} - -/* ----------------------------------------------------------------------------- - * EDID retrieval - */ - -static int adv7511_wait_for_edid(struct adv7511 *adv7511, int timeout) -{ - int ret; - - if (adv7511->i2c_main->irq) { - ret = wait_event_interruptible_timeout(adv7511->wq, - adv7511->edid_read, msecs_to_jiffies(timeout)); - } else { - for (; timeout > 0; timeout -= 25) { - ret = adv7511_irq_process(adv7511); - if (ret < 0) - break; - - if (adv7511->edid_read) - break; - - msleep(25); - } - } - - return adv7511->edid_read ? 0 : -EIO; -} - -static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block, - size_t len) -{ - struct adv7511 *adv7511 = data; - struct i2c_msg xfer[2]; - uint8_t offset; - unsigned int i; - int ret; - - if (len > 128) - return -EINVAL; - - if (adv7511->current_edid_segment != block / 2) { - unsigned int status; - - ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS, - &status); - if (ret < 0) - return ret; - - if (status != 2) { - adv7511->edid_read = false; - regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, - block); - ret = adv7511_wait_for_edid(adv7511, 200); - if (ret < 0) - return ret; - } - - /* Break this apart, hopefully more I2C controllers will - * support 64 byte transfers than 256 byte transfers - */ - - xfer[0].addr = adv7511->i2c_edid->addr; - xfer[0].flags = 0; - xfer[0].len = 1; - xfer[0].buf = &offset; - xfer[1].addr = adv7511->i2c_edid->addr; - xfer[1].flags = I2C_M_RD; - xfer[1].len = 64; - xfer[1].buf = adv7511->edid_buf; - - offset = 0; - - for (i = 0; i < 4; ++i) { - ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, - ARRAY_SIZE(xfer)); - if (ret < 0) - return ret; - else if (ret != 2) - return -EIO; - - xfer[1].buf += 64; - offset += 64; - } - - adv7511->current_edid_segment = block / 2; - } - - if (block % 2 == 0) - memcpy(buf, adv7511->edid_buf, len); - else - memcpy(buf, adv7511->edid_buf + 128, len); - - return 0; -} - -/* ----------------------------------------------------------------------------- - * Encoder operations - */ - -static int adv7511_get_modes(struct drm_encoder *encoder, - struct drm_connector *connector) -{ - struct adv7511 *adv7511 = encoder_to_adv7511(encoder); - struct edid *edid; - unsigned int count; - - /* Reading the EDID only works if the device is powered */ - if (!adv7511->powered) { - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, 0); - if (adv7511->i2c_main->irq) { - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), - ADV7511_INT0_EDID_READY); - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), - ADV7511_INT1_DDC_ERROR); - } - adv7511->current_edid_segment = -1; - } - - edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); - - if (!adv7511->powered) - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, - ADV7511_POWER_POWER_DOWN); - - kfree(adv7511->edid); - adv7511->edid = edid; - if (!edid) - return 0; - - drm_mode_connector_update_edid_property(connector, edid); - count = drm_add_edid_modes(connector, edid); - - adv7511_set_config_csc(adv7511, connector, adv7511->rgb); - - return count; -} - -static void adv7511_encoder_dpms(struct drm_encoder *encoder, int mode) -{ - struct adv7511 *adv7511 = encoder_to_adv7511(encoder); - - if (mode == DRM_MODE_DPMS_ON) - adv7511_power_on(adv7511); - else - adv7511_power_off(adv7511); -} - -static enum drm_connector_status -adv7511_encoder_detect(struct drm_encoder *encoder, - struct drm_connector *connector) -{ - struct adv7511 *adv7511 = encoder_to_adv7511(encoder); - enum drm_connector_status status; - unsigned int val; - bool hpd; - int ret; - - ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val); - if (ret < 0) - return connector_status_disconnected; - - if (val & ADV7511_STATUS_HPD) - status = connector_status_connected; - else - status = connector_status_disconnected; - - hpd = adv7511_hpd(adv7511); - - /* The chip resets itself when the cable is disconnected, so in case - * there is a pending HPD interrupt and the cable is connected there was - * at least one transition from disconnected to connected and the chip - * has to be reinitialized. */ - if (status == connector_status_connected && hpd && adv7511->powered) { - regcache_mark_dirty(adv7511->regmap); - adv7511_power_on(adv7511); - adv7511_get_modes(encoder, connector); - if (adv7511->status == connector_status_connected) - status = connector_status_disconnected; - } else { - /* Renable HPD sensing */ - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, - ADV7511_REG_POWER2_HPD_SRC_MASK, - ADV7511_REG_POWER2_HPD_SRC_BOTH); - } - - adv7511->status = status; - return status; -} - -static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, - struct drm_display_mode *mode) -{ - if (mode->clock > 165000) - return MODE_CLOCK_HIGH; - - return MODE_OK; -} - -static void adv7511_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adj_mode) -{ - struct adv7511 *adv7511 = encoder_to_adv7511(encoder); - unsigned int low_refresh_rate; - unsigned int hsync_polarity = 0; - unsigned int vsync_polarity = 0; - - if (adv7511->embedded_sync) { - unsigned int hsync_offset, hsync_len; - unsigned int vsync_offset, vsync_len; - - hsync_offset = adj_mode->crtc_hsync_start - - adj_mode->crtc_hdisplay; - vsync_offset = adj_mode->crtc_vsync_start - - adj_mode->crtc_vdisplay; - hsync_len = adj_mode->crtc_hsync_end - - adj_mode->crtc_hsync_start; - vsync_len = adj_mode->crtc_vsync_end - - adj_mode->crtc_vsync_start; - - /* The hardware vsync generator has a off-by-one bug */ - vsync_offset += 1; - - regmap_write(adv7511->regmap, ADV7511_REG_HSYNC_PLACEMENT_MSB, - ((hsync_offset >> 10) & 0x7) << 5); - regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(0), - (hsync_offset >> 2) & 0xff); - regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(1), - ((hsync_offset & 0x3) << 6) | - ((hsync_len >> 4) & 0x3f)); - regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(2), - ((hsync_len & 0xf) << 4) | - ((vsync_offset >> 6) & 0xf)); - regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(3), - ((vsync_offset & 0x3f) << 2) | - ((vsync_len >> 8) & 0x3)); - regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(4), - vsync_len & 0xff); - - hsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PHSYNC); - vsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PVSYNC); - } else { - enum adv7511_sync_polarity mode_hsync_polarity; - enum adv7511_sync_polarity mode_vsync_polarity; - - /** - * If the input signal is always low or always high we want to - * invert or let it passthrough depending on the polarity of the - * current mode. - **/ - if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC) - mode_hsync_polarity = ADV7511_SYNC_POLARITY_LOW; - else - mode_hsync_polarity = ADV7511_SYNC_POLARITY_HIGH; - - if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC) - mode_vsync_polarity = ADV7511_SYNC_POLARITY_LOW; - else - mode_vsync_polarity = ADV7511_SYNC_POLARITY_HIGH; - - if (adv7511->hsync_polarity != mode_hsync_polarity && - adv7511->hsync_polarity != - ADV7511_SYNC_POLARITY_PASSTHROUGH) - hsync_polarity = 1; - - if (adv7511->vsync_polarity != mode_vsync_polarity && - adv7511->vsync_polarity != - ADV7511_SYNC_POLARITY_PASSTHROUGH) - vsync_polarity = 1; - } - - if (mode->vrefresh <= 24000) - low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ; - else if (mode->vrefresh <= 25000) - low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ; - else if (mode->vrefresh <= 30000) - low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ; - else - low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE; - - regmap_update_bits(adv7511->regmap, 0xfb, - 0x6, low_refresh_rate << 1); - regmap_update_bits(adv7511->regmap, 0x17, - 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); - - /* - * TODO Test first order 4:2:2 to 4:4:4 up conversion method, which is - * supposed to give better results. - */ - - adv7511->f_tmds = mode->clock; -} - -static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { - .dpms = adv7511_encoder_dpms, - .mode_valid = adv7511_encoder_mode_valid, - .mode_set = adv7511_encoder_mode_set, - .detect = adv7511_encoder_detect, - .get_modes = adv7511_get_modes, -}; - -/* ----------------------------------------------------------------------------- - * Probe & remove - */ - -static int adv7511_parse_dt(struct device_node *np, - struct adv7511_link_config *config) -{ - const char *str; - int ret; - - memset(config, 0, sizeof(*config)); - - of_property_read_u32(np, "adi,input-depth", &config->input_color_depth); - if (config->input_color_depth != 8 && config->input_color_depth != 10 && - config->input_color_depth != 12) - return -EINVAL; - - ret = of_property_read_string(np, "adi,input-colorspace", &str); - if (ret < 0) - return ret; - - if (!strcmp(str, "rgb")) - config->input_colorspace = HDMI_COLORSPACE_RGB; - else if (!strcmp(str, "yuv422")) - config->input_colorspace = HDMI_COLORSPACE_YUV422; - else if (!strcmp(str, "yuv444")) - config->input_colorspace = HDMI_COLORSPACE_YUV444; - else - return -EINVAL; - - ret = of_property_read_string(np, "adi,input-clock", &str); - if (ret < 0) - return ret; - - if (!strcmp(str, "1x")) - config->input_clock = ADV7511_INPUT_CLOCK_1X; - else if (!strcmp(str, "2x")) - config->input_clock = ADV7511_INPUT_CLOCK_2X; - else if (!strcmp(str, "ddr")) - config->input_clock = ADV7511_INPUT_CLOCK_DDR; - else - return -EINVAL; - - if (config->input_colorspace == HDMI_COLORSPACE_YUV422 || - config->input_clock != ADV7511_INPUT_CLOCK_1X) { - ret = of_property_read_u32(np, "adi,input-style", - &config->input_style); - if (ret) - return ret; - - if (config->input_style < 1 || config->input_style > 3) - return -EINVAL; - - ret = of_property_read_string(np, "adi,input-justification", - &str); - if (ret < 0) - return ret; - - if (!strcmp(str, "left")) - config->input_justification = - ADV7511_INPUT_JUSTIFICATION_LEFT; - else if (!strcmp(str, "evenly")) - config->input_justification = - ADV7511_INPUT_JUSTIFICATION_EVENLY; - else if (!strcmp(str, "right")) - config->input_justification = - ADV7511_INPUT_JUSTIFICATION_RIGHT; - else - return -EINVAL; - - } else { - config->input_style = 1; - config->input_justification = ADV7511_INPUT_JUSTIFICATION_LEFT; - } - - of_property_read_u32(np, "adi,clock-delay", &config->clock_delay); - if (config->clock_delay < -1200 || config->clock_delay > 1600) - return -EINVAL; - - config->embedded_sync = of_property_read_bool(np, "adi,embedded-sync"); - - /* Hardcode the sync pulse configurations for now. */ - config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; - config->vsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH; - config->hsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH; - - return 0; -} - -static const int edid_i2c_addr = 0x7e; -static const int packet_i2c_addr = 0x70; -static const int cec_i2c_addr = 0x78; - -static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) -{ - struct adv7511_link_config link_config; - struct adv7511 *adv7511; - struct device *dev = &i2c->dev; - unsigned int val; - int ret; - - if (!dev->of_node) - return -EINVAL; - - adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); - if (!adv7511) - return -ENOMEM; - - adv7511->powered = false; - adv7511->status = connector_status_disconnected; - - ret = adv7511_parse_dt(dev->of_node, &link_config); - if (ret) - return ret; - - /* - * The power down GPIO is optional. If present, toggle it from active to - * inactive to wake up the encoder. - */ - adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); - if (IS_ERR(adv7511->gpio_pd)) - return PTR_ERR(adv7511->gpio_pd); - - if (adv7511->gpio_pd) { - mdelay(5); - gpiod_set_value_cansleep(adv7511->gpio_pd, 0); - } - - adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); - if (IS_ERR(adv7511->regmap)) - return PTR_ERR(adv7511->regmap); - - ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); - if (ret) - return ret; - dev_dbg(dev, "Rev. %d\n", val); - - ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, - ARRAY_SIZE(adv7511_fixed_registers)); - if (ret) - return ret; - - regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); - regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, - packet_i2c_addr); - regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); - adv7511_packet_disable(adv7511, 0xffff); - - adv7511->i2c_main = i2c; - adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); - if (!adv7511->i2c_edid) - return -ENOMEM; - - if (i2c->irq) { - init_waitqueue_head(&adv7511->wq); - - ret = devm_request_threaded_irq(dev, i2c->irq, NULL, - adv7511_irq_handler, - IRQF_ONESHOT, dev_name(dev), - adv7511); - if (ret) - goto err_i2c_unregister_device; - } - - /* CEC is unused for now */ - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, - ADV7511_CEC_CTRL_POWER_DOWN); - - adv7511_power_off(adv7511); - - i2c_set_clientdata(i2c, adv7511); - - adv7511_set_link_config(adv7511, &link_config); - - return 0; - -err_i2c_unregister_device: - i2c_unregister_device(adv7511->i2c_edid); - - return ret; -} - -static int adv7511_remove(struct i2c_client *i2c) -{ - struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - - i2c_unregister_device(adv7511->i2c_edid); - - kfree(adv7511->edid); - - return 0; -} - -static int adv7511_encoder_init(struct i2c_client *i2c, struct drm_device *dev, - struct drm_encoder_slave *encoder) -{ - - struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - - encoder->slave_priv = adv7511; - encoder->slave_funcs = &adv7511_encoder_funcs; - - adv7511->encoder = &encoder->base; - - return 0; -} - -static const struct i2c_device_id adv7511_i2c_ids[] = { - { "adv7511", 0 }, - { "adv7511w", 0 }, - { "adv7513", 0 }, - { } -}; -MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); - -static const struct of_device_id adv7511_of_ids[] = { - { .compatible = "adi,adv7511", }, - { .compatible = "adi,adv7511w", }, - { .compatible = "adi,adv7513", }, - { } -}; -MODULE_DEVICE_TABLE(of, adv7511_of_ids); - -static struct drm_i2c_encoder_driver adv7511_driver = { - .i2c_driver = { - .driver = { - .name = "adv7511", - .of_match_table = adv7511_of_ids, - }, - .id_table = adv7511_i2c_ids, - .probe = adv7511_probe, - .remove = adv7511_remove, - }, - - .encoder_init = adv7511_encoder_init, -}; - -static int __init adv7511_init(void) -{ - return drm_i2c_encoder_register(THIS_MODULE, &adv7511_driver); -} -module_init(adv7511_init); - -static void __exit adv7511_exit(void) -{ - drm_i2c_encoder_unregister(&adv7511_driver); -} -module_exit(adv7511_exit); - -MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); -MODULE_DESCRIPTION("ADV7511 HDMI transmitter driver"); -MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511.h index 38515b3..fccb86e 100644 --- a/drivers/gpu/drm/i2c/adv7511.h +++ b/drivers/gpu/drm/i2c/adv7511.h @@ -10,6 +10,8 @@ #define __DRM_I2C_ADV7511_H__ #include <linux/hdmi.h> +#include <linux/gpio/consumer.h> +#include <drm/drmP.h> #define ADV7511_REG_CHIP_REVISION 0x00 #define ADV7511_REG_N0 0x01 @@ -286,4 +288,43 @@ struct adv7511_video_config { struct hdmi_avi_infoframe avi_infoframe; }; +struct adv7511 { + struct i2c_client *i2c_main; + struct i2c_client *i2c_edid; + + struct regmap *regmap; + struct regmap *packet_memory_regmap; + enum drm_connector_status status; + bool powered; + + unsigned int f_tmds; + unsigned int f_audio; + + unsigned int audio_source; + + unsigned int current_edid_segment; + uint8_t edid_buf[256]; + bool edid_read; + + wait_queue_head_t wq; + struct drm_encoder *encoder; + + bool embedded_sync; + enum adv7511_sync_polarity vsync_polarity; + enum adv7511_sync_polarity hsync_polarity; + bool rgb; + + struct edid *edid; + + struct gpio_desc *gpio_pd; +}; + +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet); +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet); + +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO +int adv7511_audio_init(struct device *dev); +void adv7511_audio_exit(struct device *dev); +#endif + #endif /* __DRM_I2C_ADV7511_H__ */ diff --git a/drivers/gpu/drm/i2c/adv7511_audio.c b/drivers/gpu/drm/i2c/adv7511_audio.c new file mode 100644 index 0000000..5562ed5 --- /dev/null +++ b/drivers/gpu/drm/i2c/adv7511_audio.c @@ -0,0 +1,310 @@ +/* + * Analog Devices ADV7511 HDMI transmitter driver + * + * Copyright 2012 Analog Devices Inc. + * + * Licensed under the GPL-2. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/pm.h> +#include <linux/i2c.h> +#include <linux/spi/spi.h> +#include <linux/slab.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/initval.h> +#include <sound/tlv.h> + +#include "adv7511.h" + +static const struct snd_soc_dapm_widget adv7511_dapm_widgets[] = { + SND_SOC_DAPM_OUTPUT("TMDS"), + SND_SOC_DAPM_AIF_IN("AIFIN", "Playback", 0, SND_SOC_NOPM, 0, 0), +}; + +static const struct snd_soc_dapm_route adv7511_routes[] = { + { "TMDS", NULL, "AIFIN" }, +}; + +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs, + unsigned int *cts, unsigned int *n) +{ + switch (fs) { + case 32000: + *n = 4096; + break; + case 44100: + *n = 6272; + break; + case 48000: + *n = 6144; + break; + } + + *cts = ((f_tmds * *n) / (128 * fs)) * 1000; +} + +static int adv7511_update_cts_n(struct adv7511 *adv7511) +{ + unsigned int cts = 0; + unsigned int n = 0; + + adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n); + + regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf); + regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff); + + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0, + (cts >> 16) & 0xf); + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1, + (cts >> 8) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2, + cts & 0xff); + + return 0; +} + +static int adv7511_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_codec *codec = rtd->codec; + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + unsigned int rate; + unsigned int len; + + switch (params_rate(params)) { + case 32000: + rate = ADV7511_SAMPLE_FREQ_32000; + break; + case 44100: + rate = ADV7511_SAMPLE_FREQ_44100; + break; + case 48000: + rate = ADV7511_SAMPLE_FREQ_48000; + break; + case 88200: + rate = ADV7511_SAMPLE_FREQ_88200; + break; + case 96000: + rate = ADV7511_SAMPLE_FREQ_96000; + break; + case 176400: + rate = ADV7511_SAMPLE_FREQ_176400; + break; + case 192000: + rate = ADV7511_SAMPLE_FREQ_192000; + break; + default: + return -EINVAL; + } + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S16_LE: + len = ADV7511_I2S_SAMPLE_LEN_16; + break; + case SNDRV_PCM_FORMAT_S18_3LE: + len = ADV7511_I2S_SAMPLE_LEN_18; + break; + case SNDRV_PCM_FORMAT_S20_3LE: + len = ADV7511_I2S_SAMPLE_LEN_20; + break; + case SNDRV_PCM_FORMAT_S24_LE: + len = ADV7511_I2S_SAMPLE_LEN_24; + break; + default: + return -EINVAL; + } + + adv7511->f_audio = params_rate(params); + + adv7511_update_cts_n(adv7511); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3, + ADV7511_AUDIO_CFG3_LEN_MASK, len); + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, + ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4); + + return 0; +} + +static int adv7511_set_dai_fmt(struct snd_soc_dai *codec_dai, + unsigned int fmt) +{ + struct snd_soc_codec *codec = codec_dai->codec; + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + unsigned int audio_source, i2s_format = 0; + unsigned int invert_clock; + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_I2S; + break; + case SND_SOC_DAIFMT_RIGHT_J: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_RIGHT_J; + break; + case SND_SOC_DAIFMT_LEFT_J: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_LEFT_J; + break; + case SND_SOC_DAIFMT_SPDIF: + audio_source = ADV7511_AUDIO_SOURCE_SPDIF; + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBS_CFS: + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + invert_clock = 0; + break; + case SND_SOC_DAIFMT_IB_NF: + invert_clock = 1; + break; + default: + return -EINVAL; + } + + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70, + audio_source << 4); + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6), + invert_clock << 6); + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03, + i2s_format); + + adv7511->audio_source = audio_source; + + return 0; +} + +static int adv7511_set_bias_level(struct snd_soc_codec *codec, + enum snd_soc_bias_level level) +{ + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + + switch (level) { + case SND_SOC_BIAS_ON: + switch (adv7511->audio_source) { + case ADV7511_AUDIO_SOURCE_I2S: + break; + case ADV7511_AUDIO_SOURCE_SPDIF: + regmap_update_bits(adv7511->regmap, + ADV7511_REG_AUDIO_CONFIG, BIT(7), + BIT(7)); + break; + } + break; + case SND_SOC_BIAS_PREPARE: + if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_STANDBY) { + adv7511_packet_enable(adv7511, + ADV7511_PACKET_ENABLE_AUDIO_SAMPLE); + adv7511_packet_enable(adv7511, + ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME); + adv7511_packet_enable(adv7511, + ADV7511_PACKET_ENABLE_N_CTS); + } else { + adv7511_packet_disable(adv7511, + ADV7511_PACKET_ENABLE_AUDIO_SAMPLE); + adv7511_packet_disable(adv7511, + ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME); + adv7511_packet_disable(adv7511, + ADV7511_PACKET_ENABLE_N_CTS); + } + break; + case SND_SOC_BIAS_STANDBY: + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, + BIT(7), 0); + break; + case SND_SOC_BIAS_OFF: + break; + } + return 0; +} + +#define ADV7511_RATES (SNDRV_PCM_RATE_32000 |\ + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |\ + SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |\ + SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000) + +#define ADV7511_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S18_3LE |\ + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE) + +static const struct snd_soc_dai_ops adv7511_dai_ops = { + .hw_params = adv7511_hw_params, + /*.set_sysclk = adv7511_set_dai_sysclk,*/ + .set_fmt = adv7511_set_dai_fmt, +}; + +static struct snd_soc_dai_driver adv7511_dai = { + .name = "adv7511", + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = ADV7511_RATES, + .formats = ADV7511_FORMATS, + }, + .ops = &adv7511_dai_ops, +}; + +static int adv7511_suspend(struct snd_soc_codec *codec) +{ + return adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF); +} + +static int adv7511_resume(struct snd_soc_codec *codec) +{ + return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY); +} + +static int adv7511_probe(struct snd_soc_codec *codec) +{ + return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY); +} + +static int adv7511_remove(struct snd_soc_codec *codec) +{ + adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF); + return 0; +} + +static struct snd_soc_codec_driver adv7511_codec_driver = { + .probe = adv7511_probe, + .remove = adv7511_remove, + .suspend = adv7511_suspend, + .resume = adv7511_resume, + .set_bias_level = adv7511_set_bias_level, + + .dapm_widgets = adv7511_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(adv7511_dapm_widgets), + .dapm_routes = adv7511_routes, + .num_dapm_routes = ARRAY_SIZE(adv7511_routes), +}; + +int adv7511_audio_init(struct device *dev) +{ + return snd_soc_register_codec(dev, &adv7511_codec_driver, + &adv7511_dai, 1); +} + +void adv7511_audio_exit(struct device *dev) +{ + snd_soc_unregister_codec(dev); +} diff --git a/drivers/gpu/drm/i2c/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 index 0000000..d54256a --- /dev/null +++ b/drivers/gpu/drm/i2c/adv7511_core.c @@ -0,0 +1,1005 @@ +/* + * Analog Devices ADV7511 HDMI transmitter driver + * + * Copyright 2012 Analog Devices Inc. + * + * Licensed under the GPL-2. + */ + +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_edid.h> +#include <drm/drm_encoder_slave.h> + +#include "adv7511.h" + +static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder) +{ + return to_encoder_slave(encoder)->slave_priv; +} + +/* ADI recommended values for proper operation. */ +static const struct reg_sequence adv7511_fixed_registers[] = { + { 0x98, 0x03 }, + { 0x9a, 0xe0 }, + { 0x9c, 0x30 }, + { 0x9d, 0x61 }, + { 0xa2, 0xa4 }, + { 0xa3, 0xa4 }, + { 0xe0, 0xd0 }, + { 0xf9, 0x00 }, + { 0x55, 0x02 }, +}; + +/* ----------------------------------------------------------------------------- + * Register access + */ + +static const uint8_t adv7511_register_defaults[] = { + 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 00 */ + 0x00, 0x00, 0x01, 0x0e, 0xbc, 0x18, 0x01, 0x13, + 0x25, 0x37, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 10 */ + 0x46, 0x62, 0x04, 0xa8, 0x00, 0x00, 0x1c, 0x84, + 0x1c, 0xbf, 0x04, 0xa8, 0x1e, 0x70, 0x02, 0x1e, /* 20 */ + 0x00, 0x00, 0x04, 0xa8, 0x08, 0x12, 0x1b, 0xac, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 */ + 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0xb0, + 0x00, 0x50, 0x90, 0x7e, 0x79, 0x70, 0x00, 0x00, /* 40 */ + 0x00, 0xa8, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x02, 0x0d, 0x00, 0x00, 0x00, 0x00, /* 50 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 60 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 70 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 80 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0xc0, 0x00, 0x00, 0x00, /* 90 */ + 0x0b, 0x02, 0x00, 0x18, 0x5a, 0x60, 0x00, 0x00, + 0x00, 0x00, 0x80, 0x80, 0x08, 0x04, 0x00, 0x00, /* a0 */ + 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x40, 0x14, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* b0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* c0 */ + 0x00, 0x03, 0x00, 0x00, 0x02, 0x00, 0x01, 0x04, + 0x30, 0xff, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00, /* d0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x01, + 0x80, 0x75, 0x00, 0x00, 0x60, 0x00, 0x00, 0x00, /* e0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x75, 0x11, 0x00, /* f0 */ + 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +}; + +static bool adv7511_register_volatile(struct device *dev, unsigned int reg) +{ + switch (reg) { + case ADV7511_REG_CHIP_REVISION: + case ADV7511_REG_SPDIF_FREQ: + case ADV7511_REG_CTS_AUTOMATIC1: + case ADV7511_REG_CTS_AUTOMATIC2: + case ADV7511_REG_VIC_DETECTED: + case ADV7511_REG_VIC_SEND: + case ADV7511_REG_AUX_VIC_DETECTED: + case ADV7511_REG_STATUS: + case ADV7511_REG_GC(1): + case ADV7511_REG_INT(0): + case ADV7511_REG_INT(1): + case ADV7511_REG_PLL_STATUS: + case ADV7511_REG_AN(0): + case ADV7511_REG_AN(1): + case ADV7511_REG_AN(2): + case ADV7511_REG_AN(3): + case ADV7511_REG_AN(4): + case ADV7511_REG_AN(5): + case ADV7511_REG_AN(6): + case ADV7511_REG_AN(7): + case ADV7511_REG_HDCP_STATUS: + case ADV7511_REG_BCAPS: + case ADV7511_REG_BKSV(0): + case ADV7511_REG_BKSV(1): + case ADV7511_REG_BKSV(2): + case ADV7511_REG_BKSV(3): + case ADV7511_REG_BKSV(4): + case ADV7511_REG_DDC_STATUS: + case ADV7511_REG_EDID_READ_CTRL: + case ADV7511_REG_BSTATUS(0): + case ADV7511_REG_BSTATUS(1): + case ADV7511_REG_CHIP_ID_HIGH: + case ADV7511_REG_CHIP_ID_LOW: + return true; + } + + return false; +} + +static const struct regmap_config adv7511_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = 0xff, + .cache_type = REGCACHE_RBTREE, + .reg_defaults_raw = adv7511_register_defaults, + .num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults), + + .volatile_reg = adv7511_register_volatile, +}; + +/* ----------------------------------------------------------------------------- + * Hardware configuration + */ + +static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable, + const uint16_t *coeff, + unsigned int scaling_factor) +{ + unsigned int i; + + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), + ADV7511_CSC_UPDATE_MODE, ADV7511_CSC_UPDATE_MODE); + + if (enable) { + for (i = 0; i < 12; ++i) { + regmap_update_bits(adv7511->regmap, + ADV7511_REG_CSC_UPPER(i), + 0x1f, coeff[i] >> 8); + regmap_write(adv7511->regmap, + ADV7511_REG_CSC_LOWER(i), + coeff[i] & 0xff); + } + } + + if (enable) + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), + 0xe0, 0x80 | (scaling_factor << 5)); + else + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), + 0x80, 0x00); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), + ADV7511_CSC_UPDATE_MODE, 0); +} + +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet) +{ + if (packet & 0xff) + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, + packet, 0xff); + + if (packet & 0xff00) { + packet >>= 8; + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, + packet, 0xff); + } + + return 0; +} + +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet) +{ + if (packet & 0xff) + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, + packet, 0x00); + + if (packet & 0xff00) { + packet >>= 8; + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, + packet, 0x00); + } + + return 0; +} + +/* Coefficients for adv7511 color space conversion */ +static const uint16_t adv7511_csc_ycbcr_to_rgb[] = { + 0x0734, 0x04ad, 0x0000, 0x1c1b, + 0x1ddc, 0x04ad, 0x1f24, 0x0135, + 0x0000, 0x04ad, 0x087c, 0x1b77, +}; + +static void adv7511_set_config_csc(struct adv7511 *adv7511, + struct drm_connector *connector, + bool rgb) +{ + struct adv7511_video_config config; + bool output_format_422, output_format_ycbcr; + unsigned int mode; + uint8_t infoframe[17]; + + if (adv7511->edid) + config.hdmi_mode = drm_detect_hdmi_monitor(adv7511->edid); + else + config.hdmi_mode = false; + + hdmi_avi_infoframe_init(&config.avi_infoframe); + + config.avi_infoframe.scan_mode = HDMI_SCAN_MODE_UNDERSCAN; + + if (rgb) { + config.csc_enable = false; + config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB; + } else { + config.csc_scaling_factor = ADV7511_CSC_SCALING_4; + config.csc_coefficents = adv7511_csc_ycbcr_to_rgb; + + if ((connector->display_info.color_formats & + DRM_COLOR_FORMAT_YCRCB422) && + config.hdmi_mode) { + config.csc_enable = false; + config.avi_infoframe.colorspace = + HDMI_COLORSPACE_YUV422; + } else { + config.csc_enable = true; + config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB; + } + } + + if (config.hdmi_mode) { + mode = ADV7511_HDMI_CFG_MODE_HDMI; + + switch (config.avi_infoframe.colorspace) { + case HDMI_COLORSPACE_YUV444: + output_format_422 = false; + output_format_ycbcr = true; + break; + case HDMI_COLORSPACE_YUV422: + output_format_422 = true; + output_format_ycbcr = true; + break; + default: + output_format_422 = false; + output_format_ycbcr = false; + break; + } + } else { + mode = ADV7511_HDMI_CFG_MODE_DVI; + output_format_422 = false; + output_format_ycbcr = false; + } + + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); + + adv7511_set_colormap(adv7511, config.csc_enable, + config.csc_coefficents, + config.csc_scaling_factor); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x81, + (output_format_422 << 7) | output_format_ycbcr); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, + ADV7511_HDMI_CFG_MODE_MASK, mode); + + hdmi_avi_infoframe_pack(&config.avi_infoframe, infoframe, + sizeof(infoframe)); + + /* The AVI infoframe id is not configurable */ + regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION, + infoframe + 1, sizeof(infoframe) - 1); + + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); +} + +static void adv7511_set_link_config(struct adv7511 *adv7511, + const struct adv7511_link_config *config) +{ + /* + * The input style values documented in the datasheet don't match the + * hardware register field values :-( + */ + static const unsigned int input_styles[4] = { 0, 2, 1, 3 }; + + unsigned int clock_delay; + unsigned int color_depth; + unsigned int input_id; + + clock_delay = (config->clock_delay + 1200) / 400; + color_depth = config->input_color_depth == 8 ? 3 + : (config->input_color_depth == 10 ? 1 : 2); + + /* TODO Support input ID 6 */ + if (config->input_colorspace != HDMI_COLORSPACE_YUV422) + input_id = config->input_clock == ADV7511_INPUT_CLOCK_DDR + ? 5 : 0; + else if (config->input_clock == ADV7511_INPUT_CLOCK_DDR) + input_id = config->embedded_sync ? 8 : 7; + else if (config->input_clock == ADV7511_INPUT_CLOCK_2X) + input_id = config->embedded_sync ? 4 : 3; + else + input_id = config->embedded_sync ? 2 : 1; + + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, 0xf, + input_id); + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x7e, + (color_depth << 4) | + (input_styles[config->input_style] << 2)); + regmap_write(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG2, + config->input_justification << 3); + regmap_write(adv7511->regmap, ADV7511_REG_TIMING_GEN_SEQ, + config->sync_pulse << 2); + + regmap_write(adv7511->regmap, 0xba, clock_delay << 5); + + adv7511->embedded_sync = config->embedded_sync; + adv7511->hsync_polarity = config->hsync_polarity; + adv7511->vsync_polarity = config->vsync_polarity; + adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB; +} + +static void adv7511_power_on(struct adv7511 *adv7511) +{ + adv7511->current_edid_segment = -1; + + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, 0); + if (adv7511->i2c_main->irq) { + /* + * Documentation says the INT_ENABLE registers are reset in + * POWER_DOWN mode. My 7511w preserved the bits, however. + * Still, let's be safe and stick to the documentation. + */ + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), + ADV7511_INT0_EDID_READY); + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), + ADV7511_INT1_DDC_ERROR); + } + + /* + * Per spec it is allowed to pulse the HPD signal to indicate that the + * EDID information has changed. Some monitors do this when they wakeup + * from standby or are enabled. When the HPD goes low the adv7511 is + * reset and the outputs are disabled which might cause the monitor to + * go to standby again. To avoid this we ignore the HPD pin for the + * first few seconds after enabling the output. + */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, + ADV7511_REG_POWER2_HPD_SRC_MASK, + ADV7511_REG_POWER2_HPD_SRC_NONE); + + /* + * Most of the registers are reset during power down or when HPD is low. + */ + regcache_sync(adv7511->regmap); + + adv7511->powered = true; +} + +static void adv7511_power_off(struct adv7511 *adv7511) +{ + /* TODO: setup additional power down modes */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, + ADV7511_POWER_POWER_DOWN); + regcache_mark_dirty(adv7511->regmap); + + adv7511->powered = false; +} + +/* ----------------------------------------------------------------------------- + * Interrupt and hotplug detection + */ + +static bool adv7511_hpd(struct adv7511 *adv7511) +{ + unsigned int irq0; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); + if (ret < 0) + return false; + + if (irq0 & ADV7511_INT0_HPD) { + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), + ADV7511_INT0_HPD); + return true; + } + + return false; +} + +static int adv7511_irq_process(struct adv7511 *adv7511) +{ + unsigned int irq0, irq1; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); + if (ret < 0) + return ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); + if (ret < 0) + return ret; + + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); + regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); + + if (irq0 & ADV7511_INT0_HPD && adv7511->encoder) + drm_helper_hpd_irq_event(adv7511->encoder->dev); + + if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { + adv7511->edid_read = true; + + if (adv7511->i2c_main->irq) + wake_up_all(&adv7511->wq); + } + + return 0; +} + +static irqreturn_t adv7511_irq_handler(int irq, void *devid) +{ + struct adv7511 *adv7511 = devid; + int ret; + + ret = adv7511_irq_process(adv7511); + return ret < 0 ? IRQ_NONE : IRQ_HANDLED; +} + +/* ----------------------------------------------------------------------------- + * EDID retrieval + */ + +static int adv7511_wait_for_edid(struct adv7511 *adv7511, int timeout) +{ + int ret; + + if (adv7511->i2c_main->irq) { + ret = wait_event_interruptible_timeout(adv7511->wq, + adv7511->edid_read, msecs_to_jiffies(timeout)); + } else { + for (; timeout > 0; timeout -= 25) { + ret = adv7511_irq_process(adv7511); + if (ret < 0) + break; + + if (adv7511->edid_read) + break; + + msleep(25); + } + } + + return adv7511->edid_read ? 0 : -EIO; +} + +static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block, + size_t len) +{ + struct adv7511 *adv7511 = data; + struct i2c_msg xfer[2]; + uint8_t offset; + unsigned int i; + int ret; + + if (len > 128) + return -EINVAL; + + if (adv7511->current_edid_segment != block / 2) { + unsigned int status; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS, + &status); + if (ret < 0) + return ret; + + if (status != 2) { + adv7511->edid_read = false; + regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, + block); + ret = adv7511_wait_for_edid(adv7511, 200); + if (ret < 0) + return ret; + } + + /* Break this apart, hopefully more I2C controllers will + * support 64 byte transfers than 256 byte transfers + */ + + xfer[0].addr = adv7511->i2c_edid->addr; + xfer[0].flags = 0; + xfer[0].len = 1; + xfer[0].buf = &offset; + xfer[1].addr = adv7511->i2c_edid->addr; + xfer[1].flags = I2C_M_RD; + xfer[1].len = 64; + xfer[1].buf = adv7511->edid_buf; + + offset = 0; + + for (i = 0; i < 4; ++i) { + ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, + ARRAY_SIZE(xfer)); + if (ret < 0) + return ret; + else if (ret != 2) + return -EIO; + + xfer[1].buf += 64; + offset += 64; + } + + adv7511->current_edid_segment = block / 2; + } + + if (block % 2 == 0) + memcpy(buf, adv7511->edid_buf, len); + else + memcpy(buf, adv7511->edid_buf + 128, len); + + return 0; +} + +/* ----------------------------------------------------------------------------- + * Encoder operations + */ + +static int adv7511_get_modes(struct drm_encoder *encoder, + struct drm_connector *connector) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + struct edid *edid; + unsigned int count; + + /* Reading the EDID only works if the device is powered */ + if (!adv7511->powered) { + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, 0); + if (adv7511->i2c_main->irq) { + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), + ADV7511_INT0_EDID_READY); + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), + ADV7511_INT1_DDC_ERROR); + } + adv7511->current_edid_segment = -1; + } + + edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); + + if (!adv7511->powered) + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, + ADV7511_POWER_POWER_DOWN); + + kfree(adv7511->edid); + adv7511->edid = edid; + if (!edid) + return 0; + + drm_mode_connector_update_edid_property(connector, edid); + count = drm_add_edid_modes(connector, edid); + + adv7511_set_config_csc(adv7511, connector, adv7511->rgb); + + return count; +} + +static void adv7511_encoder_dpms(struct drm_encoder *encoder, int mode) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + + if (mode == DRM_MODE_DPMS_ON) + adv7511_power_on(adv7511); + else + adv7511_power_off(adv7511); +} + +static enum drm_connector_status +adv7511_encoder_detect(struct drm_encoder *encoder, + struct drm_connector *connector) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + enum drm_connector_status status; + unsigned int val; + bool hpd; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val); + if (ret < 0) + return connector_status_disconnected; + + if (val & ADV7511_STATUS_HPD) + status = connector_status_connected; + else + status = connector_status_disconnected; + + hpd = adv7511_hpd(adv7511); + + /* The chip resets itself when the cable is disconnected, so in case + * there is a pending HPD interrupt and the cable is connected there was + * at least one transition from disconnected to connected and the chip + * has to be reinitialized. */ + if (status == connector_status_connected && hpd && adv7511->powered) { + regcache_mark_dirty(adv7511->regmap); + adv7511_power_on(adv7511); + adv7511_get_modes(encoder, connector); + if (adv7511->status == connector_status_connected) + status = connector_status_disconnected; + } else { + /* Renable HPD sensing */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, + ADV7511_REG_POWER2_HPD_SRC_MASK, + ADV7511_REG_POWER2_HPD_SRC_BOTH); + } + + adv7511->status = status; + return status; +} + +static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, + struct drm_display_mode *mode) +{ + if (mode->clock > 165000) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + +static void adv7511_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adj_mode) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + unsigned int low_refresh_rate; + unsigned int hsync_polarity = 0; + unsigned int vsync_polarity = 0; + + if (adv7511->embedded_sync) { + unsigned int hsync_offset, hsync_len; + unsigned int vsync_offset, vsync_len; + + hsync_offset = adj_mode->crtc_hsync_start - + adj_mode->crtc_hdisplay; + vsync_offset = adj_mode->crtc_vsync_start - + adj_mode->crtc_vdisplay; + hsync_len = adj_mode->crtc_hsync_end - + adj_mode->crtc_hsync_start; + vsync_len = adj_mode->crtc_vsync_end - + adj_mode->crtc_vsync_start; + + /* The hardware vsync generator has a off-by-one bug */ + vsync_offset += 1; + + regmap_write(adv7511->regmap, ADV7511_REG_HSYNC_PLACEMENT_MSB, + ((hsync_offset >> 10) & 0x7) << 5); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(0), + (hsync_offset >> 2) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(1), + ((hsync_offset & 0x3) << 6) | + ((hsync_len >> 4) & 0x3f)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(2), + ((hsync_len & 0xf) << 4) | + ((vsync_offset >> 6) & 0xf)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(3), + ((vsync_offset & 0x3f) << 2) | + ((vsync_len >> 8) & 0x3)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(4), + vsync_len & 0xff); + + hsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PHSYNC); + vsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PVSYNC); + } else { + enum adv7511_sync_polarity mode_hsync_polarity; + enum adv7511_sync_polarity mode_vsync_polarity; + + /** + * If the input signal is always low or always high we want to + * invert or let it passthrough depending on the polarity of the + * current mode. + **/ + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC) + mode_hsync_polarity = ADV7511_SYNC_POLARITY_LOW; + else + mode_hsync_polarity = ADV7511_SYNC_POLARITY_HIGH; + + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC) + mode_vsync_polarity = ADV7511_SYNC_POLARITY_LOW; + else + mode_vsync_polarity = ADV7511_SYNC_POLARITY_HIGH; + + if (adv7511->hsync_polarity != mode_hsync_polarity && + adv7511->hsync_polarity != + ADV7511_SYNC_POLARITY_PASSTHROUGH) + hsync_polarity = 1; + + if (adv7511->vsync_polarity != mode_vsync_polarity && + adv7511->vsync_polarity != + ADV7511_SYNC_POLARITY_PASSTHROUGH) + vsync_polarity = 1; + } + + if (mode->vrefresh <= 24000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ; + else if (mode->vrefresh <= 25000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ; + else if (mode->vrefresh <= 30000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ; + else + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE; + + regmap_update_bits(adv7511->regmap, 0xfb, + 0x6, low_refresh_rate << 1); + regmap_update_bits(adv7511->regmap, 0x17, + 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); + + /* + * TODO Test first order 4:2:2 to 4:4:4 up conversion method, which is + * supposed to give better results. + */ + + adv7511->f_tmds = mode->clock; +} + +static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { + .dpms = adv7511_encoder_dpms, + .mode_valid = adv7511_encoder_mode_valid, + .mode_set = adv7511_encoder_mode_set, + .detect = adv7511_encoder_detect, + .get_modes = adv7511_get_modes, +}; + +/* ----------------------------------------------------------------------------- + * Probe & remove + */ + +static int adv7511_parse_dt(struct device_node *np, + struct adv7511_link_config *config) +{ + const char *str; + int ret; + + memset(config, 0, sizeof(*config)); + + of_property_read_u32(np, "adi,input-depth", &config->input_color_depth); + if (config->input_color_depth != 8 && config->input_color_depth != 10 && + config->input_color_depth != 12) + return -EINVAL; + + ret = of_property_read_string(np, "adi,input-colorspace", &str); + if (ret < 0) + return ret; + + if (!strcmp(str, "rgb")) + config->input_colorspace = HDMI_COLORSPACE_RGB; + else if (!strcmp(str, "yuv422")) + config->input_colorspace = HDMI_COLORSPACE_YUV422; + else if (!strcmp(str, "yuv444")) + config->input_colorspace = HDMI_COLORSPACE_YUV444; + else + return -EINVAL; + + ret = of_property_read_string(np, "adi,input-clock", &str); + if (ret < 0) + return ret; + + if (!strcmp(str, "1x")) + config->input_clock = ADV7511_INPUT_CLOCK_1X; + else if (!strcmp(str, "2x")) + config->input_clock = ADV7511_INPUT_CLOCK_2X; + else if (!strcmp(str, "ddr")) + config->input_clock = ADV7511_INPUT_CLOCK_DDR; + else + return -EINVAL; + + if (config->input_colorspace == HDMI_COLORSPACE_YUV422 || + config->input_clock != ADV7511_INPUT_CLOCK_1X) { + ret = of_property_read_u32(np, "adi,input-style", + &config->input_style); + if (ret) + return ret; + + if (config->input_style < 1 || config->input_style > 3) + return -EINVAL; + + ret = of_property_read_string(np, "adi,input-justification", + &str); + if (ret < 0) + return ret; + + if (!strcmp(str, "left")) + config->input_justification = + ADV7511_INPUT_JUSTIFICATION_LEFT; + else if (!strcmp(str, "evenly")) + config->input_justification = + ADV7511_INPUT_JUSTIFICATION_EVENLY; + else if (!strcmp(str, "right")) + config->input_justification = + ADV7511_INPUT_JUSTIFICATION_RIGHT; + else + return -EINVAL; + + } else { + config->input_style = 1; + config->input_justification = ADV7511_INPUT_JUSTIFICATION_LEFT; + } + + of_property_read_u32(np, "adi,clock-delay", &config->clock_delay); + if (config->clock_delay < -1200 || config->clock_delay > 1600) + return -EINVAL; + + config->embedded_sync = of_property_read_bool(np, "adi,embedded-sync"); + + /* Hardcode the sync pulse configurations for now. */ + config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; + config->vsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH; + config->hsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH; + + return 0; +} + +static const int edid_i2c_addr = 0x7e; +static const int packet_i2c_addr = 0x70; +static const int cec_i2c_addr = 0x78; + +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) +{ + struct adv7511_link_config link_config; + struct adv7511 *adv7511; + struct device *dev = &i2c->dev; + unsigned int val; + int ret; + + if (!dev->of_node) + return -EINVAL; + + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); + if (!adv7511) + return -ENOMEM; + + adv7511->powered = false; + adv7511->status = connector_status_disconnected; + + ret = adv7511_parse_dt(dev->of_node, &link_config); + if (ret) + return ret; + + /* + * The power down GPIO is optional. If present, toggle it from active to + * inactive to wake up the encoder. + */ + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); + if (IS_ERR(adv7511->gpio_pd)) + return PTR_ERR(adv7511->gpio_pd); + + if (adv7511->gpio_pd) { + mdelay(5); + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); + } + + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); + if (IS_ERR(adv7511->regmap)) + return PTR_ERR(adv7511->regmap); + + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); + if (ret) + return ret; + dev_dbg(dev, "Rev. %d\n", val); + + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, + ARRAY_SIZE(adv7511_fixed_registers)); + if (ret) + return ret; + + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, + packet_i2c_addr); + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); + adv7511_packet_disable(adv7511, 0xffff); + + adv7511->i2c_main = i2c; + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); + if (!adv7511->i2c_edid) + return -ENOMEM; + + if (i2c->irq) { + init_waitqueue_head(&adv7511->wq); + + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, + adv7511_irq_handler, + IRQF_ONESHOT, dev_name(dev), + adv7511); + if (ret) + goto err_i2c_unregister_device; + } + + /* CEC is unused for now */ + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, + ADV7511_CEC_CTRL_POWER_DOWN); + + adv7511_power_off(adv7511); + + i2c_set_clientdata(i2c, adv7511); + +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO + adv7511_audio_init(&i2c->dev); +#endif + + adv7511_set_link_config(adv7511, &link_config); + + /* Enable HDMI mode */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, + ADV7511_HDMI_CFG_MODE_MASK, + ADV7511_HDMI_CFG_MODE_HDMI); + + return 0; + +err_i2c_unregister_device: + i2c_unregister_device(adv7511->i2c_edid); + + return ret; +} + +static int adv7511_remove(struct i2c_client *i2c) +{ + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); + + i2c_unregister_device(adv7511->i2c_edid); + + kfree(adv7511->edid); + + return 0; +} + +static int adv7511_encoder_init(struct i2c_client *i2c, struct drm_device *dev, + struct drm_encoder_slave *encoder) +{ + + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); + + encoder->slave_priv = adv7511; + encoder->slave_funcs = &adv7511_encoder_funcs; + + adv7511->encoder = &encoder->base; + + return 0; +} + +static const struct i2c_device_id adv7511_i2c_ids[] = { + { "adv7511", 0 }, + { "adv7511w", 0 }, + { "adv7513", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); + +static const struct of_device_id adv7511_of_ids[] = { + { .compatible = "adi,adv7511", }, + { .compatible = "adi,adv7511w", }, + { .compatible = "adi,adv7513", }, + { } +}; +MODULE_DEVICE_TABLE(of, adv7511_of_ids); + +static struct drm_i2c_encoder_driver adv7511_driver = { + .i2c_driver = { + .driver = { + .name = "adv7511", + .of_match_table = adv7511_of_ids, + }, + .id_table = adv7511_i2c_ids, + .probe = adv7511_probe, + .remove = adv7511_remove, + }, + + .encoder_init = adv7511_encoder_init, +}; + +static int __init adv7511_init(void) +{ + return drm_i2c_encoder_register(THIS_MODULE, &adv7511_driver); +} +module_init(adv7511_init); + +static void __exit adv7511_exit(void) +{ + drm_i2c_encoder_unregister(&adv7511_driver); +} +module_exit(adv7511_exit); + +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); +MODULE_DESCRIPTION("ADV7511 HDMI transmitter driver"); +MODULE_LICENSE("GPL"); diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 964b7de..539c091 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -33,6 +33,7 @@ struct snd_compr_stream; #define SND_SOC_DAIFMT_DSP_B 5 /* L data MSB during FRM LRC */ #define SND_SOC_DAIFMT_AC97 6 /* AC97 */ #define SND_SOC_DAIFMT_PDM 7 /* Pulse density modulation */ +#define SND_SOC_DAIFMT_SPDIF 8 /* SPDIF */ /* left and right justified also known as MSB and LSB respectively */ #define SND_SOC_DAIFMT_MSB SND_SOC_DAIFMT_LEFT_J -- 1.9.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu @ 2016-03-29 8:05 ` Archit Taneja 2016-03-29 10:52 ` Jose Abreu 2016-03-30 9:58 ` Emil Velikov ` (2 subsequent siblings) 3 siblings, 1 reply; 23+ messages in thread From: Archit Taneja @ 2016-03-29 8:05 UTC (permalink / raw) To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel, devicetree Cc: tixy, mark.rutland, laurent.pinchart+renesas, pawel.moll, ijc+devicetree, Vineet.Gupta1, Alexey.Brodkin, lgirdwood, robh+dt, nariman, yitian.bu, wsa+renesas, broonie, Maruthi.Bayyavarapu, galak, buyitian, alexander.deucher, tiwai, perex, CARLOS.PALMINHA Hi, On 03/28/2016 08:06 PM, Jose Abreu wrote: > This patch adds audio support for the ADV7511 HDMI transmitter > using ALSA SoC. > > The code was ported from Analog Devices linux tree from > commit 1770c4a1e32b ("Merge remote-tracking branch > 'xilinx/master' into xcomm_zynq"), which is available at: > - https://github.com/analogdevicesinc/linux/ > > The main core file was renamed from adv7511.c to adv7511_core.c > so that audio and video compile into a single adv7511.ko module > and to keep up with Analog Devices kernel tree. > > The audio can be disabled using menu-config so it is possible > to use only video mode. > > The HDMI mode is automatically started at boot and the audio > (when enabled) registers as a codec into ALSA. Is there a reason why we set the mode to HDMI at probe itself? Shouldn't it be okay to set the mode later once we read the EDID off the panel? Some more comments below. > > SPDIF DAI format was also added to ASoC as it is required > by adv7511 audio. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > --- > > No changes v1 -> v2. > > drivers/gpu/drm/i2c/Kconfig | 11 + > drivers/gpu/drm/i2c/Makefile | 2 + > drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------------- > drivers/gpu/drm/i2c/adv7511.h | 41 ++ > drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++ > include/sound/soc-dai.h | 1 + > 7 files changed, 1370 insertions(+), 1024 deletions(-) > delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c <snip> > + > +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) > +{ > + struct adv7511_link_config link_config; > + struct adv7511 *adv7511; > + struct device *dev = &i2c->dev; > + unsigned int val; > + int ret; > + > + if (!dev->of_node) > + return -EINVAL; > + > + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > + if (!adv7511) > + return -ENOMEM; > + > + adv7511->powered = false; > + adv7511->status = connector_status_disconnected; > + > + ret = adv7511_parse_dt(dev->of_node, &link_config); > + if (ret) > + return ret; > + > + /* > + * The power down GPIO is optional. If present, toggle it from active to > + * inactive to wake up the encoder. > + */ > + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); > + if (IS_ERR(adv7511->gpio_pd)) > + return PTR_ERR(adv7511->gpio_pd); > + > + if (adv7511->gpio_pd) { > + mdelay(5); > + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > + } > + > + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > + if (IS_ERR(adv7511->regmap)) > + return PTR_ERR(adv7511->regmap); > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > + if (ret) > + return ret; > + dev_dbg(dev, "Rev. %d\n", val); > + > + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > + ARRAY_SIZE(adv7511_fixed_registers)); > + if (ret) > + return ret; > + > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > + packet_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > + adv7511_packet_disable(adv7511, 0xffff); > + > + adv7511->i2c_main = i2c; > + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > + if (!adv7511->i2c_edid) > + return -ENOMEM; > + > + if (i2c->irq) { > + init_waitqueue_head(&adv7511->wq); > + > + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > + adv7511_irq_handler, > + IRQF_ONESHOT, dev_name(dev), > + adv7511); > + if (ret) > + goto err_i2c_unregister_device; > + } > + > + /* CEC is unused for now */ > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > + ADV7511_CEC_CTRL_POWER_DOWN); > + > + adv7511_power_off(adv7511); > + > + i2c_set_clientdata(i2c, adv7511); > + > +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > + adv7511_audio_init(&i2c->dev); > +#endif If we intend to have more audio funcs being used by the core in the future, it would be nice to have NOP audio funcs rather than having multiple #ifdef checks in the driver when CONFIG_DRM_I2C_ADV7511_AUDIO isn't set. > + > + adv7511_set_link_config(adv7511, &link_config); > + > + /* Enable HDMI mode */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > + ADV7511_HDMI_CFG_MODE_MASK, > + ADV7511_HDMI_CFG_MODE_HDMI); > + > + return 0; > + > +err_i2c_unregister_device: > + i2c_unregister_device(adv7511->i2c_edid); > + > + return ret; > +} > + > +static int adv7511_remove(struct i2c_client *i2c) > +{ > + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > + Are we missing a call to adv7511_audio_exit() here? Thanks, Archit -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-29 8:05 ` Archit Taneja @ 2016-03-29 10:52 ` Jose Abreu 2016-03-29 17:03 ` Archit Taneja 0 siblings, 1 reply; 23+ messages in thread From: Jose Abreu @ 2016-03-29 10:52 UTC (permalink / raw) To: Archit Taneja, Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel, devicetree Cc: tixy, mark.rutland, broonie, Alexey.Brodkin, lgirdwood, yitian.bu, wsa+renesas, laurent.pinchart+renesas, nariman, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, perex, tiwai, robh+dt, galak, alexander.deucher, CARLOS.PALMINHA Hi Archit, On 29-03-2016 09:05, Archit Taneja wrote: > Hi, > > On 03/28/2016 08:06 PM, Jose Abreu wrote: >> This patch adds audio support for the ADV7511 HDMI transmitter >> using ALSA SoC. >> >> The code was ported from Analog Devices linux tree from >> commit 1770c4a1e32b ("Merge remote-tracking branch >> 'xilinx/master' into xcomm_zynq"), which is available at: >> - https://github.com/analogdevicesinc/linux/ >> >> The main core file was renamed from adv7511.c to adv7511_core.c >> so that audio and video compile into a single adv7511.ko module >> and to keep up with Analog Devices kernel tree. >> >> The audio can be disabled using menu-config so it is possible >> to use only video mode. >> >> The HDMI mode is automatically started at boot and the audio >> (when enabled) registers as a codec into ALSA. > > Is there a reason why we set the mode to HDMI at probe itself? > Shouldn't it be okay to set the mode later once we read the > EDID off the panel? > > Some more comments below. > Well, when I was using this in kernel 3.18 (with an older version of the driver) I noticed that DVI mode was being used even when HDMI was connected so I forced the driver to start in HDMI mode. There were some changes in the driver so it is possible that this is no longer needed. Should I drop it? >> >> SPDIF DAI format was also added to ASoC as it is required >> by adv7511 audio. >> >> Signed-off-by: Jose Abreu <joabreu@synopsys.com> >> --- >> >> No changes v1 -> v2. >> >> drivers/gpu/drm/i2c/Kconfig | 11 + >> drivers/gpu/drm/i2c/Makefile | 2 + >> drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------------- >> drivers/gpu/drm/i2c/adv7511.h | 41 ++ >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++ >> include/sound/soc-dai.h | 1 + >> 7 files changed, 1370 insertions(+), 1024 deletions(-) >> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > > <snip> > >> + >> +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id >> *id) >> +{ >> + struct adv7511_link_config link_config; >> + struct adv7511 *adv7511; >> + struct device *dev = &i2c->dev; >> + unsigned int val; >> + int ret; >> + >> + if (!dev->of_node) >> + return -EINVAL; >> + >> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); >> + if (!adv7511) >> + return -ENOMEM; >> + >> + adv7511->powered = false; >> + adv7511->status = connector_status_disconnected; >> + >> + ret = adv7511_parse_dt(dev->of_node, &link_config); >> + if (ret) >> + return ret; >> + >> + /* >> + * The power down GPIO is optional. If present, toggle it from active to >> + * inactive to wake up the encoder. >> + */ >> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); >> + if (IS_ERR(adv7511->gpio_pd)) >> + return PTR_ERR(adv7511->gpio_pd); >> + >> + if (adv7511->gpio_pd) { >> + mdelay(5); >> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); >> + } >> + >> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); >> + if (IS_ERR(adv7511->regmap)) >> + return PTR_ERR(adv7511->regmap); >> + >> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); >> + if (ret) >> + return ret; >> + dev_dbg(dev, "Rev. %d\n", val); >> + >> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, >> + ARRAY_SIZE(adv7511_fixed_registers)); >> + if (ret) >> + return ret; >> + >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); >> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, >> + packet_i2c_addr); >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); >> + adv7511_packet_disable(adv7511, 0xffff); >> + >> + adv7511->i2c_main = i2c; >> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); >> + if (!adv7511->i2c_edid) >> + return -ENOMEM; >> + >> + if (i2c->irq) { >> + init_waitqueue_head(&adv7511->wq); >> + >> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, >> + adv7511_irq_handler, >> + IRQF_ONESHOT, dev_name(dev), >> + adv7511); >> + if (ret) >> + goto err_i2c_unregister_device; >> + } >> + >> + /* CEC is unused for now */ >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, >> + ADV7511_CEC_CTRL_POWER_DOWN); >> + >> + adv7511_power_off(adv7511); >> + >> + i2c_set_clientdata(i2c, adv7511); >> + >> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >> + adv7511_audio_init(&i2c->dev); >> +#endif > > If we intend to have more audio funcs being used by the core in the > future, it would be nice to have NOP audio funcs rather than having > multiple #ifdef checks in the driver when CONFIG_DRM_I2C_ADV7511_AUDIO > isn't set. > I will move this ifdef to adv751_audio and use NOP functions. >> + >> + adv7511_set_link_config(adv7511, &link_config); >> + >> + /* Enable HDMI mode */ >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, >> + ADV7511_HDMI_CFG_MODE_MASK, >> + ADV7511_HDMI_CFG_MODE_HDMI); >> + >> + return 0; >> + >> +err_i2c_unregister_device: >> + i2c_unregister_device(adv7511->i2c_edid); >> + >> + return ret; >> +} >> + >> +static int adv7511_remove(struct i2c_client *i2c) >> +{ >> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >> + > > Are we missing a call to adv7511_audio_exit() here? I followed the code in Analog Devices tree where there is no call to audio_exit() but indeed you are correct. I will add this call. > > Thanks, > Archit > Thanks for your comments! Best regards, Jose Miguel Abreu ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-29 10:52 ` Jose Abreu @ 2016-03-29 17:03 ` Archit Taneja [not found] ` <56FAB557.3060900-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Archit Taneja @ 2016-03-29 17:03 UTC (permalink / raw) To: Jose Abreu, linux-snps-arc, linux-kernel, dri-devel, alsa-devel, devicetree Cc: tixy, mark.rutland, laurent.pinchart+renesas, pawel.moll, ijc+devicetree, Vineet.Gupta1, Alexey.Brodkin, lgirdwood, robh+dt, nariman, yitian.bu, wsa+renesas, broonie, Maruthi.Bayyavarapu, galak, buyitian, alexander.deucher, tiwai, perex, CARLOS.PALMINHA On 3/29/2016 4:22 PM, Jose Abreu wrote: > Hi Archit, > > On 29-03-2016 09:05, Archit Taneja wrote: >> Hi, >> >> On 03/28/2016 08:06 PM, Jose Abreu wrote: >>> This patch adds audio support for the ADV7511 HDMI transmitter >>> using ALSA SoC. >>> >>> The code was ported from Analog Devices linux tree from >>> commit 1770c4a1e32b ("Merge remote-tracking branch >>> 'xilinx/master' into xcomm_zynq"), which is available at: >>> - https://github.com/analogdevicesinc/linux/ >>> >>> The main core file was renamed from adv7511.c to adv7511_core.c >>> so that audio and video compile into a single adv7511.ko module >>> and to keep up with Analog Devices kernel tree. >>> >>> The audio can be disabled using menu-config so it is possible >>> to use only video mode. >>> >>> The HDMI mode is automatically started at boot and the audio >>> (when enabled) registers as a codec into ALSA. >> >> Is there a reason why we set the mode to HDMI at probe itself? >> Shouldn't it be okay to set the mode later once we read the >> EDID off the panel? >> >> Some more comments below. >> > > Well, when I was using this in kernel 3.18 (with an older version of the driver) > I noticed that DVI mode was being used even when HDMI was connected so I forced > the driver to start in HDMI mode. There were some changes in the driver so it is > possible that this is no longer needed. Should I drop it? Mode selection works fine with ADV7533 on a 4.5 kernel. I'm assuming it should work out of the box for ADV7511 too. We should drop this. > >>> >>> SPDIF DAI format was also added to ASoC as it is required >>> by adv7511 audio. >>> >>> Signed-off-by: Jose Abreu <joabreu@synopsys.com> >>> --- >>> >>> No changes v1 -> v2. >>> >>> drivers/gpu/drm/i2c/Kconfig | 11 + >>> drivers/gpu/drm/i2c/Makefile | 2 + >>> drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------------- >>> drivers/gpu/drm/i2c/adv7511.h | 41 ++ >>> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ >>> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++ >>> include/sound/soc-dai.h | 1 + >>> 7 files changed, 1370 insertions(+), 1024 deletions(-) >>> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c >>> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >>> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c >> >> <snip> >> >>> + >>> +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id >>> *id) >>> +{ >>> + struct adv7511_link_config link_config; >>> + struct adv7511 *adv7511; >>> + struct device *dev = &i2c->dev; >>> + unsigned int val; >>> + int ret; >>> + >>> + if (!dev->of_node) >>> + return -EINVAL; >>> + >>> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); >>> + if (!adv7511) >>> + return -ENOMEM; >>> + >>> + adv7511->powered = false; >>> + adv7511->status = connector_status_disconnected; >>> + >>> + ret = adv7511_parse_dt(dev->of_node, &link_config); >>> + if (ret) >>> + return ret; >>> + >>> + /* >>> + * The power down GPIO is optional. If present, toggle it from active to >>> + * inactive to wake up the encoder. >>> + */ >>> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); >>> + if (IS_ERR(adv7511->gpio_pd)) >>> + return PTR_ERR(adv7511->gpio_pd); >>> + >>> + if (adv7511->gpio_pd) { >>> + mdelay(5); >>> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); >>> + } >>> + >>> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); >>> + if (IS_ERR(adv7511->regmap)) >>> + return PTR_ERR(adv7511->regmap); >>> + >>> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); >>> + if (ret) >>> + return ret; >>> + dev_dbg(dev, "Rev. %d\n", val); >>> + >>> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, >>> + ARRAY_SIZE(adv7511_fixed_registers)); >>> + if (ret) >>> + return ret; >>> + >>> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); >>> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, >>> + packet_i2c_addr); >>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); >>> + adv7511_packet_disable(adv7511, 0xffff); >>> + >>> + adv7511->i2c_main = i2c; >>> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); >>> + if (!adv7511->i2c_edid) >>> + return -ENOMEM; >>> + >>> + if (i2c->irq) { >>> + init_waitqueue_head(&adv7511->wq); >>> + >>> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, >>> + adv7511_irq_handler, >>> + IRQF_ONESHOT, dev_name(dev), >>> + adv7511); >>> + if (ret) >>> + goto err_i2c_unregister_device; >>> + } >>> + >>> + /* CEC is unused for now */ >>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, >>> + ADV7511_CEC_CTRL_POWER_DOWN); >>> + >>> + adv7511_power_off(adv7511); >>> + >>> + i2c_set_clientdata(i2c, adv7511); >>> + >>> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >>> + adv7511_audio_init(&i2c->dev); >>> +#endif >> >> If we intend to have more audio funcs being used by the core in the >> future, it would be nice to have NOP audio funcs rather than having >> multiple #ifdef checks in the driver when CONFIG_DRM_I2C_ADV7511_AUDIO >> isn't set. >> > > I will move this ifdef to adv751_audio and use NOP functions. Thanks, I think it should help in the longer run. > >>> + >>> + adv7511_set_link_config(adv7511, &link_config); >>> + >>> + /* Enable HDMI mode */ >>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, >>> + ADV7511_HDMI_CFG_MODE_MASK, >>> + ADV7511_HDMI_CFG_MODE_HDMI); >>> + >>> + return 0; >>> + >>> +err_i2c_unregister_device: >>> + i2c_unregister_device(adv7511->i2c_edid); >>> + >>> + return ret; >>> +} >>> + >>> +static int adv7511_remove(struct i2c_client *i2c) >>> +{ >>> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >>> + >> >> Are we missing a call to adv7511_audio_exit() here? > > I followed the code in Analog Devices tree where there is no call to > audio_exit() but indeed you are correct. I will add this call. > Since we have 3 files for adv7511 now, could we also move the driver to a separate folder? The long term plan is to convert all the i2c slave encoder drivers to bridges. Keeping them together would be nicer when we migrate this driver to the bridge folder. Thanks, Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <56FAB557.3060900-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support [not found] ` <56FAB557.3060900-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2016-03-31 12:57 ` Jose Abreu 0 siblings, 0 replies; 23+ messages in thread From: Jose Abreu @ 2016-03-31 12:57 UTC (permalink / raw) To: Archit Taneja, Jose Abreu, linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: tixy-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A, Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E, Maruthi.Bayyavarapu-5C7GfCeVMHo, pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w, buyitian-Re5JQEeQqe8AvxtiuMwx3w, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, galak-sgV2jX0FEOL9JmXXK+q4OQ, alexander.deucher-5C7GfCeVMHo, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w Hi Archit, On 29-03-2016 18:03, Archit Taneja wrote: > > > On 3/29/2016 4:22 PM, Jose Abreu wrote: >> Hi Archit, >> >> On 29-03-2016 09:05, Archit Taneja wrote: >>> Hi, >>> >>> On 03/28/2016 08:06 PM, Jose Abreu wrote: >>>> This patch adds audio support for the ADV7511 HDMI transmitter >>>> using ALSA SoC. >>>> >>>> The code was ported from Analog Devices linux tree from >>>> commit 1770c4a1e32b ("Merge remote-tracking branch >>>> 'xilinx/master' into xcomm_zynq"), which is available at: >>>> - https://github.com/analogdevicesinc/linux/ >>>> >>>> The main core file was renamed from adv7511.c to adv7511_core.c >>>> so that audio and video compile into a single adv7511.ko module >>>> and to keep up with Analog Devices kernel tree. >>>> >>>> The audio can be disabled using menu-config so it is possible >>>> to use only video mode. >>>> >>>> The HDMI mode is automatically started at boot and the audio >>>> (when enabled) registers as a codec into ALSA. >>> >>> Is there a reason why we set the mode to HDMI at probe itself? >>> Shouldn't it be okay to set the mode later once we read the >>> EDID off the panel? >>> >>> Some more comments below. >>> >> >> Well, when I was using this in kernel 3.18 (with an older version of the driver) >> I noticed that DVI mode was being used even when HDMI was connected so I forced >> the driver to start in HDMI mode. There were some changes in the driver so it is >> possible that this is no longer needed. Should I drop it? > > Mode selection works fine with ADV7533 on a 4.5 kernel. I'm assuming it > should work out of the box for ADV7511 too. We should drop this. > > Ok, will drop. >> >>>> >>>> SPDIF DAI format was also added to ASoC as it is required >>>> by adv7511 audio. >>>> >>>> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> >>>> --- >>>> >>>> No changes v1 -> v2. >>>> >>>> drivers/gpu/drm/i2c/Kconfig | 11 + >>>> drivers/gpu/drm/i2c/Makefile | 2 + >>>> drivers/gpu/drm/i2c/adv7511.c | 1024 >>>> ----------------------------------- >>>> drivers/gpu/drm/i2c/adv7511.h | 41 ++ >>>> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ >>>> drivers/gpu/drm/i2c/adv7511_core.c | 1005 >>>> ++++++++++++++++++++++++++++++++++ >>>> include/sound/soc-dai.h | 1 + >>>> 7 files changed, 1370 insertions(+), 1024 deletions(-) >>>> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c >>> >>> <snip> >>> >>>> + >>>> +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id >>>> *id) >>>> +{ >>>> + struct adv7511_link_config link_config; >>>> + struct adv7511 *adv7511; >>>> + struct device *dev = &i2c->dev; >>>> + unsigned int val; >>>> + int ret; >>>> + >>>> + if (!dev->of_node) >>>> + return -EINVAL; >>>> + >>>> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); >>>> + if (!adv7511) >>>> + return -ENOMEM; >>>> + >>>> + adv7511->powered = false; >>>> + adv7511->status = connector_status_disconnected; >>>> + >>>> + ret = adv7511_parse_dt(dev->of_node, &link_config); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* >>>> + * The power down GPIO is optional. If present, toggle it from active to >>>> + * inactive to wake up the encoder. >>>> + */ >>>> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); >>>> + if (IS_ERR(adv7511->gpio_pd)) >>>> + return PTR_ERR(adv7511->gpio_pd); >>>> + >>>> + if (adv7511->gpio_pd) { >>>> + mdelay(5); >>>> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); >>>> + } >>>> + >>>> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); >>>> + if (IS_ERR(adv7511->regmap)) >>>> + return PTR_ERR(adv7511->regmap); >>>> + >>>> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); >>>> + if (ret) >>>> + return ret; >>>> + dev_dbg(dev, "Rev. %d\n", val); >>>> + >>>> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, >>>> + ARRAY_SIZE(adv7511_fixed_registers)); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); >>>> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, >>>> + packet_i2c_addr); >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); >>>> + adv7511_packet_disable(adv7511, 0xffff); >>>> + >>>> + adv7511->i2c_main = i2c; >>>> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); >>>> + if (!adv7511->i2c_edid) >>>> + return -ENOMEM; >>>> + >>>> + if (i2c->irq) { >>>> + init_waitqueue_head(&adv7511->wq); >>>> + >>>> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, >>>> + adv7511_irq_handler, >>>> + IRQF_ONESHOT, dev_name(dev), >>>> + adv7511); >>>> + if (ret) >>>> + goto err_i2c_unregister_device; >>>> + } >>>> + >>>> + /* CEC is unused for now */ >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, >>>> + ADV7511_CEC_CTRL_POWER_DOWN); >>>> + >>>> + adv7511_power_off(adv7511); >>>> + >>>> + i2c_set_clientdata(i2c, adv7511); >>>> + >>>> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >>>> + adv7511_audio_init(&i2c->dev); >>>> +#endif >>> >>> If we intend to have more audio funcs being used by the core in the >>> future, it would be nice to have NOP audio funcs rather than having >>> multiple #ifdef checks in the driver when CONFIG_DRM_I2C_ADV7511_AUDIO >>> isn't set. >>> >> >> I will move this ifdef to adv751_audio and use NOP functions. > > Thanks, I think it should help in the longer run. > >> >>>> + >>>> + adv7511_set_link_config(adv7511, &link_config); >>>> + >>>> + /* Enable HDMI mode */ >>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, >>>> + ADV7511_HDMI_CFG_MODE_MASK, >>>> + ADV7511_HDMI_CFG_MODE_HDMI); >>>> + >>>> + return 0; >>>> + >>>> +err_i2c_unregister_device: >>>> + i2c_unregister_device(adv7511->i2c_edid); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int adv7511_remove(struct i2c_client *i2c) >>>> +{ >>>> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >>>> + >>> >>> Are we missing a call to adv7511_audio_exit() here? >> >> I followed the code in Analog Devices tree where there is no call to >> audio_exit() but indeed you are correct. I will add this call. >> > > Since we have 3 files for adv7511 now, could we also move the driver > to a separate folder? The long term plan is to convert all the i2c > slave encoder drivers to bridges. Keeping them together would be nicer > when we migrate this driver to the bridge folder. Ok, will move to separate folder. > > Thanks, > Archit > Best regards, Jose Miguel Abreu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu 2016-03-29 8:05 ` Archit Taneja @ 2016-03-30 9:58 ` Emil Velikov 2016-04-01 17:10 ` Laurent Pinchart 2016-04-03 5:05 ` kbuild test robot 3 siblings, 0 replies; 23+ messages in thread From: Emil Velikov @ 2016-03-30 9:58 UTC (permalink / raw) To: Jose Abreu Cc: Jon Medhurst, Mark Rutland, alsa-devel, Alexey Brodkin, ML dri-devel, lgirdwood, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, Pawel Moll, Ian Campbell, Vineet.Gupta1, buyitian, Rob Herring, perex, Linux-Kernel@Vger. Kernel. Org, broonie, Kumar Gala, Alex Deucher, CARLOS.PALMINHA Hi Jose, On 28 March 2016 at 15:36, Jose Abreu <Jose.Abreu@synopsys.com> wrote: > This patch adds audio support for the ADV7511 HDMI transmitter > using ALSA SoC. > > The code was ported from Analog Devices linux tree from > commit 1770c4a1e32b ("Merge remote-tracking branch > 'xilinx/master' into xcomm_zynq"), which is available at: > - https://github.com/analogdevicesinc/linux/ > > The main core file was renamed from adv7511.c to adv7511_core.c > so that audio and video compile into a single adv7511.ko module > and to keep up with Analog Devices kernel tree. > > The audio can be disabled using menu-config so it is possible > to use only video mode. > > The HDMI mode is automatically started at boot and the audio > (when enabled) registers as a codec into ALSA. > > SPDIF DAI format was also added to ASoC as it is required > by adv7511 audio. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > --- > > No changes v1 -> v2. > > drivers/gpu/drm/i2c/Kconfig | 11 + > drivers/gpu/drm/i2c/Makefile | 2 + > drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------------- > drivers/gpu/drm/i2c/adv7511.h | 41 ++ > drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++ Please keep the file rename separate (and use -M flag when generating the patch) from the introduction of audio support. Having to check 1k LOC movement alongside the introduction of new one is a bit... suboptimal. Regards, Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu 2016-03-29 8:05 ` Archit Taneja 2016-03-30 9:58 ` Emil Velikov @ 2016-04-01 17:10 ` Laurent Pinchart 2016-04-04 9:05 ` Jose Abreu 2016-04-03 5:05 ` kbuild test robot 3 siblings, 1 reply; 23+ messages in thread From: Laurent Pinchart @ 2016-04-01 17:10 UTC (permalink / raw) To: dri-devel Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, lgirdwood, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, Jose Abreu, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, linux-kernel, broonie, galak, alexander.deucher, CARLOS.PALMINHA Hi Jose, Thank you for the patch. On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: > This patch adds audio support for the ADV7511 HDMI transmitter > using ALSA SoC. > > The code was ported from Analog Devices linux tree from > commit 1770c4a1e32b ("Merge remote-tracking branch > 'xilinx/master' into xcomm_zynq"), which is available at: > - https://github.com/analogdevicesinc/linux/ > > The main core file was renamed from adv7511.c to adv7511_core.c > so that audio and video compile into a single adv7511.ko module > and to keep up with Analog Devices kernel tree. > > The audio can be disabled using menu-config so it is possible > to use only video mode. > > The HDMI mode is automatically started at boot and the audio > (when enabled) registers as a codec into ALSA. > > SPDIF DAI format was also added to ASoC as it is required > by adv7511 audio. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > --- > > No changes v1 -> v2. > > drivers/gpu/drm/i2c/Kconfig | 11 + > drivers/gpu/drm/i2c/Makefile | 2 + > drivers/gpu/drm/i2c/adv7511.c | 1024 ------------------------------- > drivers/gpu/drm/i2c/adv7511.h | 41 ++ > drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++ Please use git-format-patch -M to detect renames if you send a new version of this series, it will help with review. > include/sound/soc-dai.h | 1 + > 7 files changed, 1370 insertions(+), 1024 deletions(-) > delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c [snip] > diff --git a/drivers/gpu/drm/i2c/adv7511_core.c > b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 > index 0000000..d54256a > --- /dev/null > +++ b/drivers/gpu/drm/i2c/adv7511_core.c [snip] > +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id > *id) +{ > + struct adv7511_link_config link_config; > + struct adv7511 *adv7511; > + struct device *dev = &i2c->dev; > + unsigned int val; > + int ret; > + > + if (!dev->of_node) > + return -EINVAL; > + > + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > + if (!adv7511) > + return -ENOMEM; > + > + adv7511->powered = false; > + adv7511->status = connector_status_disconnected; > + > + ret = adv7511_parse_dt(dev->of_node, &link_config); > + if (ret) > + return ret; > + > + /* > + * The power down GPIO is optional. If present, toggle it from active to > + * inactive to wake up the encoder. > + */ > + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); > + if (IS_ERR(adv7511->gpio_pd)) > + return PTR_ERR(adv7511->gpio_pd); > + > + if (adv7511->gpio_pd) { > + mdelay(5); > + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > + } > + > + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > + if (IS_ERR(adv7511->regmap)) > + return PTR_ERR(adv7511->regmap); > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > + if (ret) > + return ret; > + dev_dbg(dev, "Rev. %d\n", val); > + > + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > + ARRAY_SIZE(adv7511_fixed_registers)); > + if (ret) > + return ret; > + > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > + packet_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > + adv7511_packet_disable(adv7511, 0xffff); > + > + adv7511->i2c_main = i2c; > + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > + if (!adv7511->i2c_edid) > + return -ENOMEM; > + > + if (i2c->irq) { > + init_waitqueue_head(&adv7511->wq); > + > + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > + adv7511_irq_handler, > + IRQF_ONESHOT, dev_name(dev), > + adv7511); > + if (ret) > + goto err_i2c_unregister_device; > + } > + > + /* CEC is unused for now */ > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > + ADV7511_CEC_CTRL_POWER_DOWN); > + > + adv7511_power_off(adv7511); > + > + i2c_set_clientdata(i2c, adv7511); > + > +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > + adv7511_audio_init(&i2c->dev); > +#endif Shouldn't we condition this to the audio channel being somehow described in DT ? If a board doesn't route audio signals to the ADV7511 audio input, there's no need to register an audio codec. > + > + adv7511_set_link_config(adv7511, &link_config); > + > + /* Enable HDMI mode */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > + ADV7511_HDMI_CFG_MODE_MASK, > + ADV7511_HDMI_CFG_MODE_HDMI); > + > + return 0; > + > +err_i2c_unregister_device: > + i2c_unregister_device(adv7511->i2c_edid); > + > + return ret; > +} -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-04-01 17:10 ` Laurent Pinchart @ 2016-04-04 9:05 ` Jose Abreu 2016-04-04 21:41 ` Laurent Pinchart 0 siblings, 1 reply; 23+ messages in thread From: Jose Abreu @ 2016-04-04 9:05 UTC (permalink / raw) To: Laurent Pinchart, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Jose Abreu, linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, devicetree-u79uwXL29TY76Z2rM5mHXA, tixy-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A, Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E, Maruthi.Bayyavarapu-5C7GfCeVMHo, pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w, buyitian-Re5JQEeQqe8AvxtiuMwx3w, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, galak-sgV2jX0FEOL9JmXXK+q4OQ, alexander.deucher-5C7GfCeVMHo, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w Hi Laurent, On 01-04-2016 18:10, Laurent Pinchart wrote: > Hi Jose, > > Thank you for the patch. > > On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: >> This patch adds audio support for the ADV7511 HDMI transmitter >> using ALSA SoC. >> >> The code was ported from Analog Devices linux tree from >> commit 1770c4a1e32b ("Merge remote-tracking branch >> 'xilinx/master' into xcomm_zynq"), which is available at: >> - https://github.com/analogdevicesinc/linux/ >> >> The main core file was renamed from adv7511.c to adv7511_core.c >> so that audio and video compile into a single adv7511.ko module >> and to keep up with Analog Devices kernel tree. >> >> The audio can be disabled using menu-config so it is possible >> to use only video mode. >> >> The HDMI mode is automatically started at boot and the audio >> (when enabled) registers as a codec into ALSA. >> >> SPDIF DAI format was also added to ASoC as it is required >> by adv7511 audio. >> >> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> >> --- >> >> No changes v1 -> v2. >> >> drivers/gpu/drm/i2c/Kconfig | 11 + >> drivers/gpu/drm/i2c/Makefile | 2 + >> drivers/gpu/drm/i2c/adv7511.c | 1024 ------------------------------- >> drivers/gpu/drm/i2c/adv7511.h | 41 ++ >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++ > Please use git-format-patch -M to detect renames if you send a new version of > this series, it will help with review. Ok, will do that in next version. >> include/sound/soc-dai.h | 1 + >> 7 files changed, 1370 insertions(+), 1024 deletions(-) >> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > [snip] > >> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c >> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 >> index 0000000..d54256a >> --- /dev/null >> +++ b/drivers/gpu/drm/i2c/adv7511_core.c > [snip] > >> +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id >> *id) +{ >> + struct adv7511_link_config link_config; >> + struct adv7511 *adv7511; >> + struct device *dev = &i2c->dev; >> + unsigned int val; >> + int ret; >> + >> + if (!dev->of_node) >> + return -EINVAL; >> + >> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); >> + if (!adv7511) >> + return -ENOMEM; >> + >> + adv7511->powered = false; >> + adv7511->status = connector_status_disconnected; >> + >> + ret = adv7511_parse_dt(dev->of_node, &link_config); >> + if (ret) >> + return ret; >> + >> + /* >> + * The power down GPIO is optional. If present, toggle it from active to >> + * inactive to wake up the encoder. >> + */ >> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); >> + if (IS_ERR(adv7511->gpio_pd)) >> + return PTR_ERR(adv7511->gpio_pd); >> + >> + if (adv7511->gpio_pd) { >> + mdelay(5); >> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); >> + } >> + >> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); >> + if (IS_ERR(adv7511->regmap)) >> + return PTR_ERR(adv7511->regmap); >> + >> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); >> + if (ret) >> + return ret; >> + dev_dbg(dev, "Rev. %d\n", val); >> + >> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, >> + ARRAY_SIZE(adv7511_fixed_registers)); >> + if (ret) >> + return ret; >> + >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); >> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, >> + packet_i2c_addr); >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); >> + adv7511_packet_disable(adv7511, 0xffff); >> + >> + adv7511->i2c_main = i2c; >> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); >> + if (!adv7511->i2c_edid) >> + return -ENOMEM; >> + >> + if (i2c->irq) { >> + init_waitqueue_head(&adv7511->wq); >> + >> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, >> + adv7511_irq_handler, >> + IRQF_ONESHOT, dev_name(dev), >> + adv7511); >> + if (ret) >> + goto err_i2c_unregister_device; >> + } >> + >> + /* CEC is unused for now */ >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, >> + ADV7511_CEC_CTRL_POWER_DOWN); >> + >> + adv7511_power_off(adv7511); >> + >> + i2c_set_clientdata(i2c, adv7511); >> + >> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >> + adv7511_audio_init(&i2c->dev); >> +#endif > Shouldn't we condition this to the audio channel being somehow described in DT > ? If a board doesn't route audio signals to the ADV7511 audio input, there's > no need to register an audio codec. I can do this but the audio is already conditionally compiled using menuconfig. Is it really necessary to add this extra layer of condition? >> + >> + adv7511_set_link_config(adv7511, &link_config); >> + >> + /* Enable HDMI mode */ >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, >> + ADV7511_HDMI_CFG_MODE_MASK, >> + ADV7511_HDMI_CFG_MODE_HDMI); >> + >> + return 0; >> + >> +err_i2c_unregister_device: >> + i2c_unregister_device(adv7511->i2c_edid); >> + >> + return ret; >> +} Best regards, Jose Miguel Abreu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-04-04 9:05 ` Jose Abreu @ 2016-04-04 21:41 ` Laurent Pinchart 2016-04-05 11:00 ` Jose Abreu 0 siblings, 1 reply; 23+ messages in thread From: Laurent Pinchart @ 2016-04-04 21:41 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, dri-devel, lgirdwood, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, linux-kernel, broonie, galak, alexander.deucher, CARLOS.PALMINHA Hi Jose, On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote: > On 01-04-2016 18:10, Laurent Pinchart wrote: > > On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: > >> This patch adds audio support for the ADV7511 HDMI transmitter > >> using ALSA SoC. > >> > >> The code was ported from Analog Devices linux tree from > >> commit 1770c4a1e32b ("Merge remote-tracking branch > >> > >> 'xilinx/master' into xcomm_zynq"), which is available at: > >> - https://github.com/analogdevicesinc/linux/ > >> > >> The main core file was renamed from adv7511.c to adv7511_core.c > >> so that audio and video compile into a single adv7511.ko module > >> and to keep up with Analog Devices kernel tree. > >> > >> The audio can be disabled using menu-config so it is possible > >> to use only video mode. > >> > >> The HDMI mode is automatically started at boot and the audio > >> (when enabled) registers as a codec into ALSA. > >> > >> SPDIF DAI format was also added to ASoC as it is required > >> by adv7511 audio. > >> > >> Signed-off-by: Jose Abreu <joabreu@synopsys.com> > >> --- > >> > >> No changes v1 -> v2. > >> > >> drivers/gpu/drm/i2c/Kconfig | 11 + > >> drivers/gpu/drm/i2c/Makefile | 2 + > >> drivers/gpu/drm/i2c/adv7511.c | 1024 ---------------------------- > >> drivers/gpu/drm/i2c/adv7511.h | 41 ++ > >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++ > > > > Please use git-format-patch -M to detect renames if you send a new version > > of this series, it will help with review. > > Ok, will do that in next version. > > >> include/sound/soc-dai.h | 1 + > >> 7 files changed, 1370 insertions(+), 1024 deletions(-) > >> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c > >> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 > >> index 0000000..d54256a > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> +static int adv7511_probe(struct i2c_client *i2c, const struct > >> i2c_device_id *id) +{ > >> + struct adv7511_link_config link_config; > >> + struct adv7511 *adv7511; > >> + struct device *dev = &i2c->dev; > >> + unsigned int val; > >> + int ret; > >> + > >> + if (!dev->of_node) > >> + return -EINVAL; > >> + > >> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > >> + if (!adv7511) > >> + return -ENOMEM; > >> + > >> + adv7511->powered = false; > >> + adv7511->status = connector_status_disconnected; > >> + > >> + ret = adv7511_parse_dt(dev->of_node, &link_config); > >> + if (ret) > >> + return ret; > >> + > >> + /* > >> + * The power down GPIO is optional. If present, toggle it from active > >> to > >> + * inactive to wake up the encoder. > >> + */ > >> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); > >> + if (IS_ERR(adv7511->gpio_pd)) > >> + return PTR_ERR(adv7511->gpio_pd); > >> + > >> + if (adv7511->gpio_pd) { > >> + mdelay(5); > >> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > >> + } > >> + > >> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > >> + if (IS_ERR(adv7511->regmap)) > >> + return PTR_ERR(adv7511->regmap); > >> + > >> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > >> + if (ret) > >> + return ret; > >> + dev_dbg(dev, "Rev. %d\n", val); > >> + > >> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > >> + ARRAY_SIZE(adv7511_fixed_registers)); > >> + if (ret) > >> + return ret; > >> + > >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, > >> edid_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > >> + packet_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > >> + adv7511_packet_disable(adv7511, 0xffff); > >> + > >> + adv7511->i2c_main = i2c; > >> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > >> + if (!adv7511->i2c_edid) > >> + return -ENOMEM; > >> + > >> + if (i2c->irq) { > >> + init_waitqueue_head(&adv7511->wq); > >> + > >> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > >> + adv7511_irq_handler, > >> + IRQF_ONESHOT, dev_name(dev), > >> + adv7511); > >> + if (ret) > >> + goto err_i2c_unregister_device; > >> + } > >> + > >> + /* CEC is unused for now */ > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > >> + ADV7511_CEC_CTRL_POWER_DOWN); > >> + > >> + adv7511_power_off(adv7511); > >> + > >> + i2c_set_clientdata(i2c, adv7511); > >> + > >> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > >> + adv7511_audio_init(&i2c->dev); > >> +#endif > > > > Shouldn't we condition this to the audio channel being somehow described > > in DT ? If a board doesn't route audio signals to the ADV7511 audio > > input, there's no need to register an audio codec. > > I can do this but the audio is already conditionally compiled using > menuconfig. Is it really necessary to add this extra layer of condition? The idea is that enabling support for ADV7511 audio in the kernel isn't coupled with whether the system includes audio support. It would be confusing, and would also waste resources, to create a Linux sound device when no sound channel is routed on the board. > >> + > >> + adv7511_set_link_config(adv7511, &link_config); > >> + > >> + /* Enable HDMI mode */ > >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > >> + ADV7511_HDMI_CFG_MODE_MASK, > >> + ADV7511_HDMI_CFG_MODE_HDMI); > >> + > >> + return 0; > >> + > >> +err_i2c_unregister_device: > >> + i2c_unregister_device(adv7511->i2c_edid); > >> + > >> + return ret; > >> +} -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-04-04 21:41 ` Laurent Pinchart @ 2016-04-05 11:00 ` Jose Abreu 2016-04-05 16:03 ` Laurent Pinchart 0 siblings, 1 reply; 23+ messages in thread From: Jose Abreu @ 2016-04-05 11:00 UTC (permalink / raw) To: Laurent Pinchart, Jose Abreu Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, devicetree-u79uwXL29TY76Z2rM5mHXA, tixy-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A, Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E, Maruthi.Bayyavarapu-5C7GfCeVMHo, pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w, buyitian-Re5JQEeQqe8AvxtiuMwx3w, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, galak-sgV2jX0FEOL9JmXXK+q4OQ, alexander.deucher-5C7GfCeVMHo, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w Hi Laurent, On 04-04-2016 22:41, Laurent Pinchart wrote: > Hi Jose, > > On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote: >> On 01-04-2016 18:10, Laurent Pinchart wrote: >>> On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: >>>> This patch adds audio support for the ADV7511 HDMI transmitter >>>> using ALSA SoC. >>>> >>>> The code was ported from Analog Devices linux tree from >>>> commit 1770c4a1e32b ("Merge remote-tracking branch >>>> >>>> 'xilinx/master' into xcomm_zynq"), which is available at: >>>> - https://github.com/analogdevicesinc/linux/ >>>> >>>> The main core file was renamed from adv7511.c to adv7511_core.c >>>> so that audio and video compile into a single adv7511.ko module >>>> and to keep up with Analog Devices kernel tree. >>>> >>>> The audio can be disabled using menu-config so it is possible >>>> to use only video mode. >>>> >>>> The HDMI mode is automatically started at boot and the audio >>>> (when enabled) registers as a codec into ALSA. >>>> >>>> SPDIF DAI format was also added to ASoC as it is required >>>> by adv7511 audio. >>>> >>>> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> >>>> --- >>>> >>>> No changes v1 -> v2. >>>> >>>> drivers/gpu/drm/i2c/Kconfig | 11 + >>>> drivers/gpu/drm/i2c/Makefile | 2 + >>>> drivers/gpu/drm/i2c/adv7511.c | 1024 ---------------------------- >>>> drivers/gpu/drm/i2c/adv7511.h | 41 ++ >>>> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ >>>> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++ >>> Please use git-format-patch -M to detect renames if you send a new version >>> of this series, it will help with review. >> Ok, will do that in next version. >> >>>> include/sound/soc-dai.h | 1 + >>>> 7 files changed, 1370 insertions(+), 1024 deletions(-) >>>> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c >>> [snip] >>> >>>> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c >>>> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 >>>> index 0000000..d54256a >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/i2c/adv7511_core.c >>> [snip] >>> >>>> +static int adv7511_probe(struct i2c_client *i2c, const struct >>>> i2c_device_id *id) +{ >>>> + struct adv7511_link_config link_config; >>>> + struct adv7511 *adv7511; >>>> + struct device *dev = &i2c->dev; >>>> + unsigned int val; >>>> + int ret; >>>> + >>>> + if (!dev->of_node) >>>> + return -EINVAL; >>>> + >>>> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); >>>> + if (!adv7511) >>>> + return -ENOMEM; >>>> + >>>> + adv7511->powered = false; >>>> + adv7511->status = connector_status_disconnected; >>>> + >>>> + ret = adv7511_parse_dt(dev->of_node, &link_config); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* >>>> + * The power down GPIO is optional. If present, toggle it from active >>>> to >>>> + * inactive to wake up the encoder. >>>> + */ >>>> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", > GPIOD_OUT_HIGH); >>>> + if (IS_ERR(adv7511->gpio_pd)) >>>> + return PTR_ERR(adv7511->gpio_pd); >>>> + >>>> + if (adv7511->gpio_pd) { >>>> + mdelay(5); >>>> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); >>>> + } >>>> + >>>> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); >>>> + if (IS_ERR(adv7511->regmap)) >>>> + return PTR_ERR(adv7511->regmap); >>>> + >>>> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); >>>> + if (ret) >>>> + return ret; >>>> + dev_dbg(dev, "Rev. %d\n", val); >>>> + >>>> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, >>>> + ARRAY_SIZE(adv7511_fixed_registers)); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, >>>> edid_i2c_addr); >>>> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, >>>> + packet_i2c_addr); >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, > cec_i2c_addr); >>>> + adv7511_packet_disable(adv7511, 0xffff); >>>> + >>>> + adv7511->i2c_main = i2c; >>>> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); >>>> + if (!adv7511->i2c_edid) >>>> + return -ENOMEM; >>>> + >>>> + if (i2c->irq) { >>>> + init_waitqueue_head(&adv7511->wq); >>>> + >>>> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, >>>> + adv7511_irq_handler, >>>> + IRQF_ONESHOT, dev_name(dev), >>>> + adv7511); >>>> + if (ret) >>>> + goto err_i2c_unregister_device; >>>> + } >>>> + >>>> + /* CEC is unused for now */ >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, >>>> + ADV7511_CEC_CTRL_POWER_DOWN); >>>> + >>>> + adv7511_power_off(adv7511); >>>> + >>>> + i2c_set_clientdata(i2c, adv7511); >>>> + >>>> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >>>> + adv7511_audio_init(&i2c->dev); >>>> +#endif >>> Shouldn't we condition this to the audio channel being somehow described >>> in DT ? If a board doesn't route audio signals to the ADV7511 audio >>> input, there's no need to register an audio codec. >> I can do this but the audio is already conditionally compiled using >> menuconfig. Is it really necessary to add this extra layer of condition? > The idea is that enabling support for ADV7511 audio in the kernel isn't > coupled with whether the system includes audio support. It would be confusing, > and would also waste resources, to create a Linux sound device when no sound > channel is routed on the board. Ok, will do that in next version. Another question: I am facing a problem when compiling ALSA as a module, as the ADV7511 audio is not embedded into ALSA the compilation fails with undefined references to ALSA functions. The solution that I am planning to use is to add a default value to the ADV7511 kconfig entry so that the driver is compiled as module when ALSA is a module and as embedded if ALSA is embedded. Is this okay or is there another solution without moving the ADV7511 audio to the ALSA directory? > >>>> + >>>> + adv7511_set_link_config(adv7511, &link_config); >>>> + >>>> + /* Enable HDMI mode */ >>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, >>>> + ADV7511_HDMI_CFG_MODE_MASK, >>>> + ADV7511_HDMI_CFG_MODE_HDMI); >>>> + >>>> + return 0; >>>> + >>>> +err_i2c_unregister_device: >>>> + i2c_unregister_device(adv7511->i2c_edid); >>>> + >>>> + return ret; >>>> +} Best regards, Jose Miguel Abreu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-04-05 11:00 ` Jose Abreu @ 2016-04-05 16:03 ` Laurent Pinchart 0 siblings, 0 replies; 23+ messages in thread From: Laurent Pinchart @ 2016-04-05 16:03 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, dri-devel, lgirdwood, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, linux-kernel, broonie, galak, alexander.deucher, CARLOS.PALMINHA Hi Jose, On Tuesday 05 Apr 2016 12:00:54 Jose Abreu wrote: > On 04-04-2016 22:41, Laurent Pinchart wrote: > > On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote: > >> On 01-04-2016 18:10, Laurent Pinchart wrote: > >>> On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: > >>>> This patch adds audio support for the ADV7511 HDMI transmitter > >>>> using ALSA SoC. > >>>> > >>>> The code was ported from Analog Devices linux tree from > >>>> commit 1770c4a1e32b ("Merge remote-tracking branch > >>>> > >>>> 'xilinx/master' into xcomm_zynq"), which is available at: > >>>> - https://github.com/analogdevicesinc/linux/ > >>>> > >>>> The main core file was renamed from adv7511.c to adv7511_core.c > >>>> so that audio and video compile into a single adv7511.ko module > >>>> and to keep up with Analog Devices kernel tree. > >>>> > >>>> The audio can be disabled using menu-config so it is possible > >>>> to use only video mode. > >>>> > >>>> The HDMI mode is automatically started at boot and the audio > >>>> (when enabled) registers as a codec into ALSA. > >>>> > >>>> SPDIF DAI format was also added to ASoC as it is required > >>>> by adv7511 audio. > >>>> > >>>> Signed-off-by: Jose Abreu <joabreu@synopsys.com> > >>>> --- > >>>> > >>>> No changes v1 -> v2. > >>>> > >>>> drivers/gpu/drm/i2c/Kconfig | 11 + > >>>> drivers/gpu/drm/i2c/Makefile | 2 + > >>>> drivers/gpu/drm/i2c/adv7511.c | 1024 > >>>> ---------------------------- > >>>> drivers/gpu/drm/i2c/adv7511.h | 41 ++ > >>>> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > >>>> drivers/gpu/drm/i2c/adv7511_core.c | 1005 > >>>> ++++++++++++++++++++++++++++ > >>> > >>> Please use git-format-patch -M to detect renames if you send a new > >>> version > >>> of this series, it will help with review. > >> > >> Ok, will do that in next version. > >> > >>>> include/sound/soc-dai.h | 1 + > >>>> 7 files changed, 1370 insertions(+), 1024 deletions(-) > >>>> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > >>>> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > >>> > >>> [snip] > >>> > >>>> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c > >>>> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 > >>>> index 0000000..d54256a > >>>> --- /dev/null > >>>> +++ b/drivers/gpu/drm/i2c/adv7511_core.c > >>> > >>> [snip] > >>> > >>>> +static int adv7511_probe(struct i2c_client *i2c, const struct > >>>> i2c_device_id *id) +{ > >>>> + struct adv7511_link_config link_config; > >>>> + struct adv7511 *adv7511; > >>>> + struct device *dev = &i2c->dev; > >>>> + unsigned int val; > >>>> + int ret; > >>>> + > >>>> + if (!dev->of_node) > >>>> + return -EINVAL; > >>>> + > >>>> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > >>>> + if (!adv7511) > >>>> + return -ENOMEM; > >>>> + > >>>> + adv7511->powered = false; > >>>> + adv7511->status = connector_status_disconnected; > >>>> + > >>>> + ret = adv7511_parse_dt(dev->of_node, &link_config); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + /* > >>>> + * The power down GPIO is optional. If present, toggle it from active > >>>> to > >>>> + * inactive to wake up the encoder. > >>>> + */ > >>>> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", > > > > GPIOD_OUT_HIGH); > > > >>>> + if (IS_ERR(adv7511->gpio_pd)) > >>>> + return PTR_ERR(adv7511->gpio_pd); > >>>> + > >>>> + if (adv7511->gpio_pd) { > >>>> + mdelay(5); > >>>> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > >>>> + } > >>>> + > >>>> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > >>>> + if (IS_ERR(adv7511->regmap)) > >>>> + return PTR_ERR(adv7511->regmap); > >>>> + > >>>> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > >>>> + if (ret) > >>>> + return ret; > >>>> + dev_dbg(dev, "Rev. %d\n", val); > >>>> + > >>>> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > >>>> + ARRAY_SIZE(adv7511_fixed_registers)); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, > >>>> edid_i2c_addr); > >>>> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > >>>> + packet_i2c_addr); > >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, > > > > cec_i2c_addr); > > > >>>> + adv7511_packet_disable(adv7511, 0xffff); > >>>> + > >>>> + adv7511->i2c_main = i2c; > >>>> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > >>>> + if (!adv7511->i2c_edid) > >>>> + return -ENOMEM; > >>>> + > >>>> + if (i2c->irq) { > >>>> + init_waitqueue_head(&adv7511->wq); > >>>> + > >>>> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > >>>> + adv7511_irq_handler, > >>>> + IRQF_ONESHOT, dev_name(dev), > >>>> + adv7511); > >>>> + if (ret) > >>>> + goto err_i2c_unregister_device; > >>>> + } > >>>> + > >>>> + /* CEC is unused for now */ > >>>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > >>>> + ADV7511_CEC_CTRL_POWER_DOWN); > >>>> + > >>>> + adv7511_power_off(adv7511); > >>>> + > >>>> + i2c_set_clientdata(i2c, adv7511); > >>>> + > >>>> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > >>>> + adv7511_audio_init(&i2c->dev); > >>>> +#endif > >>> > >>> Shouldn't we condition this to the audio channel being somehow described > >>> in DT ? If a board doesn't route audio signals to the ADV7511 audio > >>> input, there's no need to register an audio codec. > >> > >> I can do this but the audio is already conditionally compiled using > >> menuconfig. Is it really necessary to add this extra layer of condition? > > > > The idea is that enabling support for ADV7511 audio in the kernel isn't > > coupled with whether the system includes audio support. It would be > > confusing, and would also waste resources, to create a Linux sound device > > when no sound channel is routed on the board. > > Ok, will do that in next version. Another question: I am facing a problem > when compiling ALSA as a module, as the ADV7511 audio is not embedded into > ALSA the compilation fails with undefined references to ALSA functions. The > solution that I am planning to use is to add a default value to the ADV7511 > kconfig entry so that the driver is compiled as module when ALSA is a > module and as embedded if ALSA is embedded. Is this okay or is there > another solution without moving the ADV7511 audio to the ALSA directory? You can write the Kconfig dependency as depends on DRM_I2C_ADV7511 depends on SND_SOC=y || (SND_SOC && DRM_I2C_ADV7511=m) > >>>> + > >>>> + adv7511_set_link_config(adv7511, &link_config); > >>>> + > >>>> + /* Enable HDMI mode */ > >>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > >>>> + ADV7511_HDMI_CFG_MODE_MASK, > >>>> + ADV7511_HDMI_CFG_MODE_HDMI); > >>>> + > >>>> + return 0; > >>>> + > >>>> +err_i2c_unregister_device: > >>>> + i2c_unregister_device(adv7511->i2c_edid); > >>>> + > >>>> + return ret; > >>>> +} -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support 2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu ` (2 preceding siblings ...) 2016-04-01 17:10 ` Laurent Pinchart @ 2016-04-03 5:05 ` kbuild test robot 3 siblings, 0 replies; 23+ messages in thread From: kbuild test robot @ 2016-04-03 5:05 UTC (permalink / raw) Cc: kbuild-all, linux-snps-arc, linux-kernel, dri-devel, alsa-devel, devicetree, airlied, lgirdwood, broonie, perex, tiwai, laurent.pinchart+renesas, wsa+renesas, lars, ville.syrjala, nariman, alexander.deucher, Maruthi.Bayyavarapu, buyitian, tixy, yitian.bu, Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, Vineet.Gupta1, CARLOS.PALMINHA, Jose Abreu [-- Attachment #1: Type: text/plain, Size: 1003 bytes --] Hi Jose, [auto build test ERROR on drm/drm-next] [also build test ERROR on v4.6-rc1 next-20160401] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Jose-Abreu/Add-I2S-ADV7511-audio-support-for-ARC-AXS10x-boards/20160328-224040 base: git://people.freedesktop.org/~airlied/linux.git drm-next config: x86_64-randconfig-s5-04031201 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `adv7511_audio_init': >> (.text+0x1c608b): undefined reference to `snd_soc_register_codec' drivers/built-in.o: In function `adv7511_audio_exit': >> (.text+0x1c6096): undefined reference to `snd_soc_unregister_codec' --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/octet-stream, Size: 23536 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>]
* [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support [not found] ` <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> @ 2016-03-28 14:36 ` Jose Abreu [not found] ` <538be366488bf0d7633d702f2d0bab16707b7a47.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 2016-03-29 17:31 ` Mark Brown 2016-03-28 14:36 ` [PATCH 3/3 v2] arc: axs10x: Add support for Designware I2S on DT Jose Abreu 1 sibling, 2 replies; 23+ messages in thread From: Jose Abreu @ 2016-03-28 14:36 UTC (permalink / raw) To: linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: airlied-cv59FeDIM0c, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/, lars-Qo5EllUWu/uELgA04lAiVw, ville.syrjala-VuQAYsv1563Yd54FQh9/CA, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E, alexander.deucher-5C7GfCeVMHo, Maruthi.Bayyavarapu-5C7GfCeVMHo, buyitian-Re5JQEeQqe8AvxtiuMwx3w, tixy-QSEj5FYQhm4dnm+yROfE0A, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA, Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, galak-sgV2jX0FEOL9JmXXK+q4OQ, Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w, Jose Abreu HDMI audio support was added to the AXS board using an I2S cpu driver and a custom platform driver. The platform driver supports two channels @ 16 bits with rates 32k, 44.1k and 48k. ALSA Simple audio card is used to glue the cpu, platform and codec driver (adv7511). Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> --- No changes v1 -> v2. sound/soc/dwc/Kconfig | 1 + sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 373 insertions(+), 13 deletions(-) diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig index d50e085..bc3fae7 100644 --- a/sound/soc/dwc/Kconfig +++ b/sound/soc/dwc/Kconfig @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S tristate "Synopsys I2S Device Driver" depends on CLKDEV_LOOKUP select SND_SOC_GENERIC_DMAENGINE_PCM + select SND_SIMPLE_CARD help Say Y or M if you want to add support for I2S driver for Synopsys desigwnware I2S device. The device supports upto diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index bff258d..0f2f588 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -84,11 +84,37 @@ #define MAX_CHANNEL_NUM 8 #define MIN_CHANNEL_NUM 2 +/* FPGA Version Info */ +#define FPGA_VER_INFO 0xE0011230 +#define FPGA_VER_27M 0x000FBED9 + +/* PLL registers addresses */ +#define PLL_IDIV_ADDR 0xE00100A0 +#define PLL_FBDIV_ADDR 0xE00100A4 +#define PLL_ODIV0_ADDR 0xE00100A8 +#define PLL_ODIV1_ADDR 0xE00100AC + +/* PCM definitions */ +#define BUFFER_BYTES_MAX 384000 +#define PERIOD_BYTES_MIN 2048 +#define PERIODS_MIN 8 + union dw_i2s_snd_dma_data { struct i2s_dma_data pd; struct snd_dmaengine_dai_dma_data dt; }; +struct dw_pcm_binfo { + struct snd_pcm_substream *stream; + unsigned char *dma_base; + unsigned char *dma_pointer; + unsigned int period_size_frames; + unsigned int size; + snd_pcm_uframes_t period_pointer; + unsigned int total_periods; + unsigned int current_period; +}; + struct dw_i2s_dev { void __iomem *i2s_base; struct clk *clk; @@ -100,14 +126,103 @@ struct dw_i2s_dev { struct device *dev; u32 ccr; u32 xfer_resolution; + u32 xfer_bytes; + u32 fifo_th; /* data related to DMA transfers b/w i2s and DMAC */ union dw_i2s_snd_dma_data play_dma_data; union dw_i2s_snd_dma_data capture_dma_data; struct i2s_clk_config_data config; int (*i2s_clk_cfg)(struct i2s_clk_config_data *config); + struct dw_pcm_binfo binfo; +}; + +struct dw_i2s_pll { + unsigned int rate; + unsigned int data_width; + unsigned int idiv; + unsigned int fbdiv; + unsigned int odiv0; + unsigned int odiv1; +}; + +static const struct dw_i2s_pll dw_i2s_pll_cfg_27m[] = { + /* 27Mhz */ + { 32000, 16, 0x104, 0x451, 0x10E38, 0x2000 }, + { 44100, 16, 0x104, 0x596, 0x10D35, 0x2000 }, + { 48000, 16, 0x208, 0xA28, 0x10B2C, 0x2000 }, + { 0, 0, 0, 0, 0, 0 }, }; +static const struct dw_i2s_pll dw_i2s_pll_cfg_28m[] = { + /* 28.224Mhz */ + { 32000, 16, 0x82, 0x105, 0x107DF, 0x2000 }, + { 44100, 16, 0x28A, 0x1, 0x10001, 0x2000 }, + { 48000, 16, 0xA28, 0x187, 0x10042, 0x2000 }, + { 0, 0, 0, 0, 0, 0 }, +}; + +static const struct snd_pcm_hardware dw_pcm_hw = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_BLOCK_TRANSFER, + .rates = SNDRV_PCM_RATE_32000 | + SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000, + .rate_min = 32000, + .rate_max = 48000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + .channels_min = 2, + .channels_max = 2, + .buffer_bytes_max = BUFFER_BYTES_MAX, + .period_bytes_min = PERIOD_BYTES_MIN, + .period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN, + .periods_min = PERIODS_MIN, + .periods_max = BUFFER_BYTES_MAX / PERIOD_BYTES_MIN, +}; + +static int dw_pcm_transfer(u32 *lsample, u32 *rsample, int bytes, int buf_size, + struct dw_pcm_binfo *bi) +{ + struct snd_pcm_runtime *rt = NULL; + int i; + + if (!bi) + return -EINVAL; + + rt = bi->stream->runtime; + + for (i = 0; i < buf_size; i++) { + if (bi->stream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + memcpy(&lsample[i], bi->dma_pointer, bytes); + bi->dma_pointer += bytes; + memcpy(&rsample[i], bi->dma_pointer, bytes); + bi->dma_pointer += bytes; + } else { + memcpy(bi->dma_pointer, &lsample[i], bytes); + bi->dma_pointer += bytes; + memcpy(bi->dma_pointer, &rsample[i], bytes); + bi->dma_pointer += bytes; + } + } + bi->period_pointer += bytes_to_frames(rt, bytes * 2 * buf_size); + + if (bi->period_pointer >= + (bi->period_size_frames * bi->current_period)) { + bi->current_period++; + if (bi->current_period > bi->total_periods) { + bi->dma_pointer = bi->dma_base; + bi->period_pointer = 0; + bi->current_period = 1; + } + + snd_pcm_period_elapsed(bi->stream); + } + + return 0; +} + static inline void i2s_write_reg(void __iomem *io_base, int reg, u32 val) { writel(val, io_base + reg); @@ -144,20 +259,94 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, u32 stream) } } +static irqreturn_t i2s_irq_handler(int irq, void *dev_id) +{ + struct dw_i2s_dev *dev = dev_id; + u32 isr[4], sleft[dev->fifo_th], sright[dev->fifo_th]; + int i, j; + + for (i = 0; i < 4; i++) + isr[i] = i2s_read_reg(dev->i2s_base, ISR(i)); + + for (i = 0; i < 4; i++) { + /* Copy only to/from first two channels. + * TODO: Remaining channels + */ + if ((isr[i] & 0x10) && (i == 0) && (dev->binfo.stream->stream == + SNDRV_PCM_STREAM_PLAYBACK)) { + /* TXFE - TX FIFO is empty */ + dw_pcm_transfer(sleft, sright, dev->xfer_bytes, + dev->fifo_th, &dev->binfo); + + for (j = 0; j < dev->fifo_th; j++) { + i2s_write_reg(dev->i2s_base, LRBR_LTHR(i), + sleft[j]); + i2s_write_reg(dev->i2s_base, RRBR_RTHR(i), + sright[j]); + } + } else if ((isr[i] & 0x1) && (i == 0) && + (dev->binfo.stream->stream == + SNDRV_PCM_STREAM_CAPTURE)) { + /* RSFE - RX FIFO is full */ + for (j = 0; j < dev->fifo_th; j++) { + sleft[j] = i2s_read_reg(dev->i2s_base, + LRBR_LTHR(i)); + sright[j] = i2s_read_reg(dev->i2s_base, + RRBR_RTHR(i)); + } + + dw_pcm_transfer(sleft, sright, dev->xfer_bytes, + dev->fifo_th, &dev->binfo); + } + } + + i2s_clear_irqs(dev, SNDRV_PCM_STREAM_PLAYBACK); + i2s_clear_irqs(dev, SNDRV_PCM_STREAM_CAPTURE); + + return IRQ_HANDLED; +} + +static int i2s_pll_cfg(struct i2s_clk_config_data *config) +{ + const struct dw_i2s_pll *pll_cfg; + u32 rate = config->sample_rate; + u32 data_width = config->data_width; + int i; + + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) + pll_cfg = dw_i2s_pll_cfg_27m; + else + pll_cfg = dw_i2s_pll_cfg_28m; + + for (i = 0; pll_cfg[i].rate != 0; i++) { + if ((pll_cfg[i].rate == rate) && + (pll_cfg[i].data_width == data_width)) { + writel(pll_cfg[i].idiv, (void *)PLL_IDIV_ADDR); + writel(pll_cfg[i].fbdiv, (void *)PLL_FBDIV_ADDR); + writel(pll_cfg[i].odiv0, (void *)PLL_ODIV0_ADDR); + writel(pll_cfg[i].odiv1, (void *)PLL_ODIV1_ADDR); + return 0; + } + } + + return -EINVAL; +} + static void i2s_start(struct dw_i2s_dev *dev, struct snd_pcm_substream *substream) { + struct i2s_clk_config_data *config = &dev->config; u32 i, irq; i2s_write_reg(dev->i2s_base, IER, 1); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - for (i = 0; i < 4; i++) { + for (i = 0; i < (config->chan_nr / 2); i++) { irq = i2s_read_reg(dev->i2s_base, IMR(i)); i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x30); } i2s_write_reg(dev->i2s_base, ITER, 1); } else { - for (i = 0; i < 4; i++) { + for (i = 0; i < (config->chan_nr / 2); i++) { irq = i2s_read_reg(dev->i2s_base, IMR(i)); i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x03); } @@ -195,6 +384,139 @@ static void i2s_stop(struct dw_i2s_dev *dev, } } +static int dw_pcm_open(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_pcm_runtime *rt = substream->runtime; + struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(rtd->cpu_dai); + + snd_soc_set_runtime_hwparams(substream, &dw_pcm_hw); + snd_pcm_hw_constraint_integer(rt, SNDRV_PCM_HW_PARAM_PERIODS); + + rt->hw.rate_min = 32000; + rt->hw.rate_max = 48000; + + dev->binfo.stream = substream; + rt->private_data = &dev->binfo; + return 0; +} + +static int dw_pcm_close(struct snd_pcm_substream *substream) +{ + return 0; +} + +static int dw_pcm_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *hw_params) +{ + struct snd_pcm_runtime *rt = substream->runtime; + struct dw_pcm_binfo *bi = rt->private_data; + int ret; + + ret = snd_pcm_lib_alloc_vmalloc_buffer(substream, + params_buffer_bytes(hw_params)); + if (ret < 0) + return ret; + + memset(rt->dma_area, 0, params_buffer_bytes(hw_params)); + bi->dma_base = rt->dma_area; + bi->dma_pointer = bi->dma_base; + + return 0; +} + +static int dw_pcm_hw_free(struct snd_pcm_substream *substream) +{ + int ret; + + ret = snd_pcm_lib_free_vmalloc_buffer(substream); + if (ret < 0) + return ret; + + return 0; +} + +static int dw_pcm_prepare(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *rt = substream->runtime; + struct dw_pcm_binfo *bi = rt->private_data; + u32 buffer_size_frames = 0; + + bi->period_size_frames = bytes_to_frames(rt, + snd_pcm_lib_period_bytes(substream)); + bi->size = snd_pcm_lib_buffer_bytes(substream); + buffer_size_frames = bytes_to_frames(rt, bi->size); + bi->total_periods = buffer_size_frames / bi->period_size_frames; + bi->current_period = 1; + + if ((buffer_size_frames % bi->period_size_frames) != 0) + return -EINVAL; + + return 0; +} + +static int dw_pcm_trigger(struct snd_pcm_substream *substream, int cmd) +{ + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + break; + case SNDRV_PCM_TRIGGER_STOP: + break; + default: + return -EINVAL; + } + + return 0; +} + +static snd_pcm_uframes_t dw_pcm_pointer(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *rt = substream->runtime; + struct dw_pcm_binfo *bi = rt->private_data; + + return bi->period_pointer; +} + +static struct snd_pcm_ops dw_pcm_ops = { + .open = dw_pcm_open, + .close = dw_pcm_close, + .ioctl = snd_pcm_lib_ioctl, + .hw_params = dw_pcm_hw_params, + .hw_free = dw_pcm_hw_free, + .prepare = dw_pcm_prepare, + .trigger = dw_pcm_trigger, + .pointer = dw_pcm_pointer, + .page = snd_pcm_lib_get_vmalloc_page, + .mmap = snd_pcm_lib_mmap_vmalloc, +}; + +static int dw_pcm_new(struct snd_soc_pcm_runtime *runtime) +{ + struct snd_pcm *pcm = runtime->pcm; + int ret; + + ret = snd_pcm_lib_preallocate_pages_for_all(pcm, + SNDRV_DMA_TYPE_DEV, + snd_dma_continuous_data(GFP_KERNEL), + dw_pcm_hw.buffer_bytes_max, + dw_pcm_hw.buffer_bytes_max); + if (ret < 0) + return ret; + + return 0; +} + +static void dw_pcm_free(struct snd_pcm *pcm) +{ + snd_pcm_lib_preallocate_free_for_all(pcm); +} + +static struct snd_soc_platform_driver dw_pcm_soc_platform = { + .pcm_new = dw_pcm_new, + .pcm_free = dw_pcm_free, + .ops = &dw_pcm_ops, +}; + static int dw_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { @@ -231,14 +553,16 @@ static void dw_i2s_config(struct dw_i2s_dev *dev, int stream) if (stream == SNDRV_PCM_STREAM_PLAYBACK) { i2s_write_reg(dev->i2s_base, TCR(ch_reg), dev->xfer_resolution); - i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); + i2s_write_reg(dev->i2s_base, TFCR(ch_reg), + dev->fifo_th - 1); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); } else { i2s_write_reg(dev->i2s_base, RCR(ch_reg), dev->xfer_resolution); - i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); + i2s_write_reg(dev->i2s_base, RFCR(ch_reg), + dev->fifo_th - 1); irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03); i2s_write_reg(dev->i2s_base, RER(ch_reg), 1); @@ -259,22 +583,25 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, config->data_width = 16; dev->ccr = 0x00; dev->xfer_resolution = 0x02; + dev->xfer_bytes = 0x02; break; case SNDRV_PCM_FORMAT_S24_LE: config->data_width = 24; dev->ccr = 0x08; dev->xfer_resolution = 0x04; + dev->xfer_bytes = 0x03; break; case SNDRV_PCM_FORMAT_S32_LE: config->data_width = 32; dev->ccr = 0x10; dev->xfer_resolution = 0x05; + dev->xfer_bytes = 0x04; break; default: - dev_err(dev->dev, "designware-i2s: unsuppted PCM fmt"); + dev_err(dev->dev, "designware-i2s: unsupported PCM fmt"); return -EINVAL; } @@ -316,6 +643,7 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, } } } + return 0; } @@ -498,6 +826,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, */ u32 comp1 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp1); u32 comp2 = i2s_read_reg(dev->i2s_base, dev->i2s_reg_comp2); + u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1)); u32 idx; if (dev->capability & DWC_I2S_RECORD && @@ -536,6 +865,7 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, dev->capability |= DW_I2S_SLAVE; } + dev->fifo_th = fifo_depth / 2; return 0; } @@ -620,7 +950,7 @@ static int dw_i2s_probe(struct platform_device *pdev) const struct i2s_platform_data *pdata = pdev->dev.platform_data; struct dw_i2s_dev *dev; struct resource *res; - int ret; + int ret, irq_number; struct snd_soc_dai_driver *dw_i2s_dai; const char *clk_id; @@ -643,6 +973,19 @@ static int dw_i2s_probe(struct platform_device *pdev) if (IS_ERR(dev->i2s_base)) return PTR_ERR(dev->i2s_base); + irq_number = platform_get_irq(pdev, 0); + if (irq_number <= 0) { + dev_err(&pdev->dev, "get irq fail\n"); + return -EINVAL; + } + + ret = devm_request_irq(&pdev->dev, irq_number, i2s_irq_handler, + IRQF_SHARED, "dw_i2s_irq_handler", dev); + if (ret < 0) { + dev_err(&pdev->dev, "request irq fail\n"); + return ret; + } + dev->dev = &pdev->dev; dev->i2s_reg_comp1 = I2S_COMP_PARAM_1; @@ -671,14 +1014,22 @@ static int dw_i2s_probe(struct platform_device *pdev) return -ENODEV; } } - dev->clk = devm_clk_get(&pdev->dev, clk_id); - if (IS_ERR(dev->clk)) - return PTR_ERR(dev->clk); + if (dev->i2s_clk_cfg || + of_get_property(pdev->dev.of_node, "clocks", NULL)) { + dev->clk = devm_clk_get(&pdev->dev, clk_id); + + if (IS_ERR(dev->clk)) + return PTR_ERR(dev->clk); - ret = clk_prepare_enable(dev->clk); - if (ret < 0) - return ret; + ret = clk_prepare_enable(dev->clk); + if (ret < 0) + return ret; + } else { + /* Use internal PLL config */ + dev->i2s_clk_cfg = i2s_pll_cfg; + dev->clk = NULL; + } } dev_set_drvdata(&pdev->dev, dev); @@ -690,7 +1041,14 @@ static int dw_i2s_probe(struct platform_device *pdev) } if (!pdata) { - ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0); + if (of_get_property(pdev->dev.of_node, "dmas", NULL)) { + /* Using DMA */ + ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0); + } else { + ret = snd_soc_register_platform(&pdev->dev, + &dw_pcm_soc_platform); + } + if (ret) { dev_err(&pdev->dev, "Could not register PCM: %d\n", ret); @@ -714,6 +1072,7 @@ static int dw_i2s_remove(struct platform_device *pdev) clk_disable_unprepare(dev->clk); pm_runtime_disable(&pdev->dev); + snd_soc_unregister_platform(&pdev->dev); return 0; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <538be366488bf0d7633d702f2d0bab16707b7a47.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support [not found] ` <538be366488bf0d7633d702f2d0bab16707b7a47.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> @ 2016-03-28 15:35 ` Alexey Brodkin [not found] ` <1459179345.4785.127.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Alexey Brodkin @ 2016-03-28 15:35 UTC (permalink / raw) To: Jose Abreu Cc: lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, pawel.moll-5wv7dgnIgG8@public.gmane.org, Carlos Palminha, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, alexander.deucher-5C7GfCeVMHo@public.gmane.org, tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA@public.gmane.org, linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org Hi Jose, On Mon, 2016-03-28 at 15:36 +0100, Jose Abreu wrote: > HDMI audio support was added to the AXS board using an > I2S cpu driver and a custom platform driver. > > The platform driver supports two channels @ 16 bits with > rates 32k, 44.1k and 48k. ALSA Simple audio card is used to > glue the cpu, platform and codec driver (adv7511). > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > --- > > No changes v1 -> v2. > > sound/soc/dwc/Kconfig | 1 + > sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 373 insertions(+), 13 deletions(-) > > diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig > index d50e085..bc3fae7 100644 > --- a/sound/soc/dwc/Kconfig > +++ b/sound/soc/dwc/Kconfig > @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S > tristate "Synopsys I2S Device Driver" > depends on CLKDEV_LOOKUP > select SND_SOC_GENERIC_DMAENGINE_PCM > + select SND_SIMPLE_CARD > help > Say Y or M if you want to add support for I2S driver for > Synopsys desigwnware I2S device. The device supports upto > diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c > index bff258d..0f2f588 100644 > --- a/sound/soc/dwc/designware_i2s.c > +++ b/sound/soc/dwc/designware_i2s.c > @@ -84,11 +84,37 @@ > #define MAX_CHANNEL_NUM 8 > #define MIN_CHANNEL_NUM 2 > > +/* FPGA Version Info */ > +#define FPGA_VER_INFO 0xE0011230 > +#define FPGA_VER_27M 0x000FBED9 > + > +/* PLL registers addresses */ > +#define PLL_IDIV_ADDR 0xE00100A0 > +#define PLL_FBDIV_ADDR 0xE00100A4 > +#define PLL_ODIV0_ADDR 0xE00100A8 > +#define PLL_ODIV1_ADDR 0xE00100AC Well I think all is not acceptable. See all these FPGA_VER_xxx as well as PLL_xxx are strictly ARC SDP specific things and have nothing to do with generic driver. That's so pity we don't have a driver for all clocks/PLLs on ARC SDP yet. So as of now I may only propose to use hard-coded fixed clocks as I did with ARC PGU, see "pguclk" here: http://lists.infradead.org/pipermail/linux-snps-arc/2016-March/000790.html Again I'll try to implement missing clock driver sometime soon because more and more stuff requires it but for now let's use a work-around. > +struct dw_i2s_pll { > + unsigned int rate; > + unsigned int data_width; > + unsigned int idiv; > + unsigned int fbdiv; > + unsigned int odiv0; > + unsigned int odiv1; > +}; > + > +static const struct dw_i2s_pll dw_i2s_pll_cfg_27m[] = { > + /* 27Mhz */ > + { 32000, 16, 0x104, 0x451, 0x10E38, 0x2000 }, > + { 44100, 16, 0x104, 0x596, 0x10D35, 0x2000 }, > + { 48000, 16, 0x208, 0xA28, 0x10B2C, 0x2000 }, > + { 0, 0, 0, 0, 0, 0 }, > }; > > +static const struct dw_i2s_pll dw_i2s_pll_cfg_28m[] = { > + /* 28.224Mhz */ > + { 32000, 16, 0x82, 0x105, 0x107DF, 0x2000 }, > + { 44100, 16, 0x28A, 0x1, 0x10001, 0x2000 }, > + { 48000, 16, 0xA28, 0x187, 0x10042, 0x2000 }, > + { 0, 0, 0, 0, 0, 0 }, > +}; These 2 hunks as well should go in ARC SDP clocks. > +static int i2s_pll_cfg(struct i2s_clk_config_data *config) > +{ > + const struct dw_i2s_pll *pll_cfg; > + u32 rate = config->sample_rate; > + u32 data_width = config->data_width; > + int i; > + > + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) > + pll_cfg = dw_i2s_pll_cfg_27m; > + else > + pll_cfg = dw_i2s_pll_cfg_28m; > + > + for (i = 0; pll_cfg[i].rate != 0; i++) { > + if ((pll_cfg[i].rate == rate) && > + (pll_cfg[i].data_width == data_width)) { > + writel(pll_cfg[i].idiv, (void *)PLL_IDIV_ADDR); > + writel(pll_cfg[i].fbdiv, (void *)PLL_FBDIV_ADDR); > + writel(pll_cfg[i].odiv0, (void *)PLL_ODIV0_ADDR); > + writel(pll_cfg[i].odiv1, (void *)PLL_ODIV1_ADDR); > + return 0; > + } > + } > + > + return -EINVAL; > +} Ditto. -Alexey-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <1459179345.4785.127.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support [not found] ` <1459179345.4785.127.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> @ 2016-03-28 16:07 ` Jose Abreu 0 siblings, 0 replies; 23+ messages in thread From: Jose Abreu @ 2016-03-28 16:07 UTC (permalink / raw) To: Alexey Brodkin Cc: lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, pawel.moll-5wv7dgnIgG8@public.gmane.org, Carlos Palminha, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, alexander.deucher-5C7GfCeVMHo@public.gmane.org, tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA@public.gmane.org, linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org Hi Alexey, On 28-03-2016 16:35, Alexey Brodkin wrote: > Hi Jose, > > On Mon, 2016-03-28 at 15:36 +0100, Jose Abreu wrote: >> HDMI audio support was added to the AXS board using an >> I2S cpu driver and a custom platform driver. >> >> The platform driver supports two channels @ 16 bits with >> rates 32k, 44.1k and 48k. ALSA Simple audio card is used to >> glue the cpu, platform and codec driver (adv7511). >> >> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> >> --- >> >> No changes v1 -> v2. >> >> sound/soc/dwc/Kconfig | 1 + >> sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 373 insertions(+), 13 deletions(-) >> >> diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig >> index d50e085..bc3fae7 100644 >> --- a/sound/soc/dwc/Kconfig >> +++ b/sound/soc/dwc/Kconfig >> @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S >> tristate "Synopsys I2S Device Driver" >> depends on CLKDEV_LOOKUP >> select SND_SOC_GENERIC_DMAENGINE_PCM >> + select SND_SIMPLE_CARD >> help >> Say Y or M if you want to add support for I2S driver for >> Synopsys desigwnware I2S device. The device supports upto >> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c >> index bff258d..0f2f588 100644 >> --- a/sound/soc/dwc/designware_i2s.c >> +++ b/sound/soc/dwc/designware_i2s.c >> @@ -84,11 +84,37 @@ >> #define MAX_CHANNEL_NUM 8 >> #define MIN_CHANNEL_NUM 2 >> >> +/* FPGA Version Info */ >> +#define FPGA_VER_INFO 0xE0011230 >> +#define FPGA_VER_27M 0x000FBED9 >> + >> +/* PLL registers addresses */ >> +#define PLL_IDIV_ADDR 0xE00100A0 >> +#define PLL_FBDIV_ADDR 0xE00100A4 >> +#define PLL_ODIV0_ADDR 0xE00100A8 >> +#define PLL_ODIV1_ADDR 0xE00100AC > Well I think all is not acceptable. > See all these FPGA_VER_xxx as well as PLL_xxx > are strictly ARC SDP specific things and have nothing to do with generic driver. > > That's so pity we don't have a driver for all clocks/PLLs on ARC SDP yet. > So as of now I may only propose to use hard-coded fixed clocks as I did with > ARC PGU, see "pguclk" here: > http://lists.infradead.org/pipermail/linux-snps-arc/2016-March/000790.html > > Again I'll try to implement missing clock driver sometime soon because > more and more stuff requires it but for now let's use a work-around. Yes, this is a workaround that we are using so that the driver works in ARC SDP platforms. The driver still has the functionality to operate using a clock driver (it must be declared in device tree) but if the clock handle is not declared the driver will assume that must use the internal PLL config options. This is currently the only option to make it work in ARC SDP. I will send a v3 soon without this workaround and when the missing clock drivers are implemented I will re-test this. >> +struct dw_i2s_pll { >> + unsigned int rate; >> + unsigned int data_width; >> + unsigned int idiv; >> + unsigned int fbdiv; >> + unsigned int odiv0; >> + unsigned int odiv1; >> +}; >> + >> +static const struct dw_i2s_pll dw_i2s_pll_cfg_27m[] = { >> + /* 27Mhz */ >> + { 32000, 16, 0x104, 0x451, 0x10E38, 0x2000 }, >> + { 44100, 16, 0x104, 0x596, 0x10D35, 0x2000 }, >> + { 48000, 16, 0x208, 0xA28, 0x10B2C, 0x2000 }, >> + { 0, 0, 0, 0, 0, 0 }, >> }; >> >> +static const struct dw_i2s_pll dw_i2s_pll_cfg_28m[] = { >> + /* 28.224Mhz */ >> + { 32000, 16, 0x82, 0x105, 0x107DF, 0x2000 }, >> + { 44100, 16, 0x28A, 0x1, 0x10001, 0x2000 }, >> + { 48000, 16, 0xA28, 0x187, 0x10042, 0x2000 }, >> + { 0, 0, 0, 0, 0, 0 }, >> +}; > These 2 hunks as well should go in ARC SDP clocks. > >> +static int i2s_pll_cfg(struct i2s_clk_config_data *config) >> +{ >> + const struct dw_i2s_pll *pll_cfg; >> + u32 rate = config->sample_rate; >> + u32 data_width = config->data_width; >> + int i; >> + >> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) >> + pll_cfg = dw_i2s_pll_cfg_27m; >> + else >> + pll_cfg = dw_i2s_pll_cfg_28m; >> + >> + for (i = 0; pll_cfg[i].rate != 0; i++) { >> + if ((pll_cfg[i].rate == rate) && >> + (pll_cfg[i].data_width == data_width)) { >> + writel(pll_cfg[i].idiv, (void *)PLL_IDIV_ADDR); >> + writel(pll_cfg[i].fbdiv, (void *)PLL_FBDIV_ADDR); >> + writel(pll_cfg[i].odiv0, (void *)PLL_ODIV0_ADDR); >> + writel(pll_cfg[i].odiv1, (void *)PLL_ODIV1_ADDR); >> + return 0; >> + } >> + } >> + >> + return -EINVAL; >> +} > Ditto. > > -Alexey Best regards, Jose Miguel Abreu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support 2016-03-28 14:36 ` [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI " Jose Abreu [not found] ` <538be366488bf0d7633d702f2d0bab16707b7a47.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> @ 2016-03-29 17:31 ` Mark Brown 2016-03-29 18:03 ` Jose Abreu 1 sibling, 1 reply; 23+ messages in thread From: Mark Brown @ 2016-03-29 17:31 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, lgirdwood, Alexey.Brodkin, dri-devel, linux-kernel, yitian.bu, wsa+renesas, laurent.pinchart+renesas, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, tiwai, CARLOS.PALMINHA, galak, alexander.deucher [-- Attachment #1.1: Type: text/plain, Size: 1736 bytes --] On Mon, Mar 28, 2016 at 03:36:10PM +0100, Jose Abreu wrote: > HDMI audio support was added to the AXS board using an > I2S cpu driver and a custom platform driver. > > The platform driver supports two channels @ 16 bits with > rates 32k, 44.1k and 48k. ALSA Simple audio card is used to > glue the cpu, platform and codec driver (adv7511). > sound/soc/dwc/Kconfig | 1 + > sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- Your changelog appears to describe the writing of a machine driver but this is a large patch adding code to an I2S controller driver. This means I can't review your patch since I can't tell what it is supposed to do. If you've added functionality to this driver you need to send one or more patches each of which adds a single feature to the driver together with a changelog which describes what that feature is. Glancing at the patch I'm not 100% sure that the features you're adding are part of the Synopsis device but I'm not entirely sure. > 2 files changed, 373 insertions(+), 13 deletions(-) > > diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig > index d50e085..bc3fae7 100644 > --- a/sound/soc/dwc/Kconfig > +++ b/sound/soc/dwc/Kconfig > @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S > tristate "Synopsys I2S Device Driver" > depends on CLKDEV_LOOKUP > select SND_SOC_GENERIC_DMAENGINE_PCM > + select SND_SIMPLE_CARD No, this doesn't make sense - the fact that someone has used a Synopsis I2S controller doesn't mean that they have a system which uses simple-card. If the user wants to use simple-card they need to enable it separately, this is the same pattern we follow for all CPU controller drivers. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support 2016-03-29 17:31 ` Mark Brown @ 2016-03-29 18:03 ` Jose Abreu 2016-03-29 18:22 ` Mark Brown 0 siblings, 1 reply; 23+ messages in thread From: Jose Abreu @ 2016-03-29 18:03 UTC (permalink / raw) To: Mark Brown, Jose Abreu Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, dri-devel, linux-kernel, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, lgirdwood, CARLOS.PALMINHA, galak, alexander.deucher [-- Attachment #1.1: Type: text/plain, Size: 2322 bytes --] Hi Mark, On 29-03-2016 18:31, Mark Brown wrote: > On Mon, Mar 28, 2016 at 03:36:10PM +0100, Jose Abreu wrote: >> HDMI audio support was added to the AXS board using an >> I2S cpu driver and a custom platform driver. >> >> The platform driver supports two channels @ 16 bits with >> rates 32k, 44.1k and 48k. ALSA Simple audio card is used to >> glue the cpu, platform and codec driver (adv7511). >> sound/soc/dwc/Kconfig | 1 + >> sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++-- > Your changelog appears to describe the writing of a machine driver but > this is a large patch adding code to an I2S controller driver. This > means I can't review your patch since I can't tell what it is supposed > to do. If you've added functionality to this driver you need to send > one or more patches each of which adds a single feature to the driver > together with a changelog which describes what that feature is. > > Glancing at the patch I'm not 100% sure that the features you're adding > are part of the Synopsis device but I'm not entirely sure. > The major part of this patch is the adding of an ALSA platform driver so that audio comes out of the box in AXS boards but we also added functionalities to the i2s driver and performed one bug fix related with the mask/unmask of interrupts. I will split the patches but they will depend on each other. >> 2 files changed, 373 insertions(+), 13 deletions(-) >> >> diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig >> index d50e085..bc3fae7 100644 >> --- a/sound/soc/dwc/Kconfig >> +++ b/sound/soc/dwc/Kconfig >> @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S >> tristate "Synopsys I2S Device Driver" >> depends on CLKDEV_LOOKUP >> select SND_SOC_GENERIC_DMAENGINE_PCM >> + select SND_SIMPLE_CARD > No, this doesn't make sense - the fact that someone has used a Synopsis > I2S controller doesn't mean that they have a system which uses > simple-card. If the user wants to use simple-card they need to enable > it separately, this is the same pattern we follow for all CPU controller > drivers. > I will remove this. > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Best regards, Jose Miguel Abreu [-- Attachment #1.2: Type: text/html, Size: 3471 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support 2016-03-29 18:03 ` Jose Abreu @ 2016-03-29 18:22 ` Mark Brown 2016-03-31 9:37 ` Jose Abreu 0 siblings, 1 reply; 23+ messages in thread From: Mark Brown @ 2016-03-29 18:22 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, dri-devel, linux-kernel, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, lgirdwood, CARLOS.PALMINHA, galak, alexander.deucher [-- Attachment #1.1: Type: text/plain, Size: 514 bytes --] On Tue, Mar 29, 2016 at 07:03:01PM +0100, Jose Abreu wrote: > The major part of this patch is the adding of an ALSA platform driver so that > audio comes out of the box in AXS boards but we also added functionalities to > the i2s driver and performed one bug fix related with the mask/unmask of > interrupts. I will split the patches but they will depend on each other. If you want to add a new platform driver you need to add a new platform driver, not shove the code into an existing driver for a seperate IP. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support 2016-03-29 18:22 ` Mark Brown @ 2016-03-31 9:37 ` Jose Abreu 2016-03-31 16:56 ` Mark Brown 0 siblings, 1 reply; 23+ messages in thread From: Jose Abreu @ 2016-03-31 9:37 UTC (permalink / raw) To: Mark Brown, Jose Abreu Cc: tixy, mark.rutland, alsa-devel, Alexey.Brodkin, dri-devel, linux-kernel, yitian.bu, wsa+renesas, laurent.pinchart+renesas, tiwai, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, lgirdwood, CARLOS.PALMINHA, galak, alexander.deucher Hi Mark, On 29-03-2016 19:22, Mark Brown wrote: > On Tue, Mar 29, 2016 at 07:03:01PM +0100, Jose Abreu wrote: > >> The major part of this patch is the adding of an ALSA platform driver so that >> audio comes out of the box in AXS boards but we also added functionalities to >> the i2s driver and performed one bug fix related with the mask/unmask of >> interrupts. I will split the patches but they will depend on each other. > If you want to add a new platform driver you need to add a new platform > driver, not shove the code into an existing driver for a seperate IP. > I can separate the platform driver into a new file but they will have to be compiled into the same module as the new additions to the i2s driver depend on functions of the platform driver (see i2s_irq_handler()). Or should I divide this into two modules and add a Kconfig option to the platform driver? Besides this I first wanted the driver to be compiled into the same module so that it is compatible with kernel 3.18 where simple audio card requires that platform driver == cpu driver. Best regards, Jose Miguel Abreu ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI audio support 2016-03-31 9:37 ` Jose Abreu @ 2016-03-31 16:56 ` Mark Brown 0 siblings, 0 replies; 23+ messages in thread From: Mark Brown @ 2016-03-31 16:56 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, lgirdwood, Alexey.Brodkin, dri-devel, tiwai, yitian.bu, wsa+renesas, laurent.pinchart+renesas, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, linux-kernel, CARLOS.PALMINHA, galak, alexander.deucher [-- Attachment #1.1: Type: text/plain, Size: 1258 bytes --] On Thu, Mar 31, 2016 at 10:37:20AM +0100, Jose Abreu wrote: > On 29-03-2016 19:22, Mark Brown wrote: > > If you want to add a new platform driver you need to add a new platform > > driver, not shove the code into an existing driver for a seperate IP. > I can separate the platform driver into a new file but they will have to be > compiled into the same module as the new additions to the i2s driver depend on > functions of the platform driver (see i2s_irq_handler()). Or should I divide No, that's not at all acceptable. The Designware IP is not specific to your system, you can't make it depend on your platform driver. The kernel needs to work on other people's systems too. You need to work through and/or extend the abstractions the framework provides to separate the drivers for different IPs. > this into two modules and add a Kconfig option to the platform driver? Besides > this I first wanted the driver to be compiled into the same module so that it is > compatible with kernel 3.18 where simple audio card requires that platform > driver == cpu driver. That's not OK upstream, we're working on the current kernel not on random old kernels. We don't carry compatibility code to enable current kernel code to be run on years old kernels. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3 v2] arc: axs10x: Add support for Designware I2S on DT [not found] ` <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 2016-03-28 14:36 ` [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI " Jose Abreu @ 2016-03-28 14:36 ` Jose Abreu 1 sibling, 0 replies; 23+ messages in thread From: Jose Abreu @ 2016-03-28 14:36 UTC (permalink / raw) To: linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: airlied-cv59FeDIM0c, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/, lars-Qo5EllUWu/uELgA04lAiVw, ville.syrjala-VuQAYsv1563Yd54FQh9/CA, nariman-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E, alexander.deucher-5C7GfCeVMHo, Maruthi.Bayyavarapu-5C7GfCeVMHo, buyitian-Re5JQEeQqe8AvxtiuMwx3w, tixy-QSEj5FYQhm4dnm+yROfE0A, yitian.bu-sf4bEIuTEZ8/jFmSBShxMA, Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, galak-sgV2jX0FEOL9JmXXK+q4OQ, Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w, Jose Abreu Synopsys Designware ARC SDP boards support HDMI audio output using the ADV7511 HDMI transmitter. This patchs enables audio output using Designware I2S driver, ALSA SoC simple audio card and ADV7511 codec. Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> --- Changes v1 -> v2: * This change was introduced in v2. arch/arc/boot/dts/axs10x_mb.dtsi | 49 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi index ab5d570..fc26ede 100644 --- a/arch/arc/boot/dts/axs10x_mb.dtsi +++ b/arch/arc/boot/dts/axs10x_mb.dtsi @@ -138,12 +138,23 @@ interrupts = <14>; }; - i2c@0x1e000 { - compatible = "snps,designware-i2c"; + i2s: i2s@1e000 { + compatible = "snps,designware-i2s"; reg = <0x1e000 0x100>; - clock-frequency = <400000>; - clocks = <&i2cclk>; interrupts = <15>; + #sound-dai-cells = <0>; + }; + + sound { + compatible = "simple-audio-card"; + simple-audio-card,name = "AXS10X HDMI Audio"; + simple-audio-card,format = "i2s"; + simple-audio-card,cpu { + sound-dai = <&i2s>; + }; + simple-audio-card,codec { + sound-dai = <&adv7511>; + }; }; i2c@0x1f000 { @@ -155,6 +166,36 @@ clocks = <&i2cclk>; interrupts = <16>; + adv7511: adv7511@39 { + compatible = "adi,adv7511"; + reg = <0x39>; + interrupts = <23>; + adi,input-depth = <8>; + adi,input-colorspace = "rgb"; + adi,input-clock = "1x"; + adi,clock-delay = <0x03>; + #sound-dai-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + /* RGB/YUV input */ + port@0 { + reg = <0>; + adv7511_input: endpoint { + }; + }; + + /* HDMI output */ + port@1 { + reg = <1>; + adv7511_output: endpoint { + }; + }; + }; + }; + eeprom@0x54{ compatible = "24c01"; reg = <0x54>; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards 2016-03-28 14:36 [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards Jose Abreu 2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu [not found] ` <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> @ 2016-03-29 17:00 ` Mark Brown 2 siblings, 0 replies; 23+ messages in thread From: Mark Brown @ 2016-03-29 17:00 UTC (permalink / raw) To: Jose Abreu Cc: tixy, mark.rutland, alsa-devel, lgirdwood, Alexey.Brodkin, dri-devel, linux-kernel, yitian.bu, wsa+renesas, laurent.pinchart+renesas, nariman, linux-snps-arc, devicetree, Maruthi.Bayyavarapu, pawel.moll, ijc+devicetree, Vineet.Gupta1, buyitian, robh+dt, perex, tiwai, CARLOS.PALMINHA, galak, alexander.deucher [-- Attachment #1.1: Type: text/plain, Size: 951 bytes --] On Mon, Mar 28, 2016 at 03:36:08PM +0100, Jose Abreu wrote: > ARC AXS10x platforms consist of a mainboard with several peripherals. > One of those peripherals is an HDMI output port controlled by the ADV7511 > transmitter. I'm going to tell you the same thing I tell everyone else working on HDMI audio integration: you all need to talk to each other and review each other's code and unless there is a very good reason for it there should be at least some code sharing. For example take a look at the patches Jyri Sarha has been posting recently. I don't have detailed knowledge of HDMI and the range of hardware that's out there for it but I am seeing a number of different people posting patch serieses that look a lot like each other and like they should be sharing things. Please also try to keep your CC lists reasonable, the set of people you've copied on this stuff is enormous and I'm having trouble seeing why a lot of tehm are included. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-04-05 16:03 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-28 14:36 [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards Jose Abreu
2016-03-28 14:36 ` [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Jose Abreu
2016-03-29 8:05 ` Archit Taneja
2016-03-29 10:52 ` Jose Abreu
2016-03-29 17:03 ` Archit Taneja
[not found] ` <56FAB557.3060900-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-03-31 12:57 ` Jose Abreu
2016-03-30 9:58 ` Emil Velikov
2016-04-01 17:10 ` Laurent Pinchart
2016-04-04 9:05 ` Jose Abreu
2016-04-04 21:41 ` Laurent Pinchart
2016-04-05 11:00 ` Jose Abreu
2016-04-05 16:03 ` Laurent Pinchart
2016-04-03 5:05 ` kbuild test robot
[not found] ` <cover.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2016-03-28 14:36 ` [PATCH 2/3 v2] ASoC: dwc: Add I2S HDMI " Jose Abreu
[not found] ` <538be366488bf0d7633d702f2d0bab16707b7a47.1459174494.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2016-03-28 15:35 ` Alexey Brodkin
[not found] ` <1459179345.4785.127.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2016-03-28 16:07 ` Jose Abreu
2016-03-29 17:31 ` Mark Brown
2016-03-29 18:03 ` Jose Abreu
2016-03-29 18:22 ` Mark Brown
2016-03-31 9:37 ` Jose Abreu
2016-03-31 16:56 ` Mark Brown
2016-03-28 14:36 ` [PATCH 3/3 v2] arc: axs10x: Add support for Designware I2S on DT Jose Abreu
2016-03-29 17:00 ` [PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards Mark Brown
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).