On 19/08/2019 16:47, Neil Armstrong wrote: > On 19/08/2019 16:41, Hans Verkuil wrote: >> On 8/19/19 4:38 PM, Neil Armstrong wrote: >>> Hi Hans, >>> >>> On 19/08/2019 16:05, Hans Verkuil wrote: >>>> 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> >>> >>> Did you test it on an Amlogic platform ? If yes, I don't have to ! >> >> Yes, tested on my khadas VIM2 (modified a bit to fix the issue where >> the CEC physical address wasn't invalidated correctly as discussed here >> earlier). > > Good, thanks. > > Reviewed-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > >> >> Regards, >> >> Hans >> >>> >>> Neil >>> >>>> >>>> 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) >>>>>> >>>>> >>>> >>> >> > Applying to drm-misc-next _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel