[2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
diff mbox

Message ID 20180719203549.28266-2-matthew.s.atwood@intel.com
State New
Headers show

Commit Message

Matt Atwood July 19, 2018, 8:35 p.m. UTC
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(+)

Comments

Vivi, Rodrigo July 19, 2018, 9:07 p.m. UTC | #1
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
>
Matt Atwood July 19, 2018, 9:47 p.m. UTC | #2
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

> >
Vivi, Rodrigo July 19, 2018, 10:06 p.m. UTC | #3
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
> > >

Patch
diff mbox

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;