Re: [PATCH 02/13] drm/i915: Store the converted link rates in intel_dp->supported_rates[]

From: Todd Previte <tprevite@gmail.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 02/13] drm/i915: Store the converted link rates in intel_dp->supported_rates[]
Date: Fri, 13 Mar 2015 15:58:55 -0700	[thread overview]
Message-ID: <55036BAF.1050707@gmail.com> (raw)
In-Reply-To: <1426173039-24584-3-git-send-email-ville.syrjala@linux.intel.com>


On 3/12/2015 8:10 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> No point in converting from hardware format every single time, just
> store the rates in the final format under intel_dp.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c  | 33 +++++++++++++++++++--------------
>   drivers/gpu/drm/i915/intel_drv.h |  3 ++-
>   2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1fa8cc1..d638f5e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1141,8 +1141,6 @@ static int
>   intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> -	int i = 0;
> -	uint16_t val;
>   
>   	if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
>   		/*
> @@ -1150,18 +1148,12 @@ intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
>   		 * link rate table method, so read link rates from
>   		 * supported_link_rates
>   		 */
> -		for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i) {
> -			val = le16_to_cpu(intel_dp->supported_rates[i]);
> -			if (val == 0)
> -				break;
> -
> -			sink_rates[i] = val * 200;
> -		}
> +		memcpy(sink_rates, intel_dp->supported_rates,
> +		       sizeof(intel_dp->supported_rates));
>   
> -		if (i <= 0)
> -			DRM_ERROR("No rates in SUPPORTED_LINK_RATES");
> +		return intel_dp->num_supported_rates;
>   	}
> -	return i;
> +	return 0;
>   }
>   
>   static int
> @@ -3751,10 +3743,23 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>   	    (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &	DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
>   	    (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) &&
>   	    (rev >= 0x03)) { /* eDp v1.4 or higher */
> +		__le16 supported_rates[DP_MAX_SUPPORTED_RATES];
> +		int i;
> +
>   		intel_dp_dpcd_read_wake(&intel_dp->aux,
>   				DP_SUPPORTED_LINK_RATES,
> -				intel_dp->supported_rates,
> -				sizeof(intel_dp->supported_rates));
> +				supported_rates,
> +				sizeof(supported_rates));
> +
> +		for (i = 0; i < ARRAY_SIZE(supported_rates); i++) {
> +			int val = le16_to_cpu(supported_rates[i]);
> +
> +			if (val == 0)
> +				break;
> +
> +			intel_dp->supported_rates[i] = val * 200;
> +		}
> +		intel_dp->num_supported_rates = i;
>   	}
>   	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>   	      DP_DWN_STRM_PORT_PRESENT))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c77128c..69c8437 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -627,7 +627,8 @@ struct intel_dp {
>   	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>   	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>   	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> -	__le16 supported_rates[DP_MAX_SUPPORTED_RATES];
> +	uint8_t num_supported_rates;
> +	int supported_rates[DP_MAX_SUPPORTED_RATES];
>   	struct drm_dp_aux aux;
>   	uint8_t train_set[4];
>   	int panel_power_up_delay;
The code looks good from here. Only thing to double check is where 
intel_read_sink_rates() is called and make sure it's now expecting 0 as 
a success case.

Reviewed-by: Todd Previte <tprevite@gmail.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx