On Fri, Jan 17, 2014 at 03:22:08PM +0200, Jani Nikula wrote:
On Tue, 14 Jan 2014, Thierry Reding thierry.reding@gmail.com wrote:
[...]
+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;Since DP_DPCD_REV == 0, you could use the #defines for the indexes (if you're going to send another version anyway). Ditto below for drm_dp_link_configure.
We write to DP_LINK_BW_SET in drm_dp_link_configure() so I don't think we can apply the same trick there. Also I'm not sure if it's really worth having that here. Given that it only works if we actually read DP_DPCD_REV, the number of locations where it can be done is fairly small and they will look asymmetric with respect to other functions using the drm_dp_dpcd_read/write() helpers.
So unless you feel strongly I'd prefer not to do this.
Other than that nitpick, the series looks good to me. If we face any issues migrating i915 on top of this, we can iron them out later on.
On the series,
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks!
Thierry