Re: [PATCH v2 13/13] drm/i915/dp: use readb and writeb calls for single byte DPCD access

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

On Fri, Feb 03, 2017 at 04:19:36PM +0200, Jani Nikula wrote:
> This is what we have the readb and writeb variants for. Do some minor
> return value and variable cleanup while at it.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 90ae95f2ecb2..fdd4abfc2380 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3677,9 +3677,9 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  		uint8_t frame_sync_cap;
>  
>  		dev_priv->psr.sink_support = true;
> -		drm_dp_dpcd_read(&intel_dp->aux,
> -				 DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> -				 &frame_sync_cap, 1);
> +		drm_dp_dpcd_readb(&intel_dp->aux,
> +				  DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> +				  &frame_sync_cap);
>  		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
>  		/* PSR2 needs frame sync as well */
>  		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> @@ -3749,8 +3749,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	intel_dp_set_sink_rates(intel_dp);
>  	intel_dp_set_common_rates(intel_dp);
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT,
> -			     &intel_dp->sink_count, 1) < 0)
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT,
> +			      &intel_dp->sink_count) <= 0)

As an additional change it would be nice to have a local variable
for this since we'll do '->sink_count = ->sink_count & SOMETHING' 
further down. IMO it's somewhat confusing to first read the unmasked
valuedirectly into the final location and then mask in place.

>  		return false;
>  
>  	/*
> @@ -3787,7 +3787,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  static bool
>  intel_dp_can_mst(struct intel_dp *intel_dp)
>  {
> -	u8 buf[1];
> +	u8 mstm_cap;
>  
>  	if (!i915.enable_dp_mst)
>  		return false;
> @@ -3798,10 +3798,10 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
>  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
>  		return false;
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1) != 1)
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_MSTM_CAP, &mstm_cap) != 1)
>  		return false;
>  
> -	return buf[0] & DP_MST_CAP;
> +	return mstm_cap & DP_MST_CAP;
>  }
>  
>  static void
> @@ -3947,9 +3947,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  static bool
>  intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
>  {
> -	return drm_dp_dpcd_read(&intel_dp->aux,
> -				       DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				       sink_irq_vector, 1) == 1;
> +	return drm_dp_dpcd_readb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
> +				 sink_irq_vector) == 1;
>  }
>  
>  static bool
> @@ -4012,13 +4011,13 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp)
>  static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>  {
>  	uint8_t test_pattern;
> -	uint16_t test_misc;
> +	uint8_t test_misc;
>  	__be16 h_width, v_height;
>  	int status = 0;
>  
>  	/* Read the TEST_PATTERN (DP CTS 3.1.5) */
> -	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN,
> -				  &test_pattern, 1);
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_PATTERN,
> +				   &test_pattern);
>  	if (status <= 0) {
>  		DRM_DEBUG_KMS("Test pattern read failed\n");
>  		return DP_TEST_NAK;
> @@ -4040,8 +4039,8 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>  		return DP_TEST_NAK;
>  	}
>  
> -	status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC0,
> -				  &test_misc, 1);
> +	status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_MISC0,
> +				   &test_misc);
>  	if (status <= 0) {
>  		DRM_DEBUG_KMS("TEST MISC read failed\n");
>  		return DP_TEST_NAK;
> @@ -4100,10 +4099,8 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
>  		 */
>  		block += intel_connector->detect_edid->extensions;
>  
> -		if (!drm_dp_dpcd_write(&intel_dp->aux,
> -					DP_TEST_EDID_CHECKSUM,
> -					&block->checksum,
> -					1))
> +		if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM,
> +				       block->checksum) <= 0)
>  			DRM_DEBUG_KMS("Failed to write EDID checksum\n");
>  
>  		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> -- 
> 2.1.4

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