Re: [PATCH v6 06/10] drm/display/hdmi: implement hotplug functions

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3833 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]