[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8a7da0a7-1234-4343-8538-9f7230024ca7@linaro.org>
Date: Fri, 13 Sep 2024 10:05:52 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Jonas Karlman <jonas@...boo.se>, Andrzej Hajda <andrzej.hajda@...el.com>,
Robert Foss <rfoss@...nel.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jernej Skrabec <jernej.skrabec@...il.com>
Cc: Christian Hewitt <christianshewitt@...il.com>,
Diederik de Haas <didi.debian@...ow.org>,
Christopher Obbard <chris.obbard@...labora.com>,
dri-devel@...ts.freedesktop.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 03/10] drm: bridge: dw_hdmi: Call poweron/poweroff from
atomic enable/disable
On 08/09/2024 15:28, Jonas Karlman wrote:
> Change to only call poweron/poweroff from atomic_enable/atomic_disable
> ops instead of trying to be clever by keeping a bridge_is_on state and
> poweron/off in the hotplug irq handler.
>
> The bridge is already enabled/disabled depending on connection state
> with the call to drm_helper_hpd_irq_event() in 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@...boo.se>
> ---
> v2: Update commit message
> ---
> 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 055fc9848df4..5b67640b1d0a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -169,7 +169,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 */
> @@ -2382,8 +2381,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.
> @@ -2397,30 +2394,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;
> - }
> -
> - if (force == DRM_FORCE_OFF) {
> - if (hdmi->bridge_is_on)
> - dw_hdmi_poweroff(hdmi);
> - } else {
> - if (!hdmi->bridge_is_on)
> - dw_hdmi_poweron(hdmi);
> - }
> }
>
> /*
> @@ -2545,7 +2518,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);
> }
> @@ -2954,7 +2926,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);
> @@ -2973,7 +2945,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);
> @@ -3072,7 +3044,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);
Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
Powered by blists - more mailing lists