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-ci...@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) {
>>>
>
