We're going to be doing the same probing process in nouveau for
determining downstream DP port capabilities, so let's deduplicate the
work by moving i915's code for handling this into a shared helper:
drm_dp_read_downstream_info().
Note that when we do this, we also do make some functional changes while
we're at it:
* We always clear the downstream port info before trying to read it,
just to make things easier for the caller
* We skip reading downstream port info if the DPCD indicates that we
don't support downstream port info
* We only read as many bytes as needed for the reported number of
downstream ports, no sense in reading the whole thing every time
v2:
* Fixup logic for calculating the downstream port length to account for
the fact that downstream port caps can be either 1 byte or 4 bytes
long. We can actually skip fixing the max_clock/max_bpc helpers here
since they all check for DP_DETAILED_CAP_INFO_AVAILABLE anyway.
* Fix ret code check for drm_dp_dpcd_read
v5:
* Change name from drm_dp_downstream_read_info() to
drm_dp_read_downstream_info()
* Also, add "See Also" sections for the various downstream info
functions (drm_dp_read_downstream_info(), drm_dp_downstream_max_clock(),
drm_dp_downstream_max_bpc())
Reviewed-by: Sean Paul <[email protected]>
Signed-off-by: Lyude Paul <[email protected]>
squash! drm/i915/dp: Extract drm_dp_read_downstream_info()
Signed-off-by: Lyude Paul <[email protected]>
---
drivers/gpu/drm/drm_dp_helper.c | 62 ++++++++++++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 14 +-----
include/drm/drm_dp_helper.h | 3 ++
3 files changed, 65 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 4c21cf69dad5a..f3643894ad951 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -423,6 +423,56 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
}
EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
+static u8 drm_dp_downstream_port_count(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ u8 port_count = dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_PORT_COUNT_MASK;
+
+ if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE && port_count > 4)
+ port_count = 4;
+
+ return port_count;
+}
+
+/**
+ * drm_dp_read_downstream_info() - read DPCD downstream port info if available
+ * @aux: DisplayPort AUX channel
+ * @dpcd: A cached copy of the port's DPCD
+ * @downstream_ports: buffer to store the downstream port info in
+ *
+ * See also:
+ * drm_dp_downstream_max_clock()
+ * drm_dp_downstream_max_bpc()
+ *
+ * Returns: 0 if either the downstream port info was read successfully or
+ * there was no downstream info to read, or a negative error code otherwise.
+ */
+int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS])
+{
+ int ret;
+ u8 len;
+
+ memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
+
+ /* No downstream info to read */
+ if (!drm_dp_is_branch(dpcd) ||
+ dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
+ !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+ return 0;
+
+ len = drm_dp_downstream_port_count(dpcd);
+ if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)
+ len *= 4;
+
+ ret = drm_dp_dpcd_read(aux, DP_DOWNSTREAM_PORT_0, downstream_ports, len);
+ if (ret < 0)
+ return ret;
+
+ return ret == len ? 0 : -EIO;
+}
+EXPORT_SYMBOL(drm_dp_read_downstream_info);
+
/**
* drm_dp_downstream_max_clock() - extract branch device max
* pixel rate for legacy VGA
@@ -431,7 +481,11 @@ EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
* @dpcd: DisplayPort configuration data
* @port_cap: port capabilities
*
- * Returns max clock in kHz on success or 0 if max clock not defined
+ * See also:
+ * drm_dp_read_downstream_info()
+ * drm_dp_downstream_max_bpc()
+ *
+ * Returns: Max clock in kHz on success or 0 if max clock not defined
*/
int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4])
@@ -462,7 +516,11 @@ EXPORT_SYMBOL(drm_dp_downstream_max_clock);
* @dpcd: DisplayPort configuration data
* @port_cap: port capabilities
*
- * Returns max bpc on success or 0 if max bpc not defined
+ * See also:
+ * drm_dp_read_downstream_info()
+ * drm_dp_downstream_max_clock()
+ *
+ * Returns: Max bpc on success or 0 if max bpc not defined
*/
int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
const u8 port_cap[4])
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4c7314b7a84e4..9c4b806af8c79 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4685,18 +4685,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
return false;
}
- if (!drm_dp_is_branch(intel_dp->dpcd))
- return true; /* native DP sink */
-
- if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
- return true; /* no per-port downstream info */
-
- if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
- intel_dp->downstream_ports,
- DP_MAX_DOWNSTREAM_PORTS) < 0)
- return false; /* downstream port status fetch failed */
-
- return true;
+ return drm_dp_read_downstream_info(&intel_dp->aux, intel_dp->dpcd,
+ intel_dp->downstream_ports) == 0;
}
static bool
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5c28199248626..b8716b200666f 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1613,6 +1613,9 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
u8 real_edid_checksum);
+int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]);
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.26.2
On Wed, 26 Aug 2020, Lyude Paul <[email protected]> wrote:
> We're going to be doing the same probing process in nouveau for
> determining downstream DP port capabilities, so let's deduplicate the
> work by moving i915's code for handling this into a shared helper:
> drm_dp_read_downstream_info().
>
> Note that when we do this, we also do make some functional changes while
> we're at it:
> * We always clear the downstream port info before trying to read it,
> just to make things easier for the caller
> * We skip reading downstream port info if the DPCD indicates that we
> don't support downstream port info
> * We only read as many bytes as needed for the reported number of
> downstream ports, no sense in reading the whole thing every time
>
> v2:
> * Fixup logic for calculating the downstream port length to account for
> the fact that downstream port caps can be either 1 byte or 4 bytes
> long. We can actually skip fixing the max_clock/max_bpc helpers here
> since they all check for DP_DETAILED_CAP_INFO_AVAILABLE anyway.
> * Fix ret code check for drm_dp_dpcd_read
> v5:
> * Change name from drm_dp_downstream_read_info() to
> drm_dp_read_downstream_info()
> * Also, add "See Also" sections for the various downstream info
> functions (drm_dp_read_downstream_info(), drm_dp_downstream_max_clock(),
> drm_dp_downstream_max_bpc())
>
> Reviewed-by: Sean Paul <[email protected]>
> Signed-off-by: Lyude Paul <[email protected]>
>
> squash! drm/i915/dp: Extract drm_dp_read_downstream_info()
Whoops!
>
> Signed-off-by: Lyude Paul <[email protected]>
> ---
> drivers/gpu/drm/drm_dp_helper.c | 62 ++++++++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +-----
> include/drm/drm_dp_helper.h | 3 ++
> 3 files changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 4c21cf69dad5a..f3643894ad951 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -423,6 +423,56 @@ bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
> }
> EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
>
> +static u8 drm_dp_downstream_port_count(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> +{
> + u8 port_count = dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_PORT_COUNT_MASK;
> +
> + if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE && port_count > 4)
> + port_count = 4;
> +
> + return port_count;
> +}
> +
> +/**
> + * drm_dp_read_downstream_info() - read DPCD downstream port info if available
> + * @aux: DisplayPort AUX channel
> + * @dpcd: A cached copy of the port's DPCD
> + * @downstream_ports: buffer to store the downstream port info in
> + *
> + * See also:
> + * drm_dp_downstream_max_clock()
> + * drm_dp_downstream_max_bpc()
> + *
> + * Returns: 0 if either the downstream port info was read successfully or
> + * there was no downstream info to read, or a negative error code otherwise.
> + */
> +int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
> + const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS])
> +{
> + int ret;
> + u8 len;
> +
> + memset(downstream_ports, 0, DP_MAX_DOWNSTREAM_PORTS);
> +
> + /* No downstream info to read */
> + if (!drm_dp_is_branch(dpcd) ||
> + dpcd[DP_DPCD_REV] < DP_DPCD_REV_10 ||
> + !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> + return 0;
Generally I think stuff like this is easier and faster to read with
multiple if statements and early returns, but *shrug*.
I really hope we didn't have a reason to not check
DP_DWN_STRM_PORT_PRESENT here... there's been a lot of junk branch
devices in the past. Fingers crossed.
Reviewed-by: Jani Nikula <[email protected]>
> +
> + len = drm_dp_downstream_port_count(dpcd);
> + if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)
> + len *= 4;
> +
> + ret = drm_dp_dpcd_read(aux, DP_DOWNSTREAM_PORT_0, downstream_ports, len);
> + if (ret < 0)
> + return ret;
> +
> + return ret == len ? 0 : -EIO;
> +}
> +EXPORT_SYMBOL(drm_dp_read_downstream_info);
> +
> /**
> * drm_dp_downstream_max_clock() - extract branch device max
> * pixel rate for legacy VGA
> @@ -431,7 +481,11 @@ EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
> * @dpcd: DisplayPort configuration data
> * @port_cap: port capabilities
> *
> - * Returns max clock in kHz on success or 0 if max clock not defined
> + * See also:
> + * drm_dp_read_downstream_info()
> + * drm_dp_downstream_max_bpc()
> + *
> + * Returns: Max clock in kHz on success or 0 if max clock not defined
> */
> int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> const u8 port_cap[4])
> @@ -462,7 +516,11 @@ EXPORT_SYMBOL(drm_dp_downstream_max_clock);
> * @dpcd: DisplayPort configuration data
> * @port_cap: port capabilities
> *
> - * Returns max bpc on success or 0 if max bpc not defined
> + * See also:
> + * drm_dp_read_downstream_info()
> + * drm_dp_downstream_max_clock()
> + *
> + * Returns: Max bpc on success or 0 if max bpc not defined
> */
> int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> const u8 port_cap[4])
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4c7314b7a84e4..9c4b806af8c79 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4685,18 +4685,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> return false;
> }
>
> - if (!drm_dp_is_branch(intel_dp->dpcd))
> - return true; /* native DP sink */
> -
> - if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
> - return true; /* no per-port downstream info */
> -
> - if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
> - intel_dp->downstream_ports,
> - DP_MAX_DOWNSTREAM_PORTS) < 0)
> - return false; /* downstream port status fetch failed */
> -
> - return true;
> + return drm_dp_read_downstream_info(&intel_dp->aux, intel_dp->dpcd,
> + intel_dp->downstream_ports) == 0;
> }
>
> static bool
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5c28199248626..b8716b200666f 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1613,6 +1613,9 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
> bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
> u8 real_edid_checksum);
>
> +int drm_dp_read_downstream_info(struct drm_dp_aux *aux,
> + const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]);
> 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],
--
Jani Nikula, Intel Open Source Graphics Center