Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
@ 2019-08-13  9:32 Hans Verkuil
  2019-08-13  9:34 ` Hans Verkuil
  2019-08-13 11:43 ` Hans Verkuil
  0 siblings, 2 replies; 8+ messages in thread
From: Hans Verkuil @ 2019-08-13  9:32 UTC (permalink / raw)
  To: Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart, Sean Paul

When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
address was not invalidated when the HDMI cable was unplugged. Some more
digging showed that meson uses meson_dw_hdmi.c to handle the HPD.

Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
CEC physical address to that function, so that it is independent of where
the HPD interrupt happens.

Tested with both a AML-S905X-CC and a Khadas VIM2 board.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Note: an alternative would be to make a new dw-hdmi function such as
dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
I decided not to do that since this patch is minimally invasive, but
that can obviously be changed if that approach is preferred.
---
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index c5a854af54f8..e899b31e1432 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
 		dw_hdmi_update_power(hdmi);
 		dw_hdmi_update_phy_mask(hdmi);
 	}
+	if (!hpd && !rx_sense) {
+		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
+
+		if (notifier)
+			cec_notifier_phys_addr_invalidate(notifier);
+	}
+
 	mutex_unlock(&hdmi->mutex);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
@@ -2369,14 +2376,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) {
-			struct cec_notifier *notifier;
-
-			notifier = READ_ONCE(hdmi->cec_notifier);
-			if (notifier)
-				cec_notifier_phys_addr_invalidate(notifier);
-		}
 	}

 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13  9:32 [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense() Hans Verkuil
@ 2019-08-13  9:34 ` Hans Verkuil
  2019-08-13 10:18   ` Jonas Karlman
  2019-08-13 11:43 ` Hans Verkuil
  1 sibling, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2019-08-13  9:34 UTC (permalink / raw)
  To: Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Dariusz Marcinkiewicz

CC Dariusz as well, since this issue was discovered when testing his
CEC patches.

Regards,

	Hans

On 8/13/19 11:32 AM, Hans Verkuil wrote:
> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
> address was not invalidated when the HDMI cable was unplugged. Some more
> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
> 
> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
> CEC physical address to that function, so that it is independent of where
> the HPD interrupt happens.
> 
> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> Note: an alternative would be to make a new dw-hdmi function such as
> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
> I decided not to do that since this patch is minimally invasive, but
> that can obviously be changed if that approach is preferred.
> ---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index c5a854af54f8..e899b31e1432 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>  		dw_hdmi_update_power(hdmi);
>  		dw_hdmi_update_phy_mask(hdmi);
>  	}
> +	if (!hpd && !rx_sense) {
> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
> +
> +		if (notifier)
> +			cec_notifier_phys_addr_invalidate(notifier);
> +	}
> +
>  	mutex_unlock(&hdmi->mutex);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
> @@ -2369,14 +2376,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) {
> -			struct cec_notifier *notifier;
> -
> -			notifier = READ_ONCE(hdmi->cec_notifier);
> -			if (notifier)
> -				cec_notifier_phys_addr_invalidate(notifier);
> -		}
>  	}
> 
>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13  9:34 ` Hans Verkuil
@ 2019-08-13 10:18   ` Jonas Karlman
  2019-08-13 10:49     ` Hans Verkuil
  2019-08-13 11:27     ` Hans Verkuil
  0 siblings, 2 replies; 8+ messages in thread
From: Jonas Karlman @ 2019-08-13 10:18 UTC (permalink / raw)
  To: Hans Verkuil, Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Dariusz Marcinkiewicz

As an alternative I have a patch [1] to submit that moves cec_notifier_phys_addr_invalidate() call
from dw_hdmi_irq() to dw_hdmi_connector_detect() in order to address an issue with
stale CEC phys addr and stale EDID/ELD data after TV or AVR uses a 100ms HPD pulse
to signal EDID has changed, full patchset at [2].

At the moment CEC phys address is invalidated directly at HPD, leaving the address as invalid
after a 100ms HPD pulse, phys address may later be restored to a valid phys address when
get_modes() is called by drm core.

Should I wait on your and related patches to be merged before submitting my series?

[1] https://github.com/Kwiboo/linux-rockchip/commit/2f4f99c82983e70952668c21f1c56a0241bd75f2
[2] https://github.com/Kwiboo/linux-rockchip/compare/next-20190813...next-20190813-cec-eld

Regards,
Jonas

On 2019-08-13 11:34, Hans Verkuil wrote:
> CC Dariusz as well, since this issue was discovered when testing his
> CEC patches.
>
> Regards,
>
> 	Hans
>
> On 8/13/19 11:32 AM, Hans Verkuil wrote:
>> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
>> address was not invalidated when the HDMI cable was unplugged. Some more
>> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
>>
>> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
>> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
>> CEC physical address to that function, so that it is independent of where
>> the HPD interrupt happens.
>>
>> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> Note: an alternative would be to make a new dw-hdmi function such as
>> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
>> I decided not to do that since this patch is minimally invasive, but
>> that can obviously be changed if that approach is preferred.
>> ---
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index c5a854af54f8..e899b31e1432 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>  		dw_hdmi_update_power(hdmi);
>>  		dw_hdmi_update_phy_mask(hdmi);
>>  	}
>> +	if (!hpd && !rx_sense) {
>> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
>> +
>> +		if (notifier)
>> +			cec_notifier_phys_addr_invalidate(notifier);
>> +	}
>> +
>>  	mutex_unlock(&hdmi->mutex);
>>  }
>>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>> @@ -2369,14 +2376,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) {
>> -			struct cec_notifier *notifier;
>> -
>> -			notifier = READ_ONCE(hdmi->cec_notifier);
>> -			if (notifier)
>> -				cec_notifier_phys_addr_invalidate(notifier);
>> -		}
>>  	}
>>
>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13 10:18   ` Jonas Karlman
@ 2019-08-13 10:49     ` Hans Verkuil
  2019-08-13 11:27     ` Hans Verkuil
  1 sibling, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2019-08-13 10:49 UTC (permalink / raw)
  To: Jonas Karlman, Maling list - DRI developers, Neil Armstrong
  Cc: Dariusz Marcinkiewicz, Sean Paul, Laurent Pinchart,
	Linux Media Mailing List

Hi Jonas,

On 8/13/19 12:18 PM, Jonas Karlman wrote:
> As an alternative I have a patch [1] to submit that moves cec_notifier_phys_addr_invalidate() call
> from dw_hdmi_irq() to dw_hdmi_connector_detect() in order to address an issue with
> stale CEC phys addr and stale EDID/ELD data after TV or AVR uses a 100ms HPD pulse
> to signal EDID has changed, full patchset at [2].
> 
> At the moment CEC phys address is invalidated directly at HPD, leaving the address as invalid
> after a 100ms HPD pulse, phys address may later be restored to a valid phys address when
> get_modes() is called by drm core.
> 
> Should I wait on your and related patches to be merged before submitting my series?

Let me test your patch on my meson device and see if it solves this issue as well.

I'll get back to you later today.

Regards,

	Hans

> 
> [1] https://github.com/Kwiboo/linux-rockchip/commit/2f4f99c82983e70952668c21f1c56a0241bd75f2
> [2] https://github.com/Kwiboo/linux-rockchip/compare/next-20190813...next-20190813-cec-eld
> 
> Regards,
> Jonas
> 
> On 2019-08-13 11:34, Hans Verkuil wrote:
>> CC Dariusz as well, since this issue was discovered when testing his
>> CEC patches.
>>
>> Regards,
>>
>> 	Hans
>>
>> On 8/13/19 11:32 AM, Hans Verkuil wrote:
>>> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
>>> address was not invalidated when the HDMI cable was unplugged. Some more
>>> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
>>>
>>> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
>>> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
>>> CEC physical address to that function, so that it is independent of where
>>> the HPD interrupt happens.
>>>
>>> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
>>>
>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>> ---
>>> Note: an alternative would be to make a new dw-hdmi function such as
>>> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
>>> I decided not to do that since this patch is minimally invasive, but
>>> that can obviously be changed if that approach is preferred.
>>> ---
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index c5a854af54f8..e899b31e1432 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>  		dw_hdmi_update_power(hdmi);
>>>  		dw_hdmi_update_phy_mask(hdmi);
>>>  	}
>>> +	if (!hpd && !rx_sense) {
>>> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
>>> +
>>> +		if (notifier)
>>> +			cec_notifier_phys_addr_invalidate(notifier);
>>> +	}
>>> +
>>>  	mutex_unlock(&hdmi->mutex);
>>>  }
>>>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>>> @@ -2369,14 +2376,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) {
>>> -			struct cec_notifier *notifier;
>>> -
>>> -			notifier = READ_ONCE(hdmi->cec_notifier);
>>> -			if (notifier)
>>> -				cec_notifier_phys_addr_invalidate(notifier);
>>> -		}
>>>  	}
>>>
>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13 10:18   ` Jonas Karlman
  2019-08-13 10:49     ` Hans Verkuil
@ 2019-08-13 11:27     ` Hans Verkuil
  2019-08-13 11:41       ` Jonas Karlman
  1 sibling, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2019-08-13 11:27 UTC (permalink / raw)
  To: Jonas Karlman, Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Dariusz Marcinkiewicz

On 8/13/19 12:18 PM, Jonas Karlman wrote:
> As an alternative I have a patch [1] to submit that moves cec_notifier_phys_addr_invalidate() call
> from dw_hdmi_irq() to dw_hdmi_connector_detect() in order to address an issue with
> stale CEC phys addr and stale EDID/ELD data after TV or AVR uses a 100ms HPD pulse
> to signal EDID has changed, full patchset at [2].
> 
> At the moment CEC phys address is invalidated directly at HPD, leaving the address as invalid
> after a 100ms HPD pulse, phys address may later be restored to a valid phys address when
> get_modes() is called by drm core.
> 
> Should I wait on your and related patches to be merged before submitting my series?

Your patch fixes this issue as well, so just ignore my patch and submit your series.
Please CC me when you post your series.

Regards,

	Hans

> 
> [1] https://github.com/Kwiboo/linux-rockchip/commit/2f4f99c82983e70952668c21f1c56a0241bd75f2
> [2] https://github.com/Kwiboo/linux-rockchip/compare/next-20190813...next-20190813-cec-eld
> 
> Regards,
> Jonas
> 
> On 2019-08-13 11:34, Hans Verkuil wrote:
>> CC Dariusz as well, since this issue was discovered when testing his
>> CEC patches.
>>
>> Regards,
>>
>> 	Hans
>>
>> On 8/13/19 11:32 AM, Hans Verkuil wrote:
>>> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
>>> address was not invalidated when the HDMI cable was unplugged. Some more
>>> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
>>>
>>> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
>>> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
>>> CEC physical address to that function, so that it is independent of where
>>> the HPD interrupt happens.
>>>
>>> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
>>>
>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>> ---
>>> Note: an alternative would be to make a new dw-hdmi function such as
>>> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
>>> I decided not to do that since this patch is minimally invasive, but
>>> that can obviously be changed if that approach is preferred.
>>> ---
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index c5a854af54f8..e899b31e1432 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>  		dw_hdmi_update_power(hdmi);
>>>  		dw_hdmi_update_phy_mask(hdmi);
>>>  	}
>>> +	if (!hpd && !rx_sense) {
>>> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
>>> +
>>> +		if (notifier)
>>> +			cec_notifier_phys_addr_invalidate(notifier);
>>> +	}
>>> +
>>>  	mutex_unlock(&hdmi->mutex);
>>>  }
>>>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>>> @@ -2369,14 +2376,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) {
>>> -			struct cec_notifier *notifier;
>>> -
>>> -			notifier = READ_ONCE(hdmi->cec_notifier);
>>> -			if (notifier)
>>> -				cec_notifier_phys_addr_invalidate(notifier);
>>> -		}
>>>  	}
>>>
>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13 11:27     ` Hans Verkuil
@ 2019-08-13 11:41       ` Jonas Karlman
  0 siblings, 0 replies; 8+ messages in thread
