diff mbox series

[3/5] drm: dw-hdmi: update CEC phys addr and EDID on HPD event

Message ID AM0PR06MB4004285BAA935D14F1A68787ACBF0@AM0PR06MB4004.eurprd06.prod.outlook.com (mailing list archive)
State New, archived
Headers show
Series drm: dw-hdmi: update CEC phys addr, EDID and ELD on HPD event | expand

Commit Message

Jonas Karlman Sept. 1, 2019, 4:14 p.m. UTC
Update CEC phys addr and EDID on HPD event, fixes lost CEC phys addr and
stale EDID when HDMI cable is unplugged/replugged or AVR is powered on/off.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Neil Armstrong Sept. 2, 2019, 8:10 a.m. UTC | #1
On 01/09/2019 18:14, Jonas Karlman wrote:
> Update CEC phys addr and EDID on HPD event, fixes lost CEC phys addr and
> stale EDID when HDMI cable is unplugged/replugged or AVR is powered on/off.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 91d86ddd6624..0f07be1b5914 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2189,6 +2189,7 @@ static int dw_hdmi_connector_update_edid(struct drm_connector *connector,
>  static enum drm_connector_status
>  dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>  {
> +	enum drm_connector_status status;
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
>  					     connector);
>  
> @@ -2198,7 +2199,14 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>  	dw_hdmi_update_phy_mask(hdmi);
>  	mutex_unlock(&hdmi->mutex);
>  
> -	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
> +	status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
> +
> +	if (status == connector_status_connected)
> +		dw_hdmi_connector_update_edid(connector, false);
> +	else
> +		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> +
> +	return status;
>  }
>  
>  static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
> @@ -2438,12 +2446,6 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>  		dw_hdmi_setup_rx_sense(hdmi,
>  				       phy_stat & HDMI_PHY_HPD,
>  				       phy_stat & HDMI_PHY_RX_SENSE);
> -
> -		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
> -			mutex_lock(&hdmi->cec_notifier_mutex);
> -			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> -			mutex_unlock(&hdmi->cec_notifier_mutex);
> -		}
>  	}
>  
>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> 

Won't it possibly trigger twice edid readouts on each HPD event,
one for CEC and one for modes ? It seems sane but not very efficient to me...

Anyway:
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Jonas Karlman Sept. 2, 2019, 10:49 a.m. UTC | #2
On 2019-09-02 10:10, Neil Armstrong wrote:
> On 01/09/2019 18:14, Jonas Karlman wrote:
>> Update CEC phys addr and EDID on HPD event, fixes lost CEC phys addr and
>> stale EDID when HDMI cable is unplugged/replugged or AVR is powered on/off.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index 91d86ddd6624..0f07be1b5914 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2189,6 +2189,7 @@ static int dw_hdmi_connector_update_edid(struct drm_connector *connector,
>>  static enum drm_connector_status
>>  dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>>  {
>> +	enum drm_connector_status status;
>>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
>>  					     connector);
>>  
>> @@ -2198,7 +2199,14 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>>  	dw_hdmi_update_phy_mask(hdmi);
>>  	mutex_unlock(&hdmi->mutex);
>>  
>> -	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
>> +	status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
>> +
>> +	if (status == connector_status_connected)
>> +		dw_hdmi_connector_update_edid(connector, false);
>> +	else
>> +		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>> +
>> +	return status;
>>  }
>>  
>>  static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>> @@ -2438,12 +2446,6 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>>  		dw_hdmi_setup_rx_sense(hdmi,
>>  				       phy_stat & HDMI_PHY_HPD,
>>  				       phy_stat & HDMI_PHY_RX_SENSE);
>> -
>> -		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
>> -			mutex_lock(&hdmi->cec_notifier_mutex);
>> -			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>> -			mutex_unlock(&hdmi->cec_notifier_mutex);
>> -		}
>>  	}
>>  
>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>
> Won't it possibly trigger twice edid readouts on each HPD event,
> one for CEC and one for modes ? It seems sane but not very efficient to me...

Yes there may be EDID readout twice with this change in case both detect and get_modes gets called.
Guess we could save the EDID readout in detect and reuse it in get_modes or similar?

The issue observed is that CEC phys addr never gets updated after an invalidation (HPD)
until get_modes is called, when TV/AVR triggers an EDID refresh using a 100ms HPD pulse
or an AVR is powered on/off the CEC phys addr, EDID and ELD never gets refreshed as
only detect is called and not get_modes.

Regards,
Jonas

>
> Anyway:
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 91d86ddd6624..0f07be1b5914 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2189,6 +2189,7 @@  static int dw_hdmi_connector_update_edid(struct drm_connector *connector,
 static enum drm_connector_status
 dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
+	enum drm_connector_status status;
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
 					     connector);
 
@@ -2198,7 +2199,14 @@  dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	dw_hdmi_update_phy_mask(hdmi);
 	mutex_unlock(&hdmi->mutex);
 
-	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+	status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+
+	if (status == connector_status_connected)
+		dw_hdmi_connector_update_edid(connector, false);
+	else
+		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
+
+	return status;
 }
 
 static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
@@ -2438,12 +2446,6 @@  static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 		dw_hdmi_setup_rx_sense(hdmi,
 				       phy_stat & HDMI_PHY_HPD,
 				       phy_stat & HDMI_PHY_RX_SENSE);
-
-		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
-			mutex_lock(&hdmi->cec_notifier_mutex);
-			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
-			mutex_unlock(&hdmi->cec_notifier_mutex);
-		}
 	}
 
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {