From patchwork Thu Aug 18 14:32:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9288409 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 31021600CB for ; Thu, 18 Aug 2016 14:53:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22BB928F53 for ; Thu, 18 Aug 2016 14:53:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 17A2E28F5C; Thu, 18 Aug 2016 14:53:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id DEA9F28F53 for ; Thu, 18 Aug 2016 14:53:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B3A226EA82; Thu, 18 Aug 2016 14:53:21 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0B0366EA72 for ; Thu, 18 Aug 2016 14:32:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=2GUz1QlGyiA0RahhXvKInEHzpLo5aZ4DMfDAxBBfaOI=; b=OrOF+lvB7hhGYerXfW+eJPuGcWSOxycQ/Ue0zpM53xLKeLuTAMVG9EHWFidXHu2wfjWjeQqkfmbVU2vFNxV6jEN1aFtAOnwCs8ASYSbo5C70BA2iz2qxM3zsSsYdYBaLgXVFCQWXbdbgnnHDnLYVpWoAlsgy/+FAW3KZM6BDzMI=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:53369) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1baOMv-0001zY-0G; Thu, 18 Aug 2016 15:32:29 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1baOMr-0007A7-Ob; Thu, 18 Aug 2016 15:32:25 +0100 Date: Thu, 18 Aug 2016 15:32:25 +0100 From: Russell King - ARM Linux To: Vladimir Zapolskiy Subject: Re: [RESEND PATCH v2 1/3] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface Message-ID: <20160818143225.GY1041@n2100.armlinux.org.uk> References: <1471379184-32252-1-git-send-email-vladimir_zapolskiy@mentor.com> <1471379205-32304-1-git-send-email-vladimir_zapolskiy@mentor.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1471379205-32304-1-git-send-email-vladimir_zapolskiy@mentor.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Mailman-Approved-At: Thu, 18 Aug 2016 14:53:20 +0000 Cc: Fabio Estevam , Daniel Vetter , dri-devel@lists.freedesktop.org, Dave Airlie X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 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 #include #include +#include #include #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 +#include +#include +#include +#include +#include + +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 "); +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 #include #include -#include #include #include #include @@ -26,6 +25,7 @@ #include #include #include +#include #include