Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 > > Acked-by: Hans Verkuil Tested-by: Hans Verkuil >>> >>> 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 > >> >> 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 = >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(_info, connector); >> + >> +notifier = cec_notifier_conn_register(hdmi->dev, NULL, >> _info); >> +if (!notifier) >> +return -ENOMEM; >> + >> +mutex_lock(>cec_notifier_mutex); >> +hdmi->cec_notifier = notifier; >> +mutex_unlock(>cec_notifier_mutex); >> + >> return 0; >> } >> >> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) >> +{ >> +struct dw_hdmi *hdmi = bridge->driver_private; >> + >> +mutex_lock(>cec_notifier_mutex); >> +cec_notifier_conn_unregister(hdmi->cec_notifier); >> +hdmi->cec_notifier = NULL; >> +mutex_unlock(>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 |
Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 Acked-by: Hans Verkuil >>> >>> Tested-by: Hans Verkuil >> >> 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 > > 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 = >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(_info, connector); > + > + notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); > + if (!notifier) > + return -ENOMEM; > + > + mutex_lock(>cec_notifier_mutex); > + hdmi->cec_notifier = notifier; > + mutex_unlock(>cec_notifier_mutex); > + > return 0; > } > > +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) > +{ > + struct dw_hdmi *hdmi = bridge->driver_private; > + > + mutex_lock(>cec_notifier_mutex); > + cec_notifier_conn_unregister(hdmi->cec_notifier); > + hdmi->cec_notifier = NULL; > + mutex_unlock(>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(>cec_notifier_mutex); > + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); > + mutex_unlock(>cec_notifier_mutex); > + } > } > > if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { > @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > > mutex_init(>mutex); > mutex_init(>audio_mutex); >
Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 >>> >>> Acked-by: Hans Verkuil >> >> Tested-by: Hans Verkuil > > 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). 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 = >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(_info, connector); + + notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); + if (!notifier) + return -ENOMEM; + + mutex_lock(>cec_notifier_mutex); + hdmi->cec_notifier = notifier; + mutex_unlock(>cec_notifier_mutex); + return 0; } +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) +{ + struct dw_hdmi *hdmi = bridge->driver_private; + + mutex_lock(>cec_notifier_mutex); + cec_notifier_conn_unregister(hdmi->cec_notifier); + hdmi->cec_notifier = NULL; + mutex_unlock(>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(>cec_notifier_mutex); + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); + mutex_unlock(>cec_notifier_mutex); + } } if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, mutex_init(>mutex); mutex_init(>audio_mutex); + mutex_init(>cec_notifier_mutex); spin_lock_init(>audio_lock); ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); @@ -2693,12 +2720,6 @@
Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 >> >> Acked-by: Hans Verkuil > > Tested-by: Hans Verkuil Did you test it on an Amlogic platform ? If yes, I don't have to ! 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 = >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(_info, connector); >>> + >>> + notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); >>> + if (!notifier) >>> + return -ENOMEM; >>> + >>> + mutex_lock(>cec_notifier_mutex); >>> + hdmi->cec_notifier = notifier; >>> + mutex_unlock(>cec_notifier_mutex); >>> + >>> return 0; >>> } >>> >>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) >>> +{ >>> + struct dw_hdmi *hdmi = bridge->driver_private; >>> + >>> + mutex_lock(>cec_notifier_mutex); >>> + cec_notifier_conn_unregister(hdmi->cec_notifier); >>> + hdmi->cec_notifier = NULL; >>> + mutex_unlock(>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(>cec_notifier_mutex); >>> + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); >>> + mutex_unlock(>cec_notifier_mutex); >>> + } >>> } >>> >>> if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { >>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, >>> >>> mutex_init(>mutex); >>> mutex_init(>audio_mutex); >>> + mutex_init(>cec_notifier_mutex); >>> spin_lock_init(>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
Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 > > Acked-by: Hans Verkuil Tested-by: Hans Verkuil 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 = >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(_info, connector); >> + >> +notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); >> +if (!notifier) >> +return -ENOMEM; >> + >> +mutex_lock(>cec_notifier_mutex); >> +hdmi->cec_notifier = notifier; >> +mutex_unlock(>cec_notifier_mutex); >> + >> return 0; >> } >> >> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) >> +{ >> +struct dw_hdmi *hdmi = bridge->driver_private; >> + >> +mutex_lock(>cec_notifier_mutex); >> +cec_notifier_conn_unregister(hdmi->cec_notifier); >> +hdmi->cec_notifier = NULL; >> +mutex_unlock(>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(>cec_notifier_mutex); >> +cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); >> +mutex_unlock(>cec_notifier_mutex); >> +} >> } >> >> if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { >> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, >> >> mutex_init(>mutex); >> mutex_init(>audio_mutex); >> +mutex_init(>cec_notifier_mutex); >> spin_lock_init(>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; >>
Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 Acked-by: Hans Verkuil 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 = >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(_info, connector); > + > + notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); > + if (!notifier) > + return -ENOMEM; > + > + mutex_lock(>cec_notifier_mutex); > + hdmi->cec_notifier = notifier; > + mutex_unlock(>cec_notifier_mutex); > + > return 0; > } > > +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) > +{ > + struct dw_hdmi *hdmi = bridge->driver_private; > + > + mutex_lock(>cec_notifier_mutex); > + cec_notifier_conn_unregister(hdmi->cec_notifier); > + hdmi->cec_notifier = NULL; > + mutex_unlock(>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(>cec_notifier_mutex); > + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); > + mutex_unlock(>cec_notifier_mutex); > + } > } > > if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { > @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > > mutex_init(>mutex); > mutex_init(>audio_mutex); > + mutex_init(>cec_notifier_mutex); > spin_lock_init(>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
[PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
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 --- 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 = >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(_info, connector); + + notifier = cec_notifier_conn_register(hdmi->dev, NULL, _info); + if (!notifier) + return -ENOMEM; + + mutex_lock(>cec_notifier_mutex); + hdmi->cec_notifier = notifier; + mutex_unlock(>cec_notifier_mutex); + return 0; } +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge) +{ + struct dw_hdmi *hdmi = bridge->driver_private; + + mutex_lock(>cec_notifier_mutex); + cec_notifier_conn_unregister(hdmi->cec_notifier); + hdmi->cec_notifier = NULL; + mutex_unlock(>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(>cec_notifier_mutex); + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier); + mutex_unlock(>cec_notifier_mutex); + } } if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev, mutex_init(>mutex); mutex_init(>audio_mutex); + mutex_init(>cec_notifier_mutex); spin_lock_init(>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,