diff mbox

[v6,09/10] drm/i915: Update bits per component for display info

Message ID 1467803094-10473-10-git-send-email-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kahola July 6, 2016, 11:04 a.m. UTC
DisplayPort branch device may define max supported bits per
component. Update display info based on this value if bpc
is defined.

v2: cleanup to match the drm_dp_helper.c patches introduced
    earlier in this series

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Daniel Vetter July 12, 2016, 1:51 p.m. UTC | #1
On Wed, Jul 06, 2016 at 02:04:53PM +0300, Mika Kahola wrote:
> DisplayPort branch device may define max supported bits per
> component. Update display info based on this value if bpc
> is defined.
> 
> v2: cleanup to match the drm_dp_helper.c patches introduced
>     earlier in this series
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 76a654e..53ec844 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3932,6 +3932,14 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>  	uint8_t *dpcd = intel_dp->dpcd;
>  	uint8_t type;
>  
> +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> +		int bpc = drm_dp_downstream_max_bpc(dpcd,
> +						    intel_dp->downstream_ports);
> +
> +		if (bpc > 0)
> +			intel_dp->attached_connector->base.display_info.bpc = bpc;
> +	}

I think a function in the dp helpers to correctly fill out the connector's
display info would be neater.
-Daniel

> +
>  	if (!intel_dp_get_dpcd(intel_dp))
>  		return connector_status_disconnected;
>  
> @@ -3968,6 +3976,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>  			return connector_status_unknown;
>  	}
>  
> +
>  	/* Anything else is out of spec, warn and ignore */
>  	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
>  	return connector_status_disconnected;
> -- 
> 1.9.1
>
Mika Kahola Aug. 2, 2016, 11:23 a.m. UTC | #2
On Tue, 2016-07-12 at 15:51 +0200, Daniel Vetter wrote:
> On Wed, Jul 06, 2016 at 02:04:53PM +0300, Mika Kahola wrote:
> > DisplayPort branch device may define max supported bits per
> > component. Update display info based on this value if bpc
> > is defined.
> > 
> > v2: cleanup to match the drm_dp_helper.c patches introduced
> >     earlier in this series
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 76a654e..53ec844 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3932,6 +3932,14 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> >  	uint8_t *dpcd = intel_dp->dpcd;
> >  	uint8_t type;
> >  
> > +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> > +		int bpc = drm_dp_downstream_max_bpc(dpcd,
> > +						    intel_dp->downstream_ports);
> > +
> > +		if (bpc > 0)
> > +			intel_dp->attached_connector->base.display_info.bpc = bpc;
> > +	}
> 
> I think a function in the dp helpers to correctly fill out the connector's
> display info would be neater.
> -Daniel
Ok. I can move this stuff to be part of dp helper routine.
Cheers,
Mika
> 
> > +
> >  	if (!intel_dp_get_dpcd(intel_dp))
> >  		return connector_status_disconnected;
> >  
> > @@ -3968,6 +3976,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> >  			return connector_status_unknown;
> >  	}
> >  
> > +
> >  	/* Anything else is out of spec, warn and ignore */
> >  	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
> >  	return connector_status_disconnected;
> > -- 
> > 1.9.1
> > 
>
Ville Syrjälä Aug. 2, 2016, 11:41 a.m. UTC | #3
On Tue, Aug 02, 2016 at 02:23:13PM +0300, Mika Kahola wrote:
> On Tue, 2016-07-12 at 15:51 +0200, Daniel Vetter wrote:
> > On Wed, Jul 06, 2016 at 02:04:53PM +0300, Mika Kahola wrote:
> > > DisplayPort branch device may define max supported bits per
> > > component. Update display info based on this value if bpc
> > > is defined.
> > > 
> > > v2: cleanup to match the drm_dp_helper.c patches introduced
> > >     earlier in this series
> > > 
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 76a654e..53ec844 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3932,6 +3932,14 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > >  	uint8_t *dpcd = intel_dp->dpcd;
> > >  	uint8_t type;
> > >  
> > > +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> > > +		int bpc = drm_dp_downstream_max_bpc(dpcd,
> > > +						    intel_dp->downstream_ports);
> > > +
> > > +		if (bpc > 0)
> > > +			intel_dp->attached_connector->base.display_info.bpc = bpc;
> > > +	}
> > 
> > I think a function in the dp helpers to correctly fill out the connector's
> > display info would be neater.
> > -Daniel
> Ok. I can move this stuff to be part of dp helper routine.

I'm not sure frobbing with display_info is a good idea here since that
that one has so far been the EDID parsers domain. The ordering between
all this stuff is already very poorly defined/buggy, so some bigger
cleanup is probably needed. I have some patches to the EDID parser that
I should send out that at least make it a bit clearer where it fills out
the display_info stuff.

> Cheers,
> Mika
> > 
> > > +
> > >  	if (!intel_dp_get_dpcd(intel_dp))
> > >  		return connector_status_disconnected;
> > >  
> > > @@ -3968,6 +3976,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > >  			return connector_status_unknown;
> > >  	}
> > >  
> > > +
> > >  	/* Anything else is out of spec, warn and ignore */
> > >  	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
> > >  	return connector_status_disconnected;
> > > -- 
> > > 1.9.1
> > > 
> > 
> 
> -- 
> Mika Kahola - Intel OTC
Daniel Vetter Aug. 2, 2016, 1:18 p.m. UTC | #4
On Tue, Aug 02, 2016 at 02:41:25PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 02, 2016 at 02:23:13PM +0300, Mika Kahola wrote:
> > On Tue, 2016-07-12 at 15:51 +0200, Daniel Vetter wrote:
> > > On Wed, Jul 06, 2016 at 02:04:53PM +0300, Mika Kahola wrote:
> > > > DisplayPort branch device may define max supported bits per
> > > > component. Update display info based on this value if bpc
> > > > is defined.
> > > > 
> > > > v2: cleanup to match the drm_dp_helper.c patches introduced
> > > >     earlier in this series
> > > > 
> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 76a654e..53ec844 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -3932,6 +3932,14 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > > >  	uint8_t *dpcd = intel_dp->dpcd;
> > > >  	uint8_t type;
> > > >  
> > > > +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> > > > +		int bpc = drm_dp_downstream_max_bpc(dpcd,
> > > > +						    intel_dp->downstream_ports);
> > > > +
> > > > +		if (bpc > 0)
> > > > +			intel_dp->attached_connector->base.display_info.bpc = bpc;
> > > > +	}
> > > 
> > > I think a function in the dp helpers to correctly fill out the connector's
> > > display info would be neater.
> > > -Daniel
> > Ok. I can move this stuff to be part of dp helper routine.
> 
> I'm not sure frobbing with display_info is a good idea here since that
> that one has so far been the EDID parsers domain. The ordering between
> all this stuff is already very poorly defined/buggy, so some bigger
> cleanup is probably needed. I have some patches to the EDID parser that
> I should send out that at least make it a bit clearer where it fills out
> the display_info stuff.

+1 for bigger cleanup. A helper which does all the dp sink detection
(grabs edid, dpcd and everything else and then computes restrictions)
would be _real_ awesome I think. Even better if you can trick some other
driver into using it too.

Tomeu from collabora just volunteered himself to clean up the analogix dp
support and switch over to the helpers. He's rather active on irc, so
great guinea-pig to volutneer for this and work together with.
-Daniel

> 
> > Cheers,
> > Mika
> > > 
> > > > +
> > > >  	if (!intel_dp_get_dpcd(intel_dp))
> > > >  		return connector_status_disconnected;
> > > >  
> > > > @@ -3968,6 +3976,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > > >  			return connector_status_unknown;
> > > >  	}
> > > >  
> > > > +
> > > >  	/* Anything else is out of spec, warn and ignore */
> > > >  	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
> > > >  	return connector_status_disconnected;
> > > > -- 
> > > > 1.9.1
> > > > 
> > > 
> > 
> > -- 
> > Mika Kahola - Intel OTC
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 76a654e..53ec844 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3932,6 +3932,14 @@  intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	uint8_t *dpcd = intel_dp->dpcd;
 	uint8_t type;
 
+	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
+		int bpc = drm_dp_downstream_max_bpc(dpcd,
+						    intel_dp->downstream_ports);
+
+		if (bpc > 0)
+			intel_dp->attached_connector->base.display_info.bpc = bpc;
+	}
+
 	if (!intel_dp_get_dpcd(intel_dp))
 		return connector_status_disconnected;
 
@@ -3968,6 +3976,7 @@  intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 			return connector_status_unknown;
 	}
 
+
 	/* Anything else is out of spec, warn and ignore */
 	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
 	return connector_status_disconnected;