From: Jonas Karlman @ 2019-08-13 11:41 UTC (permalink / raw)
  To: Hans Verkuil, Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Dariusz Marcinkiewicz

On 2019-08-13 13:27, Hans Verkuil wrote:
> On 8/13/19 12:18 PM, Jonas Karlman wrote:
>> As an alternative I have a patch [1] to submit that moves cec_notifier_phys_addr_invalidate() call
>> from dw_hdmi_irq() to dw_hdmi_connector_detect() in order to address an issue with
>> stale CEC phys addr and stale EDID/ELD data after TV or AVR uses a 100ms HPD pulse
>> to signal EDID has changed, full patchset at [2].
>>
>> At the moment CEC phys address is invalidated directly at HPD, leaving the address as invalid
>> after a 100ms HPD pulse, phys address may later be restored to a valid phys address when
>> get_modes() is called by drm core.
>>
>> Should I wait on your and related patches to be merged before submitting my series?
> Your patch fixes this issue as well, so just ignore my patch and submit your series.
> Please CC me when you post your series.

Thanks for testing, I will include you in CC when I submit my series later today or tomorrow.

Regards,
Jonas

>
> Regards,
>
> 	Hans
>
>> [1] https://github.com/Kwiboo/linux-rockchip/commit/2f4f99c82983e70952668c21f1c56a0241bd75f2
>> [2] https://github.com/Kwiboo/linux-rockchip/compare/next-20190813...next-20190813-cec-eld
>>
>> Regards,
>> Jonas
>>
>> On 2019-08-13 11:34, Hans Verkuil wrote:
>>> CC Dariusz as well, since this issue was discovered when testing his
>>> CEC patches.
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>> On 8/13/19 11:32 AM, Hans Verkuil wrote:
>>>> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
>>>> address was not invalidated when the HDMI cable was unplugged. Some more
>>>> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
>>>>
>>>> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
>>>> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
>>>> CEC physical address to that function, so that it is independent of where
>>>> the HPD interrupt happens.
>>>>
>>>> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
>>>>
>>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>> ---
>>>> Note: an alternative would be to make a new dw-hdmi function such as
>>>> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
>>>> I decided not to do that since this patch is minimally invasive, but
>>>> that can obviously be changed if that approach is preferred.
>>>> ---
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index c5a854af54f8..e899b31e1432 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>>  		dw_hdmi_update_power(hdmi);
>>>>  		dw_hdmi_update_phy_mask(hdmi);
>>>>  	}
>>>> +	if (!hpd && !rx_sense) {
>>>> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
>>>> +
>>>> +		if (notifier)
>>>> +			cec_notifier_phys_addr_invalidate(notifier);
>>>> +	}
>>>> +
>>>>  	mutex_unlock(&hdmi->mutex);
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>>>> @@ -2369,14 +2376,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) {
>>>> -			struct cec_notifier *notifier;
>>>> -
>>>> -			notifier = READ_ONCE(hdmi->cec_notifier);
>>>> -			if (notifier)
>>>> -				cec_notifier_phys_addr_invalidate(notifier);
>>>> -		}
>>>>  	}
>>>>
>>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13  9:32 [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense() Hans Verkuil
  2019-08-13  9:34 ` Hans Verkuil
@ 2019-08-13 11:43 ` Hans Verkuil
  2019-08-13 11:56   ` Neil Armstrong
  1 sibling, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2019-08-13 11:43 UTC (permalink / raw)
  To: Maling list - DRI developers, Neil Armstrong
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Jonas Karlman

On 8/13/19 11:32 AM, Hans Verkuil wrote:
> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
> address was not invalidated when the HDMI cable was unplugged. Some more
> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
> 
> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
> CEC physical address to that function, so that it is independent of where
> the HPD interrupt happens.
> 
> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Please disregard this patch, Jonas Karlman will post a different series
which will fix this in a different way.

Regards,

	Hans

> ---
> Note: an alternative would be to make a new dw-hdmi function such as
> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
> I decided not to do that since this patch is minimally invasive, but
> that can obviously be changed if that approach is preferred.
> ---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index c5a854af54f8..e899b31e1432 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>  		dw_hdmi_update_power(hdmi);
>  		dw_hdmi_update_phy_mask(hdmi);
>  	}
> +	if (!hpd && !rx_sense) {
> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
> +
> +		if (notifier)
> +			cec_notifier_phys_addr_invalidate(notifier);
> +	}
> +
>  	mutex_unlock(&hdmi->mutex);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
> @@ -2369,14 +2376,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) {
> -			struct cec_notifier *notifier;
> -
> -			notifier = READ_ONCE(hdmi->cec_notifier);
> -			if (notifier)
> -				cec_notifier_phys_addr_invalidate(notifier);
> -		}
>  	}
> 
>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()
  2019-08-13 11:43 ` Hans Verkuil
@ 2019-08-13 11:56   ` Neil Armstrong
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-08-13 11:56 UTC (permalink / raw)
  To: Hans Verkuil, Maling list - DRI developers
  Cc: Linux Media Mailing List, Andrzej Hajda, Laurent Pinchart,
	Sean Paul, Jonas Karlman

Hi Hans,

On 13/08/2019 13:43, Hans Verkuil wrote:
> On 8/13/19 11:32 AM, Hans Verkuil wrote:
>> When testing CEC on the AML-S905X-CC board I noticed that the CEC physical
>> address was not invalidated when the HDMI cable was unplugged. Some more
>> digging showed that meson uses meson_dw_hdmi.c to handle the HPD.
>>
>> Both dw_hdmi_irq() and dw_hdmi_top_thread_irq() (in meson_dw_hdmi.c) call
>> the dw_hdmi_setup_rx_sense() function. So move the code to invalidate the
>> CEC physical address to that function, so that it is independent of where
>> the HPD interrupt happens.
>>
>> Tested with both a AML-S905X-CC and a Khadas VIM2 board.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Please disregard this patch, Jonas Karlman will post a different series
> which will fix this in a different way.

Noted, thanks.

Neil

> 
> Regards,
> 
> 	Hans
> 
>> ---
>> Note: an alternative would be to make a new dw-hdmi function such as
>> dw_hdmi_cec_phys_addr_invalidate() that is called from meson_dw_hdmi.c.
>> I decided not to do that since this patch is minimally invasive, but
>> that can obviously be changed if that approach is preferred.
>> ---
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index c5a854af54f8..e899b31e1432 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2329,6 +2329,13 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>  		dw_hdmi_update_power(hdmi);
>>  		dw_hdmi_update_phy_mask(hdmi);
>>  	}
>> +	if (!hpd && !rx_sense) {
>> +		struct cec_notifier *notifier = READ_ONCE(hdmi->cec_notifier);
>> +
>> +		if (notifier)
>> +			cec_notifier_phys_addr_invalidate(notifier);
>> +	}
>> +
>>  	mutex_unlock(&hdmi->mutex);
>>  }
>>  EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>> @@ -2369,14 +2376,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) {
>> -			struct cec_notifier *notifier;
>> -
>> -			notifier = READ_ONCE(hdmi->cec_notifier);
>> -			if (notifier)
>> -				cec_notifier_phys_addr_invalidate(notifier);
>> -		}
>>  	}
>>
>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-13  9:32 [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense() Hans Verkuil
2019-08-13  9:34 ` Hans Verkuil
2019-08-13 10:18   ` Jonas Karlman
2019-08-13 10:49     ` Hans Verkuil
2019-08-13 11:27     ` Hans Verkuil
2019-08-13 11:41       ` Jonas Karlman
2019-08-13 11:43 ` Hans Verkuil
2019-08-13 11:56   ` Neil Armstrong

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox