Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support
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
---
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
+
+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, 0x);
+
+ 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
Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support
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
---
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
+
+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, 0x);
+
+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
futur
Re: [PATCH] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
Hi,
On 10/14/2016 6:57 PM, Eugeniy Paltsev wrote:
ARC PGU driver starts crashing on initialization after
'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
of "drm_i2c_encoder_driver" structure, which doesn't exist after
adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
So, when we call "encoder_init" function from this structure driver
crashes.
The conversion doesn't seem entirely correct. Some comments below.
Bootlog:
->8
[drm] Initialized drm 1.1.0 20060810
arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e00
Path: (null)
CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8
task: 9a058000 task.stack: 9a032000
[ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8
[EFA ]: 0x0004
[BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
[ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230
[STAT32]: 0x0846 : K DE E2 E1
BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x
LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x
r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11
r03: 0x r04: 0x9a010f54 r05: 0x
r06: 0x0001 r07: 0x r08: 0x0028
r09: 0x0001 r10: 0x0007 r11: 0x0054
r12: 0x720a3033
Stack Trace:
drm_atomic_helper_connector_dpms+0xa4/0x230
arcpgu_drm_hdmi_init+0xbc/0x228
arcpgu_probe+0x168/0x244
platform_drv_probe+0x26/0x64
really_probe+0x1f0/0x32c
__driver_attach+0xa8/0xd0
bus_for_each_dev+0x3c/0x74
bus_add_driver+0xc2/0x184
driver_register+0x50/0xec
do_one_initcall+0x3a/0x120
kernel_init_freeable+0x108/0x1a0
->8
Fix ARC PGU driver to be able work with drm bridge hdmi encoder
interface. The hdmi connector code isn't needed anymore as we expect
the adv7511 bridge driver to create/manage the connector.
Signed-off-by: Eugeniy Paltsev
---
drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +-
1 file changed, 35 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c
b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index b7a8b2a..48a6c63 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -14,104 +14,54 @@
*
*/
+#include
#include
#include
#include
#include "arcpgu.h"
-struct arcpgu_drm_connector {
- struct drm_connector connector;
- struct drm_encoder_slave *encoder_slave;
-};
-
-static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
+static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+ struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
{
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_get_modes: cannot find slave encoder for
connector\n");
- return 0;
- }
-
- sfuncs = slave->slave_funcs;
- if (sfuncs->get_modes == NULL)
- return 0;
-
- return sfuncs->get_modes(&slave->base, connector);
-}
-
-static enum drm_connector_status
-arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
-{
- enum drm_connector_status status = connector_status_unknown;
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
-
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_detect: cannot find slave encoder for
connector\n");
- return status;
- }
+ struct drm_bridge *bridge = encoder->bridge;
- sfuncs = slave->slave_funcs;
- if (sfuncs && sfuncs->detect)
- return sfuncs->detect(&slave->base, connector);
-
- dev_err(connector->dev->dev, "connector_detect: could not detect slave
funcs\n");
- return status;
+ bridge->funcs->mode_set(bridge, mode, adjusted_mode);
The bridge functions shouldn't really be called by the kms driver.
They're called automatically by the drm core for the bridge attached
to the encoder. See mode_fixup in drivers/gpu/drm/drm_atomic_helper.c
}
-static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
-{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_hel
Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
ARC PGU driver starts crashing on initialization after
'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
of "drm_i2c_encoder_driver" structure, which doesn't exist after
adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
So, when we call "encoder_init" function from this structure driver
crashes.
Bootlog:
->8
[drm] Initialized drm 1.1.0 20060810
arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e00
Path: (null)
CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8
task: 9a058000 task.stack: 9a032000
[ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8
[EFA ]: 0x0004
[BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
[ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230
[STAT32]: 0x0846 : K DE E2 E1
BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x
LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x
r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11
r03: 0x r04: 0x9a010f54 r05: 0x
r06: 0x0001 r07: 0x r08: 0x0028
r09: 0x0001 r10: 0x0007 r11: 0x0054
r12: 0x720a3033
Stack Trace:
drm_atomic_helper_connector_dpms+0xa4/0x230
arcpgu_drm_hdmi_init+0xbc/0x228
arcpgu_probe+0x168/0x244
platform_drv_probe+0x26/0x64
really_probe+0x1f0/0x32c
__driver_attach+0xa8/0xd0
bus_for_each_dev+0x3c/0x74
bus_add_driver+0xc2/0x184
driver_register+0x50/0xec
do_one_initcall+0x3a/0x120
kernel_init_freeable+0x108/0x1a0
->8
Fix ARC PGU driver to be able work with drm bridge hdmi encoder
interface. The hdmi connector code isn't needed anymore as we expect
the adv7511 bridge driver to create/manage the connector.
Looks good now.
Reviewed-by: Archit Taneja
Signed-off-by: Eugeniy Paltsev
---
Changes for v2:
- remove bridge functions call from kms driver
- get rid of drm_encoder_slave interface
- coding style cleanup
drivers/gpu/drm/arc/arcpgu_hdmi.c | 159 --
1 file changed, 17 insertions(+), 142 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c
b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index b7a8b2a..b69c66b 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -14,170 +14,45 @@
*
*/
-#include
+#include
#include
-#include
#include "arcpgu.h"
-struct arcpgu_drm_connector {
- struct drm_connector connector;
- struct drm_encoder_slave *encoder_slave;
-};
-
-static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
-{
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_get_modes: cannot find slave encoder for
connector\n");
- return 0;
- }
-
- sfuncs = slave->slave_funcs;
- if (sfuncs->get_modes == NULL)
- return 0;
-
- return sfuncs->get_modes(&slave->base, connector);
-}
-
-static enum drm_connector_status
-arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
-{
- enum drm_connector_status status = connector_status_unknown;
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
-
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_detect: cannot find slave encoder for
connector\n");
- return status;
- }
-
- sfuncs = slave->slave_funcs;
- if (sfuncs && sfuncs->detect)
- return sfuncs->detect(&slave->base, connector);
-
- dev_err(connector->dev->dev, "connector_detect: could not detect slave
funcs\n");
- return status;
-}
-
-static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
-{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_helper_funcs
-arcpgu_drm_connector_helper_funcs = {
- .get_modes = arcpgu_drm_connector_get_modes,
-};
-
-static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
-
