Re: [PATCH v2 12/14] drm/msm/hdmi: drop hpd-gpios support

[Jessica Zhang <quic_jesszhan@quicinc.com>]

On 5/22/2024 3:51 AM, Dmitry Baryshkov wrote:
> Supporting simultaneous check of native HPD and the external GPIO proved
> to be less stable than just native HPD. Drop the hpd-gpios support,
> leaving just the native HPD support. In case the native HPD doesn't work
> the user is urged to switch to specifying the HPD property to the
> hdmi-connector device.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c     | 14 +++-------
>   drivers/gpu/drm/msm/hdmi/hdmi.h     |  2 --
>   drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 53 +++----------------------------------
>   3 files changed, 7 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index e160a23e962e..a9437054c015 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -468,17 +468,9 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
>   				     "failed to get extp clock\n");
>   
> -	hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
> -	/* This will catch e.g. -EPROBE_DEFER */
> -	if (IS_ERR(hdmi->hpd_gpiod))
> -		return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
> -				     "failed to get hpd gpio\n");
> -
> -	if (!hdmi->hpd_gpiod)
> -		DBG("failed to get HPD gpio");
> -
> -	if (hdmi->hpd_gpiod)
> -		gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
> +	if (of_find_property(dev->of_node, "hpd-gpios", NULL) ||
> +	    of_find_property(dev->of_node, "hpd-gpio", NULL))
> +		dev_warn(dev, "hpd-gpios is not supported anymore, please migrate to the hdmi-connector\n");
>   
>   	ret = msm_hdmi_get_phy(hdmi);
>   	if (ret) {
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> index 2a98efa8b6bd..268ff8604423 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> @@ -52,8 +52,6 @@ struct hdmi {
>   	struct clk_bulk_data *pwr_clks;
>   	struct clk *extp_clk;
>   
> -	struct gpio_desc *hpd_gpiod;
> -
>   	struct hdmi_phy *phy;
>   	struct device *phy_dev;
>   
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> index 32e447267e3b..d3353c6148ed 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> @@ -69,9 +69,6 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
>   	int ret;
>   	unsigned long flags;
>   
> -	if (hdmi->hpd_gpiod)
> -		gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
> -
>   	ret = pm_runtime_resume_and_get(dev);
>   	if (ret)
>   		return ret;
> @@ -144,8 +141,11 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
>   	}
>   }
>   
> -static enum drm_connector_status detect_reg(struct hdmi *hdmi)
> +enum drm_connector_status msm_hdmi_bridge_detect(
> +		struct drm_bridge *bridge)
>   {
> +	struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> +	struct hdmi *hdmi = hdmi_bridge->hdmi;
>   	uint32_t hpd_int_status = 0;
>   	int ret;
>   
> @@ -161,48 +161,3 @@ static enum drm_connector_status detect_reg(struct hdmi *hdmi)
>   	return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ?
>   			connector_status_connected : connector_status_disconnected;
>   }
> -
> -#define HPD_GPIO_INDEX	2
> -static enum drm_connector_status detect_gpio(struct hdmi *hdmi)
> -{
> -	return gpiod_get_value(hdmi->hpd_gpiod) ?
> -			connector_status_connected :
> -			connector_status_disconnected;
> -}
> -
> -enum drm_connector_status msm_hdmi_bridge_detect(
> -		struct drm_bridge *bridge)
> -{
> -	struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
> -	struct hdmi *hdmi = hdmi_bridge->hdmi;
> -	enum drm_connector_status stat_gpio, stat_reg;
> -	int retry = 20;
> -
> -	/*
> -	 * some platforms may not have hpd gpio. Rely only on the status
> -	 * provided by REG_HDMI_HPD_INT_STATUS in this case.
> -	 */
> -	if (!hdmi->hpd_gpiod)
> -		return detect_reg(hdmi);
> -
> -	do {
> -		stat_gpio = detect_gpio(hdmi);
> -		stat_reg  = detect_reg(hdmi);
> -
> -		if (stat_gpio == stat_reg)
> -			break;
> -
> -		mdelay(10);
> -	} while (--retry);
> -
> -	/* the status we get from reading gpio seems to be more reliable,
> -	 * so trust that one the most if we didn't manage to get hdmi and
> -	 * gpio status to agree:
> -	 */
> -	if (stat_gpio != stat_reg) {
> -		DBG("HDMI_HPD_INT_STATUS tells us: %d", stat_reg);
> -		DBG("hpd gpio tells us: %d", stat_gpio);
> -	}
> -
> -	return stat_gpio;
> -}
> 
> -- 
> 2.39.2
>