| Message ID | 20180719203549.28266-2-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 19, 2018 at 01:35:49PM -0700, matthew.s.atwood@intel.com wrote: > From: Matt Atwood <matthew.s.atwood@intel.com> > > According to DP spec (2.9.3.1 of DP 1.4) if > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD > 02200h through 0220Fh shall contain the DPRX's true capability. These > values will match 00000h through 0000Fh, except for DPCD_REV, > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT. > > Read from DPCD once for all 3 values as this is an expensive operation. > Spec mentions that all of address space 02200h through 0220Fh should > contain the right information however currently only 3 values can > differ. > > There is no address space in the intel_dp->dpcd struct for addresses > 02200h through 0220Fh, and since so much of the data is a identical, > simply overwrite the values stored in 00000h through 0000Fh with the > values that can be overwritten from addresses 02200h through 0220Fh. > > This patch helps with backward compatibility for devices pre DP1.3. > > v2: read only dpcd values which can be affected, remove incorrect check, > split into drm include changes into separate patch, commit message, > verbose debugging statements during overwrite. > > v3: white space fixes > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 37 +++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index dde92e4af5d3..a31fbbbd7954 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp) > sizeof(intel_dp->dpcd)) < 0) > return false; /* aux transfer failed */ > We never know what vendors can do with reserved bits. We should never assume they are zero. So we shouldn't do any of below unless it is newer than DP 1.3. > + if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) { > + uint8_t dpcd_ext[6]; > + > + DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n"); > + > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV, > + &dpcd_ext, sizeof(dpcd_ext)) < 0) > + return false; /* aux transfer failed */ > + > + if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV], > + sizeof(u8))) { > + DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n", > + intel_dp->dpcd[DP_DPCD_REV], > + dpcd_ext[DP_DPCD_REV]); > + memcpy(&intel_dp->dpcd[DP_DPCD_REV], > + &dpcd_ext[DP_DPCD_REV], > + sizeof(u8)); > + } > + if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE], > + &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) { > + DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n", > + intel_dp->dpcd[DP_MAX_LINK_RATE], > + dpcd_ext[DP_MAX_LINK_RATE]); > + memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE], > + &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8)); > + } > + if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) { > + DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present previous value %2x new value %2x\n", > + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], > + dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]); > + memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], > + sizeof(u8)); > + } > + } > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > > return intel_dp->dpcd[DP_DPCD_REV] != 0; > -- > 2.17.1 >
On Thu, 2018-07-19 at 14:07 -0700, Rodrigo Vivi wrote: > On Thu, Jul 19, 2018 at 01:35:49PM -0700, matthew.s.atwood@intel.com > wrote: > > From: Matt Atwood <matthew.s.atwood@intel.com> > > > > According to DP spec (2.9.3.1 of DP 1.4) if > > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in > > DPCD > > 02200h through 0220Fh shall contain the DPRX's true capability. > > These > > values will match 00000h through 0000Fh, except for DPCD_REV, > > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT. > > > > Read from DPCD once for all 3 values as this is an expensive > > operation. > > Spec mentions that all of address space 02200h through 0220Fh > > should > > contain the right information however currently only 3 values can > > differ. > > > > There is no address space in the intel_dp->dpcd struct for > > addresses > > 02200h through 0220Fh, and since so much of the data is a > > identical, > > simply overwrite the values stored in 00000h through 0000Fh with > > the > > values that can be overwritten from addresses 02200h through > > 0220Fh. > > > > This patch helps with backward compatibility for devices pre DP1.3. > > > > v2: read only dpcd values which can be affected, remove incorrect > > check, > > split into drm include changes into separate patch, commit message, > > verbose debugging statements during overwrite. > > > > v3: white space fixes > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 37 > > +++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index dde92e4af5d3..a31fbbbd7954 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp > > *intel_dp) > > sizeof(intel_dp->dpcd)) < 0) > > return false; /* aux transfer failed */ > > > > We never know what vendors can do with reserved bits. We should never > assume > they are zero. So we shouldn't do any of below unless it is newer > than DP 1.3. I think you mean newer than DP1.2? > > > + if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > > + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) { > > + uint8_t dpcd_ext[6]; > > + > > + DRM_DEBUG_KMS("DPCD: Extended Receiver Capability > > Field Present, accessing 02200h through 022FFh\n"); > > + > > + if (drm_dp_dpcd_read(&intel_dp->aux, > > DP_DP13_DPCD_REV, > > + &dpcd_ext, sizeof(dpcd_ext)) > > < 0) > > + return false; /* aux transfer failed */ > > + > > + if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], > > &dpcd_ext[DP_DPCD_REV], > > + sizeof(u8))) { > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > Revision previous value %2x new value %2x\n", > > + intel_dp->dpcd[DP_DPCD_REV], > > + dpcd_ext[DP_DPCD_REV]); > > + memcpy(&intel_dp->dpcd[DP_DPCD_REV], > > + &dpcd_ext[DP_DPCD_REV], > > + sizeof(u8)); > > + } > > + if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE], > > + &dpcd_ext[DP_MAX_LINK_RATE], > > sizeof(u8))) { > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > Max Link Rate previous value %2x new value %2x\n", > > + intel_dp- > > >dpcd[DP_MAX_LINK_RATE], > > + dpcd_ext[DP_MAX_LINK_RATE]); > > + memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE], > > + &dpcd_ext[DP_MAX_LINK_RATE], > > sizeof(u8)); > > + } > > + if (memcmp(&intel_dp- > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], > > sizeof(u8))) { > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > Downstream Port Present previous value %2x new value %2x\n", > > + intel_dp- > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + dpcd_ext[DP_DOWNSTREAMPORT_P > > RESENT]); > > + memcpy(&intel_dp- > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT > > ], > > + sizeof(u8)); > > + } > > + } > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp- > > >dpcd), intel_dp->dpcd); > > > > return intel_dp->dpcd[DP_DPCD_REV] != 0; > > -- > > 2.17.1 > >
On Thu, Jul 19, 2018 at 02:47:59PM -0700, Atwood, Matthew S wrote: > On Thu, 2018-07-19 at 14:07 -0700, Rodrigo Vivi wrote: > > On Thu, Jul 19, 2018 at 01:35:49PM -0700, matthew.s.atwood@intel.com > > wrote: > > > From: Matt Atwood <matthew.s.atwood@intel.com> > > > > > > According to DP spec (2.9.3.1 of DP 1.4) if > > > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in > > > DPCD > > > 02200h through 0220Fh shall contain the DPRX's true capability. > > > These > > > values will match 00000h through 0000Fh, except for DPCD_REV, > > > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT. > > > > > > Read from DPCD once for all 3 values as this is an expensive > > > operation. > > > Spec mentions that all of address space 02200h through 0220Fh > > > should > > > contain the right information however currently only 3 values can > > > differ. > > > > > > There is no address space in the intel_dp->dpcd struct for > > > addresses > > > 02200h through 0220Fh, and since so much of the data is a > > > identical, > > > simply overwrite the values stored in 00000h through 0000Fh with > > > the > > > values that can be overwritten from addresses 02200h through > > > 0220Fh. > > > > > > This patch helps with backward compatibility for devices pre DP1.3. > > > > > > v2: read only dpcd values which can be affected, remove incorrect > > > check, > > > split into drm include changes into separate patch, commit message, > > > verbose debugging statements during overwrite. > > > > > > v3: white space fixes > > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 37 > > > +++++++++++++++++++++++++++++++++ > > > 1 file changed, 37 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index dde92e4af5d3..a31fbbbd7954 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp > > > *intel_dp) > > > sizeof(intel_dp->dpcd)) < 0) > > > return false; /* aux transfer failed */ > > > > > > > We never know what vendors can do with reserved bits. We should never > > assume > > they are zero. So we shouldn't do any of below unless it is newer > > than DP 1.3. > I think you mean newer than DP1.2? yeap, sorry.. >= DP1.3 > > > > > + if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > > > + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) { > > > + uint8_t dpcd_ext[6]; > > > + > > > + DRM_DEBUG_KMS("DPCD: Extended Receiver Capability > > > Field Present, accessing 02200h through 022FFh\n"); > > > + > > > + if (drm_dp_dpcd_read(&intel_dp->aux, > > > DP_DP13_DPCD_REV, > > > + &dpcd_ext, sizeof(dpcd_ext)) > > > < 0) > > > + return false; /* aux transfer failed */ > > > + > > > + if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], > > > &dpcd_ext[DP_DPCD_REV], > > > + sizeof(u8))) { > > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > > Revision previous value %2x new value %2x\n", > > > + intel_dp->dpcd[DP_DPCD_REV], > > > + dpcd_ext[DP_DPCD_REV]); > > > + memcpy(&intel_dp->dpcd[DP_DPCD_REV], > > > + &dpcd_ext[DP_DPCD_REV], > > > + sizeof(u8)); > > > + } > > > + if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE], > > > + &dpcd_ext[DP_MAX_LINK_RATE], > > > sizeof(u8))) { > > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > > Max Link Rate previous value %2x new value %2x\n", > > > + intel_dp- > > > >dpcd[DP_MAX_LINK_RATE], > > > + dpcd_ext[DP_MAX_LINK_RATE]); > > > + memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE], > > > + &dpcd_ext[DP_MAX_LINK_RATE], > > > sizeof(u8)); > > > + } > > > + if (memcmp(&intel_dp- > > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], > > > sizeof(u8))) { > > > + DRM_DEBUG_KMS("DPCD: new value for DPCD > > > Downstream Port Present previous value %2x new value %2x\n", > > > + intel_dp- > > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > > + dpcd_ext[DP_DOWNSTREAMPORT_P > > > RESENT]); > > > + memcpy(&intel_dp- > > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > > + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT > > > ], > > > + sizeof(u8)); > > > + } > > > + } > > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp- > > > >dpcd), intel_dp->dpcd); > > > > > > return intel_dp->dpcd[DP_DPCD_REV] != 0; > > > -- > > > 2.17.1 > > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index dde92e4af5d3..a31fbbbd7954 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp) sizeof(intel_dp->dpcd)) < 0) return false; /* aux transfer failed */ + if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] & + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) { + uint8_t dpcd_ext[6]; + + DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n"); + + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV, + &dpcd_ext, sizeof(dpcd_ext)) < 0) + return false; /* aux transfer failed */ + + if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV], + sizeof(u8))) { + DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n", + intel_dp->dpcd[DP_DPCD_REV], + dpcd_ext[DP_DPCD_REV]); + memcpy(&intel_dp->dpcd[DP_DPCD_REV], + &dpcd_ext[DP_DPCD_REV], + sizeof(u8)); + } + if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE], + &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) { + DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n", + intel_dp->dpcd[DP_MAX_LINK_RATE], + dpcd_ext[DP_MAX_LINK_RATE]); + memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE], + &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8)); + } + if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) { + DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present previous value %2x new value %2x\n", + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], + dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]); + memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT], + &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], + sizeof(u8)); + } + } DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); return intel_dp->dpcd[DP_DPCD_REV] != 0;