[PATCH v2 1/5] drm/dp: Pull drm_dp_link_power_up/down from Tegra to common drm_dp_helper

Dmitry Baryshkov dmitry.baryshkov at oss.qualcomm.com
Thu Mar 27 12:54:37 UTC 2025


On 27/03/2025 14:39, Andy Yan wrote:
> 
> Hello Dmitry,
>       Could you take this series? If so, merging it earlier can avoid future conflicts from other patches.
> Besides, I can update my DP driver based on drm-misc-next.

I'd really like an ack from Thiery or Mikko.

If tere is none (and no objections), I'll push it on Monday.

> 
> At 2025-03-18 14:34:35, "Andy Yan" <andyshrk at 163.com> wrote:
>> From: Andy Yan <andy.yan at rock-chips.com>
>>
>> The helper functions drm_dp_link_power_up/down were moved to Tegra
>> DRM in commit 9a42c7c647a9 ("drm/tegra: Move drm_dp_link helpers to Tegra DRM")".
>>
>> Now since more and more users are duplicating the same code in their
>> own drivers, it's time to make them as DRM DP common helpers again.
>>
>> Signed-off-by: Andy Yan <andy.yan at rock-chips.com>
>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
>> ---
>>
>> Changes in v2:
>> - Fix commit message as suggested by Dmitry
>>
>> drivers/gpu/drm/display/drm_dp_helper.c | 69 +++++++++++++++++++++++++
>> drivers/gpu/drm/tegra/dp.c              | 67 ------------------------
>> drivers/gpu/drm/tegra/dp.h              |  2 -
>> drivers/gpu/drm/tegra/sor.c             |  4 +-
>> include/drm/display/drm_dp_helper.h     |  2 +
>> 5 files changed, 73 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>> index dbce1c3f4969..e5dec67e5fca 100644
>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>> @@ -838,6 +838,75 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux,
>> }
>> EXPORT_SYMBOL(drm_dp_dpcd_read_phy_link_status);
>>
>> +/**
>> + * drm_dp_link_power_up() - power up a DisplayPort link
>> + * @aux: DisplayPort AUX channel
>> + * @revision: DPCD revision supported on the link
>> + *
>> + * Returns 0 on success or a negative error code on failure.
>> + */
>> +int drm_dp_link_power_up(struct drm_dp_aux *aux, unsigned char revision)
>> +{
>> +	u8 value;
>> +	int err;
>> +
>> +	/* DP_SET_POWER register is only available on DPCD v1.1 and later */
>> +	if (revision < DP_DPCD_REV_11)
>> +		return 0;
>> +
>> +	err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	value &= ~DP_SET_POWER_MASK;
>> +	value |= DP_SET_POWER_D0;
>> +
>> +	err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	/*
>> +	 * According to the DP 1.1 specification, a "Sink Device must exit the
>> +	 * power saving state within 1 ms" (Section 2.5.3.1, Table 5-52, "Sink
>> +	 * Control Field" (register 0x600).
>> +	 */
>> +	usleep_range(1000, 2000);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_link_power_up);
>> +
>> +/**
>> + * drm_dp_link_power_down() - power down a DisplayPort link
>> + * @aux: DisplayPort AUX channel
>> + * @revision: DPCD revision supported on the link
>> + *
>> + * Returns 0 on success or a negative error code on failure.
>> + */
>> +int drm_dp_link_power_down(struct drm_dp_aux *aux, unsigned char revision)
>> +{
>> +	u8 value;
>> +	int err;
>> +
>> +	/* DP_SET_POWER register is only available on DPCD v1.1 and later */
>> +	if (revision < DP_DPCD_REV_11)
>> +		return 0;
>> +
>> +	err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	value &= ~DP_SET_POWER_MASK;
>> +	value |= DP_SET_POWER_D3;
>> +
>> +	err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_link_power_down);
>> +
>> static int read_payload_update_status(struct drm_dp_aux *aux)
>> {
>> 	int ret;
>> diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
>> index 08fbd8f151a1..990e744b0923 100644
>> --- a/drivers/gpu/drm/tegra/dp.c
>> +++ b/drivers/gpu/drm/tegra/dp.c
>> @@ -255,73 +255,6 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>> 	return 0;
>> }
>>
>> -/**
>> - * drm_dp_link_power_up() - power up a DisplayPort link
>> - * @aux: DisplayPort AUX channel
>> - * @link: pointer to a structure containing the link configuration
>> - *
>> - * Returns 0 on success or a negative error code on failure.
>> - */
>> -int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link)
>> -{
>> -	u8 value;
>> -	int err;
>> -
>> -	/* DP_SET_POWER register is only available on DPCD v1.1 and later */
>> -	if (link->revision < 0x11)
>> -		return 0;
>> -
>> -	err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
>> -	if (err < 0)
>> -		return err;
>> -
>> -	value &= ~DP_SET_POWER_MASK;
>> -	value |= DP_SET_POWER_D0;
>> -
>> -	err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> -	if (err < 0)
>> -		return err;
>> -
>> -	/*
>> -	 * According to the DP 1.1 specification, a "Sink Device must exit the
>> -	 * power saving state within 1 ms" (Section 2.5.3.1, Table 5-52, "Sink
>> -	 * Control Field" (register 0x600).
>> -	 */
>> -	usleep_range(1000, 2000);
>> -
>> -	return 0;
>> -}
>> -
>> -/**
>> - * drm_dp_link_power_down() - power down a DisplayPort link
>> - * @aux: DisplayPort AUX channel
>> - * @link: pointer to a structure containing the link configuration
>> - *
>> - * Returns 0 on success or a negative error code on failure.
>> - */
>> -int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link)
>> -{
>> -	u8 value;
>> -	int err;
>> -
>> -	/* DP_SET_POWER register is only available on DPCD v1.1 and later */
>> -	if (link->revision < 0x11)
>> -		return 0;
>> -
>> -	err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value);
>> -	if (err < 0)
>> -		return err;
>> -
>> -	value &= ~DP_SET_POWER_MASK;
>> -	value |= DP_SET_POWER_D3;
>> -
>> -	err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value);
>> -	if (err < 0)
>> -		return err;
>> -
>> -	return 0;
>> -}
>> -
>> /**
>>   * drm_dp_link_configure() - configure a DisplayPort link
>>   * @aux: DisplayPort AUX channel
>> diff --git a/drivers/gpu/drm/tegra/dp.h b/drivers/gpu/drm/tegra/dp.h
>> index cb12ed0c54e7..695060cafac0 100644
>> --- a/drivers/gpu/drm/tegra/dp.h
>> +++ b/drivers/gpu/drm/tegra/dp.h
>> @@ -164,8 +164,6 @@ int drm_dp_link_remove_rate(struct drm_dp_link *link, unsigned long rate);
>> void drm_dp_link_update_rates(struct drm_dp_link *link);
>>
>> int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
>> -int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>> -int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>> int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
>> int drm_dp_link_choose(struct drm_dp_link *link,
>> 		       const struct drm_display_mode *mode,
>> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
>> index f98f70eda906..21f3dfdcc5c9 100644
>> --- a/drivers/gpu/drm/tegra/sor.c
>> +++ b/drivers/gpu/drm/tegra/sor.c
>> @@ -2666,7 +2666,7 @@ static void tegra_sor_dp_disable(struct drm_encoder *encoder)
>> 	 * the AUX transactions would just be timing out.
>> 	 */
>> 	if (output->connector.status != connector_status_disconnected) {
>> -		err = drm_dp_link_power_down(sor->aux, &sor->link);
>> +		err = drm_dp_link_power_down(sor->aux, sor->link.revision);
>> 		if (err < 0)
>> 			dev_err(sor->dev, "failed to power down link: %d\n",
>> 				err);
>> @@ -2882,7 +2882,7 @@ static void tegra_sor_dp_enable(struct drm_encoder *encoder)
>> 	else
>> 		dev_dbg(sor->dev, "link training succeeded\n");
>>
>> -	err = drm_dp_link_power_up(sor->aux, &sor->link);
>> +	err = drm_dp_link_power_up(sor->aux, sor->link.revision);
>> 	if (err < 0)
>> 		dev_err(sor->dev, "failed to power up DP link: %d\n", err);
>>
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index 5ae4241959f2..f9dabce484a7 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -566,6 +566,8 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>> int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux,
>> 				     enum drm_dp_phy dp_phy,
>> 				     u8 link_status[DP_LINK_STATUS_SIZE]);
>> +int drm_dp_link_power_up(struct drm_dp_aux *aux, unsigned char revision);
>> +int drm_dp_link_power_down(struct drm_dp_aux *aux, unsigned char revision);
>>
>> int drm_dp_dpcd_write_payload(struct drm_dp_aux *aux,
>> 			      int vcpid, u8 start_time_slot, u8 time_slot_count);
>> -- 
>> 2.43.0


-- 
With best wishes
Dmitry


More information about the dri-devel mailing list