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

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

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