Re: [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Aug 11, 2016 at 12:43:39PM +0300, Mika Kahola wrote:
> On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> > On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > > 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.
> > > 
> > > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > >     function (Daniel)
> > > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> > > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > >     Code restructuring (Ville)
> > > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > > 
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 25 ++++++++++++++++++++++++-
> > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 21b04c3..e990c8b 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > >  	return (max_link_clock * max_lanes * 8) / 10;
> > >  }
> > >  
> > > +static int
> > > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
> > > +{
> > 
> > I would just
> > 
> > {
> > 	int max_dotclk = dev_priv->max_dotclk_freq;
> > 
> > 	ds_max_dotclk = ...;
> > 	if (ds_dotclk != 0)
> > 		max_dotclk = min(max_dotclk, ds_max_dotclk);
> > 
> > 	return max_dotclk;
> > }
> > 
> > > +	int ds_dotclk;
> > > +	int type;
> > > +
> > > +	ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > > +						intel_dp->downstream_ports);
> > > +
> > > +	if (ds_dotclk == 0)
> > > +		return dotclk;
> > > +
> > > +	type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > > +	
> > > +	if (type != DP_DS_PORT_TYPE_VGA)
> > > +		return dotclk;
> > 
> > Why isn't drm_dp_downstream_max_clock() handling all of it already?
> > Why are we even checking for !=VGA?
> The routine drm_dp_downstream_max_clock returns the clock rate which can
> be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
> need to have a check for this as we are only interested to update VGA
> dotclock value.

We should handle it all. Actually I'm not even sure how we're supposed
to deal with the downstream port max TMDS clock since for HDMI that
depends on the bpc, but since this is about a DP->HDMI conversion, I
don't know if we have to take the downstream port max TMDS clock into
account when choosing the bpc over the DP link as well. I suppose that's
possible if the dongle can't change change the bpc, and instead just
passes things through. I think this is one of those places where the
DP spec is way too unclear. But for DP->VGA there is no clock going out
the other end, so it must be just about the limits of the DP input or
the DAC.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel