My understanding and testing based on Rockchip has shown that rx_sense
has no significant impact before and after this change.
>
>>
>> drm core will call the force/detect ops and enable/disable based on
>> forced/HPD state, so I am not expecting any difference in how this
>> currently works.
>>
>> This change to only poweron/setup in atomic_enable should also fix
>> issues/situations where dw-hdmi was poweron too early during bootup in
>> irq handler, before the drm driver was fully probed.
I may have been wrong here, there is a check for disabled here so it
should not have setup() before atomic_enable().
Still we should ensure configure happen in atomic_enable(). The
hpd_irq_event/hpd_notify calls will trigger a detect() and signal core
if the bridge should be enabled/disabled when connector status changes.
>
> Hmm, but I thought the code wouldn't poweron the bridge is rxsense was 0
> even if a mode was set, this won't work like this anymore right ?
This is what I thought was happening, and the comment in code seem to
indicate that. However, the code only seem to care about HPD=1 status to
poweron() and possible both HPD=0 + RXSENSE=0 to poweroff().
To make matter more complex core also mainly care about detect() status
and most/all related drivers seem to only use the the HPD status.
So I would say that there should not be any changes in impact of the
rxsense signal.
Will also send a new patch in a v2 for dw_hdmi_irq() that may be related:
- if (phy_stat & HDMI_PHY_HPD)
+ if ((intr_stat & HDMI_IH_PHY_STAT0_HPD) &&
+ (phy_stat & HDMI_PHY_HPD))
status = connector_status_connected;
or there may be a status_connected event triggered when rxsense goes
from 1 to 0 and a second one with status_disconnected shortly after when
hpd goes from 1 to 0 when the cable is removed.
Regards,
Jonas
>
> Neil
>
>>
>> Regards,
>> Jonas
>>
>>>
>>> Neil
>>>
>>>> -
>>>> - if (force == DRM_FORCE_OFF) {
>>>> - if (hdmi->bridge_is_on)
>>>> - dw_hdmi_poweroff(hdmi);
>>>> - } else {
>>>> - if (!hdmi->bridge_is_on)
>>>> - dw_hdmi_poweron(hdmi);
>>>> - }
>>>> }
>>>>
>>>> /*
>>>> @@ -2546,7 +2519,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>>>>
>>>> mutex_lock(&hdmi->mutex);
>>>> hdmi->force = connector->force;
>>>> - dw_hdmi_update_power(hdmi);
>>>> dw_hdmi_update_phy_mask(hdmi);
>>>> mutex_unlock(&hdmi->mutex);
>>>> }
>>>> @@ -2955,7 +2927,7 @@ static void dw_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
>>>> mutex_lock(&hdmi->mutex);
>>>> hdmi->disabled = true;
>>>> hdmi->curr_conn = NULL;
>>>> - dw_hdmi_update_power(hdmi);
>>>> + dw_hdmi_poweroff(hdmi);
>>>> dw_hdmi_update_phy_mask(hdmi);
>>>> handle_plugged_change(hdmi, false);
>>>> mutex_unlock(&hdmi->mutex);
>>>> @@ -2974,7 +2946,7 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
>>>> mutex_lock(&hdmi->mutex);
>>>> hdmi->disabled = false;
>>>> hdmi->curr_conn = connector;
>>>> - dw_hdmi_update_power(hdmi);
>>>> + dw_hdmi_poweron(hdmi);
>>>> dw_hdmi_update_phy_mask(hdmi);
>>>> handle_plugged_change(hdmi, true);
>>>> mutex_unlock(&hdmi->mutex);
>>>> @@ -3073,7 +3045,6 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
>>>> if (hpd)
>>>> hdmi->rxsense = true;
>>>>
>>>> - dw_hdmi_update_power(hdmi);
>>>> dw_hdmi_update_phy_mask(hdmi);
>>>> }
>>>> mutex_unlock(&hdmi->mutex);
>>>
>>
>
