diff mbox

[v7,4/5] drm: Update bits per component for display info

Message ID 1470661230-15988-5-git-send-email-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kahola, Mika Aug. 8, 2016, 1 p.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
v3: Fill bpc for connector's display info in separate
    drm_dp_helper function (Daniel)

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c |  3 +++
 include/drm/drm_dp_helper.h     |  4 ++++
 3 files changed, 31 insertions(+)

Comments

Ville Syrjälä Aug. 11, 2016, 7:22 a.m. UTC | #1 
On Mon, Aug 08, 2016 at 04:00:29PM +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
> v3: Fill bpc for connector's display info in separate
>     drm_dp_helper function (Daniel)
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c |  3 +++
>  include/drm/drm_dp_helper.h     |  4 ++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 1f36016..a2c46ed 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -514,6 +514,30 @@ int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
>  
>  /**
> + * drm_dp_downstream_update_max_bpc() - extract branch device max
> + *                                      bits per component and update
> + *                                      connector max bpc
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + * @connector: Connector info
> + * Returns current max bpc on success
> + */
> +int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +				     const u8 port_cap[4],
> +				     struct drm_connector *connector)
> +{
> +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> +		int bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
> +
> +		if (bpc > 0)
> +			connector->display_info.bpc = bpc;
> +	}
> +
> +	return connector->display_info.bpc;
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_update_max_bpc);
> +
> +/**
>   * drm_dp_downstream_hw_rev() - read DP branch device HW revision
>   * @aux: DisplayPort AUX channel
>   *
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e990c8b..763e2f5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3985,6 +3985,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>  	uint8_t *dpcd = intel_dp->dpcd;
>  	uint8_t type;
>  
> +	drm_dp_downstream_update_max_bpc(dpcd, intel_dp->downstream_ports,
> +					 &intel_dp->attached_connector->base);
> +

And it will be promptly overwritten by the EDID parser. Like I said
before, touching display_info from elsewhere is a bad idea right now.
We need to lay out some real rules on how the EDID parser vs. DP
helpers must be called if we are to share that information between
the two.

For now, I would just utilize the DP helper in the compute_config
to limit the bpc.

>  	if (!intel_dp_get_dpcd(intel_dp))
>  		return connector_status_disconnected;
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 45366aa..5491a9b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -26,6 +26,7 @@
>  #include <linux/types.h>
>  #include <linux/i2c.h>
>  #include <linux/delay.h>
> +#include <drm/drm_crtc.h>
>  
>  /*
>   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
> @@ -827,6 +828,9 @@ int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  				const u8 port_cap[4]);
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  			      const u8 port_cap[4]);
> +int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +				     const u8 port_cap[4],
> +				     struct drm_connector *connector);
>  int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
>  int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  			     struct drm_dp_aux *aux, struct drm_dp_link *link);
> -- 
> 1.9.1
Daniel Vetter Aug. 11, 2016, 8:53 a.m. UTC | #2 
On Thu, Aug 11, 2016 at 10:22:27AM +0300, Ville Syrjälä wrote:
> On Mon, Aug 08, 2016 at 04:00:29PM +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
> > v3: Fill bpc for connector's display info in separate
> >     drm_dp_helper function (Daniel)
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 24 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c |  3 +++
> >  include/drm/drm_dp_helper.h     |  4 ++++
> >  3 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 1f36016..a2c46ed 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -514,6 +514,30 @@ int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> >  EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
> >  
> >  /**
> > + * drm_dp_downstream_update_max_bpc() - extract branch device max
> > + *                                      bits per component and update
> > + *                                      connector max bpc
> > + * @dpcd: DisplayPort configuration data
> > + * @port_cap: port capabilities
> > + * @connector: Connector info
> > + * Returns current max bpc on success
> > + */
> > +int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > +				     const u8 port_cap[4],
> > +				     struct drm_connector *connector)
> > +{
> > +	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
> > +		int bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
> > +
> > +		if (bpc > 0)
> > +			connector->display_info.bpc = bpc;
> > +	}
> > +
> > +	return connector->display_info.bpc;
> > +}
> > +EXPORT_SYMBOL(drm_dp_downstream_update_max_bpc);
> > +
> > +/**
> >   * drm_dp_downstream_hw_rev() - read DP branch device HW revision
> >   * @aux: DisplayPort AUX channel
> >   *
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index e990c8b..763e2f5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3985,6 +3985,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> >  	uint8_t *dpcd = intel_dp->dpcd;
> >  	uint8_t type;
> >  
> > +	drm_dp_downstream_update_max_bpc(dpcd, intel_dp->downstream_ports,
> > +					 &intel_dp->attached_connector->base);
> > +
> 
> And it will be promptly overwritten by the EDID parser. Like I said
> before, touching display_info from elsewhere is a bad idea right now.
> We need to lay out some real rules on how the EDID parser vs. DP
> helpers must be called if we are to share that information between
> the two.
> 
> For now, I would just utilize the DP helper in the compute_config
> to limit the bpc.

I think we need one dp helper which does _all_ the sink detection, and in
the right order. Edid reading dpcd decoding, all that. And then stuff is
all into relevant structures like display_info, but also some new (or well
I think we have parts of it already) dp_sink struct. Otherwise this will
stay fragile, especially once other drivers start using it.
-Daniel

> 
> >  	if (!intel_dp_get_dpcd(intel_dp))
> >  		return connector_status_disconnected;
> >  
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 45366aa..5491a9b 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -26,6 +26,7 @@
> >  #include <linux/types.h>
> >  #include <linux/i2c.h>
> >  #include <linux/delay.h>
> > +#include <drm/drm_crtc.h>
> >  
> >  /*
> >   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
> > @@ -827,6 +828,9 @@ int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> >  				const u8 port_cap[4]);
> >  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> >  			      const u8 port_cap[4]);
> > +int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > +				     const u8 port_cap[4],
> > +				     struct drm_connector *connector);
> >  int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
> >  int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> >  			     struct drm_dp_aux *aux, struct drm_dp_link *link);
> > -- 
> > 1.9.1
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 1f36016..a2c46ed 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -514,6 +514,30 @@  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
 
 /**
+ * drm_dp_downstream_update_max_bpc() - extract branch device max
+ *                                      bits per component and update
+ *                                      connector max bpc
+ * @dpcd: DisplayPort configuration data
+ * @port_cap: port capabilities
+ * @connector: Connector info
+ * Returns current max bpc on success
+ */
+int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+				     const u8 port_cap[4],
+				     struct drm_connector *connector)
+{
+	if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) {
+		int bpc = drm_dp_downstream_max_bpc(dpcd, port_cap);
+
+		if (bpc > 0)
+			connector->display_info.bpc = bpc;
+	}
+
+	return connector->display_info.bpc;
+}
+EXPORT_SYMBOL(drm_dp_downstream_update_max_bpc);
+
+/**
  * drm_dp_downstream_hw_rev() - read DP branch device HW revision
  * @aux: DisplayPort AUX channel
  *
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e990c8b..763e2f5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3985,6 +3985,9 @@  intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	uint8_t *dpcd = intel_dp->dpcd;
 	uint8_t type;
 
+	drm_dp_downstream_update_max_bpc(dpcd, intel_dp->downstream_ports,
+					 &intel_dp->attached_connector->base);
+
 	if (!intel_dp_get_dpcd(intel_dp))
 		return connector_status_disconnected;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 45366aa..5491a9b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -26,6 +26,7 @@ 
 #include <linux/types.h>
 #include <linux/i2c.h>
 #include <linux/delay.h>
+#include <drm/drm_crtc.h>
 
 /*
  * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
@@ -827,6 +828,9 @@  int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 				const u8 port_cap[4]);
 int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 			      const u8 port_cap[4]);
+int drm_dp_downstream_update_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+				     const u8 port_cap[4],
+				     struct drm_connector *connector);
 int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]);
 int drm_dp_downstream_hw_rev(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 			     struct drm_dp_aux *aux, struct drm_dp_link *link);