[Intel-gfx] [PATCH v2 4/8] drm/i915/dp: Print full branch/sink descriptor

[Jani Nikula]

On Mon, 24 Oct 2016, Imre Deak <imre.deak at intel.com> wrote:
> Extend the branch/sink descriptor info with the missing device ID
> field. While at it also read out all the descriptor registers in one
> transfer and make the debug print more compact.
>
> v2: (Jani)
> - Cache the descriptor in intel_dp.
> - Split out this change into a separate patch.
>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 61 +++++++++++++---------------------------
>  drivers/gpu/drm/i915/intel_drv.h | 10 +++++++
>  2 files changed, 30 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1991250..726fdf2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1451,34 +1451,33 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  	DRM_DEBUG_KMS("common rates: %s\n", str);
>  }
>  
> -static void intel_dp_print_hw_revision(struct intel_dp *intel_dp)
> +static bool
> +__intel_dp_read_desc(struct intel_dp *intel_dp, struct intel_dp_desc *desc)
>  {
> -	uint8_t rev;
> -	int len;
> +	u32 base = drm_dp_is_branch(intel_dp->dpcd) ? DP_BRANCH_OUI :
> +						      DP_SINK_OUI;
>  
> -	if (!drm_dp_is_branch(intel_dp->dpcd))
> -		return;
> -
> -	len = drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_HW_REV, &rev, 1);
> -	if (len < 0)
> -		return;
> -
> -	DRM_DEBUG_KMS("sink hw revision: %d.%d\n", (rev & 0xf0) >> 4, rev & 0xf);
> +	return drm_dp_dpcd_read(&intel_dp->aux, base, desc, sizeof(*desc)) ==
> +	       sizeof(*desc);
>  }
>  
> -static void intel_dp_print_sw_revision(struct intel_dp *intel_dp)
> +static bool intel_dp_read_desc(struct intel_dp *intel_dp)
>  {
> -	uint8_t rev[2];
> -	int len;
> +	struct intel_dp_desc *desc = &intel_dp->desc;
> +	bool oui_sup = intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] &
> +		       DP_OUI_SUPPORT;
>  
> -	if (!drm_dp_is_branch(intel_dp->dpcd))
> -		return;
> +	if (__intel_dp_read_desc(intel_dp, desc) < 0)

The bool returned from __intel_dp_read_desc will never be less than 0...

There's no point in reading the desc if oui_sup is false, is there? All
of desc should read all zeros in that case, not just OUI.

I guess the desc should be reset to zeros at detect or somewhere. I'm
sure we have other stuff we fail to reset too.

> +		return false;
>  
> -	len = drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_SW_REV, &rev, 2);
> -	if (len < 0)
> -		return;
> +	DRM_DEBUG_KMS("DP %s: OUI %*phD%s dev-ID %.*s HW-rev %d.%d SW-rev %d.%d\n",

Maybe some form of %*pE would be more defensive for device id? See
Documentation/printk-formats.txt.

> +		      drm_dp_is_branch(intel_dp->dpcd) ? "branch" : "sink",
> +		      (int)sizeof(desc->oui), desc->oui, oui_sup ? "" : "(NS)",
> +		      (int)sizeof(desc->device_id), desc->device_id,
> +		      desc->hw_rev >> 4, desc->hw_rev & 0xf,
> +		      desc->sw_major_rev, desc->sw_minor_rev);
>  
> -	DRM_DEBUG_KMS("sink sw revision: %d.%d\n", rev[0], rev[1]);
> +	return true;
>  }
>  
>  static int rate_to_index(int find, const int *rates)
> @@ -3621,23 +3620,6 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	return true;
>  }
>  
> -static void
> -intel_dp_probe_oui(struct intel_dp *intel_dp)
> -{
> -	bool is_branch = drm_dp_is_branch(intel_dp->dpcd);
> -	u8 buf[3];
> -
> -	if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
> -		return;
> -
> -	if (drm_dp_dpcd_read(&intel_dp->aux,
> -			     is_branch ? DP_BRANCH_OUI : DP_SINK_OUI,
> -			     buf, 3) == 3)
> -		DRM_DEBUG_KMS("%s OUI: %02hx%02hx%02hx\n",
> -			      is_branch ? "Branch" : "Sink",
> -			      buf[0], buf[1], buf[2]);
> -}
> -
>  static bool
>  intel_dp_can_mst(struct intel_dp *intel_dp)
>  {
> @@ -4416,10 +4398,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  
>  	intel_dp_print_rates(intel_dp);
>  
> -	intel_dp_probe_oui(intel_dp);
> -
> -	intel_dp_print_hw_revision(intel_dp);
> -	intel_dp_print_sw_revision(intel_dp);
> +	intel_dp_read_desc(intel_dp);
>  
>  	intel_dp_configure_mst(intel_dp);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 16b33f5..4c9f953 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -883,6 +883,14 @@ enum link_m_n_set {
>  	M2_N2
>  };
>  
> +struct intel_dp_desc {
> +	u8 oui[3];
> +	u8 device_id[6];
> +	u8 hw_rev;
> +	u8 sw_major_rev;
> +	u8 sw_minor_rev;
> +} __packed;
> +
>  struct intel_dp {
>  	i915_reg_t output_reg;
>  	i915_reg_t aux_ch_ctl_reg;
> @@ -905,6 +913,8 @@ struct intel_dp {
>  	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
>  	uint8_t num_sink_rates;
>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
> +	/* sink or branch descriptor */
> +	struct intel_dp_desc desc;
>  	struct drm_dp_aux aux;
>  	uint8_t train_set[4];
>  	int panel_power_up_delay;

-- 
Jani Nikula, Intel Open Source Technology Center