On Tue, Aug 16, 2016 at 11:26:43PM +0300, Vladimir Zapolskiy wrote: > This change is needed to properly lock I2C bus driver, which serves > DDC. > > The change fixes an overflow over zero of I2C bus driver user counter: > > root@imx6q:~# lsmod > Not tainted > dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000 > dw_hdmi_imx 3498 0 - Live 0xbf00d000 > dw_hdmi 16398 2 dw_hdmi_ahb_audio,dw_hdmi_imx, Live 0xbf004000 > i2c_imx 16687 0 - Live 0xbf017000 > > root@imx6q:~# rmmod dw_hdmi_imx > root@imx6q:~# lsmod > Not tainted > dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000 > dw_hdmi 16398 1 dw_hdmi_ahb_audio, Live 0xbf004000 > i2c_imx 16687 -1 - Live 0xbf017000 > ^^ > > root@imx6q:~# rmmod i2c_imx > rmmod: ERROR: Module i2c_imx is in use > > Note that prior to this change put_device() coupled with > of_find_i2c_adapter_by_node() was missing on error path of > dw_hdmi_bind(), added i2c_put_adapter() there along with the change. I *guess* this is the right thing, but it would be nice to see the results with the patch applied in the commit description. Nevertheless: Acked-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx> I'd also like to see the DDC bits extracted from the various imx drivers, because this is surely common code - I've had this floating around for a few years but never got around to finishing it off and submitting it. If folk think this is a good idea, and want to take the idea on, that's fine by me. drivers/gpu/drm/Makefile | 2 + drivers/gpu/drm/bridge/dw-hdmi.c | 98 +++++++++---------------------------- drivers/gpu/drm/drm_ddc_connector.c | 91 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/imx/imx-tve.c | 59 ++++++---------------- include/drm/drm_ddc_connector.h | 33 +++++++++++++ 5 files changed, 163 insertions(+), 120 deletions(-) diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 0238bf8bc8c3..e31c72f86f8c 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -21,6 +21,8 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o +drm-y += drm_ddc_connector.o + drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ drm_kms_helper_common.o drm_dp_dual_mode_helper.o \ diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c index 65dd102689ef..786c0c076585 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/dw-hdmi.c @@ -27,6 +27,7 @@ #include <drm/drm_crtc_helper.h> #include <drm/drm_edid.h> #include <drm/drm_encoder_slave.h> +#include <drm/drm_ddc_connector.h> #include <drm/bridge/dw_hdmi.h> #include "dw-hdmi.h" @@ -103,7 +104,7 @@ struct hdmi_data_info { }; struct dw_hdmi { - struct drm_connector connector; + struct drm_ddc_connector *ddc_conn; struct drm_encoder *encoder; struct drm_bridge *bridge; @@ -124,10 +125,7 @@ struct dw_hdmi { bool phy_enabled; struct drm_display_mode previous_mode; - struct i2c_adapter *ddc; void __iomem *regs; - bool sink_is_hdmi; - bool sink_has_audio; struct mutex mutex; /* for state below and previous_mode */ enum drm_connector_force force; /* mutex-protected force state */ @@ -862,7 +860,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi) bool cscon; /*check csc whether needed activated in HDMI mode */ - cscon = hdmi->sink_is_hdmi && is_color_space_conversion(hdmi); + cscon = hdmi->ddc_conn->sink_is_hdmi && is_color_space_conversion(hdmi); /* HDMI Phy spec says to do the phy initialization sequence twice */ for (i = 0; i < 2; i++) { @@ -1039,7 +1037,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, HDMI_FC_INVIDCONF_IN_I_P_INTERLACED : HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE; - inv_val |= hdmi->sink_is_hdmi ? + inv_val |= hdmi->ddc_conn->sink_is_hdmi ? HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE : HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE; @@ -1238,7 +1236,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) /* HDMI Initialization Step B.3 */ dw_hdmi_enable_video_path(hdmi); - if (hdmi->sink_has_audio) { + if (hdmi->ddc_conn->sink_has_audio) { dev_dbg(hdmi->dev, "sink has audio support\n"); /* HDMI Initialization Step E - Configure audio */ @@ -1262,7 +1260,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) hdmi_tx_hdcp_config(hdmi); dw_hdmi_clear_overflow(hdmi); - if (hdmi->cable_plugin && hdmi->sink_is_hdmi) + if (hdmi->cable_plugin && hdmi->ddc_conn->sink_is_hdmi) hdmi_enable_overflow_interrupts(hdmi); return 0; @@ -1438,8 +1436,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge) static enum drm_connector_status dw_hdmi_connector_detect(struct drm_connector *connector, bool force) { - struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, - connector); + struct dw_hdmi *hdmi = drm_ddc_private(connector); mutex_lock(&hdmi->mutex); hdmi->force = DRM_FORCE_UNSPECIFIED; @@ -1451,43 +1448,11 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force) connector_status_connected : connector_status_disconnected; } -static int dw_hdmi_connector_get_modes(struct drm_connector *connector) -{ - struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, - connector); - struct edid *edid; - int ret = 0; - - if (!hdmi->ddc) - return 0; - - edid = drm_get_edid(connector, hdmi->ddc); - if (edid) { - dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", - edid->width_cm, edid->height_cm); - - hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); - hdmi->sink_has_audio = drm_detect_monitor_audio(edid); - drm_mode_connector_update_edid_property(connector, edid); - hdmi_event_new_edid(hdmi->dev, edid, 0); - ret = drm_add_edid_modes(connector, edid); - /* Store the ELD */ - drm_edid_to_eld(connector, edid); - hdmi_event_new_eld(hdmi->dev, connector->eld); - kfree(edid); - } else { - dev_dbg(hdmi->dev, "failed to get edid\n"); - } - - return ret; -} - static enum drm_mode_status dw_hdmi_connector_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { - struct dw_hdmi *hdmi = container_of(connector, - struct dw_hdmi, connector); + struct dw_hdmi *hdmi = drm_ddc_private(connector); enum drm_mode_status mode_status = MODE_OK; /* We don't support double-clocked modes */ @@ -1500,16 +1465,9 @@ dw_hdmi_connector_mode_valid(struct drm_connector *connector, return mode_status; } -static void dw_hdmi_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - static void dw_hdmi_connector_force(struct drm_connector *connector) { - struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, - connector); + struct dw_hdmi *hdmi = drm_ddc_private(connector); mutex_lock(&hdmi->mutex); hdmi->force = connector->force; @@ -1522,7 +1480,7 @@ static const struct drm_connector_funcs dw_hdmi_connector_funcs = { .dpms = drm_atomic_helper_connector_dpms, .fill_modes = drm_helper_probe_single_connector_modes, .detect = dw_hdmi_connector_detect, - .destroy = dw_hdmi_connector_destroy, + .destroy = drm_ddc_connector_destroy, .force = dw_hdmi_connector_force, .reset = drm_atomic_helper_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, @@ -1530,7 +1488,7 @@ static const struct drm_connector_funcs dw_hdmi_connector_funcs = { }; static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = { - .get_modes = dw_hdmi_connector_get_modes, + .get_modes = drm_ddc_connector_get_modes, .mode_valid = dw_hdmi_connector_mode_valid, .best_encoder = drm_atomic_helper_best_encoder, }; @@ -1669,16 +1627,16 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi) } encoder->bridge = bridge; - hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD; + hdmi->ddc_conn->connector.polled = DRM_CONNECTOR_POLL_HPD; drm_connector_helper_add(&hdmi->connector, &dw_hdmi_connector_helper_funcs); - drm_connector_init(drm, &hdmi->connector, - &dw_hdmi_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA); + drm_ddc_connector_init(drm, hdmi->ddc_conn, + &dw_hdmi_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); - drm_mode_connector_attach_encoder(&hdmi->connector, encoder); + drm_mode_connector_attach_encoder(&hdmi->ddc_conn->connector, encoder); return 0; } @@ -1691,7 +1649,6 @@ int dw_hdmi_bind(struct device *dev, struct device *master, struct drm_device *drm = data; struct device_node *np = dev->of_node; struct platform_device_info pdevinfo; - struct device_node *ddc_node; struct dw_hdmi_audio_data audio; struct dw_hdmi *hdmi; int ret; @@ -1701,7 +1658,11 @@ int dw_hdmi_bind(struct device *dev, struct device *master, if (!hdmi) return -ENOMEM; - hdmi->connector.interlace_allowed = 1; + hdmi->ddc_conn = drm_ddc_connector_create(drm, np, hdmi); + if (IS_ERR(hdmi->ddc_conn)) + return PTR_ERR(hdmi->ddc_conn); + + hdmi->ddc_conn->connector.interlace_allowed = 1; hdmi->plat_data = plat_data; hdmi->dev = dev; @@ -1732,19 +1693,6 @@ int dw_hdmi_bind(struct device *dev, struct device *master, return -EINVAL; } - ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); - if (ddc_node) { - hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node); - of_node_put(ddc_node); - if (!hdmi->ddc) { - dev_dbg(hdmi->dev, "failed to read ddc node\n"); - return -EPROBE_DEFER; - } - - } else { - dev_dbg(hdmi->dev, "no ddc property found\n"); - } - hdmi->regs = devm_ioremap_resource(dev, iores); if (IS_ERR(hdmi->regs)) return PTR_ERR(hdmi->regs); @@ -1826,7 +1774,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master, audio.base = hdmi->regs; audio.irq = irq; audio.hdmi = hdmi; - audio.eld = hdmi->connector.eld; + audio.eld = hdmi->ddc_conn->connector.eld; pdevinfo.data = &audio; pdevinfo.size_data = sizeof(audio); @@ -1865,12 +1813,10 @@ void dw_hdmi_unbind(struct device *dev, struct device *master, void *data) /* Disable all interrupts */ hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); - hdmi->connector.funcs->destroy(&hdmi->connector); hdmi->encoder->funcs->destroy(hdmi->encoder); clk_disable_unprepare(hdmi->iahb_clk); clk_disable_unprepare(hdmi->isfr_clk); - i2c_put_adapter(hdmi->ddc); } EXPORT_SYMBOL_GPL(dw_hdmi_unbind); diff --git a/drivers/gpu/drm/drm_ddc_connector.c b/drivers/gpu/drm/drm_ddc_connector.c new file mode 100644 index 000000000000..1476db8cd549 --- /dev/null +++ b/drivers/gpu/drm/drm_ddc_connector.c @@ -0,0 +1,89 @@ +#include <linux/i2c.h> +#include <linux/module.h> +#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_edid.h> +#include <drm/drm_ddc_connector.h> + +enum drm_connector_status +drm_ddc_connector_always_connected(struct drm_connector *connector, bool force) +{ + return connector_status_connected; +} +EXPORT_SYMBOL_GPL(drm_ddc_connector_always_connected); + +int drm_ddc_connector_get_modes(struct drm_connector *connector) +{ + struct drm_ddc_connector *ddc_conn = to_ddc_conn(connector); + struct edid *edid; + int ret = 0; + + if (!ddc_conn->ddc) + return 0; + + edid = drm_get_edid(connector, ddc_conn->ddc); + if (edid) { + ddc_conn->sink_is_hdmi = drm_detect_hdmi_monitor(edid); + ddc_conn->sink_has_audio = drm_detect_monitor_audio(edid); + drm_mode_connector_update_edid_property(connector, edid); + hdmi_event_new_edid(hdmi->dev, edid, 0); + ret = drm_add_edid_modes(connector, edid); + /* Store the ELD */ + drm_edid_to_eld(connector, edid); + hdmi_event_new_eld(hdmi->dev, connector->eld); + kfree(edid); + } + + return ret; +} +EXPORT_SYMBOL_GPL(drm_ddc_connector_get_modes); + +void drm_ddc_connector_destroy(struct drm_connector *connector) +{ + struct drm_ddc_connector *ddc_conn = to_ddc_conn(connector); + + drm_connector_unregister(connector); + drm_connector_cleanup(connector); + if (ddc_conn->ddc) + i2c_put_adapter(ddc_conn->ddc); + kfree(ddc_conn); +} +EXPORT_SYMBOL_GPL(drm_ddc_connector_destroy); + +void drm_ddc_connector_init(struct drm_device *drm, + struct drm_ddc_connector *ddc_conn, + struct drm_connector_funcs *funcs, int connector_type) +{ + drm_connector_init(drm, &ddc_conn->connector, funcs, connector_type); +} +EXPORT_SYMBOL_GPL(drm_ddc_connector_init); + +struct drm_ddc_connector *drm_ddc_connector_create(struct drm_device *drm, + struct device_node *np, void *private) +{ + struct drm_ddc_connector *ddc_conn; + struct device_node *ddc_node; + + ddc_conn = kzalloc(sizeof(*ddc_conn), GFP_KERNEL); + if (!ddc_conn) + return ERR_PTR(-ENOMEM); + + ddc_conn->private = private; + + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); + if (ddc_node) { + ddc_conn->ddc = of_get_i2c_adapter_by_node(ddc_node); + of_node_put(ddc_node); + if (!ddc_conn->ddc) { + kfree(ddc_conn); + return ERR_PTR(-EPROBE_DEFER); + } + } + + return ddc_conn; +} +EXPORT_SYMBOL_GPL(drm_ddc_connector_create); + +MODULE_AUTHOR("Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>"); +MODULE_DESCRIPTION("Generic DRM DDC connector module"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index 5e875944ffa2..e1955e4cd90b 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -17,7 +17,6 @@ #include <linux/clk-provider.h> #include <linux/component.h> #include <linux/module.h> -#include <linux/i2c.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/spinlock.h> @@ -26,6 +25,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_fb_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_ddc_connector.h> #include <video/imx-ipu-v3.h> #include "imx-drm.h" @@ -104,7 +104,7 @@ enum { }; struct imx_tve { - struct drm_connector connector; + struct drm_ddc_connector *ddc_conn; struct drm_encoder encoder; struct device *dev; spinlock_t lock; /* register lock */ @@ -115,7 +115,6 @@ struct imx_tve { struct regmap *regmap; struct regulator *dac_reg; - struct i2c_adapter *ddc; struct clk *clk; struct clk *di_sel_clk; struct clk_hw clk_hw_di; @@ -227,31 +226,6 @@ static int tve_setup_vga(struct imx_tve *tve) TVE_TVDAC_TEST_MODE_MASK, 1); } -static enum drm_connector_status imx_tve_connector_detect( - struct drm_connector *connector, bool force) -{ - return connector_status_connected; -} - -static int imx_tve_connector_get_modes(struct drm_connector *connector) -{ - struct imx_tve *tve = con_to_tve(connector); - struct edid *edid; - int ret = 0; - - if (!tve->ddc) - return 0; - - edid = drm_get_edid(connector, tve->ddc); - if (edid) { - drm_mode_connector_update_edid_property(connector, edid); - ret = drm_add_edid_modes(connector, edid); - kfree(edid); - } - - return ret; -} - static int imx_tve_connector_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { @@ -277,7 +251,7 @@ static int imx_tve_connector_mode_valid(struct drm_connector *connector, static struct drm_encoder *imx_tve_connector_best_encoder( struct drm_connector *connector) { - struct imx_tve *tve = con_to_tve(connector); + struct imx_tve *tve = drm_ddc_private(connector); return &tve->encoder; } @@ -352,15 +326,15 @@ static int imx_tve_atomic_check(struct drm_encoder *encoder, static const struct drm_connector_funcs imx_tve_connector_funcs = { .dpms = drm_atomic_helper_connector_dpms, .fill_modes = drm_helper_probe_single_connector_modes, - .detect = imx_tve_connector_detect, - .destroy = imx_drm_connector_destroy, + .detect = drm_ddc_connector_always_connected, + .destroy = drm_ddc_connector_destroy, .reset = drm_atomic_helper_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = { - .get_modes = imx_tve_connector_get_modes, + .get_modes = drm_ddc_connector_get_modes, .best_encoder = imx_tve_connector_best_encoder, .mode_valid = imx_tve_connector_mode_valid, }; @@ -499,12 +473,13 @@ static int imx_tve_register(struct drm_device *drm, struct imx_tve *tve) drm_encoder_init(drm, &tve->encoder, &imx_tve_encoder_funcs, encoder_type, NULL); - drm_connector_helper_add(&tve->connector, + drm_connector_helper_add(&tve->ddc_conn->connector, &imx_tve_connector_helper_funcs); - drm_connector_init(drm, &tve->connector, &imx_tve_connector_funcs, - DRM_MODE_CONNECTOR_VGA); + drm_ddc_connector_init(drm, tve->ddc_conn, &imx_tve_connector_funcs, + DRM_MODE_CONNECTOR_VGA); - drm_mode_connector_attach_encoder(&tve->connector, &tve->encoder); + drm_mode_connector_attach_encoder(&tve->ddc_conn->connector, + &tve->encoder); return 0; } @@ -553,7 +528,6 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = data; struct device_node *np = dev->of_node; - struct device_node *ddc_node; struct imx_tve *tve; struct resource *res; void __iomem *base; @@ -565,15 +539,13 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) if (!tve) return -ENOMEM; + tve->ddc_conn = drm_ddc_connector_create(drm, np, tve); + if (IS_ERR(tve->ddc_conn)) + return PTR_ERR(tve->ddc_conn); + tve->dev = dev; spin_lock_init(&tve->lock); - ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); - if (ddc_node) { - tve->ddc = of_find_i2c_adapter_by_node(ddc_node); - of_node_put(ddc_node); - } - tve->mode = of_get_tve_mode(np); if (tve->mode != TVE_MODE_VGA) { dev_err(dev, "only VGA mode supported, currently\n"); @@ -685,7 +657,6 @@ static void imx_tve_unbind(struct device *dev, struct device *master, { struct imx_tve *tve = dev_get_drvdata(dev); - tve->connector.funcs->destroy(&tve->connector); tve->encoder.funcs->destroy(&tve->encoder); if (!IS_ERR(tve->dac_reg)) diff --git a/include/drm/drm_ddc_connector.h b/include/drm/drm_ddc_connector.h new file mode 100644 index 000000000000..fb8cdf66d5fd --- /dev/null +++ b/include/drm/drm_ddc_connector.h @@ -0,0 +1,33 @@ +#ifndef DRM_DDC_CONNECTOR_H +#define DRM_DDC_CONNECTOR_H + +#include <drm/drm_crtc.h> + +struct drm_ddc_connector { + struct i2c_adapter *ddc; + struct drm_connector connector; + unsigned int sink_is_hdmi:1; + unsigned int sink_has_audio:1; + void *private; +}; + +#define to_ddc_conn(c) container_of(c, struct drm_ddc_connector, connector) + +enum drm_connector_status drm_ddc_connector_always_connected( + struct drm_connector *connector, bool force); +int drm_ddc_connector_get_modes(struct drm_connector *connector); +void drm_ddc_connector_init(struct drm_device *drm, + struct drm_ddc_connector *ddc_conn, + struct drm_connector_funcs *funcs, int connector_type); +void drm_ddc_connector_destroy(struct drm_connector *connector); +struct drm_ddc_connector *drm_ddc_connector_create(struct drm_device *drm, + struct device_node *np, void *private); + +static inline void *drm_ddc_private(struct drm_connector *connector) +{ + struct drm_ddc_connector *ddc_conn = to_ddc_conn(connector); + + return ddc_conn->private; +} + +#endif -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel