Re: [Intel-gfx] [PATCH v7 4/5] drm: Update bits per component for display info

2016-08-11 Thread Daniel Vetter 
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 
> > ---
> >  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,
> > +_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 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  /*
> >   * 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 4/5] drm: Update bits per component for display info

2016-08-11 Thread Ville Syrjälä 
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 
> ---
>  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,
> +  _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 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 4/5] drm: Update bits per component for display info

2016-08-08 Thread Mika Kahola 
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 
---
 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,
+_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 
 #include 
 #include 
+#include 
 
 /*
  * 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx