Re: [PATCH] drm/bridge: dw-hdmi: move cec PA invalidation to dw_hdmi_setup_rx_sense()

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

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
>