Re: [PATCH v2 07/12] drm/rockchip: dw_dp: Implement out-of-band HPD handling

From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Cc: "Sandy Huang" <hjc@rock-chips.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Andy Yan" <andy.yan@rock-chips.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Andrzej Hajda" <andrzej.hajda@intel.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Robert Foss" <rfoss@kernel.org>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Dmitry Baryshkov" <dmitry.baryshkov@oss.qualcomm.com>,
	"Luca Ceresoli" <luca.ceresoli@bootlin.com>,
	"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com>,
	"Damon Ding" <damon.ding@rock-chips.com>,
	"Dmitry Baryshkov" <lumag@kernel.org>,
	"Alexey Charkov" <alchark@gmail.com>,
	dri-devel@lists.freedesktop.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@collabora.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 07/12] drm/rockchip: dw_dp: Implement out-of-band HPD handling
Date: Fri, 12 Jun 2026 18:37:42 +0200	[thread overview]
Message-ID: <aiw0VKuj6Y6tpr9f@venus> (raw)
In-Reply-To: <4f4ff085-80ae-4144-b040-be9b136574ee@rock-chips.com>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

Hello Chaoyi,

On Tue, Jun 02, 2026 at 04:30:59PM +0800, Chaoyi Chen wrote:
> On 5/1/2026 6:20 AM, Sebastian Reichel wrote:
> > Implement out-of-band hotplug handling, which will be used to receive
> > external hotplug information from the USB-C state machine. This is
> > currently handled by the USBDP PHY, which brings quite some trouble
> > as the register being accessed requires the power-domain from the DP
> > controller and also requires custom TypeC HPD info parsing in the
> > USBDP PHY driver.
> > 
> > In contrast to the USBDP PHY this does not just enable the hotplug
> > signal when a DP AltMode capable adapter is plugged in, but instead
> > properly detects if a cable is plugged in for things like USB-C to
> > HDMI adapters.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> [...]
> 
> > +static void dw_dp_rockchip_hpd_sw_cfg(void *data, bool hpd)
> > +{
> > +	struct rockchip_dw_dp *dp = data;
> > +	u32 hpd_reg = dp->pdata->hpd_reg[dp->id];
> > +
> > +	dev_dbg(dp->dev, "Force HPD connected=%s\n", str_yes_no(hpd));
> > +
> > +	dp->hpd_cfg = hpd;
> > +
> > +	regmap_write(dp->vo_grf, hpd_reg,
> > +		     FIELD_PREP_WM16_CONST(ROCKCHIP_VO_GRF_DP_SINK_HPD_CFG, dp->hpd_cfg));
> 
> FIELD_PREP_WM16() should be used here because "dp->hpd_cfg" is not
> a constant expression.
> 
> Other patches in this series have similar issues. Doesn't your
> compiler warn you about this? Thanks.

The code builds warning free for me. Also FIELD_PREP_WM16_CONST
requies that the mask is constant, which is the first argument
(i.e. ROCKCHIP_VO_GRF_DP_SINK_HPD_CFG in this case). The second
argument should be fine as a variable as far as I can tell.

But since FIELD_PREP_WM16 is shorter anyways, gives better error
handling and can be used for all occuances in the dw-dp rockchip
glue I will use that instead.

Thanks,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]