On 16/04/2021 09:46, Tomi Valkeinen wrote:
> Hi Hans,
>
> On 02/03/2021 18:23, Hans Verkuil wrote:
>> Add bridge connector_attach/detach ops. These ops are called when a
>> bridge is attached or detached to a drm_connector. These ops can be
>> used to register and unregister an HDMI CEC adapter for a bridge that
>> supports CEC.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_bridge_connector.c | 9 +++++++++
>> include/drm/drm_bridge.h | 27 ++++++++++++++++++++++++++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
>> index 791379816837..07db71d4f5b3 100644
>> --- a/drivers/gpu/drm/drm_bridge_connector.c
>> +++ b/drivers/gpu/drm/drm_bridge_connector.c
>> @@ -203,6 +203,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
>> {
>> struct drm_bridge_connector *bridge_connector =
>> to_drm_bridge_connector(connector);
>> + struct drm_bridge *bridge;
>> +
>> + drm_for_each_bridge_in_chain(bridge_connector->encoder, bridge)
>> + if (bridge->funcs->connector_detach)
>> + bridge->funcs->connector_detach(bridge, connector);
>>
>> if (bridge_connector->bridge_hpd) {
>> struct drm_bridge *hpd = bridge_connector->bridge_hpd;
>> @@ -375,6 +380,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>> connector->polled = DRM_CONNECTOR_POLL_CONNECT
>> | DRM_CONNECTOR_POLL_DISCONNECT;
>>
>> + drm_for_each_bridge_in_chain(encoder, bridge)
>> + if (bridge->funcs->connector_attach)
>> + bridge->funcs->connector_attach(bridge, connector);
>> +
>> return connector;
>> }
>> EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 2195daa289d2..3320a6ebd253 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -629,6 +629,33 @@ struct drm_bridge_funcs {
>> * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
>> */
>> void (*hpd_disable)(struct drm_bridge *bridge);
>> +
>> + /**
>> + * @connector_attach:
>> + *
>> + * This callback is invoked whenever our bridge is being attached to a
>> + * &drm_connector. This is where an HDMI CEC adapter can be registered.
>> + * Note that this callback expects that this op always succeeds. Since
>> + * HDMI CEC support is an optional feature, any failure to register a
>> + * CEC adapter must be ignored since video output will still work
>> + * without CEC.
>> + *
>
> Even if CEC support is optional, the callback itself is generic.
> Wouldn't it be better to make this function return an error, and for
> CEC, just return 0 if CEC won't get registered correctly?
I'll do that.
>
> Also, I personally like things to fail if something doesn't go right,
> instead of continuing, if that thing is never supposed to happen in
> normal situations. E.g. if CEC registration fails because we're out of
> memory, I think the op should fail too.
If that happens you have no video output. And that's a lot more important
than CEC! As you suggested, I'll have the cec connector_attach just return
0.
Regards,
Hans
>
> Tomi
>
