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 <narmstrong@xxxxxxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
