Re: [PATCH v13 5/9] drm/synopsys+ingenic: repair hot plug detection

[Paul Cercueil <paul@crapouillou.net>]

Hi Nikolaus,

Le mer., févr. 2 2022 at 17:31:19 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> so that it calls drm_kms_helper_hotplug_event().
> 
> We need to set .poll_enabled but that struct component
> can only be accessed in the core code. Hence we add a public
> setter function.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
>  drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++
>  include/drm/bridge/dw_hdmi.h              | 1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 54d8fdad395f5..52e7cd2e020d3 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -3216,6 +3216,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi 
> *hdmi)
>  	return 0;
>  }
> 
> +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
> +{
> +	if (hdmi->bridge.dev)
> +		hdmi->bridge.dev->mode_config.poll_enabled = enable;
> +	else
> +		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
> +
>  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  			      const struct dw_hdmi_plat_data *plat_data)
>  {
> diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c 
> b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> index 34e986dd606cf..90547a28dc5c7 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> @@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, 
> void *data,
>  	if (mode->clock > 216000)
>  		return MODE_CLOCK_HIGH;
> 
> +	dw_hdmi_enable_poll(hdmi, true);
> +

It would be a better idea to move this patch before the patch that 
creates ingenic-dw-hdmi.c. Then you wouldn't have to patch a file that 
was just introduced.

As for the patch itself, I guess it's fine, but is that really needed? 
My understanding is that it's the hdmi-connector's job to poll.

Cheers,
-Paul

>  	return MODE_OK;
>  }
> 
> diff --git a/include/drm/bridge/dw_hdmi.h 
> b/include/drm/bridge/dw_hdmi.h
> index 2a1f85f9a8a3f..963960794b40e 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -196,5 +196,6 @@ enum drm_connector_status 
> dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
>  void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
>  			    bool force, bool disabled, bool rxsense);
>  void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
> 
>  #endif /* __IMX_HDMI_H__ */
> --
> 2.33.0
>