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

Ville Syrjälä
2016-08-11T07:22:27Z

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

--
Ville Syrjälä
Intel OTC