Our detect callback has a bunch of operations to perform depending on the current and last status of the connector, such a setting the CEC physical address or enabling the scrambling again.
This is currently dealt with a bunch of if / else statetements that make it fairly difficult to read and extend.
Let's move all that logic to a function of its own.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 66 ++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 98f15b11f135..7f44ef08f83a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -182,17 +182,53 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, + enum drm_connector_status status) +{ + struct drm_connector *connector = &vc4_hdmi->connector; + struct edid *edid; + + /* + * NOTE: This function should really be called with + * vc4_hdmi->mutex held, but doing so results in reentrancy + * issues since cec_s_phys_addr_from_edid might call + * .adap_enable, which leads to that funtion being called with + * our mutex held. + * + * Concurrency isn't an issue at the moment since we don't share + * any state with any of the other frameworks so we can ignore + * the lock for now. + */ + + if (status == connector->status) + return; + + if (status == connector_status_disconnected) { + cec_phys_addr_invalidate(vc4_hdmi->cec_adap); + return; + } + + edid = drm_get_edid(connector, vc4_hdmi->ddc); + if (!edid) + return; + + cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); + kfree(edid); + + vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); +} + static enum drm_connector_status vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) { struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); - bool connected = false; + enum drm_connector_status status = connector_status_disconnected;
/* * NOTE: This function should really take vc4_hdmi->mutex, but * doing so results in reentrancy issues since - * cec_s_phys_addr_from_edid might call .adap_enable, which - * leads to that funtion being called with our mutex held. + * vc4_hdmi_handle_hotplug() can call into other functions that + * would take the mutex while it's held here. * * Concurrency isn't an issue at the moment since we don't share * any state with any of the other frameworks so we can ignore @@ -203,7 +239,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
if (vc4_hdmi->hpd_gpio) { if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) - connected = true; + status = connector_status_connected; } else { unsigned long flags; u32 hotplug; @@ -213,27 +249,13 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
if (hotplug & VC4_HDMI_HOTPLUG_CONNECTED) - connected = true; + status = connector_status_connected; }
- if (connected) { - if (connector->status != connector_status_connected) { - struct edid *edid = drm_get_edid(connector, vc4_hdmi->ddc); - - if (edid) { - cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); - kfree(edid); - } - } - - vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); - pm_runtime_put(&vc4_hdmi->pdev->dev); - return connector_status_connected; - } - - cec_phys_addr_invalidate(vc4_hdmi->cec_adap); + vc4_hdmi_handle_hotplug(vc4_hdmi, status); pm_runtime_put(&vc4_hdmi->pdev->dev); - return connector_status_disconnected; + + return status; }
static void vc4_hdmi_connector_destroy(struct drm_connector *connector)