Hi Neil,
On 2024-06-24 11:56, neil.armstrong@xxxxxxxxxx wrote:
> On 24/06/2024 11:41, Jonas Karlman wrote:
>> Hi Neil,
>>
>> On 2024-06-24 11:23, Neil Armstrong wrote:
>>> On 11/06/2024 17:50, Jonas Karlman wrote:
>>>> Change to only call poweron/poweroff from atomic_enable/atomic_disable
>>>> ops instead of trying to keep a bridge_is_on state and poweron/off in
>>>> the hotplug irq handler.
>>>>
>>>> A benefit of this is that drm mode_config mutex is always held at
>>>> poweron/off, something that may reduce the need for our own mutex.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx>
>>>> ---
>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 33 ++---------------------
>>>> 1 file changed, 2 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index 9f2bc932c371..34bc6f4754b8 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -172,7 +172,6 @@ struct dw_hdmi {
>>>> enum drm_connector_force force; /* mutex-protected force state */
>>>> struct drm_connector *curr_conn;/* current connector (only valid when !disabled) */
>>>> bool disabled; /* DRM has disabled our bridge */
>>>> - bool bridge_is_on; /* indicates the bridge is on */
>>>> bool rxsense; /* rxsense state */
>>>> u8 phy_mask; /* desired phy int mask settings */
>>>> u8 mc_clkdis; /* clock disable register */
>>>> @@ -2383,8 +2382,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>>>>
>>>> static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
>>>> {
>>>> - hdmi->bridge_is_on = true;
>>>> -
>>>> /*
>>>> * The curr_conn field is guaranteed to be valid here, as this function
>>>> * is only be called when !hdmi->disabled.
>>>> @@ -2398,30 +2395,6 @@ static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
>>>> hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>>>> hdmi->phy.enabled = false;
>>>> }
>>>> -
>>>> - hdmi->bridge_is_on = false;
>>>> -}
>>>> -
>>>> -static void dw_hdmi_update_power(struct dw_hdmi *hdmi)
>>>> -{
>>>> - int force = hdmi->force;
>>>> -
>>>> - if (hdmi->disabled) {
>>>> - force = DRM_FORCE_OFF;
>>>> - } else if (force == DRM_FORCE_UNSPECIFIED) {
>>>> - if (hdmi->rxsense)
>>>> - force = DRM_FORCE_ON;
>>>> - else
>>>> - force = DRM_FORCE_OFF;
>>>> - }
>>>
>>> This means we always poweron the bridge even if rxsense is false ?
>>
>> If I follow the logic there should not be any difference, beside that
>> power on now only happen in atomic_enable instead of sometime in irq
>> handler.
>>
>> hdmi->rxsense is set to true based on hpd in dw_hdmi_setup_rx_sense().
>> For both meson and dw-hdmi this means HPD=1 and not rxsense=1.
>
> Yeah I know, I was worried for other platforms using rxsense
