| Message ID | 20190820011721.30136-5-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand |
On Tue, 20 Aug 2019 04:16:35 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > To support implementation of DRM connectors on top of DRM bridges > instead of by bridges, the drm_bridge needs to expose new operations and > data: > > - Output detection, hot-plug notification, mode retrieval and EDID > retrieval operations > - Bitmask of supported operations > - Bridge output type > - I2C adapter for DDC access > > Add and document these. > > Three new bridge helper functions are also added to handle hot plug > notification in a way that is as transparent as possible for the > bridges. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > Changes since v1: > > - Make .hpd_enable() and .hpd_disable() optional > - Rename .lost_hotplug() to .hpd_notify() > - Add ddc field to drm_bridge > --- > drivers/gpu/drm/drm_bridge.c | 94 +++++++++++++++++++ > include/drm/drm_bridge.h | 177 ++++++++++++++++++++++++++++++++++- > 2 files changed, 270 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index cba537c99e43..cf1fbed88410 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -70,6 +70,8 @@ static LIST_HEAD(bridge_list); > */ > void drm_bridge_add(struct drm_bridge *bridge) > { > + mutex_init(&bridge->hpd_mutex); > + > mutex_lock(&bridge_lock); > list_add_tail(&bridge->list, &bridge_list); > mutex_unlock(&bridge_lock); > @@ -86,6 +88,8 @@ void drm_bridge_remove(struct drm_bridge *bridge) > mutex_lock(&bridge_lock); > list_del_init(&bridge->list); > mutex_unlock(&bridge_lock); > + > + mutex_destroy(&bridge->hpd_mutex); > } > EXPORT_SYMBOL(drm_bridge_remove); > > @@ -462,6 +466,96 @@ void drm_atomic_bridge_enable(struct drm_bridge *bridge, > } > EXPORT_SYMBOL(drm_atomic_bridge_enable); > > +/** > + * drm_bridge_hpd_enable - enable hot plug detection for the bridge > + * @bridge: bridge control structure > + * @cb: hot-plug detection callback > + * @data: data to be passed to the hot-plug detection callback > + * > + * Call &drm_bridge_funcs.hpd_enable if implemented and register the given @cb > + * and @data as hot plug notification callback. From now on the @cb will be > + * called with @data when an output status change is detected by the bridge, > + * until hot plug notification gets disabled with drm_bridge_hpd_disable(). > + * > + * Hot plug detection is supported only if the DRM_BRIDGE_OP_HPD flag is set in > + * bridge->ops. This function shall not be called when the flag is not set. > + * > + * Only one hot plug detection callback can be registered at a time, it is an > + * error to call this function when hot plug detection is already enabled for > + * the bridge. > + */ > +void drm_bridge_hpd_enable(struct drm_bridge *bridge, > + void (*cb)(void *data, > + enum drm_connector_status status), > + void *data) > +{ > + if (!bridge || !(bridge->ops & DRM_BRIDGE_OP_HPD)) > + return; > + > + mutex_lock(&bridge->hpd_mutex); > + > + if (WARN(bridge->hpd_cb, "Hot plug detection already enabled\n")) > + goto unlock; > + > + bridge->hpd_cb = cb; > + bridge->hpd_data = data; > + > + if (bridge->funcs->hpd_enable) > + bridge->funcs->hpd_enable(bridge); > + > +unlock: > + mutex_unlock(&bridge->hpd_mutex); > +} > +EXPORT_SYMBOL_GPL(drm_bridge_hpd_enable); > + > +/** > + * drm_bridge_hpd_disable - disable hot plug detection for the bridge > + * @bridge: bridge control structure > + * > + * Call &drm_bridge_funcs.hpd_disable if implemented and unregister the hot > + * plug detection callback previously registered with drm_bridge_hpd_enable(). > + * Once this function returns the callback will not be called by the bridge > + * when an output status change occurs. > + * > + * Hot plug detection is supported only if the DRM_BRIDGE_OP_HPD flag is set in > + * bridge->ops. This function shall not be called when the flag is not set. > + */ > +void drm_bridge_hpd_disable(struct drm_bridge *bridge) > +{ > + if (!bridge || !(bridge->ops & DRM_BRIDGE_OP_HPD)) > + return; > + > + mutex_lock(&bridge->hpd_mutex); > + if (bridge->funcs->hpd_disable) > + bridge->funcs->hpd_disable(bridge); > + > + bridge->hpd_cb = NULL; > + bridge->hpd_data = NULL; > + mutex_unlock(&bridge->hpd_mutex); > +} > +EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > + > +/** > + * drm_bridge_hpd_notify - notify hot plug detection events > + * @bridge: bridge control structure > + * @status: output connection status > + * > + * Bridge drivers shall call this function to report hot plug events when they > + * detect a change in the output status, when hot plug detection has been > + * enabled by drm_bridge_hpd_enable(). > + * > + * 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) > +{ > + 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); > + > #ifdef CONFIG_OF > /** > * of_drm_find_bridge - find the bridge corresponding to the device node in > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 7616f6562fe4..7db6ca0fd75d 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -23,14 +23,16 @@ > #ifndef __DRM_BRIDGE_H__ > #define __DRM_BRIDGE_H__ > > -#include <linux/list.h> > #include <linux/ctype.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > #include <drm/drm_mode_object.h> > #include <drm/drm_modes.h> > > struct drm_bridge; > struct drm_bridge_timings; > struct drm_panel; > +struct i2c_adapter; > > /** > * struct drm_bridge_funcs - drm_bridge control functions > @@ -334,6 +336,111 @@ struct drm_bridge_funcs { > */ > void (*atomic_post_disable)(struct drm_bridge *bridge, > struct drm_atomic_state *state); > + > + /** > + * @detect: > + * > + * Check if anything is attached to the bridge output. > + * > + * This callback is optional, if not implemented the bridge will be > + * considered as always having a component attached to its output. > + * Bridges that implement this callback shall set the > + * DRM_BRIDGE_OP_DETECT flag in their &drm_bridge->ops. > + * > + * RETURNS: > + * > + * drm_connector_status indicating the bridge output status. > + */ > + enum drm_connector_status (*detect)(struct drm_bridge *bridge); > + > + /** > + * @get_modes: > + * > + * Fill all modes currently valid for the sink into the &drm_connector > + * with drm_mode_probed_add(). > + * > + * The @get_modes callback is mostly intended to support non-probable > + * displays such as many fixed panels. Bridges that support reading > + * EDID shall leave @get_modes unimplemented and implement the > + * &drm_bridge_funcs->get_edid callback instead. > + * > + * This callback is optional. Bridges that implement it shall set the > + * DRM_BRIDGE_OP_MODES flag in their &drm_bridge->ops. > + * > + * RETURNS: > + * > + * The number of modes added by calling drm_mode_probed_add(). > + */ > + int (*get_modes)(struct drm_bridge *bridge, > + struct drm_connector *connector); > + > + /** > + * @get_edid: > + * > + * Read and parse the EDID data of the connected display. > + * > + * The @get_edid callback is the preferred way of reporting mode > + * information for a display connected to the bridge output. Bridges > + * that support readind EDID shall implement this callback and leave > + * the @get_modes callback unimplemented. > + * > + * The caller of this operation shall first verify the output > + * connection status and refrain from reading EDID from a disconnected > + * output. > + * > + * This callback is optional. Bridges that implement it shall set the > + * DRM_BRIDGE_OP_EDID flag in their &drm_bridge->ops. > + * > + * RETURNS: > + * > + * An edid structure newly allocated with kmalloc() (or similar) on > + * success, or NULL otherwise. The caller is responsible for freeing > + * the returned edid structure with kfree(). > + */ > + struct edid *(*get_edid)(struct drm_bridge *bridge, > + struct drm_connector *connector); > + > + /** > + * @hpd_notify: > + * > + * Notify the bridge of hot plug detection. > + * > + * This callback is optional, it may be implemented by bridges that > + * need to be notified of display connection or disconnection for > + * internal reasons. One use case is to reset the internal state of CEC > + * controllers for HDMI bridges. > + */ > + void (*hpd_notify)(struct drm_bridge *bridge, > + enum drm_connector_status status); > + > + /** > + * @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 > + * connection status, until hot plug detection gets disabled with > + * @hpd_disable. > + * > + * This callback is optional and shall only be implemented by bridges > + * that support hot-plug notification without polling. Bridges that > + * implement it shall also implement the @hpd_disable callback and set > + * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. > + */ > + void (*hpd_enable)(struct drm_bridge *bridge); > + > + /** > + * @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 > + * connection status occurs. > + * > + * This callback is optional and shall only be implemented by bridges > + * that support hot-plug notification without polling. Bridges that > + * implement it shall also implement the @hpd_enable callback and set > + * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. > + */ > + void (*hpd_disable)(struct drm_bridge *bridge); > }; > > /** > @@ -372,6 +479,39 @@ struct drm_bridge_timings { > bool dual_link; > }; > > +/** > + * enum drm_bridge_ops - Bitmask of operations supported by the bridge > + */ > +enum drm_bridge_ops { > + /** > + * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to > + * its output. Bridges that set this flag shall implement the > + * &drm_bridge_funcs->detect callback. > + */ > + DRM_BRIDGE_OP_DETECT = BIT(0), > + /** > + * @DRM_BRIDGE_OP_EDID: The bridge can retrieve the EDID of the display > + * connected to its output. Bridges that set this flag shall implement > + * the &drm_bridge_funcs->get_edid callback. > + */ > + DRM_BRIDGE_OP_EDID = BIT(1), > + /** > + * @DRM_BRIDGE_OP_HPD: The bridge can detect hot-plug and hot-unplug > + * without requiring polling. Bridges that set this flag shall > + * implement the &drm_bridge_funcs->hpd_enable and > + * &drm_bridge_funcs->hpd_disable callbacks if they support enabling > + * and disabling hot-plug detection dynamically. > + */ > + DRM_BRIDGE_OP_HPD = BIT(2), > + /** > + * @DRM_BRIDGE_OP_MODES: The bridge can retrieving the modes supported > + * by the display at its output. This does not include readind EDID > + * which is separately covered by @DRM_BRIDGE_OP_EDID. Bridges that set > + * this flag shall implement the &drm_bridge_funcs->get_modes callback. > + */ > + DRM_BRIDGE_OP_MODES = BIT(3), > +}; > + > /** > * struct drm_bridge - central DRM bridge control structure > */ > @@ -398,6 +538,33 @@ struct drm_bridge { > const struct drm_bridge_funcs *funcs; > /** @driver_private: pointer to the bridge driver's internal context */ > void *driver_private; > + /** @ops: bitmask of operations supported by the bridge */ > + enum drm_bridge_ops ops; > + /** > + * @type: Type of the connection at the bridge output > + * (DRM_MODE_CONNECTOR_*). For bridges at the end of this chain this > + * identifies the type of connected display. > + */ > + int type; > + /** > + * @ddc: Associated I2C adapter for DDC access, if any. > + */ > + struct i2c_adapter *ddc; > + /** private: */ > + /** > + * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields. > + */ > + struct mutex hpd_mutex; > + /** > + * @hpd_cb: Hot plug detection callback, registered with > + * drm_bridge_hpd_enable(). > + */ > + void (*hpd_cb)(void *data, enum drm_connector_status status); > + /** > + * @hpd_data: Private data passed to the Hot plug detection callback > + * @hpd_cb. > + */ > + void *hpd_data; > }; > > void drm_bridge_add(struct drm_bridge *bridge); > @@ -428,6 +595,14 @@ void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge, > void drm_atomic_bridge_enable(struct drm_bridge *bridge, > struct drm_atomic_state *state); > > +void drm_bridge_hpd_enable(struct drm_bridge *bridge, > + void (*cb)(void *data, > + 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); > + > #ifdef CONFIG_DRM_PANEL_BRIDGE > struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, > u32 connector_type);
On Tue, 20 Aug 2019 04:16:35 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > +/** > + * enum drm_bridge_ops - Bitmask of operations supported by the bridge > + */ > +enum drm_bridge_ops { > + /** > + * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to > + * its output. Bridges that set this flag shall implement the > + * &drm_bridge_funcs->detect callback. > + */ > + DRM_BRIDGE_OP_DETECT = BIT(0), > + /** > + * @DRM_BRIDGE_OP_EDID: The bridge can retrieve the EDID of the display > + * connected to its output. Bridges that set this flag shall implement > + * the &drm_bridge_funcs->get_edid callback. > + */ > + DRM_BRIDGE_OP_EDID = BIT(1), > + /** > + * @DRM_BRIDGE_OP_HPD: The bridge can detect hot-plug and hot-unplug > + * without requiring polling. Bridges that set this flag shall > + * implement the &drm_bridge_funcs->hpd_enable and > + * &drm_bridge_funcs->hpd_disable callbacks if they support enabling > + * and disabling hot-plug detection dynamically. > + */ > + DRM_BRIDGE_OP_HPD = BIT(2), > + /** > + * @DRM_BRIDGE_OP_MODES: The bridge can retrieving the modes supported > + * by the display at its output. This does not include readind EDID > + * which is separately covered by @DRM_BRIDGE_OP_EDID. Bridges that set > + * this flag shall implement the &drm_bridge_funcs->get_modes callback. > + */ > + DRM_BRIDGE_OP_MODES = BIT(3), > +}; > + > /** > * struct drm_bridge - central DRM bridge control structure > */ > @@ -398,6 +538,33 @@ struct drm_bridge { > const struct drm_bridge_funcs *funcs; > /** @driver_private: pointer to the bridge driver's internal context */ > void *driver_private; > + /** @ops: bitmask of operations supported by the bridge */ > + enum drm_bridge_ops ops; Just realized that you're not checking ->ops and ->funcs consistency at attach() time (you're only checking ->ops in the tfp410 driver before calling ->funcs->detect() and ->funcs->get_modes() and they might be NULL). > + /** > + * @type: Type of the connection at the bridge output > + * (DRM_MODE_CONNECTOR_*). For bridges at the end of this chain this > + * identifies the type of connected display. > + */ > + int type; > + /** > + * @ddc: Associated I2C adapter for DDC access, if any. > + */ > + struct i2c_adapter *ddc; > + /** private: */ > + /** > + * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields. > + */ > + struct mutex hpd_mutex; > + /** > + * @hpd_cb: Hot plug detection callback, registered with > + * drm_bridge_hpd_enable(). > + */ > + void (*hpd_cb)(void *data, enum drm_connector_status status); > + /** > + * @hpd_data: Private data passed to the Hot plug detection callback > + * @hpd_cb. > + */ > + void *hpd_data; > };
Hi Boris, On Thu, Aug 22, 2019 at 06:41:25PM +0200, Boris Brezillon wrote: > On Tue, 20 Aug 2019 04:16:35 +0300 Laurent Pinchart wrote: > > > +/** > > + * enum drm_bridge_ops - Bitmask of operations supported by the bridge > > + */ > > +enum drm_bridge_ops { > > + /** > > + * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to > > + * its output. Bridges that set this flag shall implement the > > + * &drm_bridge_funcs->detect callback. > > + */ > > + DRM_BRIDGE_OP_DETECT = BIT(0), > > + /** > > + * @DRM_BRIDGE_OP_EDID: The bridge can retrieve the EDID of the display > > + * connected to its output. Bridges that set this flag shall implement > > + * the &drm_bridge_funcs->get_edid callback. > > + */ > > + DRM_BRIDGE_OP_EDID = BIT(1), > > + /** > > + * @DRM_BRIDGE_OP_HPD: The bridge can detect hot-plug and hot-unplug > > + * without requiring polling. Bridges that set this flag shall > > + * implement the &drm_bridge_funcs->hpd_enable and > > + * &drm_bridge_funcs->hpd_disable callbacks if they support enabling > > + * and disabling hot-plug detection dynamically. > > + */ > > + DRM_BRIDGE_OP_HPD = BIT(2), > > + /** > > + * @DRM_BRIDGE_OP_MODES: The bridge can retrieving the modes supported > > + * by the display at its output. This does not include readind EDID > > + * which is separately covered by @DRM_BRIDGE_OP_EDID. Bridges that set > > + * this flag shall implement the &drm_bridge_funcs->get_modes callback. > > + */ > > + DRM_BRIDGE_OP_MODES = BIT(3), > > +}; > > + > > /** > > * struct drm_bridge - central DRM bridge control structure > > */ > > @@ -398,6 +538,33 @@ struct drm_bridge { > > const struct drm_bridge_funcs *funcs; > > /** @driver_private: pointer to the bridge driver's internal context */ > > void *driver_private; > > + /** @ops: bitmask of operations supported by the bridge */ > > + enum drm_bridge_ops ops; > > Just realized that you're not checking ->ops and ->funcs consistency at > attach() time (you're only checking ->ops in the tfp410 driver before > calling ->funcs->detect() and ->funcs->get_modes() and they might be > NULL). Do you think that's an issue ? I would then expect the driver to crash right away during development and the issue to be fixed. > > + /** > > + * @type: Type of the connection at the bridge output > > + * (DRM_MODE_CONNECTOR_*). For bridges at the end of this chain this > > + * identifies the type of connected display. > > + */ > > + int type; > > + /** > > + * @ddc: Associated I2C adapter for DDC access, if any. > > + */ > > + struct i2c_adapter *ddc; > > + /** private: */ > > + /** > > + * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields. > > + */ > > + struct mutex hpd_mutex; > > + /** > > + * @hpd_cb: Hot plug detection callback, registered with > > + * drm_bridge_hpd_enable(). > > + */ > > + void (*hpd_cb)(void *data, enum drm_connector_status status); > > + /** > > + * @hpd_data: Private data passed to the Hot plug detection callback > > + * @hpd_cb. > > + */ > > + void *hpd_data; > > };
On Fri, 23 Aug 2019 03:34:06 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Boris, > > On Thu, Aug 22, 2019 at 06:41:25PM +0200, Boris Brezillon wrote: > > On Tue, 20 Aug 2019 04:16:35 +0300 Laurent Pinchart wrote: > > > > > +/** > > > + * enum drm_bridge_ops - Bitmask of operations supported by the bridge > > > + */ > > > +enum drm_bridge_ops { > > > + /** > > > + * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to > > > + * its output. Bridges that set this flag shall implement the > > > + * &drm_bridge_funcs->detect callback. > > > + */ > > > + DRM_BRIDGE_OP_DETECT = BIT(0), > > > + /** > > > + * @DRM_BRIDGE_OP_EDID: The bridge can retrieve the EDID of the display > > > + * connected to its output. Bridges that set this flag shall implement > > > + * the &drm_bridge_funcs->get_edid callback. > > > + */ > > > + DRM_BRIDGE_OP_EDID = BIT(1), > > > + /** > > > + * @DRM_BRIDGE_OP_HPD: The bridge can detect hot-plug and hot-unplug > > > + * without requiring polling. Bridges that set this flag shall > > > + * implement the &drm_bridge_funcs->hpd_enable and > > > + * &drm_bridge_funcs->hpd_disable callbacks if they support enabling > > > + * and disabling hot-plug detection dynamically. > > > + */ > > > + DRM_BRIDGE_OP_HPD = BIT(2), > > > + /** > > > + * @DRM_BRIDGE_OP_MODES: The bridge can retrieving the modes supported > > > + * by the display at its output. This does not include readind EDID > > > + * which is separately covered by @DRM_BRIDGE_OP_EDID. Bridges that set > > > + * this flag shall implement the &drm_bridge_funcs->get_modes callback. > > > + */ > > > + DRM_BRIDGE_OP_MODES = BIT(3), > > > +}; > > > + > > > /** > > > * struct drm_bridge - central DRM bridge control structure > > > */ > > > @@ -398,6 +538,33 @@ struct drm_bridge { > > > const struct drm_bridge_funcs *funcs; > > > /** @driver_private: pointer to the bridge driver's internal context */ > > > void *driver_private; > > > + /** @ops: bitmask of operations supported by the bridge */ > > > + enum drm_bridge_ops ops; > > > > Just realized that you're not checking ->ops and ->funcs consistency at > > attach() time (you're only checking ->ops in the tfp410 driver before > > calling ->funcs->detect() and ->funcs->get_modes() and they might be > > NULL). > > Do you think that's an issue ? I would then expect the driver to crash > right away during development and the issue to be fixed. I tend to add those checks to avoid random NULL pointer exceptions, but if others are fine with this solution, I'm fine too. > > > > + /** > > > + * @type: Type of the connection at the bridge output > > > + * (DRM_MODE_CONNECTOR_*). For bridges at the end of this chain this > > > + * identifies the type of connected display. > > > + */ > > > + int type; > > > + /** > > > + * @ddc: Associated I2C adapter for DDC access, if any. > > > + */ > > > + struct i2c_adapter *ddc; > > > + /** private: */ > > > + /** > > > + * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields. > > > + */ > > > + struct mutex hpd_mutex; > > > + /** > > > + * @hpd_cb: Hot plug detection callback, registered with > > > + * drm_bridge_hpd_enable(). > > > + */ > > > + void (*hpd_cb)(void *data, enum drm_connector_status status); > > > + /** > > > + * @hpd_data: Private data passed to the Hot plug detection callback > > > + * @hpd_cb. > > > + */ > > > + void *hpd_data; > > > }; >
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index cba537c99e43..cf1fbed88410 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -70,6 +70,8 @@ static LIST_HEAD(bridge_list); */ void drm_bridge_add(struct drm_bridge *bridge) { + mutex_init(&bridge->hpd_mutex); + mutex_lock(&bridge_lock); list_add_tail(&bridge->list, &bridge_list); mutex_unlock(&bridge_lock); @@ -86,6 +88,8 @@ void drm_bridge_remove(struct drm_bridge *bridge) mutex_lock(&bridge_lock); list_del_init(&bridge->list); mutex_unlock(&bridge_lock); + + mutex_destroy(&bridge->hpd_mutex); } EXPORT_SYMBOL(drm_bridge_remove); @@ -462,6 +466,96 @@ void drm_atomic_bridge_enable(struct drm_bridge *bridge, } EXPORT_SYMBOL(drm_atomic_bridge_enable); +/** + * drm_bridge_hpd_enable - enable hot plug detection for the bridge + * @bridge: bridge control structure + * @cb: hot-plug detection callback + * @data: data to be passed to the hot-plug detection callback + * + * Call &drm_bridge_funcs.hpd_enable if implemented and register the given @cb + * and @data as hot plug notification callback. From now on the @cb will be + * called with @data when an output status change is detected by the bridge, + * until hot plug notification gets disabled with drm_bridge_hpd_disable(). + * + * Hot plug detection is supported only if the DRM_BRIDGE_OP_HPD flag is set in + * bridge->ops. This function shall not be called when the flag is not set. + * + * Only one hot plug detection callback can be registered at a time, it is an + * error to call this function when hot plug detection is already enabled for + * the bridge. + */ +void drm_bridge_hpd_enable(struct drm_bridge *bridge, + void (*cb)(void *data, + enum drm_connector_status status), + void *data) +{ + if (!bridge || !(bridge->ops & DRM_BRIDGE_OP_HPD)) + return; + + mutex_lock(&bridge->hpd_mutex); + + if (WARN(bridge->hpd_cb, "Hot plug detection already enabled\n")) + goto unlock; + + bridge->hpd_cb = cb; + bridge->hpd_data = data; + + if (bridge->funcs->hpd_enable) + bridge->funcs->hpd_enable(bridge); + +unlock: + mutex_unlock(&bridge->hpd_mutex); +} +EXPORT_SYMBOL_GPL(drm_bridge_hpd_enable); + +/** + * drm_bridge_hpd_disable - disable hot plug detection for the bridge + * @bridge: bridge control structure + * + * Call &drm_bridge_funcs.hpd_disable if implemented and unregister the hot + * plug detection callback previously registered with drm_bridge_hpd_enable(). + * Once this function returns the callback will not be called by the bridge + * when an output status change occurs. + * + * Hot plug detection is supported only if the DRM_BRIDGE_OP_HPD flag is set in + * bridge->ops. This function shall not be called when the flag is not set. + */ +void drm_bridge_hpd_disable(struct drm_bridge *bridge) +{ + if (!bridge || !(bridge->ops & DRM_BRIDGE_OP_HPD)) + return; + + mutex_lock(&bridge->hpd_mutex); + if (bridge->funcs->hpd_disable) + bridge->funcs->hpd_disable(bridge); + + bridge->hpd_cb = NULL; + bridge->hpd_data = NULL; + mutex_unlock(&bridge->hpd_mutex); +} +EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); + +/** + * drm_bridge_hpd_notify - notify hot plug detection events + * @bridge: bridge control structure + * @status: output connection status + * + * Bridge drivers shall call this function to report hot plug events when they + * detect a change in the output status, when hot plug detection has been + * enabled by drm_bridge_hpd_enable(). + * + * 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) +{ + 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); + #ifdef CONFIG_OF /** * of_drm_find_bridge - find the bridge corresponding to the device node in diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 7616f6562fe4..7db6ca0fd75d 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -23,14 +23,16 @@ #ifndef __DRM_BRIDGE_H__ #define __DRM_BRIDGE_H__ -#include <linux/list.h> #include <linux/ctype.h> +#include <linux/list.h> +#include <linux/mutex.h> #include <drm/drm_mode_object.h> #include <drm/drm_modes.h> struct drm_bridge; struct drm_bridge_timings; struct drm_panel; +struct i2c_adapter; /** * struct drm_bridge_funcs - drm_bridge control functions @@ -334,6 +336,111 @@ struct drm_bridge_funcs { */ void (*atomic_post_disable)(struct drm_bridge *bridge, struct drm_atomic_state *state); + + /** + * @detect: + * + * Check if anything is attached to the bridge output. + * + * This callback is optional, if not implemented the bridge will be + * considered as always having a component attached to its output. + * Bridges that implement this callback shall set the + * DRM_BRIDGE_OP_DETECT flag in their &drm_bridge->ops. + * + * RETURNS: + * + * drm_connector_status indicating the bridge output status. + */ + enum drm_connector_status (*detect)(struct drm_bridge *bridge); + + /** + * @get_modes: + * + * Fill all modes currently valid for the sink into the &drm_connector + * with drm_mode_probed_add(). + * + * The @get_modes callback is mostly intended to support non-probable + * displays such as many fixed panels. Bridges that support reading + * EDID shall leave @get_modes unimplemented and implement the + * &drm_bridge_funcs->get_edid callback instead. + * + * This callback is optional. Bridges that implement it shall set the + * DRM_BRIDGE_OP_MODES flag in their &drm_bridge->ops. + * + * RETURNS: + * + * The number of modes added by calling drm_mode_probed_add(). + */ + int (*get_modes)(struct drm_bridge *bridge, + struct drm_connector *connector); + + /** + * @get_edid: + * + * Read and parse the EDID data of the connected display. + * + * The @get_edid callback is the preferred way of reporting mode + * information for a display connected to the bridge output. Bridges + * that support readind EDID shall implement this callback and leave + * the @get_modes callback unimplemented. + * + * The caller of this operation shall first verify the output + * connection status and refrain from reading EDID from a disconnected + * output. + * + * This callback is optional. Bridges that implement it shall set the + * DRM_BRIDGE_OP_EDID flag in their &drm_bridge->ops. + * + * RETURNS: + * + * An edid structure newly allocated with kmalloc() (or similar) on + * success, or NULL otherwise. The caller is responsible for freeing + * the returned edid structure with kfree(). + */ + struct edid *(*get_edid)(struct drm_bridge *bridge, + struct drm_connector *connector); + + /** + * @hpd_notify: + * + * Notify the bridge of hot plug detection. + * + * This callback is optional, it may be implemented by bridges that + * need to be notified of display connection or disconnection for + * internal reasons. One use case is to reset the internal state of CEC + * controllers for HDMI bridges. + */ + void (*hpd_notify)(struct drm_bridge *bridge, + enum drm_connector_status status); + + /** + * @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 + * connection status, until hot plug detection gets disabled with + * @hpd_disable. + * + * This callback is optional and shall only be implemented by bridges + * that support hot-plug notification without polling. Bridges that + * implement it shall also implement the @hpd_disable callback and set + * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. + */ + void (*hpd_enable)(struct drm_bridge *bridge); + + /** + * @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 + * connection status occurs. + * + * This callback is optional and shall only be implemented by bridges + * that support hot-plug notification without polling. Bridges that + * implement it shall also implement the @hpd_enable callback and set + * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops. + */ + void (*hpd_disable)(struct drm_bridge *bridge); }; /** @@ -372,6 +479,39 @@ struct drm_bridge_timings { bool dual_link; }; +/** + * enum drm_bridge_ops - Bitmask of operations supported by the bridge + */ +enum drm_bridge_ops { + /** + * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to + * its output. Bridges that set this flag shall implement the + * &drm_bridge_funcs->detect callback. + */ + DRM_BRIDGE_OP_DETECT = BIT(0), + /** + * @DRM_BRIDGE_OP_EDID: The bridge can retrieve the EDID of the display + * connected to its output. Bridges that set this flag shall implement + * the &drm_bridge_funcs->get_edid callback. + */ + DRM_BRIDGE_OP_EDID = BIT(1), + /** + * @DRM_BRIDGE_OP_HPD: The bridge can detect hot-plug and hot-unplug + * without requiring polling. Bridges that set this flag shall + * implement the &drm_bridge_funcs->hpd_enable and + * &drm_bridge_funcs->hpd_disable callbacks if they support enabling + * and disabling hot-plug detection dynamically. + */ + DRM_BRIDGE_OP_HPD = BIT(2), + /** + * @DRM_BRIDGE_OP_MODES: The bridge can retrieving the modes supported + * by the display at its output. This does not include readind EDID + * which is separately covered by @DRM_BRIDGE_OP_EDID. Bridges that set + * this flag shall implement the &drm_bridge_funcs->get_modes callback. + */ + DRM_BRIDGE_OP_MODES = BIT(3), +}; + /** * struct drm_bridge - central DRM bridge control structure */ @@ -398,6 +538,33 @@ struct drm_bridge { const struct drm_bridge_funcs *funcs; /** @driver_private: pointer to the bridge driver's internal context */ void *driver_private; + /** @ops: bitmask of operations supported by the bridge */ + enum drm_bridge_ops ops; + /** + * @type: Type of the connection at the bridge output + * (DRM_MODE_CONNECTOR_*). For bridges at the end of this chain this + * identifies the type of connected display. + */ + int type; + /** + * @ddc: Associated I2C adapter for DDC access, if any. + */ + struct i2c_adapter *ddc; + /** private: */ + /** + * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields. + */ + struct mutex hpd_mutex; + /** + * @hpd_cb: Hot plug detection callback, registered with + * drm_bridge_hpd_enable(). + */ + void (*hpd_cb)(void *data, enum drm_connector_status status); + /** + * @hpd_data: Private data passed to the Hot plug detection callback + * @hpd_cb. + */ + void *hpd_data; }; void drm_bridge_add(struct drm_bridge *bridge); @@ -428,6 +595,14 @@ void drm_atomic_bridge_pre_enable(struct drm_bridge *bridge, void drm_atomic_bridge_enable(struct drm_bridge *bridge, struct drm_atomic_state *state); +void drm_bridge_hpd_enable(struct drm_bridge *bridge, + void (*cb)(void *data, + 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); + #ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, u32 connector_type);
To support implementation of DRM connectors on top of DRM bridges instead of by bridges, the drm_bridge needs to expose new operations and data: - Output detection, hot-plug notification, mode retrieval and EDID retrieval operations - Bitmask of supported operations - Bridge output type - I2C adapter for DDC access Add and document these. Three new bridge helper functions are also added to handle hot plug notification in a way that is as transparent as possible for the bridges. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes since v1: - Make .hpd_enable() and .hpd_disable() optional - Rename .lost_hotplug() to .hpd_notify() - Add ddc field to drm_bridge --- drivers/gpu/drm/drm_bridge.c | 94 +++++++++++++++++++ include/drm/drm_bridge.h | 177 ++++++++++++++++++++++++++++++++++- 2 files changed, 270 insertions(+), 1 deletion(-)