[Intel-gfx] [PATCH v3 02/12] drm: Read DPCD receiver capability for DP to VGA converter

[Ville Syrjälä]

On Mon, May 23, 2016 at 01:50:47PM +0300, Mika Kahola wrote:
> Read from DPCD receiver capability field the max allowed
> pixel clock and bits per component for DP to VGA converter.
> 
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c  | 46 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  include/drm/drm_dp_helper.h      | 21 ++++++++++++++++++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index eeaf5a7..c5bec6f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -412,6 +412,52 @@ int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  }
>  EXPORT_SYMBOL(drm_dp_link_power_down);
>  
> +/*
> + * drm_dp_bd() - read DisplayPort Receiver Capability Fields for
> + * DP branch devices
> + * @aux: DisplayPort AUX channel
> + * @bd: pointer to a structure containing DP branch device information
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_bd(struct drm_dp_aux *aux, struct drm_dp_bd *bd)
> +{
> +	uint8_t info[4];
> +	uint8_t dfp;
> +	bool detailed_cap_info;
> +	int err, size;
> +
> +	err = drm_dp_dpcd_read(aux, DP_DOWNSTREAMPORT_PRESENT, &dfp, sizeof(dfp));
> +	if (err < 0)
> +		return err;
> +
> +	bd->present = dfp & 0x1;
> +
> +	if (!bd->present)
> +		return 0;
> +
> +	detailed_cap_info = dfp & DP_DETAILED_CAP_INFO_AVAILABLE;
> +
> +	size = detailed_cap_info ? 4 : 1;
> +
> +	err = drm_dp_dpcd_read(aux, DP_DOWNSTREAM_PORT_0, info, size);
> +	if (err < 0)
> +		return err;
> +
> +	bd->type = info[0] & DP_DS_PORT_TYPE_MASK;
> +	bd->hpd = info[0] & DP_DS_PORT_HPD;
> +
> +	if (detailed_cap_info) {
> +		if (bd->type & DP_DS_PORT_TYPE_VGA) {
> +			bd->dfp.vga.dot_clk = info[1] * 8 * 1000;
> +			bd->dfp.vga.bpc = info[2] & DP_DS_VGA_MAX_BPC_MASK;
> +		}
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_bd);
> +
>  /**
>   * drm_dp_link_configure() - configure a DisplayPort link
>   * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0741b2d..f85914b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -886,6 +886,8 @@ struct intel_dp {
>  
>  	bool train_set_valid;
>  
> +	struct drm_dp_bd bd;
> +
>  	/* Displayport compliance testing */
>  	unsigned long compliance_test_type;
>  	unsigned long compliance_test_data;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index e384c7f..d3e78a5 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -802,10 +802,31 @@ struct drm_dp_link {
>  	unsigned long capabilities;
>  };
>  
> +/*
> + * DP to VGA
> + */
> +struct drm_dp_vga {
> +	int dot_clk;
> +	uint8_t bpc;
> +};
> +
> +/*
> + * Branch device
> + */
> +struct drm_dp_bd {
> +	bool present;
> +	int type;
> +	bool hpd;
> +	union {
> +		struct drm_dp_vga vga;
> +	} dfp;

I still don't see any real point in adding this thing. I'd just keep
the common dp helper style and pass the port caps to each function
that parses one thing from there. Eg.

int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
			      const u8 port_cap[4]);
int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
				const u8 port_cap[4]);
etc.

> +};
> +
>  int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
> +int drm_dp_bd(struct drm_dp_aux *aux, struct drm_dp_bd *bd);
>  
>  int drm_dp_aux_register(struct drm_dp_aux *aux);
>  void drm_dp_aux_unregister(struct drm_dp_aux *aux);
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC