| Message ID | 20241206-drm-bridge-hdmi-connector-v6-6-50dc145a9c06@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: add DRM HDMI Codec framework | expand |
On Fri, Dec 06, 2024 at 12:16:00PM +0200, Dmitry Baryshkov wrote: > The HDMI Connectors need to perform a variety of tasks when the HDMI > connector state changes. Such tasks include setting or invalidating CEC > address, notifying HDMI codec driver, updating scrambler data, etc. > > Implementing such tasks in a driver-specific callbacks is error prone. > Start implementing the generic helper function (currently handling only > the HDMI Codec framework) to be used by drivers utilizing HDMI Connector > framework. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 61 +++++++++++++++++++++++++ > include/drm/display/drm_hdmi_state_helper.h | 8 ++++ > 2 files changed, 69 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > index 80bf2829ba89b5f84fed4fa9eb1d6302e10a4f9e..4cdeb63688b9e48acd8e8ae87a45b6253f7dd12b 100644 > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > @@ -769,3 +769,64 @@ drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *con > return ret; > } > EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_clear_audio_infoframe); > + > +/** > + * drm_atomic_helper_connector_hdmi_hotplug_edid - Handle the hotplug event for the HDMI connector passing custom EDID > + * @connector: A pointer to the HDMI connector > + * @status: Connection status > + * @drm_edid: EDID to process > + * > + * This function should be called as a part of the .detect() / .detect_ctx() > + * and .force() callbacks, updating the HDMI-specific connector's data. Most > + * drivers should be able to use @drm_atomic_helper_connector_hdmi_hotplug() > + * instead. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int > +drm_atomic_helper_connector_hdmi_hotplug_edid(struct drm_connector *connector, > + enum drm_connector_status status, > + const struct drm_edid *drm_edid) > +{ > + if (status == connector_status_disconnected) { > + // TODO: also handle CEC and scramber, HDMI sink disconnected. > + drm_connector_hdmi_codec_plugged_notify(connector, false); > + } > + > + drm_edid_connector_update(connector, drm_edid); > + > + if (status == connector_status_connected) { > + // TODO: also handle CEC and scramber, HDMI sink is now connected. > + drm_connector_hdmi_codec_plugged_notify(connector, true); > + } > + > + return 0; > +} > +EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug_edid); I think we discussed it in a previous version's thread after you sent that one, but I'd rather have that helper call an edid retrieval function than passing it edids. Also, EDIDs are mandatory for HDMI, so I'd call the function drm_atomic_helper_connector_hdmi_hotplug. > +/** > + * drm_atomic_helper_connector_hdmi_hotplug - Handle the hotplug event for the HDMI connector > + * @connector: A pointer to the HDMI connector > + * @status: Connection status > + * > + * This function should be called as a part of the .detect() / .detect_ctx() > + * and .force() callbacks, updating the HDMI-specific connector's data. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int > +drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector, > + enum drm_connector_status status) > +{ > + const struct drm_edid *drm_edid; > + int ret; > + > + drm_edid = drm_edid_read(connector); > + ret = drm_atomic_helper_connector_hdmi_hotplug_edid(connector, status, drm_edid); > + drm_edid_free(drm_edid); Oh. Why do we need the two variants? Or is it to deal with drivers that don't set connector->ddc? Maxime
On Mon, Dec 16, 2024 at 06:20:04PM +0100, Maxime Ripard wrote: > On Fri, Dec 06, 2024 at 12:16:00PM +0200, Dmitry Baryshkov wrote: > > The HDMI Connectors need to perform a variety of tasks when the HDMI > > connector state changes. Such tasks include setting or invalidating CEC > > address, notifying HDMI codec driver, updating scrambler data, etc. > > > > Implementing such tasks in a driver-specific callbacks is error prone. > > Start implementing the generic helper function (currently handling only > > the HDMI Codec framework) to be used by drivers utilizing HDMI Connector > > framework. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 61 +++++++++++++++++++++++++ > > include/drm/display/drm_hdmi_state_helper.h | 8 ++++ > > 2 files changed, 69 insertions(+) > > > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > index 80bf2829ba89b5f84fed4fa9eb1d6302e10a4f9e..4cdeb63688b9e48acd8e8ae87a45b6253f7dd12b 100644 > > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > @@ -769,3 +769,64 @@ drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *con > > return ret; > > } > > EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_clear_audio_infoframe); > > + > > +/** > > + * drm_atomic_helper_connector_hdmi_hotplug_edid - Handle the hotplug event for the HDMI connector passing custom EDID > > + * @connector: A pointer to the HDMI connector > > + * @status: Connection status > > + * @drm_edid: EDID to process > > + * > > + * This function should be called as a part of the .detect() / .detect_ctx() > > + * and .force() callbacks, updating the HDMI-specific connector's data. Most > > + * drivers should be able to use @drm_atomic_helper_connector_hdmi_hotplug() > > + * instead. > > + * > > + * Returns: > > + * Zero on success, error code on failure. > > + */ > > +int > > +drm_atomic_helper_connector_hdmi_hotplug_edid(struct drm_connector *connector, > > + enum drm_connector_status status, > > + const struct drm_edid *drm_edid) > > +{ > > + if (status == connector_status_disconnected) { > > + // TODO: also handle CEC and scramber, HDMI sink disconnected. > > + drm_connector_hdmi_codec_plugged_notify(connector, false); > > + } > > + > > + drm_edid_connector_update(connector, drm_edid); > > + > > + if (status == connector_status_connected) { > > + // TODO: also handle CEC and scramber, HDMI sink is now connected. > > + drm_connector_hdmi_codec_plugged_notify(connector, true); > > + } > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug_edid); > > I think we discussed it in a previous version's thread after you sent > that one, but I'd rather have that helper call an edid retrieval > function than passing it edids. Ack, I forgot to add the hook. I'll do that for the next version. I'll add the .read_edid callback to the HDMI functions. > > Also, EDIDs are mandatory for HDMI, so I'd call the function > drm_atomic_helper_connector_hdmi_hotplug. See below. > > > +/** > > + * drm_atomic_helper_connector_hdmi_hotplug - Handle the hotplug event for the HDMI connector > > + * @connector: A pointer to the HDMI connector > > + * @status: Connection status > > + * > > + * This function should be called as a part of the .detect() / .detect_ctx() > > + * and .force() callbacks, updating the HDMI-specific connector's data. > > + * > > + * Returns: > > + * Zero on success, error code on failure. > > + */ > > +int > > +drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector, > > + enum drm_connector_status status) > > +{ > > + const struct drm_edid *drm_edid; > > + int ret; > > + > > + drm_edid = drm_edid_read(connector); > > + ret = drm_atomic_helper_connector_hdmi_hotplug_edid(connector, status, drm_edid); > > + drm_edid_free(drm_edid); > > Oh. Why do we need the two variants? Or is it to deal with drivers that > don't set connector->ddc? Yes. There are enough HDMI bridge drivers that would provide the callback instead of the DDC bus. Adding callback should solve that.
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c index 80bf2829ba89b5f84fed4fa9eb1d6302e10a4f9e..4cdeb63688b9e48acd8e8ae87a45b6253f7dd12b 100644 --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c @@ -769,3 +769,64 @@ drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *con return ret; } EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_clear_audio_infoframe); + +/** + * drm_atomic_helper_connector_hdmi_hotplug_edid - Handle the hotplug event for the HDMI connector passing custom EDID + * @connector: A pointer to the HDMI connector + * @status: Connection status + * @drm_edid: EDID to process + * + * This function should be called as a part of the .detect() / .detect_ctx() + * and .force() callbacks, updating the HDMI-specific connector's data. Most + * drivers should be able to use @drm_atomic_helper_connector_hdmi_hotplug() + * instead. + * + * Returns: + * Zero on success, error code on failure. + */ +int +drm_atomic_helper_connector_hdmi_hotplug_edid(struct drm_connector *connector, + enum drm_connector_status status, + const struct drm_edid *drm_edid) +{ + if (status == connector_status_disconnected) { + // TODO: also handle CEC and scramber, HDMI sink disconnected. + drm_connector_hdmi_codec_plugged_notify(connector, false); + } + + drm_edid_connector_update(connector, drm_edid); + + if (status == connector_status_connected) { + // TODO: also handle CEC and scramber, HDMI sink is now connected. + drm_connector_hdmi_codec_plugged_notify(connector, true); + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug_edid); + +/** + * drm_atomic_helper_connector_hdmi_hotplug - Handle the hotplug event for the HDMI connector + * @connector: A pointer to the HDMI connector + * @status: Connection status + * + * This function should be called as a part of the .detect() / .detect_ctx() + * and .force() callbacks, updating the HDMI-specific connector's data. + * + * Returns: + * Zero on success, error code on failure. + */ +int +drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector, + enum drm_connector_status status) +{ + const struct drm_edid *drm_edid; + int ret; + + drm_edid = drm_edid_read(connector); + ret = drm_atomic_helper_connector_hdmi_hotplug_edid(connector, status, drm_edid); + drm_edid_free(drm_edid); + + return ret; +} +EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug); diff --git a/include/drm/display/drm_hdmi_state_helper.h b/include/drm/display/drm_hdmi_state_helper.h index d6d65da6d8f9ee46de33114cce2d6fbe6098a862..4ffd40d73d50d89449508b7a5ce5836a596638a1 100644 --- a/include/drm/display/drm_hdmi_state_helper.h +++ b/include/drm/display/drm_hdmi_state_helper.h @@ -6,8 +6,11 @@ struct drm_atomic_state; struct drm_connector; struct drm_connector_state; +struct drm_edid; struct hdmi_audio_infoframe; +enum drm_connector_status; + void __drm_atomic_helper_connector_hdmi_reset(struct drm_connector *connector, struct drm_connector_state *new_conn_state); @@ -19,6 +22,11 @@ int drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector int drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *connector); int drm_atomic_helper_connector_hdmi_update_infoframes(struct drm_connector *connector, struct drm_atomic_state *state); +int drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector, + enum drm_connector_status status); +int drm_atomic_helper_connector_hdmi_hotplug_edid(struct drm_connector *connector, + enum drm_connector_status status, + const struct drm_edid *drm_edid); enum drm_mode_status drm_hdmi_connector_mode_valid(struct drm_connector *connector,
The HDMI Connectors need to perform a variety of tasks when the HDMI connector state changes. Such tasks include setting or invalidating CEC address, notifying HDMI codec driver, updating scrambler data, etc. Implementing such tasks in a driver-specific callbacks is error prone. Start implementing the generic helper function (currently handling only the HDMI Codec framework) to be used by drivers utilizing HDMI Connector framework. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/display/drm_hdmi_state_helper.c | 61 +++++++++++++++++++++++++ include/drm/display/drm_hdmi_state_helper.h | 8 ++++ 2 files changed, 69 insertions(+)