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

Tue May 10 09:54:05 UTC 2016

On Tue, May 10, 2016 at 12:44:22PM +0300, Mika Kahola wrote:
> On Tue, 2016-05-03 at 16:28 +0200, Daniel Vetter wrote:
> > 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?
> 
> dfp - Downstream-Facing Port

I thought "digital flat panel" (as in the DFP standard and whatnot),
which was a bit confusing. So a bit of an overloaded TLA that one.
If we need such a substruct I might just call it downstream_port
or something along those lines.

> > > 
> > > 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
> 
> Ok. Two votes for moving this stuff into the dp helpers. I'll do a
> respin with this one. 
> 
> Cheers,
> Mika
> 
> > 
> > > 
> > > > +	}
> > > > +}
> > > > +
> > > >  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
> > 
> 
> -- 
> Mika Kahola - Intel OTC

-- 
Ville Syrjälä
Intel OTC