On Wed, 26 Aug 2020, Lyude Paul lyude@redhat.com 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 sean@poorly.run Signed-off-by: Lyude Paul lyude@redhat.com
squash! drm/i915/dp: Extract drm_dp_read_downstream_info()
Whoops!
Signed-off-by: Lyude Paul lyude@redhat.com
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 jani.nikula@intel.com
- 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],