[Intel-gfx] [PATCH 1/3] drm/i915: Check pixel rate for DP to VGA dongle

[Daniel Vetter]

On Tue, May 03, 2016 at 04:23:34PM +0300, Ville Syrjälä wrote:
> On Tue, May 03, 2016 at 02:46:36PM +0300, Mika Kahola wrote:
> > Prep work to improve DP branch device handling.
> > 
> > Filter out a mode that exceeds the max pixel rate setting
> > for DP to VGA dongle. This is defined in DPCD register 0x81
> > if detailed cap info i.e. info field is 4 bytes long and
> > it is available for DP downstream port.
> > 
> > The register defines the pixel rate divided by 8 in MP/s.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 34 ++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  9 +++++++++
> >  2 files changed, 43 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3633002..74a04ce 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -201,6 +201,13 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >  	int max_rate, mode_rate, max_lanes, max_link_clock;
> >  	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> >  
> > +	/* DP to VGA dongle may define max pixel rate in DPCD */
> > +	if (intel_dp->dfp.present &&
> > +	    intel_dp->dfp.detailed_cap_info &&
> > +	    (intel_dp->dfp.type & DP_DS_PORT_TYPE_VGA) &&
> > +	    (intel_dp->dfp.dot_clk > 0))
> > +		max_dotclk = min(max_dotclk, intel_dp->dfp.dot_clk);
> 
> What's dfp?
> 
> Looks like most of this stuff is not really needed. Just storing a max
> dotclock per downstream port would seem to suffice.
> 
> > +
> >  	if (is_edp(intel_dp) && fixed_mode) {
> >  		if (mode->hdisplay > fixed_mode->hdisplay)
> >  			return MODE_PANEL;
> > @@ -4566,6 +4573,28 @@ static const struct drm_encoder_funcs intel_dp_enc_funcs = {
> >  	.destroy = intel_dp_encoder_destroy,
> >  };
> >  
> > +static void intel_dp_get_dfp(struct intel_dp *intel_dp)
> > +{
> > +	uint8_t dfp_info[4];
> > +
> > +	intel_dp->dfp.detailed_cap_info = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE;
> > +
> > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, dfp_info, 4) < 0) {
> > +		intel_dp->dfp.present = false;
> > +		intel_dp->dfp.detailed_cap_info = false;
> > +		return; /* aux transfer failed */
> > +	}
> > +
> > +	intel_dp->dfp.type = dfp_info[0] & DP_DS_PORT_TYPE_MASK;
> > +
> > +	if (intel_dp->dfp.detailed_cap_info) {
> > +		if (intel_dp->dfp.type & DP_DS_PORT_TYPE_VGA) {
> > +			intel_dp->dfp.dot_clk = dfp_info[1] * 8 * 1000;
> > +			DRM_DEBUG_KMS("max pixel rate for VGA is %d kHz\n", intel_dp->dfp.dot_clk);
> > +		}
> 
> I would suggest putting this sort of stuff into the dp helper. I once
> started to hatch something to deal with these downstream port limits,
> but never finished it. I pushed my WIP stuff (mostly ideas how to parse
> these port caps) to 
> 
> git://github.com/vsyrjala/linux.git dp_downstream_ports
> 
> maybe you can to see if there's anything useful for you there.

Seconded on at least moving the computation into the dp helpers. i915.ko
really should only ask the helpers for the final result, maybe with an
intermediate step to cache the dp aux register stuff. There's already some
structures in the dp helpers to store sink state, we could start using
those (unfortunately they're not agreeing on what the canonical one should
be).
-Daniel

> 
> > +	}
> > +}
> > +
> >  enum irqreturn
> >  intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  {
> > @@ -4599,6 +4628,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  	power_domain = intel_display_port_aux_power_domain(intel_encoder);
> >  	intel_display_power_get(dev_priv, power_domain);
> >  
> > +	intel_dp->dfp.present = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & 0x1;
> > +
> > +	if (intel_dp->dfp.present)
> > +		intel_dp_get_dfp(intel_dp);
> > +
> >  	if (long_hpd) {
> >  		/* indicate that we need to restart link training */
> >  		intel_dp->train_set_valid = false;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 21dee3f..9798a59 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -794,6 +794,13 @@ enum link_m_n_set {
> >  	M2_N2
> >  };
> >  
> > +struct intel_dp_dfp {
> > +	bool present;
> > +	int type;
> > +	bool detailed_cap_info;
> > +	int dot_clk; /* pixel rate for VGA dongles */
> > +};
> > +
> >  struct intel_dp {
> >  	i915_reg_t output_reg;
> >  	i915_reg_t aux_ch_ctl_reg;
> > @@ -861,6 +868,8 @@ struct intel_dp {
> >  
> >  	bool train_set_valid;
> >  
> > +	struct intel_dp_dfp dfp;
> > +
> >  	/* Displayport compliance testing */
> >  	unsigned long compliance_test_type;
> >  	unsigned long compliance_test_data;
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch