Re: [PATCH v5 09/10] drm/rockchip: dw_hdmi_qp: Register HPD IRQ after connector setup

Cristian Ciocaltea
2026-04-28T13:58:28Z

On 4/28/26 4:41 AM, Dmitry Baryshkov wrote:
> On Sun, Apr 26, 2026 at 03:20:21AM +0300, Cristian Ciocaltea wrote:
>> Move devm_request_threaded_irq() to the end of bind(), after
>> drm_bridge_connector_init() and drm_connector_attach_encoder(), to
>> ensure all DRM resources are ready before HPD interrupts can fire.
>>
>> While at it, add error handling for drm_connector_attach_encoder().
>>
>> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
>> Tested-by: Maud Spierings <maud_spierings@hotmail.com>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> index 618d2aaa5c7d..fbbe26f8730c 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> @@ -577,14 +577,6 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>> if (irq < 0)
>> return irq;
>>
>> - ret = devm_request_threaded_irq(dev, irq,
>> - cfg->ctrl_ops->hardirq_callback,
>> - cfg->ctrl_ops->irq_callback,
>> - IRQF_SHARED, "dw-hdmi-qp-hpd",
>> - hdmi);
>> - if (ret)
>> - return ret;
>> -
>> drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
>> ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
>> if (ret)
>> @@ -602,7 +594,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>> return dev_err_probe(dev, PTR_ERR(connector),
>> "Failed to init bridge connector\n");
>>
>> - return drm_connector_attach_encoder(connector, encoder);
>> + ret = drm_connector_attach_encoder(connector, encoder);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to attach connector\n");
>> +
>> + return devm_request_threaded_irq(dev, irq,
>> + cfg->ctrl_ops->hardirq_callback,
>> + cfg->ctrl_ops->irq_callback,
>> + IRQF_SHARED, "dw-hdmi-qp-hpd",
>> + hdmi);
>
> And if requesting of the IRQ fails, you have a lot to unwind. It might
> be better to request IRQ early enough with the IRQF_NO_AUTOEN flag and
> then enable_irq() here.

Indeed, that'd be a better approach.

Thanks,
Cristian