From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751345AbdAQOy2 (ORCPT ); Tue, 17 Jan 2017 09:54:28 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:34834 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751099AbdAQOyC (ORCPT ); Tue, 17 Jan 2017 09:54:02 -0500 From: Laurent Pinchart To: Neil Armstrong Cc: dri-devel@lists.freedesktop.org, laurent.pinchart+renesas@ideasonboard.com, Jose.Abreu@synopsys.com, kieran.bingham@ideasonboard.com, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [RFC/RFT PATCH 2/4] drm/bridge: dw-hdmi: Add support for custom PHY handling Date: Tue, 17 Jan 2017 16:54:16 +0200 Message-ID: <1766052.45cMAnqkLu@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <1484656294-6140-3-git-send-email-narmstrong@baylibre.com> References: <1484656294-6140-1-git-send-email-narmstrong@baylibre.com> <1484656294-6140-3-git-send-email-narmstrong@baylibre.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Neil, Thank you for the patch. On Tuesday 17 Jan 2017 13:31:32 Neil Armstrong wrote: > The Synopsys DesignWare HDMI TX Controller support various Transceivers > (PHY) attached to the controller, but also allows fully custom PHYs to be > connected. > > Add PHY init, disable functions in plat_data to handle fully custom PHY > init. > > Some custom PHYs also handles the HPD and RxSense separately and do not use > the internal signals exported in the Controller's IRQ stat, so a read_hpd > is provided to switch to HPD status polling. > To complete the implementation, add a function to mimic the Controllers > interrupt HPD and RxSense handling from the vendor PHY wrapper code. I believe this goes in the right direction. PHY handling needs to be decoupled from the TX controller. As you've noticed I've taken a first step in that direction with "drm: bridge: dw-hdmi: Refactor PHY power handling", but that's not enough. Issues were reported with that patch which I plan to rework. If that's fine with you, I'll rebase it on top of this patch (that I will likely modify) and plan to get the result ready for v4.12. > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/bridge/dw-hdmi.c | 78 +++++++++++++++++++++++++++++-------- > include/drm/bridge/dw_hdmi.h | 8 +++++ > 2 files changed, 70 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c > b/drivers/gpu/drm/bridge/dw-hdmi.c index 13747fe..923e250 100644 > --- a/drivers/gpu/drm/bridge/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c > @@ -1434,9 +1434,18 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct > drm_display_mode *mode) hdmi_av_composer(hdmi, mode); > > /* HDMI Initializateion Step B.2 */ > - ret = dw_hdmi_phy_init(hdmi); > - if (ret) > - return ret; > + if (hdmi->plat_data->hdmi_phy_init) { > + ret = hdmi->plat_data->hdmi_phy_init(hdmi, hdmi->plat_data, > + &hdmi->previous_mode); > + if (ret) > + return ret; > + > + hdmi->phy_enabled = true; > + } else { > + ret = dw_hdmi_phy_init(hdmi); > + if (ret) > + return ret; > + } > > /* HDMI Initialization Step B.3 */ > dw_hdmi_enable_video_path(hdmi); > @@ -1551,7 +1560,11 @@ static void dw_hdmi_poweron(struct dw_hdmi *hdmi) > > static void dw_hdmi_poweroff(struct dw_hdmi *hdmi) > { > - dw_hdmi_phy_disable(hdmi); > + if (hdmi->phy_enabled && hdmi->plat_data->hdmi_phy_disable) { > + hdmi->plat_data->hdmi_phy_disable(hdmi, hdmi->plat_data); > + hdmi->phy_enabled = false; > + } else > + dw_hdmi_phy_disable(hdmi); > hdmi->bridge_is_on = false; > } > > @@ -1593,6 +1606,9 @@ static void dw_hdmi_update_phy_mask(struct dw_hdmi > *hdmi) { > u8 old_mask = hdmi->phy_mask; > > + if (hdmi->plat_data && hdmi->plat_data->hdmi_read_hpd) > + return; > + > if (hdmi->force || hdmi->disabled || !hdmi->rxsense) > hdmi->phy_mask |= HDMI_PHY_RX_SENSE; > else > @@ -1614,6 +1630,11 @@ static void dw_hdmi_update_phy_mask(struct dw_hdmi > *hdmi) dw_hdmi_update_phy_mask(hdmi); > mutex_unlock(&hdmi->mutex); > > + if (hdmi->plat_data && hdmi->plat_data->hdmi_read_hpd) > + return hdmi->plat_data->hdmi_read_hpd(hdmi, hdmi->plat_data) ? > + connector_status_connected : > + connector_status_disconnected; > + > return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ? > connector_status_connected : connector_status_disconnected; > } > @@ -1901,6 +1922,26 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) > } > }; > > +void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense) > +{ > + struct dw_hdmi *hdmi = dev_get_drvdata(dev); > + > + mutex_lock(&hdmi->mutex); > + > + if (!hdmi->disabled && !hdmi->force) { > + if (!rx_sense) > + hdmi->rxsense = false; > + > + if (hpd) > + hdmi->rxsense = true; > + > + dw_hdmi_update_power(hdmi); > + dw_hdmi_update_phy_mask(hdmi); > + } > + mutex_unlock(&hdmi->mutex); > +} > +EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense); > + > static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > { > unsigned int i; > @@ -1921,7 +1962,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > return -ENODEV; > } > > - if (!hdmi->phy->configure && !hdmi->plat_data->configure_phy) { > + if (!hdmi->phy->configure && > + !hdmi->plat_data->configure_phy && > + !hdmi->plat_data->hdmi_phy_init) { > dev_err(hdmi->dev, "%s requires platform support\n", > hdmi->phy->name); > return -ENODEV; > @@ -2101,15 +2144,17 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > hdmi->ddc = NULL; > } > > - /* > - * Configure registers related to HDMI interrupt > - * generation before registering IRQ. > - */ > - hdmi_writeb(hdmi, HDMI_PHY_HPD | HDMI_PHY_RX_SENSE, HDMI_PHY_POL0); > + if (!hdmi->plat_data || !hdmi->plat_data->hdmi_read_hpd) { > + /* > + * Configure registers related to HDMI interrupt > + * generation before registering IRQ. > + */ > + hdmi_writeb(hdmi, HDMI_PHY_HPD | HDMI_PHY_RX_SENSE, HDMI_PHY_POL0); > > - /* Clear Hotplug interrupts */ > - hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE, > - HDMI_IH_PHY_STAT0); > + /* Clear Hotplug interrupts */ > + hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE, > + HDMI_IH_PHY_STAT0); > + } > > hdmi->bridge.driver_private = hdmi; > hdmi->bridge.funcs = &dw_hdmi_bridge_funcs; > @@ -2119,9 +2164,10 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > if (ret) > goto err_iahb; > > - /* Unmute interrupts */ > - hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE), > - HDMI_IH_MUTE_PHY_STAT0); > + if (!hdmi->plat_data || !hdmi->plat_data->hdmi_read_hpd) > + /* Unmute interrupts */ > + hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE), > + HDMI_IH_MUTE_PHY_STAT0); > > memset(&pdevinfo, 0, sizeof(pdevinfo)); > pdevinfo.parent = dev; > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > index 163842d..d6a0ab3 100644 > --- a/include/drm/bridge/dw_hdmi.h > +++ b/include/drm/bridge/dw_hdmi.h > @@ -61,6 +61,13 @@ struct dw_hdmi_plat_data { > enum drm_mode_status (*mode_valid)(struct drm_connector *connector, > struct drm_display_mode *mode); > struct regmap *regm; > + int (*hdmi_phy_init)(struct dw_hdmi *hdmi, > + const struct dw_hdmi_plat_data *data, > + struct drm_display_mode *mode); > + void (*hdmi_phy_disable)(struct dw_hdmi *hdmi, > + const struct dw_hdmi_plat_data *data); > + bool (*hdmi_read_hpd)(struct dw_hdmi *hdmi, > + const struct dw_hdmi_plat_data *data); > }; > > int dw_hdmi_probe(struct platform_device *pdev, > @@ -70,6 +77,7 @@ int dw_hdmi_probe(struct platform_device *pdev, > int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder, > const struct dw_hdmi_plat_data *plat_data); > > +void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense); > void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate); > void dw_hdmi_audio_enable(struct dw_hdmi *hdmi); > void dw_hdmi_audio_disable(struct dw_hdmi *hdmi); -- Regards, Laurent Pinchart