Re: [PATCH v8 3/3] drm/rockchip: Add basic RK3588 HDMI output support

[Cristian Ciocaltea]

Hi Jonas,

On 9/29/24 3:34 AM, Jonas Karlman wrote:
> Hi Cristian,
> 
> On 2024-09-29 00:36, Cristian Ciocaltea wrote:
>> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
>> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
>> Samsung IP block.
>>
>> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
>> without audio, CEC or any of the HDMI 2.1 specific features.
>>
>> Co-developed-by: Algea Cao <algea.cao@xxxxxxxxxxxxxx>
>> Signed-off-by: Algea Cao <algea.cao@xxxxxxxxxxxxxx>
>> Tested-by: Heiko Stuebner <heiko@xxxxxxxxx>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/rockchip/Kconfig               |   9 +
>>  drivers/gpu/drm/rockchip/Makefile              |   1 +
>>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 425 +++++++++++++++++++++++++
>>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c    |   2 +
>>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h    |   1 +
>>  5 files changed, 438 insertions(+)
>>
> 
> [snip]
> 
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> new file mode 100644
>> index 000000000000..6103d30d40fb
> 
> [snip]
> 
>> +static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id)
>> +{
>> +	struct rockchip_hdmi_qp *hdmi = dev_id;
>> +	u32 intr_stat, val;
>> +	int debounce_ms;
>> +
>> +	regmap_read(hdmi->regmap, RK3588_GRF_SOC_STATUS1, &intr_stat);
>> +	if (!intr_stat)
>> +		return IRQ_NONE;
>> +
>> +	val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_CLR,
>> +			    RK3588_HDMI0_HPD_INT_CLR);
>> +	regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val);
>> +
>> +	debounce_ms = intr_stat & RK3588_HDMI0_LEVEL_INT ? 150 : 20;
>> +	mod_delayed_work(system_wq, &hdmi->hpd_work,
>> +			 msecs_to_jiffies(debounce_ms));
> 
> If I am understanding this correctly this will wait for 150 ms after HPD
> goes high and 20 ms after HPD goes low to trigger the hpd_work.
> 
> Would it not make more sense to be the other way around? In order to
> reduce unnecessary HOTPLUG=1 uevents from being triggered during short
> EDID refresh pulses? At least that is what I am playing around with for
> dw-hdmi.

Sorry for my late reply, I'm on vacation but eventually managed to get
access to a display, so I took the opportunity to prepare v9 [1].

The debouncing setup was borrowed from downstream driver and I haven't
payed much attention to it, but now that you pointed this out I think we
could simplify it and just use 150 ms in both cases.

Thanks,
Cristian

[1]: https://lore.kernel.org/all/20241010-b4-rk3588-bridge-upstream-v9-0-68c47dde0410@xxxxxxxxxxxxx/