We had _power_up(), but drivers also need to be able to power down.
Signed-off-by: Rob Clark robdclark@gmail.com --- This lets me drop edp_sink_power_state() from msm eDP code, and use the helpers instead.
drivers/gpu/drm/drm_dp_helper.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_dp_helper.h | 1 + 2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 959e207..82122ec 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -353,6 +353,37 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link) 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 diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 11f8c84..7e25030 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -586,6 +586,7 @@ struct drm_dp_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_aux_register(struct drm_dp_aux *aux);
On Tue, Dec 02, 2014 at 10:46:42AM -0500, Rob Clark wrote:
We had _power_up(), but drivers also need to be able to power down.
Signed-off-by: Rob Clark robdclark@gmail.com
This lets me drop edp_sink_power_state() from msm eDP code, and use the helpers instead.
drivers/gpu/drm/drm_dp_helper.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_dp_helper.h | 1 + 2 files changed, 32 insertions(+)
Reviewed-by: Thierry Reding treding@nvidia.com
On Tue, Dec 2, 2014 at 10:46 AM, Rob Clark robdclark@gmail.com wrote:
We had _power_up(), but drivers also need to be able to power down.
Signed-off-by: Rob Clark robdclark@gmail.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
This lets me drop edp_sink_power_state() from msm eDP code, and use the helpers instead.
drivers/gpu/drm/drm_dp_helper.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_dp_helper.h | 1 + 2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 959e207..82122ec 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -353,6 +353,37 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link) 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
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 11f8c84..7e25030 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -586,6 +586,7 @@ struct drm_dp_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_aux_register(struct drm_dp_aux *aux);
1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 02 Dec 2014, Rob Clark robdclark@gmail.com wrote:
We had _power_up(), but drivers also need to be able to power down.
Patch looks good, I'm just hijacking the thread to talk about the _power_up() counterpart. Sorry. ;)
First, I'm not sure it's all right or sensible to read DP_SET_POWER first when the sink is sleeping. Shouldn't it just write DP_SET_POWER_D0 to DP_SET_POWER?
Second, I think _power_up() should retry the wake up to ensure an AUX reply. (If we decide it's all right to read DP_SET_POWER first, I think it should have a retry too.)
DP v1.2 section 5.1.5 (section 5.2.5 is also relevant):
""" The Source device must write the value of 1h to DPCD Address 600h of the downstream device via AUX CH to switch the uPacket RX of the downstream device out of power-save mode. A uPacket RX of a downstream device in a power-save mode may not reply to this AUX request transaction. The uPacket RX of a downstream device in a power-save mode for an open, box-to-box connection is allowed to take up to 1ms till it is ready to reply to the AUX request transaction. Therefore, the Source device must retry until the 1ms wake-up timeout period of the Sink device expires. """
This also means that _probe() probably should not be used on sinks that are in sleep. Therefore _power_up() after _probe() seems redundant too (one example in tegra_output_sor_enable).
I seem to have reviewed the _power_up() and _probe() patch, so you can blame me just as well...
BR, Jani.
Signed-off-by: Rob Clark robdclark@gmail.com
This lets me drop edp_sink_power_state() from msm eDP code, and use the helpers instead.
drivers/gpu/drm/drm_dp_helper.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_dp_helper.h | 1 + 2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 959e207..82122ec 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -353,6 +353,37 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link) 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
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 11f8c84..7e25030 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -586,6 +586,7 @@ struct drm_dp_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_aux_register(struct drm_dp_aux *aux);
1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Jani Nikula -
Rob Clark -
Thierry Reding