Re: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
On Tue, 2018-07-17 at 15:34 -0700, Dhinakaran Pandiyan wrote: > On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote: > > > > From: Matt Atwood > > > > 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 0h through Fh, 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 0h through Fh 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, > I still see 6 bytes read and 3 copied. Ignore this, the original patch was reading 16B. Thanks for clarifying Matt. > > > > > remove incorrect check, > > split into drm include changes into separate patch, commit message, > > verbose debugging statements during overwrite. > > > > Signed-off-by: Matt Atwood > > --- > > 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..364cf41a8b89 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(_dp->aux, > > DP_DP13_DPCD_REV, > > + _ext, sizeof(dpcd_ext)) < 0) > > + return false; /* aux transfer failed */ > > + > > + if (memcmp(_dp->dpcd[DP_DPCD_REV], > > _ext[DP_DPCD_REV], > > + sizeof(u8))) > Why use memcmp and memcmpy if it's just one byte? You could just use > "==" > I believe this is what Jani suggested. if (memcmp(old_dpcd, new_dpcd, sizeof(new_dpcd)) { DRM_DEBUG_KMS(); memcpy(old_dpcd, new_dpcd, sizeof(new_dpcd); } We lose the information about which specific fields in the 6 bytes changed, but that's okay IMO. > > > > { > > + 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(_dp->dpcd[DP_DPCD_REV], > > + _ext[DP_DPCD_REV], > > + sizeof(u8)); > > + } > > + if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE], > > + _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(_dp->dpcd[DP_MAX_LINK_RATE], > > + _ext[DP_MAX_LINK_RATE], > > sizeof(u8)); > > + } > > + if (memcmp(_dp- > > > > > > dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + _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 > > RE > > SENT]); > > + memcpy(_dp- > > > > > > dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + _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; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
On Tue, Jul 17, 2018 at 03:34:35PM -0700, Dhinakaran Pandiyan wrote: > On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote: > > From: Matt Atwood > > > > 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 0h through Fh, 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 0h through Fh 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, > > I still see 6 bytes read and 3 copied. +1 > > > remove incorrect check, > > split into drm include changes into separate patch, commit message, > > verbose debugging statements during overwrite. > > > > Signed-off-by: Matt Atwood > > --- > > 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..364cf41a8b89 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(_dp->aux, > > DP_DP13_DPCD_REV, > > + _ext, sizeof(dpcd_ext)) < 0) > > + return false; /* aux transfer failed */ > > + > > + if (memcmp(_dp->dpcd[DP_DPCD_REV], > > _ext[DP_DPCD_REV], > > + sizeof(u8))) > > Why use memcmp and memcmpy if it's just one byte? You could just use > "==" I think == should work here, but why not memcmp anyways?! > > > { > > + 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(_dp->dpcd[DP_DPCD_REV], > > + _ext[DP_DPCD_REV], > > + sizeof(u8)); > > + } > > + if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE], > > + _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(_dp->dpcd[DP_MAX_LINK_RATE], > > + _ext[DP_MAX_LINK_RATE], > > sizeof(u8)); > > + } > > + if (memcmp(_dp- > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + _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_PRE > > SENT]); > > + memcpy(_dp- > > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > > + _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; > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote: > From: Matt Atwood > > 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 0h through Fh, 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 0h through Fh 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, I still see 6 bytes read and 3 copied. > remove incorrect check, > split into drm include changes into separate patch, commit message, > verbose debugging statements during overwrite. > > Signed-off-by: Matt Atwood > --- > 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..364cf41a8b89 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(_dp->aux, > DP_DP13_DPCD_REV, > + _ext, sizeof(dpcd_ext)) < 0) > + return false; /* aux transfer failed */ > + > + if (memcmp(_dp->dpcd[DP_DPCD_REV], > _ext[DP_DPCD_REV], > + sizeof(u8))) Why use memcmp and memcmpy if it's just one byte? You could just use "==" > { > + 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(_dp->dpcd[DP_DPCD_REV], > + _ext[DP_DPCD_REV], > + sizeof(u8)); > + } > + if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE], > + _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(_dp->dpcd[DP_MAX_LINK_RATE], > + _ext[DP_MAX_LINK_RATE], > sizeof(u8)); > + } > + if (memcmp(_dp- > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > + _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_PRE > SENT]); > + memcpy(_dp- > >dpcd[DP_DOWNSTREAMPORT_PRESENT], > + _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; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel