On 8/19/19 11:32 AM, Hans Verkuil wrote: > On 8/14/19 12:45 PM, Dariusz Marcinkiewicz wrote: >> Use the new cec_notifier_conn_(un)register() functions to >> (un)register the notifier for the HDMI connector, and fill in >> the cec_connector_info. >> >> Changes since v6: >> - move cec_notifier_conn_unregister to a bridge detach >> function, >> - add a mutex protecting a CEC notifier. >> Changes since v4: >> - typo fix >> Changes since v2: >> - removed unnecessary NULL check before a call to >> cec_notifier_conn_unregister, >> - use cec_notifier_phys_addr_invalidate to invalidate physical >> address. >> Changes since v1: >> Add memory barrier to make sure that the notifier >> becomes visible to the irq thread once it is fully >> constructed. >> >> Signed-off-by: Dariusz Marcinkiewicz <darekm@xxxxxxxxxx> > > Acked-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> Tested-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> Regards, Hans > > Regards, > > Hans > >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++-------- >> 1 file changed, 30 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 83b94b66e464e..55162c9092f71 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -190,6 +190,7 @@ struct dw_hdmi { >> void (*enable_audio)(struct dw_hdmi *hdmi); >> void (*disable_audio)(struct dw_hdmi *hdmi); >> >> + struct mutex cec_notifier_mutex; >> struct cec_notifier *cec_notifier; >> }; >> >> @@ -2194,6 +2195,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge) >> struct dw_hdmi *hdmi = bridge->driver_private; >> struct drm_encoder *encoder = bridge->encoder; >> struct drm_connector *connector = &hdmi->connector; >> + struct cec_connector_info conn_info; >> + struct cec_notifier *notifier; >> >> connector->interlace_allowed = 1; >> connector->polled = DRM_CONNECTOR_POLL_HPD; >> @@ -2207,9 +2210,29 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge) >> >> drm_connector_attach_encoder(connector, encoder); >> >> + cec_fill_conn_info_from_drm(&conn_info, connector); >> + >> + notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info); >> + if (!notifier) >> + return -ENOMEM; >> + >> + mutex_lock(&hdmi->cec_notifier_mutex); >> + hdmi->cec_notifier = notifier; >> + mutex_unlock(&hdmi->cec_notifier_mutex); >> + >> return 0; >> } >> >> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) >> +{ >> + struct dw_hdmi *hdmi = bridge->driver_private; >> + >> + mutex_lock(&hdmi->cec_notifier_mutex); >> + cec_notifier_conn_unregister(hdmi->cec_notifier); >> + hdmi->cec_notifier = NULL; >> + mutex_unlock(&hdmi->cec_notifier_mutex); >> +} >> + >> static enum drm_mode_status >> dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge, >> const struct drm_display_mode *mode) >> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge) >> >> static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { >> .attach = dw_hdmi_bridge_attach, >> + .detach = dw_hdmi_bridge_detach, >> .enable = dw_hdmi_bridge_enable, >> .disable = dw_hdmi_bridge_disable, >> .mode_set = dw_hdmi_bridge_mode_set, >> @@ -2373,9 +2397,11 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) >> phy_stat & HDMI_PHY_HPD, >> phy_stat & HDMI_PHY_RX_SENSE); >> >> - if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) >> - cec_notifier_set_phys_addr(hdmi->cec_notifier, >> - CEC_PHYS_ADDR_INVALID); >> + if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) { >> + mutex_lock(&hdmi->cec_notifier_mutex); >> + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); >> + mutex_unlock(&hdmi->cec_notifier_mutex); >> + } >> } >> >> if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { >> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, >> >> mutex_init(&hdmi->mutex); >> mutex_init(&hdmi->audio_mutex); >> + mutex_init(&hdmi->cec_notifier_mutex); >> spin_lock_init(&hdmi->audio_lock); >> >> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); >> @@ -2693,12 +2720,6 @@ __dw_hdmi_probe(struct platform_device *pdev, >> if (ret) >> goto err_iahb; >> >> - hdmi->cec_notifier = cec_notifier_get(dev); >> - if (!hdmi->cec_notifier) { >> - ret = -ENOMEM; >> - goto err_iahb; >> - } >> - >> /* >> * To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator >> * N and cts values before enabling phy >> @@ -2796,9 +2817,6 @@ __dw_hdmi_probe(struct platform_device *pdev, >> hdmi->ddc = NULL; >> } >> >> - if (hdmi->cec_notifier) >> - cec_notifier_put(hdmi->cec_notifier); >> - >> clk_disable_unprepare(hdmi->iahb_clk); >> if (hdmi->cec_clk) >> clk_disable_unprepare(hdmi->cec_clk); >> @@ -2820,9 +2838,6 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) >> /* Disable all interrupts */ >> hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); >> >> - if (hdmi->cec_notifier) >> - cec_notifier_put(hdmi->cec_notifier); >> - >> clk_disable_unprepare(hdmi->iahb_clk); >> clk_disable_unprepare(hdmi->isfr_clk); >> if (hdmi->cec_clk) >> >