Re: [PATCH 3/4] drm/msm: expose edid to hdmi cec adapter

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 18/04/2023 21:10, Arnaud Vrac wrote:
> When edid has been read after hpd, pass it to the cec adapter so that it
> can extract the physical address of the device on the cec bus.
> Invalidate the physical address when hpd is low.

If there is another bridge in a chain (e.g. display-connector) which 
handles HPD, then the msm_hdmi_bridge_detect() might get skipped. Please 
also add the hpd_notify() callback which invalidate the physical 
address. See adv7511, which does that both in its own HPD path and in 
the hpd_notify().

> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 ++
>   drivers/gpu/drm/msm/hdmi/hdmi_hpd.c    | 17 +++++++++++++----
>   2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 9b1391d27ed39..efc3bd4908e83 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -7,6 +7,7 @@
>   #include <linux/delay.h>
>   #include <drm/drm_bridge_connector.h>
>   #include <drm/drm_edid.h>
> +#include <media/cec.h>
>   
>   #include "msm_kms.h"
>   #include "hdmi.h"
> @@ -256,6 +257,7 @@ static struct edid *msm_hdmi_bridge_get_edid(struct drm_bridge *bridge,
>   	hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl | HDMI_CTRL_ENABLE);
>   
>   	edid = drm_get_edid(connector, hdmi->i2c);
> +	cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
>   
>   	hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
>   
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> index bfa827b479897..cb3eb2625ff63 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> @@ -7,6 +7,7 @@
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/pinctrl/consumer.h>
> +#include <media/cec.h>
>   
>   #include "msm_kms.h"
>   #include "hdmi.h"
> @@ -230,15 +231,17 @@ enum drm_connector_status msm_hdmi_bridge_detect(
>   {
>   	struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
>   	struct hdmi *hdmi = hdmi_bridge->hdmi;
> -	enum drm_connector_status stat_gpio, stat_reg;
> +	enum drm_connector_status 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);
> +	if (!hdmi->hpd_gpiod) {
> +		status = detect_reg(hdmi);
> +		goto out;
> +	}
>   
>   	do {
>   		stat_gpio = detect_gpio(hdmi);
> @@ -259,5 +262,11 @@ enum drm_connector_status msm_hdmi_bridge_detect(
>   		DBG("hpd gpio tells us: %d", stat_gpio);
>   	}
>   
> -	return stat_gpio;
> +	status = stat_gpio;
> +
> +out:
> +	if (!status)
> +		cec_phys_addr_invalidate(hdmi->cec_adap);
> +
> +	return status;
>   }
> 

-- 
With best wishes
Dmitry