Re: [PATCH RESEND v4 4/4] drm/rockchip: dw_hdmi_qp: Do not send HPD events for all connectors

[Heiko Stuebner <heiko@sntech.de>]

Am Montag, 23. März 2026, 18:45:30 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> In order to optimize the HPD event handling and run the detect cycle on
> the affected connector only, make use of
> drm_connector_helper_hpd_irq_event() instead of
> drm_helper_hpd_irq_event().
> 
> Additionally, move devm_request_threaded_irq() after bridge connector
> initialization.

nit: Using "and", "additionally", etc in commit messages, is a strong
indicator, things should be separate commits.

Especially as the sentence above just explains the "what" but not the
why (interrupt firing, before the device is actually there probably).

If there are no other changes requested, they can stay together though,
the "why" is somewhat obvious here.


Both changes themself look fine, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> Tested-by: Maud Spierings <maud_spierings@hotmail.com>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 44 ++++++++++++--------------
>  1 file changed, 20 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 1a09bcc96c3e..65dfaee15178 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -92,6 +92,7 @@ struct rockchip_hdmi_qp {
>  	struct regmap *regmap;
>  	struct regmap *vo_regmap;
>  	struct rockchip_encoder encoder;
> +	struct drm_connector *connector;
>  	struct dw_hdmi_qp *hdmi;
>  	struct phy *phy;
>  	struct gpio_desc *frl_enable_gpio;
> @@ -251,14 +252,10 @@ static void dw_hdmi_qp_rk3588_hpd_work(struct work_struct *work)
>  	struct rockchip_hdmi_qp *hdmi = container_of(work,
>  						     struct rockchip_hdmi_qp,
>  						     hpd_work.work);
> -	struct drm_device *drm = hdmi->encoder.encoder.dev;
> -	bool changed;
> +	bool changed = drm_connector_helper_hpd_irq_event(hdmi->connector);
>  
> -	if (drm) {
> -		changed = drm_helper_hpd_irq_event(drm);
> -		if (changed)
> -			dev_dbg(hdmi->dev, "connector status changed\n");
> -	}
> +	if (changed)
> +		dev_dbg(hdmi->dev, "connector status changed\n");
>  }
>  
>  static irqreturn_t dw_hdmi_qp_rk3576_hardirq(int irq, void *dev_id)
> @@ -466,13 +463,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	struct dw_hdmi_qp_plat_data plat_data = {};
>  	const struct rockchip_hdmi_qp_cfg *cfg;
>  	struct drm_device *drm = data;
> -	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  	struct rockchip_hdmi_qp *hdmi;
>  	struct resource *res;
>  	struct clk_bulk_data *clks;
>  	struct clk *ref_clk;
> -	int ret, irq, i;
> +	int ret, hpd_irq, i;
>  
>  	if (!pdev->dev.of_node)
>  		return -ENODEV;
> @@ -573,17 +569,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	if (plat_data.cec_irq < 0)
>  		return plat_data.cec_irq;
>  
> -	irq = platform_get_irq_byname(pdev, "hpd");
> -	if (irq < 0)
> -		return irq;
> -
> -	ret = devm_request_threaded_irq(hdmi->dev, irq,
> -					cfg->ctrl_ops->hardirq_callback,
> -					cfg->ctrl_ops->irq_callback,
> -					IRQF_SHARED, "dw-hdmi-qp-hpd",
> -					hdmi);
> -	if (ret)
> -		return ret;
> +	hpd_irq = platform_get_irq_byname(pdev, "hpd");
> +	if (hpd_irq < 0)
> +		return hpd_irq;
>  
>  	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
>  	drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
> @@ -597,12 +585,20 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  				     "Failed to bind dw-hdmi-qp");
>  	}
>  
> -	connector = drm_bridge_connector_init(drm, encoder);
> -	if (IS_ERR(connector))
> -		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
> +	hdmi->connector = drm_bridge_connector_init(drm, encoder);
> +	if (IS_ERR(hdmi->connector))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->connector),
>  				     "Failed to init bridge connector\n");
>  
> -	return drm_connector_attach_encoder(connector, encoder);
> +	ret = drm_connector_attach_encoder(hdmi->connector, encoder);
> +	if (ret)
> +		return ret;
> +
> +	return devm_request_threaded_irq(hdmi->dev, hpd_irq,
> +					 cfg->ctrl_ops->hardirq_callback,
> +					 cfg->ctrl_ops->irq_callback,
> +					 IRQF_SHARED, "dw-hdmi-qp-hpd",
> +					 hdmi);
>  }
>  
>  static void dw_hdmi_qp_rockchip_unbind(struct device *dev,
> 
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip