diff mbox series

[v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

Message ID 20181129220058.19636-1-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series [v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT | expand

Commit Message

Navare, Manasi Nov. 29, 2018, 10 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
v4: make path dependent on DPCD revision > 1.2
v5: split into function, removed DPCD rev check
v6: add debugging prints for early exit conditions
v7 (From Manasi):
* Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
* Exit early (Jani N)
v8 (From Manasi):
* Get rid of superfluous debug prints (Jani N)
* Print entire base DPCD before memcpy (Jani N)
v9 (From Manasi):
* Add uniform newlines (Rodrigo)

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Tested-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Matt Atwood Nov. 29, 2018, 10:54 p.m. UTC | #1
On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> v4: make path dependent on DPCD revision > 1.2
> v5: split into function, removed DPCD rev check
> v6: add debugging prints for early exit conditions
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
> v8 (From Manasi):
> * Get rid of superfluous debug prints (Jani N)
> * Print entire base DPCD before memcpy (Jani N)
> v9 (From Manasi):
> * Add uniform newlines (Rodrigo)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 38
> +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 38a6e82153fd..b7c4d38089b5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * Prior to DP1.3 the bit represented by
> +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> +	 * if it is set DP_DPCD_REV at 0000h could be at a value less
> than
> +	 * the true capability of the panel. The only way to check is
> to
> +	 * then compare 0000h and 2200h.
> +	 */
> +	if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> +	      DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
I strongly disagree with removing the debug statements. While the spec
may be clear, real world products have real world gotchas that can
silently fail for a long time. The print statements would affect less
then 1% of panels. Why can't we support more verbose debugging
statements here?
> +		return;
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> sizeof(dpcd_ext)) {
> +		DRM_ERROR("DPCD failed read at extended
> capabilities\n");
> +		return;
> +	}
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> +		DRM_DEBUG_KMS("DPCD extended DPCD rev less than base
> DPCD rev\n");
> +		return;
> +	}
> +
> +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> +		return;
> +
> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
I'f we're doing a Base DPCD dump to dmesg, might as well do the new one
too and have it all in one place.
> +
> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
I disagree with this method. I specifically did each register that
*could* change to avoid panels that may not follow spec. While this is
more spec compliant, I'd prefer an approach that doesnt allow the panel
to do things improperly.
> +}
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3998,6 +4034,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  			     sizeof(intel_dp->dpcd)) < 0)
>  		return false; /* aux transfer failed */
>  
> +	intel_dp_extended_receiver_capabilities(intel_dp);
> +
>  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd),
> intel_dp->dpcd);
>  
>  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
Manasi, thanks for babysitting this patch while I was on vacation.
Rodrigo Vivi Nov. 29, 2018, 11:37 p.m. UTC | #2
On Thu, Nov 29, 2018 at 10:54:50PM +0000, Atwood, Matthew S wrote:
> On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> > v4: make path dependent on DPCD revision > 1.2
> > v5: split into function, removed DPCD rev check
> > v6: add debugging prints for early exit conditions
> > v7 (From Manasi):
> > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > * Exit early (Jani N)
> > v8 (From Manasi):
> > * Get rid of superfluous debug prints (Jani N)
> > * Print entire base DPCD before memcpy (Jani N)
> > v9 (From Manasi):
> > * Add uniform newlines (Rodrigo)
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 38
> > +++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 38a6e82153fd..b7c4d38089b5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> > *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	u8 dpcd_ext[6];
> > +
> > +	/*
> > +	 * Prior to DP1.3 the bit represented by
> > +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> > +	 * if it is set DP_DPCD_REV at 0000h could be at a value less
> > than
> > +	 * the true capability of the panel. The only way to check is
> > to
> > +	 * then compare 0000h and 2200h.
> > +	 */
> > +	if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +	      DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
> I strongly disagree with removing the debug statements. While the spec
> may be clear, real world products have real world gotchas that can
> silently fail for a long time. The print statements would affect less
> then 1% of panels. Why can't we support more verbose debugging
> statements here?

Well, I'm also in favor of the more verbose approach. Specially with
so many bad panels we got out there already.

But in the end if we print all the Base DPCD I believe we
will have all information we need anyway right?

> > +		return;
> > +
> > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> > sizeof(dpcd_ext)) {
> > +		DRM_ERROR("DPCD failed read at extended
> > capabilities\n");
> > +		return;
> > +	}
> > +
> > +	if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> > +		DRM_DEBUG_KMS("DPCD extended DPCD rev less than base
> > DPCD rev\n");
> > +		return;
> > +	}
> > +
> > +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> > +		return;
> > +
> > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> I'f we're doing a Base DPCD dump to dmesg, might as well do the new one
> too and have it all in one place.

I initially had the same feeling here, but then I noticed that
the new one is printed right after this function is called.
So I believe this is a clean enough way. But any patch can be on
top.

> > +
> > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> I disagree with this method. I specifically did each register that
> *could* change to avoid panels that may not follow spec. While this is
> more spec compliant, I'd prefer an approach that doesnt allow the panel
> to do things improperly.

I don't have strong feelings on one or the other approach.
But the situation with the author disagreeing with own patch
doesn't seem right.

> > +}
> > +
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3998,6 +4034,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  			     sizeof(intel_dp->dpcd)) < 0)
> >  		return false; /* aux transfer failed */
> >  
> > +	intel_dp_extended_receiver_capabilities(intel_dp);
> > +
> >  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd),
> > intel_dp->dpcd);
> >  
> >  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> Manasi, thanks for babysitting this patch while I was on vacation.

maybe we should split in 2 patches for a clean and accurate
history? :/

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Matt Atwood Dec. 3, 2018, 11:54 p.m. UTC | #3
On Thu, 2018-11-29 at 15:37 -0800, Rodrigo Vivi wrote:
> On Thu, Nov 29, 2018 at 10:54:50PM +0000, Atwood, Matthew S wrote:
> > On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> > > v4: make path dependent on DPCD revision > 1.2
> > > v5: split into function, removed DPCD rev check
> > > v6: add debugging prints for early exit conditions
> > > v7 (From Manasi):
> > > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext)
> > > (Jani N)
> > > * Exit early (Jani N)
> > > v8 (From Manasi):
> > > * Get rid of superfluous debug prints (Jani N)
> > > * Print entire base DPCD before memcpy (Jani N)
> > > v9 (From Manasi):
> > > * Add uniform newlines (Rodrigo)
> > > 
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 38
> > > +++++++++++++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 38a6e82153fd..b7c4d38089b5 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> > > *encoder,
> > >  	}
> > >  }
> > >  
> > > +static void
> > > +intel_dp_extended_receiver_capabilities(struct intel_dp
> > > *intel_dp)
> > > +{
> > > +	u8 dpcd_ext[6];
> > > +
> > > +	/*
> > > +	 * Prior to DP1.3 the bit represented by
> > > +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> > > +	 * if it is set DP_DPCD_REV at 0000h could be at a value less
> > > than
> > > +	 * the true capability of the panel. The only way to check is
> > > to
> > > +	 * then compare 0000h and 2200h.
> > > +	 */
> > > +	if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > > +	      DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
> > 
> > I strongly disagree with removing the debug statements. While the
> > spec
> > may be clear, real world products have real world gotchas that can
> > silently fail for a long time. The print statements would affect
> > less
> > then 1% of panels. Why can't we support more verbose debugging
> > statements here?
> 
> Well, I'm also in favor of the more verbose approach. Specially with
> so many bad panels we got out there already.
> 
> But in the end if we print all the Base DPCD I believe we
> will have all information we need anyway right?
Sure.
> 
> > > +		return;
> > > +
> > > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > > +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> > > sizeof(dpcd_ext)) {
> > > +		DRM_ERROR("DPCD failed read at extended
> > > capabilities\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> > > +		DRM_DEBUG_KMS("DPCD extended DPCD rev less than base
> > > DPCD rev\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> > > +		return;
> > > +
> > > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > 
> > I'f we're doing a Base DPCD dump to dmesg, might as well do the new
> > one
> > too and have it all in one place.
> 
> I initially had the same feeling here, but then I noticed that
> the new one is printed right after this function is called.
> So I believe this is a clean enough way. But any patch can be on
> top.
You're right this is fine
> 
> > > +
> > > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> > 
> > I disagree with this method. I specifically did each register that
> > *could* change to avoid panels that may not follow spec. While this
> > is
> > more spec compliant, I'd prefer an approach that doesnt allow the
> > panel
> > to do things improperly.
> 
> I don't have strong feelings on one or the other approach.
> But the situation with the author disagreeing with own patch
> doesn't seem right.
I'm fine with merging it.
> 
> > > +}
> > > +
> > >  bool
> > >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> > >  {
> > > @@ -3998,6 +4034,8 @@ intel_dp_read_dpcd(struct intel_dp
> > > *intel_dp)
> > >  			     sizeof(intel_dp->dpcd)) < 0)
> > >  		return false; /* aux transfer failed */
> > >  
> > > +	intel_dp_extended_receiver_capabilities(intel_dp);
> > > +
> > >  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd),
> > > intel_dp->dpcd);
> > >  
> > >  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> > 
> > Manasi, thanks for babysitting this patch while I was on vacation.
> 
> maybe we should split in 2 patches for a clean and accurate
> history? :/
Rather just have this done now I think. 
> 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Navare, Manasi Dec. 6, 2018, 3:50 p.m. UTC | #4
Thanks for the patch and the review, pushed to dinq

