Re: [PATCH v3 13/13] drm/tegra: Move drm_dp_link helpers to Tegra DRM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 21, 2019 at 04:34:37PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
> 
> During the discussion of patches that enhance the drm_dp_link helpers it
> was concluded that these helpers aren't very useful to begin with. After
> all other drivers have been converted not to use these helpers anymore,
> move these helpers into the last remaining user: Tegra DRM.
> 
> If at some point these helpers are deemed more widely useful, they can
> be moved out into the DRM DP helpers again.
> 
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>

I didn't check in detail whether you moved it all without changes :-)

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 128 ------------------------------
>  drivers/gpu/drm/tegra/Makefile  |   1 +
>  drivers/gpu/drm/tegra/dp.c      | 133 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tegra/dp.h      |  26 +++++++
>  drivers/gpu/drm/tegra/dpaux.c   |   1 +
>  drivers/gpu/drm/tegra/sor.c     |   1 +
>  include/drm/drm_dp_helper.h     |  16 ----
>  7 files changed, 162 insertions(+), 144 deletions(-)
>  create mode 100644 drivers/gpu/drm/tegra/dp.c
>  create mode 100644 drivers/gpu/drm/tegra/dp.h
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index f567141aff54..2c7870aef469 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -351,134 +351,6 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> -/**
> - * drm_dp_link_probe() - probe a DisplayPort link for capabilities
> - * @aux: DisplayPort AUX channel
> - * @link: pointer to structure in which to return link capabilities
> - *
> - * The structure filled in by this function can usually be passed directly
> - * into drm_dp_link_power_up() and drm_dp_link_configure() to power up and
> - * configure the link based on the link's capabilities.
> - *
> - * Returns 0 on success or a negative error code on failure.
> - */
> -int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> -{
> -	u8 values[3];
> -	int err;
> -
> -	memset(link, 0, sizeof(*link));
> -
> -	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
> -	if (err < 0)
> -		return err;
> -
> -	link->revision = values[0];
> -	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
> -	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
> -
> -	if (values[2] & DP_ENHANCED_FRAME_CAP)
> -		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_dp_link_probe);
> -
> -/**
> - * 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;
> -}
> -EXPORT_SYMBOL(drm_dp_link_power_up);
> -
> -/**
> - * 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;
> -}
> -EXPORT_SYMBOL(drm_dp_link_power_down);
> -
> -/**
> - * drm_dp_link_configure() - configure 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_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
> -{
> -	u8 values[2];
> -	int err;
> -
> -	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
> -	values[1] = link->num_lanes;
> -
> -	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> -		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> -
> -	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values));
> -	if (err < 0)
> -		return err;
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_dp_link_configure);
> -
>  /**
>   * drm_dp_downstream_max_clock() - extract branch device max
>   *                                 pixel rate for legacy VGA
> diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> index 33c463e8d49f..d6cf202414f0 100644
> --- a/drivers/gpu/drm/tegra/Makefile
> +++ b/drivers/gpu/drm/tegra/Makefile
> @@ -5,6 +5,7 @@ tegra-drm-y := \
>  	drm.o \
>  	gem.o \
>  	fb.o \
> +	dp.o \
>  	hub.o \
>  	plane.o \
>  	dc.o \
> diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
> new file mode 100644
> index 000000000000..50ba967ebcbd
> --- /dev/null
> +++ b/drivers/gpu/drm/tegra/dp.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright (C) 2013-2019 NVIDIA Corporation
> + * Copyright (C) 2015 Rob Clark
> + */
> +
> +#include <drm/drm_dp_helper.h>
> +
> +#include "dp.h"
> +
> +/**
> + * drm_dp_link_probe() - probe a DisplayPort link for capabilities
> + * @aux: DisplayPort AUX channel
> + * @link: pointer to structure in which to return link capabilities
> + *
> + * The structure filled in by this function can usually be passed directly
> + * into drm_dp_link_power_up() and drm_dp_link_configure() to power up and
> + * configure the link based on the link's capabilities.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> +{
> +	u8 values[3];
> +	int err;
> +
> +	memset(link, 0, sizeof(*link));
> +
> +	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
> +	if (err < 0)
> +		return err;
> +
> +	link->revision = values[0];
> +	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
> +	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
> +
> +	if (values[2] & DP_ENHANCED_FRAME_CAP)
> +		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> +
> +	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
> + * @link: pointer to a structure containing the link configuration
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
> +{
> +	u8 values[2];
> +	int err;
> +
> +	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
> +	values[1] = link->num_lanes;
> +
> +	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> +		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> +
> +	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values));
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/tegra/dp.h b/drivers/gpu/drm/tegra/dp.h
> new file mode 100644
> index 000000000000..ca99a21d9686
> --- /dev/null
> +++ b/drivers/gpu/drm/tegra/dp.h
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright (C) 2013-2019 NVIDIA Corporation.
> + * Copyright (C) 2015 Rob Clark
> + */
> +
> +#ifndef DRM_TEGRA_DP_H
> +#define DRM_TEGRA_DP_H 1
> +
> +struct drm_dp_aux;
> +
> +#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
> +
> +struct drm_dp_link {
> +	unsigned char revision;
> +	unsigned int rate;
> +	unsigned int num_lanes;
> +	unsigned long capabilities;
> +};
> +
> +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);
> +
> +#endif
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index a0f6f9b0d258..1144605c9737 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -22,6 +22,7 @@
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_panel.h>
>  
> +#include "dp.h"
>  #include "dpaux.h"
>  #include "drm.h"
>  #include "trace.h"
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 57693260245e..91d5c5041d2c 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -25,6 +25,7 @@
>  #include <drm/drm_scdc_helper.h>
>  
>  #include "dc.h"
> +#include "dp.h"
>  #include "drm.h"
>  #include "hda.h"
>  #include "sor.h"
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 04f6c0bb0274..51ecb5112ef8 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1455,22 +1455,6 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
>  int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  				 u8 status[DP_LINK_STATUS_SIZE]);
>  
> -/*
> - * DisplayPort link
> - */
> -#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
> -
> -struct drm_dp_link {
> -	unsigned char revision;
> -	unsigned int rate;
> -	unsigned int num_lanes;
> -	unsigned long capabilities;
> -};
> -
> -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_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  				const u8 port_cap[4]);
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux