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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Mika Kahola <mika.kahola@intel.com>
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
	jim.bride@linux.intel.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle
Date: Thu, 11 Aug 2016 10:18:37 +0300	[thread overview]
Message-ID: <20160811071837.GC4329@intel.com> (raw)
In-Reply-To: <1470661230-15988-4-git-send-email-mika.kahola@intel.com>

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?

> +
> +	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

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