* [PATCHv9 0/2] drm: tda998x: use cec_notifier_conn_(un)register @ 2019-10-17 7:28 ` Hans Verkuil 0 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: Dariusz Marcinkiewicz, Laurent Pinchart, dri-devel, Russell King This splits the previous v7.2 patch (1) into two parts: one that replaces cec_notifier_get/put by cec_notifier_conn_(un)register, and one that sets the connector info. That second patch moves the CEC notifier code to tda998x_bridge_detach, but Laurent is making changes in that area and prefers that this isn't implemented yet. Dariusz, can you comment on the use of the mutex in the second patch? This second patch won't be merged for v5.5 so you can take your time :-) But the replacement of the cec_notifier_get/put functions is something that needs to be finished for v5.5. By splitting this patch up I can merge the first half, but delay the second half. This tda driver just won't be able to report the connector information in the meantime. Changes since v8: - Moved the mutex addition to the second patch where I think it actually belongs. Regards, Hans (1) https://patchwork.linuxtv.org/patch/58464/ Dariusz Marcinkiewicz (2): drm: tda998x: use cec_notifier_conn_(un)register drm: tda998x: set the connector info drivers/gpu/drm/i2c/tda998x_drv.c | 38 ++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv9 0/2] drm: tda998x: use cec_notifier_conn_(un)register @ 2019-10-17 7:28 ` Hans Verkuil 0 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: dri-devel, Dariusz Marcinkiewicz, Daniel Vetter, Russell King, Laurent Pinchart This splits the previous v7.2 patch (1) into two parts: one that replaces cec_notifier_get/put by cec_notifier_conn_(un)register, and one that sets the connector info. That second patch moves the CEC notifier code to tda998x_bridge_detach, but Laurent is making changes in that area and prefers that this isn't implemented yet. Dariusz, can you comment on the use of the mutex in the second patch? This second patch won't be merged for v5.5 so you can take your time :-) But the replacement of the cec_notifier_get/put functions is something that needs to be finished for v5.5. By splitting this patch up I can merge the first half, but delay the second half. This tda driver just won't be able to report the connector information in the meantime. Changes since v8: - Moved the mutex addition to the second patch where I think it actually belongs. Regards, Hans (1) https://patchwork.linuxtv.org/patch/58464/ Dariusz Marcinkiewicz (2): drm: tda998x: use cec_notifier_conn_(un)register drm: tda998x: set the connector info drivers/gpu/drm/i2c/tda998x_drv.c | 38 ++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv9 1/2] drm: tda998x: use cec_notifier_conn_(un)register 2019-10-17 7:28 ` Hans Verkuil @ 2019-10-17 7:28 ` Hans Verkuil -1 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: Russell King, dri-devel, Laurent Pinchart, Hans Verkuil, Dariusz Marcinkiewicz From: Dariusz Marcinkiewicz <darekm@google.com> Use the new cec_notifier_conn_(un)register() functions to (un)register the notifier for the HDMI connector. Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/gpu/drm/i2c/tda998x_drv.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 84c6d4c91c65..dde8decb52a6 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -805,8 +805,8 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) tda998x_edid_delay_start(priv); } else { schedule_work(&priv->detect_work); - cec_notifier_set_phys_addr(priv->cec_notify, - CEC_PHYS_ADDR_INVALID); + cec_notifier_phys_addr_invalidate( + priv->cec_notify); } handled = true; @@ -1790,8 +1790,7 @@ static void tda998x_destroy(struct device *dev) i2c_unregister_device(priv->cec); - if (priv->cec_notify) - cec_notifier_put(priv->cec_notify); + cec_notifier_conn_unregister(priv->cec_notify); } static int tda998x_create(struct device *dev) @@ -1916,7 +1915,7 @@ static int tda998x_create(struct device *dev) cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); } - priv->cec_notify = cec_notifier_get(dev); + priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); if (!priv->cec_notify) { ret = -ENOMEM; goto fail; -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv9 1/2] drm: tda998x: use cec_notifier_conn_(un)register @ 2019-10-17 7:28 ` Hans Verkuil 0 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: dri-devel, Dariusz Marcinkiewicz, Daniel Vetter, Russell King, Laurent Pinchart, Hans Verkuil From: Dariusz Marcinkiewicz <darekm@google.com> Use the new cec_notifier_conn_(un)register() functions to (un)register the notifier for the HDMI connector. Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/gpu/drm/i2c/tda998x_drv.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 84c6d4c91c65..dde8decb52a6 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -805,8 +805,8 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) tda998x_edid_delay_start(priv); } else { schedule_work(&priv->detect_work); - cec_notifier_set_phys_addr(priv->cec_notify, - CEC_PHYS_ADDR_INVALID); + cec_notifier_phys_addr_invalidate( + priv->cec_notify); } handled = true; @@ -1790,8 +1790,7 @@ static void tda998x_destroy(struct device *dev) i2c_unregister_device(priv->cec); - if (priv->cec_notify) - cec_notifier_put(priv->cec_notify); + cec_notifier_conn_unregister(priv->cec_notify); } static int tda998x_create(struct device *dev) @@ -1916,7 +1915,7 @@ static int tda998x_create(struct device *dev) cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); } - priv->cec_notify = cec_notifier_get(dev); + priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); if (!priv->cec_notify) { ret = -ENOMEM; goto fail; -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv9 2/2] drm: tda998x: set the connector info 2019-10-17 7:28 ` Hans Verkuil @ 2019-10-17 7:28 ` Hans Verkuil -1 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: Russell King, dri-devel, Laurent Pinchart, Hans Verkuil, Dariusz Marcinkiewicz From: Dariusz Marcinkiewicz <darekm@google.com> Fill in the cec_connector_info when calling cec_notifier_conn_register(). Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index dde8decb52a6..b0620842fa3a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -82,6 +82,9 @@ struct tda998x_priv { u8 audio_port_enable[AUDIO_ROUTE_NUM]; struct tda9950_glue cec_glue; struct gpio_desc *calib; + + /* protect cec_notify */ + struct mutex cec_notify_mutex; struct cec_notifier *cec_notify; }; @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) tda998x_edid_delay_start(priv); } else { schedule_work(&priv->detect_work); + + mutex_lock(&priv->cec_notify_mutex); cec_notifier_phys_addr_invalidate( priv->cec_notify); + mutex_unlock(&priv->cec_notify_mutex); } handled = true; @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, struct drm_device *drm) { struct drm_connector *connector = &priv->connector; + struct cec_connector_info conn_info; + struct cec_notifier *notifier; int ret; connector->interlace_allowed = 1; @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, if (ret) return ret; + cec_fill_conn_info_from_drm(&conn_info, connector); + + notifier = cec_notifier_conn_register(priv->cec_glue.parent, + NULL, &conn_info); + if (!notifier) + return -ENOMEM; + + mutex_lock(&priv->cec_notify_mutex); + priv->cec_notify = notifier; + mutex_unlock(&priv->cec_notify_mutex); + drm_connector_attach_encoder(&priv->connector, priv->bridge.encoder); @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) { struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); + mutex_lock(&priv->cec_notify_mutex); + cec_notifier_conn_unregister(priv->cec_notify); + priv->cec_notify = NULL; + mutex_unlock(&priv->cec_notify_mutex); + drm_connector_cleanup(&priv->connector); } @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) cancel_work_sync(&priv->detect_work); i2c_unregister_device(priv->cec); - - cec_notifier_conn_unregister(priv->cec_notify); } static int tda998x_create(struct device *dev) @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) mutex_init(&priv->mutex); /* protect the page access */ mutex_init(&priv->audio_mutex); /* protect access from audio thread */ mutex_init(&priv->edid_mutex); + mutex_init(&priv->cec_notify_mutex); INIT_LIST_HEAD(&priv->bridge.list); init_waitqueue_head(&priv->edid_delay_waitq); timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); } - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); - if (!priv->cec_notify) { - ret = -ENOMEM; - goto fail; - } - priv->cec_glue.parent = dev; priv->cec_glue.data = priv; priv->cec_glue.init = tda998x_cec_hook_init; -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv9 2/2] drm: tda998x: set the connector info @ 2019-10-17 7:28 ` Hans Verkuil 0 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 7:28 UTC (permalink / raw) To: linux-media Cc: dri-devel, Dariusz Marcinkiewicz, Daniel Vetter, Russell King, Laurent Pinchart, Hans Verkuil From: Dariusz Marcinkiewicz <darekm@google.com> Fill in the cec_connector_info when calling cec_notifier_conn_register(). Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index dde8decb52a6..b0620842fa3a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -82,6 +82,9 @@ struct tda998x_priv { u8 audio_port_enable[AUDIO_ROUTE_NUM]; struct tda9950_glue cec_glue; struct gpio_desc *calib; + + /* protect cec_notify */ + struct mutex cec_notify_mutex; struct cec_notifier *cec_notify; }; @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) tda998x_edid_delay_start(priv); } else { schedule_work(&priv->detect_work); + + mutex_lock(&priv->cec_notify_mutex); cec_notifier_phys_addr_invalidate( priv->cec_notify); + mutex_unlock(&priv->cec_notify_mutex); } handled = true; @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, struct drm_device *drm) { struct drm_connector *connector = &priv->connector; + struct cec_connector_info conn_info; + struct cec_notifier *notifier; int ret; connector->interlace_allowed = 1; @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, if (ret) return ret; + cec_fill_conn_info_from_drm(&conn_info, connector); + + notifier = cec_notifier_conn_register(priv->cec_glue.parent, + NULL, &conn_info); + if (!notifier) + return -ENOMEM; + + mutex_lock(&priv->cec_notify_mutex); + priv->cec_notify = notifier; + mutex_unlock(&priv->cec_notify_mutex); + drm_connector_attach_encoder(&priv->connector, priv->bridge.encoder); @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) { struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); + mutex_lock(&priv->cec_notify_mutex); + cec_notifier_conn_unregister(priv->cec_notify); + priv->cec_notify = NULL; + mutex_unlock(&priv->cec_notify_mutex); + drm_connector_cleanup(&priv->connector); } @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) cancel_work_sync(&priv->detect_work); i2c_unregister_device(priv->cec); - - cec_notifier_conn_unregister(priv->cec_notify); } static int tda998x_create(struct device *dev) @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) mutex_init(&priv->mutex); /* protect the page access */ mutex_init(&priv->audio_mutex); /* protect access from audio thread */ mutex_init(&priv->edid_mutex); + mutex_init(&priv->cec_notify_mutex); INIT_LIST_HEAD(&priv->bridge.list); init_waitqueue_head(&priv->edid_delay_waitq); timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); } - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); - if (!priv->cec_notify) { - ret = -ENOMEM; - goto fail; - } - priv->cec_glue.parent = dev; priv->cec_glue.data = priv; priv->cec_glue.init = tda998x_cec_hook_init; -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv9 2/2] drm: tda998x: set the connector info 2019-10-17 7:28 ` Hans Verkuil @ 2019-10-17 8:11 ` Russell King - ARM Linux admin -1 siblings, 0 replies; 10+ messages in thread From: Russell King - ARM Linux admin @ 2019-10-17 8:11 UTC (permalink / raw) To: Hans Verkuil Cc: Dariusz Marcinkiewicz, Laurent Pinchart, dri-devel, linux-media On Thu, Oct 17, 2019 at 09:28:42AM +0200, Hans Verkuil wrote: > From: Dariusz Marcinkiewicz <darekm@google.com> > > Fill in the cec_connector_info when calling cec_notifier_conn_register(). > > Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> > Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index dde8decb52a6..b0620842fa3a 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -82,6 +82,9 @@ struct tda998x_priv { > u8 audio_port_enable[AUDIO_ROUTE_NUM]; > struct tda9950_glue cec_glue; > struct gpio_desc *calib; > + > + /* protect cec_notify */ > + struct mutex cec_notify_mutex; > struct cec_notifier *cec_notify; > }; > > @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) > tda998x_edid_delay_start(priv); > } else { > schedule_work(&priv->detect_work); > + > + mutex_lock(&priv->cec_notify_mutex); > cec_notifier_phys_addr_invalidate( > priv->cec_notify); > + mutex_unlock(&priv->cec_notify_mutex); > } > > handled = true; > @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, > struct drm_device *drm) > { > struct drm_connector *connector = &priv->connector; > + struct cec_connector_info conn_info; > + struct cec_notifier *notifier; > int ret; > > connector->interlace_allowed = 1; > @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, > if (ret) > return ret; > > + cec_fill_conn_info_from_drm(&conn_info, connector); > + > + notifier = cec_notifier_conn_register(priv->cec_glue.parent, > + NULL, &conn_info); > + if (!notifier) > + return -ENOMEM; > + > + mutex_lock(&priv->cec_notify_mutex); > + priv->cec_notify = notifier; > + mutex_unlock(&priv->cec_notify_mutex); You haven't taken on board what I said about the mutex in this instance. > + > drm_connector_attach_encoder(&priv->connector, > priv->bridge.encoder); > > @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) > { > struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); > > + mutex_lock(&priv->cec_notify_mutex); > + cec_notifier_conn_unregister(priv->cec_notify); > + priv->cec_notify = NULL; > + mutex_unlock(&priv->cec_notify_mutex); > + > drm_connector_cleanup(&priv->connector); > } > > @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) > cancel_work_sync(&priv->detect_work); > > i2c_unregister_device(priv->cec); > - > - cec_notifier_conn_unregister(priv->cec_notify); > } > > static int tda998x_create(struct device *dev) > @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) > mutex_init(&priv->mutex); /* protect the page access */ > mutex_init(&priv->audio_mutex); /* protect access from audio thread */ > mutex_init(&priv->edid_mutex); > + mutex_init(&priv->cec_notify_mutex); > INIT_LIST_HEAD(&priv->bridge.list); > init_waitqueue_head(&priv->edid_delay_waitq); > timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); > @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) > cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); > } > > - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); > - if (!priv->cec_notify) { > - ret = -ENOMEM; > - goto fail; > - } > - > priv->cec_glue.parent = dev; > priv->cec_glue.data = priv; > priv->cec_glue.init = tda998x_cec_hook_init; > -- > 2.23.0 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv9 2/2] drm: tda998x: set the connector info @ 2019-10-17 8:11 ` Russell King - ARM Linux admin 0 siblings, 0 replies; 10+ messages in thread From: Russell King - ARM Linux admin @ 2019-10-17 8:11 UTC (permalink / raw) To: Hans Verkuil Cc: linux-media, dri-devel, Dariusz Marcinkiewicz, Daniel Vetter, Laurent Pinchart On Thu, Oct 17, 2019 at 09:28:42AM +0200, Hans Verkuil wrote: > From: Dariusz Marcinkiewicz <darekm@google.com> > > Fill in the cec_connector_info when calling cec_notifier_conn_register(). > > Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> > Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index dde8decb52a6..b0620842fa3a 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -82,6 +82,9 @@ struct tda998x_priv { > u8 audio_port_enable[AUDIO_ROUTE_NUM]; > struct tda9950_glue cec_glue; > struct gpio_desc *calib; > + > + /* protect cec_notify */ > + struct mutex cec_notify_mutex; > struct cec_notifier *cec_notify; > }; > > @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) > tda998x_edid_delay_start(priv); > } else { > schedule_work(&priv->detect_work); > + > + mutex_lock(&priv->cec_notify_mutex); > cec_notifier_phys_addr_invalidate( > priv->cec_notify); > + mutex_unlock(&priv->cec_notify_mutex); > } > > handled = true; > @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, > struct drm_device *drm) > { > struct drm_connector *connector = &priv->connector; > + struct cec_connector_info conn_info; > + struct cec_notifier *notifier; > int ret; > > connector->interlace_allowed = 1; > @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, > if (ret) > return ret; > > + cec_fill_conn_info_from_drm(&conn_info, connector); > + > + notifier = cec_notifier_conn_register(priv->cec_glue.parent, > + NULL, &conn_info); > + if (!notifier) > + return -ENOMEM; > + > + mutex_lock(&priv->cec_notify_mutex); > + priv->cec_notify = notifier; > + mutex_unlock(&priv->cec_notify_mutex); You haven't taken on board what I said about the mutex in this instance. > + > drm_connector_attach_encoder(&priv->connector, > priv->bridge.encoder); > > @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) > { > struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); > > + mutex_lock(&priv->cec_notify_mutex); > + cec_notifier_conn_unregister(priv->cec_notify); > + priv->cec_notify = NULL; > + mutex_unlock(&priv->cec_notify_mutex); > + > drm_connector_cleanup(&priv->connector); > } > > @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) > cancel_work_sync(&priv->detect_work); > > i2c_unregister_device(priv->cec); > - > - cec_notifier_conn_unregister(priv->cec_notify); > } > > static int tda998x_create(struct device *dev) > @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) > mutex_init(&priv->mutex); /* protect the page access */ > mutex_init(&priv->audio_mutex); /* protect access from audio thread */ > mutex_init(&priv->edid_mutex); > + mutex_init(&priv->cec_notify_mutex); > INIT_LIST_HEAD(&priv->bridge.list); > init_waitqueue_head(&priv->edid_delay_waitq); > timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); > @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) > cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); > } > > - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); > - if (!priv->cec_notify) { > - ret = -ENOMEM; > - goto fail; > - } > - > priv->cec_glue.parent = dev; > priv->cec_glue.data = priv; > priv->cec_glue.init = tda998x_cec_hook_init; > -- > 2.23.0 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv9 2/2] drm: tda998x: set the connector info 2019-10-17 8:11 ` Russell King - ARM Linux admin @ 2019-10-17 8:42 ` Hans Verkuil -1 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 8:42 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Dariusz Marcinkiewicz, Laurent Pinchart, dri-devel, linux-media On 10/17/19 10:11 AM, Russell King - ARM Linux admin wrote: > On Thu, Oct 17, 2019 at 09:28:42AM +0200, Hans Verkuil wrote: >> From: Dariusz Marcinkiewicz <darekm@google.com> >> >> Fill in the cec_connector_info when calling cec_notifier_conn_register(). >> >> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> >> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> --- >> drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- >> 1 file changed, 25 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c >> index dde8decb52a6..b0620842fa3a 100644 >> --- a/drivers/gpu/drm/i2c/tda998x_drv.c >> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c >> @@ -82,6 +82,9 @@ struct tda998x_priv { >> u8 audio_port_enable[AUDIO_ROUTE_NUM]; >> struct tda9950_glue cec_glue; >> struct gpio_desc *calib; >> + >> + /* protect cec_notify */ >> + struct mutex cec_notify_mutex; >> struct cec_notifier *cec_notify; >> }; >> >> @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) >> tda998x_edid_delay_start(priv); >> } else { >> schedule_work(&priv->detect_work); >> + >> + mutex_lock(&priv->cec_notify_mutex); >> cec_notifier_phys_addr_invalidate( >> priv->cec_notify); >> + mutex_unlock(&priv->cec_notify_mutex); >> } >> >> handled = true; >> @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, >> struct drm_device *drm) >> { >> struct drm_connector *connector = &priv->connector; >> + struct cec_connector_info conn_info; >> + struct cec_notifier *notifier; >> int ret; >> >> connector->interlace_allowed = 1; >> @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, >> if (ret) >> return ret; >> >> + cec_fill_conn_info_from_drm(&conn_info, connector); >> + >> + notifier = cec_notifier_conn_register(priv->cec_glue.parent, >> + NULL, &conn_info); >> + if (!notifier) >> + return -ENOMEM; >> + >> + mutex_lock(&priv->cec_notify_mutex); >> + priv->cec_notify = notifier; >> + mutex_unlock(&priv->cec_notify_mutex); > > You haven't taken on board what I said about the mutex in this > instance. That's because I didn't. See the cover letter. I need this info from the author of the patch, Dariusz. Once I have that, and we agree with the reasoning, then I'll post a new patch for it. For now all I am interested in is getting patch 1/2 merged. Patch 2/2 won't be merged any time soon. Regards, Hans > >> + >> drm_connector_attach_encoder(&priv->connector, >> priv->bridge.encoder); >> >> @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) >> { >> struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); >> >> + mutex_lock(&priv->cec_notify_mutex); >> + cec_notifier_conn_unregister(priv->cec_notify); >> + priv->cec_notify = NULL; >> + mutex_unlock(&priv->cec_notify_mutex); >> + >> drm_connector_cleanup(&priv->connector); >> } >> >> @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) >> cancel_work_sync(&priv->detect_work); >> >> i2c_unregister_device(priv->cec); >> - >> - cec_notifier_conn_unregister(priv->cec_notify); >> } >> >> static int tda998x_create(struct device *dev) >> @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) >> mutex_init(&priv->mutex); /* protect the page access */ >> mutex_init(&priv->audio_mutex); /* protect access from audio thread */ >> mutex_init(&priv->edid_mutex); >> + mutex_init(&priv->cec_notify_mutex); >> INIT_LIST_HEAD(&priv->bridge.list); >> init_waitqueue_head(&priv->edid_delay_waitq); >> timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); >> @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) >> cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); >> } >> >> - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); >> - if (!priv->cec_notify) { >> - ret = -ENOMEM; >> - goto fail; >> - } >> - >> priv->cec_glue.parent = dev; >> priv->cec_glue.data = priv; >> priv->cec_glue.init = tda998x_cec_hook_init; >> -- >> 2.23.0 >> >> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv9 2/2] drm: tda998x: set the connector info @ 2019-10-17 8:42 ` Hans Verkuil 0 siblings, 0 replies; 10+ messages in thread From: Hans Verkuil @ 2019-10-17 8:42 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-media, dri-devel, Dariusz Marcinkiewicz, Daniel Vetter, Laurent Pinchart On 10/17/19 10:11 AM, Russell King - ARM Linux admin wrote: > On Thu, Oct 17, 2019 at 09:28:42AM +0200, Hans Verkuil wrote: >> From: Dariusz Marcinkiewicz <darekm@google.com> >> >> Fill in the cec_connector_info when calling cec_notifier_conn_register(). >> >> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> >> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> --- >> drivers/gpu/drm/i2c/tda998x_drv.c | 33 +++++++++++++++++++++++-------- >> 1 file changed, 25 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c >> index dde8decb52a6..b0620842fa3a 100644 >> --- a/drivers/gpu/drm/i2c/tda998x_drv.c >> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c >> @@ -82,6 +82,9 @@ struct tda998x_priv { >> u8 audio_port_enable[AUDIO_ROUTE_NUM]; >> struct tda9950_glue cec_glue; >> struct gpio_desc *calib; >> + >> + /* protect cec_notify */ >> + struct mutex cec_notify_mutex; >> struct cec_notifier *cec_notify; >> }; >> >> @@ -805,8 +808,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) >> tda998x_edid_delay_start(priv); >> } else { >> schedule_work(&priv->detect_work); >> + >> + mutex_lock(&priv->cec_notify_mutex); >> cec_notifier_phys_addr_invalidate( >> priv->cec_notify); >> + mutex_unlock(&priv->cec_notify_mutex); >> } >> >> handled = true; >> @@ -1331,6 +1337,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv, >> struct drm_device *drm) >> { >> struct drm_connector *connector = &priv->connector; >> + struct cec_connector_info conn_info; >> + struct cec_notifier *notifier; >> int ret; >> >> connector->interlace_allowed = 1; >> @@ -1347,6 +1355,17 @@ static int tda998x_connector_init(struct tda998x_priv *priv, >> if (ret) >> return ret; >> >> + cec_fill_conn_info_from_drm(&conn_info, connector); >> + >> + notifier = cec_notifier_conn_register(priv->cec_glue.parent, >> + NULL, &conn_info); >> + if (!notifier) >> + return -ENOMEM; >> + >> + mutex_lock(&priv->cec_notify_mutex); >> + priv->cec_notify = notifier; >> + mutex_unlock(&priv->cec_notify_mutex); > > You haven't taken on board what I said about the mutex in this > instance. That's because I didn't. See the cover letter. I need this info from the author of the patch, Dariusz. Once I have that, and we agree with the reasoning, then I'll post a new patch for it. For now all I am interested in is getting patch 1/2 merged. Patch 2/2 won't be merged any time soon. Regards, Hans > >> + >> drm_connector_attach_encoder(&priv->connector, >> priv->bridge.encoder); >> >> @@ -1366,6 +1385,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge) >> { >> struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); >> >> + mutex_lock(&priv->cec_notify_mutex); >> + cec_notifier_conn_unregister(priv->cec_notify); >> + priv->cec_notify = NULL; >> + mutex_unlock(&priv->cec_notify_mutex); >> + >> drm_connector_cleanup(&priv->connector); >> } >> >> @@ -1789,8 +1813,6 @@ static void tda998x_destroy(struct device *dev) >> cancel_work_sync(&priv->detect_work); >> >> i2c_unregister_device(priv->cec); >> - >> - cec_notifier_conn_unregister(priv->cec_notify); >> } >> >> static int tda998x_create(struct device *dev) >> @@ -1811,6 +1833,7 @@ static int tda998x_create(struct device *dev) >> mutex_init(&priv->mutex); /* protect the page access */ >> mutex_init(&priv->audio_mutex); /* protect access from audio thread */ >> mutex_init(&priv->edid_mutex); >> + mutex_init(&priv->cec_notify_mutex); >> INIT_LIST_HEAD(&priv->bridge.list); >> init_waitqueue_head(&priv->edid_delay_waitq); >> timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0); >> @@ -1915,12 +1938,6 @@ static int tda998x_create(struct device *dev) >> cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD); >> } >> >> - priv->cec_notify = cec_notifier_conn_register(dev, NULL, NULL); >> - if (!priv->cec_notify) { >> - ret = -ENOMEM; >> - goto fail; >> - } >> - >> priv->cec_glue.parent = dev; >> priv->cec_glue.data = priv; >> priv->cec_glue.init = tda998x_cec_hook_init; >> -- >> 2.23.0 >> >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-17 8:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-17 7:28 [PATCHv9 0/2] drm: tda998x: use cec_notifier_conn_(un)register Hans Verkuil 2019-10-17 7:28 ` Hans Verkuil 2019-10-17 7:28 ` [PATCHv9 1/2] " Hans Verkuil 2019-10-17 7:28 ` Hans Verkuil 2019-10-17 7:28 ` [PATCHv9 2/2] drm: tda998x: set the connector info Hans Verkuil 2019-10-17 7:28 ` Hans Verkuil 2019-10-17 8:11 ` Russell King - ARM Linux admin 2019-10-17 8:11 ` Russell King - ARM Linux admin 2019-10-17 8:42 ` Hans Verkuil 2019-10-17 8:42 ` Hans Verkuil
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.