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.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,

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 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..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(&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))) 
  

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(&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_PRE
  

SENT]);

• 	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;