Hi Mihail,
On Fri, Dec 13, 2019 at 01:15:12AM +0200, Laurent Pinchart wrote:
> On Wed, Dec 11, 2019 at 11:01:37AM +0000, Mihail Atanassov wrote:
> > On Tuesday, 10 December 2019 22:57:04 GMT Laurent Pinchart 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@xxxxxxxxxxxxxxxx>
> > > Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> > > ---
> > > Changes since v2:
> > >
> > > - Add wrappers around the .detect(), .get_modes() and .get_edid()
> > > operations
> > > - Warn bridge drivers about valid usage of the connector argument to
> > > .get_modes() and .get_edid()
> > >
> > > 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 | 162 +++++++++++++++++++++++++++++
> > > include/drm/drm_bridge.h | 193 ++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 354 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index c2cf0c90fa26..473353bd762f 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -70,6 +70,8 @@ static LIST_HEAD(bridge_list);
[snip]
> > > +/**
> > > + * 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)
> >
> > Is this check necessary given the DRM_BRIDGE_OP_HPD check? Would a
> >
> > if (WARN_ON(!bridge->funcs->hpd_enable))
> > return;
> >
> > suffice? Similarly for the check in _disable below.
>
> You're right. I'll do so and move it out of the hpd_mutex-protected
> section.
Actually the operations are not mandatory even if DRM_BRIDGE_OP_HPD is
set, they only have to be implemented if the bridge is capable of
enabling and disabling HPD detection dynamically. If it can't, it can
always generate HPD events and they will be blocked by
drm_bridge_hpd_notify().
> > > + 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);
[snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