Manasi`

On Thu, Nov 29, 2018 at 02:00:58PM -0800, Manasi Navare 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
> v4: make path dependent on DPCD revision > 1.2
> v5: split into function, removed DPCD rev check
> v6: add debugging prints for early exit conditions
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
> v8 (From Manasi):
> * Get rid of superfluous debug prints (Jani N)
> * Print entire base DPCD before memcpy (Jani N)
> v9 (From Manasi):
> * Add uniform newlines (Rodrigo)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 38a6e82153fd..b7c4d38089b5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * Prior to DP1.3 the bit represented by
> +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> +	 * the true capability of the panel. The only way to check is to
> +	 * then compare 0000h and 2200h.
> +	 */
> +	if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> +	      DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
> +		return;
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> +		DRM_ERROR("DPCD failed read at extended capabilities\n");
> +		return;
> +	}
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> +		DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> +		return;
> +	}
> +
> +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> +		return;
> +
> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> +
> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> +}
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3998,6 +4034,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  			     sizeof(intel_dp->dpcd)) < 0)
>  		return false; /* aux transfer failed */
>  
> +	intel_dp_extended_receiver_capabilities(intel_dp);
> +
>  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
>  
>  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> -- 
> 2.19.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 38a6e82153fd..b7c4d38089b5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3991,6 +3991,42 @@  intel_dp_link_down(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
+{
+	u8 dpcd_ext[6];
+
+	/*
+	 * Prior to DP1.3 the bit represented by
+	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
+	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
+	 * the true capability of the panel. The only way to check is to
+	 * then compare 0000h and 2200h.
+	 */
+	if (!(intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+	      DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT))
+		return;
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
+			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
+		DRM_ERROR("DPCD failed read at extended capabilities\n");
+		return;
+	}
+
+	if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
+		DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
+		return;
+	}
+
+	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
+		return;
+
+	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
+		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
+
+	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
+}
+
 bool
 intel_dp_read_dpcd(struct intel_dp *intel_dp)
 {
@@ -3998,6 +4034,8 @@  intel_dp_read_dpcd(struct intel_dp *intel_dp)
 			     sizeof(intel_dp->dpcd)) < 0)
 		return false; /* aux transfer failed */
 
+	intel_dp_extended_receiver_capabilities(intel_dp);
+
 	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
 
 	return intel_dp->dpcd[DP_DPCD_REV] != 0;