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

Mika Kahola
2016-08-11T09:43:39Z

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.

Cheers,
Mika
>
> > +
> > + return min(dotclk, ds_dotclk);
> > +}
> > +
> > static enum drm_mode_status
> > intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> > @@ -199,7 +219,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> > int target_clock = mode->clock;
> > int max_rate, mode_rate, max_lanes, max_link_clock;
> > - int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> > + int max_dotclk;
> > +
> > + max_dotclk = intel_dp_downstream_max_dotclock(intel_dp,
> > + to_i915(connector->dev)->max_dotclk_freq);
> >
> > if (is_edp(intel_dp) && fixed_mode) {
> > if (mode->hdisplay > fixed_mode->hdisplay)
> > --
> > 1.9.1