drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion.
Signed-off-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else { - drm_bridge_hpd_notify(&adv7511->bridge, status); + drm_bridge_hpd_cb(&adv7511->bridge, status); } } } diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } }
@@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else { - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } }
diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge;
- drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge));
return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock);
- drm_bridge_hpd_notify(<9611uxc->bridge, - connected ? - connector_status_connected : - connector_status_disconnected); + drm_bridge_hpd_cb(<9611uxc->bridge, + connected ? + connector_status_connected : + connector_status_disconnected); } }
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev); - drm_bridge_hpd_notify(&hdmi->bridge, status); + drm_bridge_hpd_cb(&hdmi->bridge, status); } }
diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge;
- drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge));
return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
/** - * drm_bridge_hpd_notify - notify hot plug detection events + * drm_bridge_hpd_cb - notify hot plug detection events * @bridge: bridge control structure * @status: output connection status * @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); * * This function shall be called in a context that can sleep. */ -void drm_bridge_hpd_notify(struct drm_bridge *bridge, - enum drm_connector_status status) +void drm_bridge_hpd_cb(struct drm_bridge *bridge, + enum drm_connector_status status) { mutex_lock(&bridge->hpd_mutex); if (bridge->hpd_cb) bridge->hpd_cb(bridge->hpd_data, status); mutex_unlock(&bridge->hpd_mutex); } -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb);
#ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call - * drm_bridge_hpd_notify() each time a change is detected in the output + * drm_bridge_hpd_cb() each time a change is detected in the output * connection status, until hot plug detection gets disabled with * @hpd_disable. * @@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge - * shall not call drm_bridge_hpd_notify() when a change in the output + * shall not call drm_bridge_hpd_cb() when a change in the output * connection status occurs. * * This callback is optional and shall only be implemented by bridges @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge, - enum drm_connector_status status); +void drm_bridge_hpd_cb(struct drm_bridge *bridge, + enum drm_connector_status status);
#ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
Hey Dafna,
Thanks for submitting a cleanup patch, it is much appreciated. It looks good to me, feel free to add my r-b.
I'm not going to merge this right away, but will let this patch soak for a while to let other people have a look at it.
On Tue, 30 Mar 2021 at 13:52, Dafna Hirschfeld dafna.hirschfeld@collabora.com wrote:
drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion.
Signed-off-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else {
drm_bridge_hpd_notify(&adv7511->bridge, status);
drm_bridge_hpd_cb(&adv7511->bridge, status); } }} diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); }}
@@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else {
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } }diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge;
drm_bridge_hpd_notify(bridge, display_connector_detect(bridge));
drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); return IRQ_HANDLED;} diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock);
drm_bridge_hpd_notify(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);
drm_bridge_hpd_cb(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected); }}
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev);
drm_bridge_hpd_notify(&hdmi->bridge, status);
drm_bridge_hpd_cb(&hdmi->bridge, status); } }diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge;
drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge));
drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); return IRQ_HANDLED;} diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
/**
- drm_bridge_hpd_notify - notify hot plug detection events
- drm_bridge_hpd_cb - notify hot plug detection events
- @bridge: bridge control structure
- @status: output connection status
@@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
- This function shall be called in a context that can sleep.
*/ -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status)+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status){ mutex_lock(&bridge->hpd_mutex); if (bridge->hpd_cb) bridge->hpd_cb(bridge->hpd_data, status); mutex_unlock(&bridge->hpd_mutex); } -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb);
#ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call
* drm_bridge_hpd_notify() each time a change is detected in the output
* drm_bridge_hpd_cb() each time a change is detected in the output * connection status, until hot plug detection gets disabled with * @hpd_disable. *@@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge
* shall not call drm_bridge_hpd_notify() when a change in the output
* shall not call drm_bridge_hpd_cb() when a change in the output * connection status occurs. * * This callback is optional and shall only be implemented by bridges@@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status);#ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); -- 2.17.1
Reviewed-by: Robert Foss robert.foss@linaro.org
On Tue, 30 Mar 2021 at 14:28, Robert Foss robert.foss@linaro.org wrote:
Hey Dafna,
Thanks for submitting a cleanup patch, it is much appreciated. It looks good to me, feel free to add my r-b.
I'm not going to merge this right away, but will let this patch soak for a while to let other people have a look at it.
On Tue, 30 Mar 2021 at 13:52, Dafna Hirschfeld dafna.hirschfeld@collabora.com wrote:
drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion.
Signed-off-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else {
drm_bridge_hpd_notify(&adv7511->bridge, status);
drm_bridge_hpd_cb(&adv7511->bridge, status); } }} diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); }}
@@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else {
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } }diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge;
drm_bridge_hpd_notify(bridge, display_connector_detect(bridge));
drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); return IRQ_HANDLED;} diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock);
drm_bridge_hpd_notify(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);
drm_bridge_hpd_cb(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected); }}
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev);
drm_bridge_hpd_notify(&hdmi->bridge, status);
drm_bridge_hpd_cb(&hdmi->bridge, status); } }diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge;
drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge));
drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); return IRQ_HANDLED;} diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
/**
- drm_bridge_hpd_notify - notify hot plug detection events
- drm_bridge_hpd_cb - notify hot plug detection events
- @bridge: bridge control structure
- @status: output connection status
@@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
- This function shall be called in a context that can sleep.
*/ -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status)+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status){ mutex_lock(&bridge->hpd_mutex); if (bridge->hpd_cb) bridge->hpd_cb(bridge->hpd_data, status); mutex_unlock(&bridge->hpd_mutex); } -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb);
#ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call
* drm_bridge_hpd_notify() each time a change is detected in the output
* drm_bridge_hpd_cb() each time a change is detected in the output * connection status, until hot plug detection gets disabled with * @hpd_disable. *@@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge
* shall not call drm_bridge_hpd_notify() when a change in the output
* shall not call drm_bridge_hpd_cb() when a change in the output * connection status occurs. * * This callback is optional and shall only be implemented by bridges@@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status);#ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); -- 2.17.1
Hi Dafna,
Thank you for the patch.
On Tue, Mar 30, 2021 at 01:52:00PM +0200, Dafna Hirschfeld wrote:
drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion.
Signed-off-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else {
drm_bridge_hpd_notify(&adv7511->bridge, status);
drm_bridge_hpd_cb(&adv7511->bridge, status);} diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp));}
@@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else {
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp));diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge;
- drm_bridge_hpd_notify(bridge, display_connector_detect(bridge));
drm_bridge_hpd_cb(bridge, display_connector_detect(bridge));
return IRQ_HANDLED;
} diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock);
drm_bridge_hpd_notify(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);
drm_bridge_hpd_cb(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);}
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev);
drm_bridge_hpd_notify(&hdmi->bridge, status);
drm_bridge_hpd_cb(&hdmi->bridge, status);diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge;
- drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge));
drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge));
return IRQ_HANDLED;
} diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
/**
- drm_bridge_hpd_notify - notify hot plug detection events
- drm_bridge_hpd_cb - notify hot plug detection events
This function is still documented as notifying hot plug detection events, so drm_bridge_hpd_cb() isn't a great name :-S I do agree there's confusion with the current naming scheme though.
bridge->hpd_cb() is an internal callback, not part of bridge ops, so I'd rather not expose its name in the public drm_bridge_hpd_notify() API. Could we find a better naming scheme ?
- @bridge: bridge control structure
- @status: output connection status
@@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
- This function shall be called in a context that can sleep.
*/ -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status)+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status){ mutex_lock(&bridge->hpd_mutex); if (bridge->hpd_cb) bridge->hpd_cb(bridge->hpd_data, status); mutex_unlock(&bridge->hpd_mutex); } -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb);
#ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call
* drm_bridge_hpd_notify() each time a change is detected in the output
* drm_bridge_hpd_cb() each time a change is detected in the output
- connection status, until hot plug detection gets disabled with
- @hpd_disable.
@@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge
* shall not call drm_bridge_hpd_notify() when a change in the output
* shall not call drm_bridge_hpd_cb() when a change in the output
- connection status occurs.
- This callback is optional and shall only be implemented by bridges
@@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status);#ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
Hi
On 06.04.21 14:40, Laurent Pinchart wrote:
Hi Dafna,
Thank you for the patch.
On Tue, Mar 30, 2021 at 01:52:00PM +0200, Dafna Hirschfeld wrote:
drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion.
Signed-off-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else {
drm_bridge_hpd_notify(&adv7511->bridge, status);
drm_bridge_hpd_cb(&adv7511->bridge, status);diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp));@@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else {
drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp));diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge;
- drm_bridge_hpd_notify(bridge, display_connector_detect(bridge));
drm_bridge_hpd_cb(bridge, display_connector_detect(bridge));
return IRQ_HANDLED; }
diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock);
drm_bridge_hpd_notify(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);
drm_bridge_hpd_cb(<9611uxc->bridge,connected ?connector_status_connected :connector_status_disconnected);diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev);
drm_bridge_hpd_notify(&hdmi->bridge, status);
drm_bridge_hpd_cb(&hdmi->bridge, status);diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge;
- drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge));
drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge));
return IRQ_HANDLED; }
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
/**
- drm_bridge_hpd_notify - notify hot plug detection events
- drm_bridge_hpd_cb - notify hot plug detection events
This function is still documented as notifying hot plug detection events, so drm_bridge_hpd_cb() isn't a great name :-S I do agree there's confusion with the current naming scheme though.
bridge->hpd_cb() is an internal callback, not part of bridge ops, so I'd rather not expose its name in the public drm_bridge_hpd_notify() API. Could we find a better naming scheme ?
I that bridge->funcs->hpd_notify is called only from drm_bridge_connector_hpd_notify which is called from both the hpd cb and from the 'detect' of the bridge connector. But 'detect' is not necesseraly by hpd right? I mean there is other ways apart of hpd to detect a connctor right? If that is the case then bridge->funcs->hpd_notify is also not a good name.
Also, I see that in some places the function drm_bridge_connector_init is called without later calling the function drm_bridge_connector_enable_hpd. Is there a use case in which we want to create a bridge connector without enabeling the hpd_cb ?
Regarding the naming, maybe we can replace 'drm_bridge_hpd_notify' with 'drm_bridge_hpd' ?
Thanks, Dafna
- @bridge: bridge control structure
- @status: output connection status
@@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
- This function shall be called in a context that can sleep.
*/ -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status)+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status)-EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb);
#ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call
* drm_bridge_hpd_notify() each time a change is detected in the output
* drm_bridge_hpd_cb() each time a change is detected in the output
- connection status, until hot plug detection gets disabled with
- @hpd_disable.
@@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge
* shall not call drm_bridge_hpd_notify() when a change in the output
* shall not call drm_bridge_hpd_cb() when a change in the output
- connection status occurs.
- This callback is optional and shall only be implemented by bridges
@@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);+void drm_bridge_hpd_cb(struct drm_bridge *bridge,
enum drm_connector_status status);#ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
dri-devel@lists.freedesktop.org
-
Dafna Hirschfeld -
Laurent Pinchart -
Robert Foss