Re: [PATCH v4 7/9] drm: tegra: use cec_notifier_conn_(un)register

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

Hi Dariusz,

One comment below:

On 7/16/19 2:27 PM, Dariusz Marcinkiewicz wrote:
> Use the new cec_notifier_conn_(un)register() functions to
> (un)register the notifier for the HDMI connector, and fill in
> the cec_connector_info.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> ---
>  drivers/gpu/drm/tegra/output.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index 9c2b9dad55c30..ea92e72280868 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -80,6 +80,9 @@ tegra_output_connector_detect(struct drm_connector *connector, bool force)
>  
>  void tegra_output_connector_destroy(struct drm_connector *connector)
>  {
> +	struct tegra_output *output = connector_to_output(connector);
> +
> +	cec_notifier_conn_unregister(output->cec);
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);
>  }
> @@ -174,18 +177,11 @@ int tegra_output_probe(struct tegra_output *output)
>  		disable_irq(output->hpd_irq);
>  	}
>  
> -	output->cec = cec_notifier_get(output->dev);
> -	if (!output->cec)
> -		return -ENOMEM;
> -
>  	return 0;
>  }
>  
>  void tegra_output_remove(struct tegra_output *output)
>  {
> -	if (output->cec)
> -		cec_notifier_put(output->cec);
> -
>  	if (gpio_is_valid(output->hpd_gpio)) {
>  		free_irq(output->hpd_irq, output);
>  		gpio_free(output->hpd_gpio);
> @@ -197,6 +193,7 @@ void tegra_output_remove(struct tegra_output *output)
>  
>  int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  {
> +	struct cec_connector_info conn_info;
>  	int err;
>  
>  	if (output->panel) {
> @@ -212,6 +209,13 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  	if (gpio_is_valid(output->hpd_gpio))
>  		enable_irq(output->hpd_irq);
>  
> +	cec_fill_conn_info_from_drm(&conn_info, &output->connector);
> +

This isn't right: tegra_output_init() is called for various output types, but the
CEC adapter should only be created for an HDMI connector. So there should be
a check on the connector type before registering a cec notifier.

Regards,

	Hans

> +	output->cec = cec_notifier_conn_register(output->dev, NULL, &conn_info);
> +	if (!output->cec)
> +		return -ENOMEM;
> +
> +
>  	return 0;
>  }
>  
>