Re: [RFC 11/12] drm/i915: Remove I915_READ_NOTRACE

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 07 Jun 2019, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Only a few call sites remain which have been converted to uncore mmio
> accessors and so the macro can be removed.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/gvt/debugfs.c  |  4 +--
>  drivers/gpu/drm/i915/gvt/firmware.c |  5 ++--
>  drivers/gpu/drm/i915/i915_drv.c     |  6 ++--
>  drivers/gpu/drm/i915/i915_drv.h     |  1 -
>  drivers/gpu/drm/i915/i915_pmu.c     |  8 ++++--
>  drivers/gpu/drm/i915/intel_dp.c     | 43 +++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_gmbus.c  | 11 ++++----
>  7 files changed, 43 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c b/drivers/gpu/drm/i915/gvt/debugfs.c
> index 8a9606f91e68..2fb7b73b260d 100644
> --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> @@ -58,12 +58,12 @@ static int mmio_offset_compare(void *priv,
>  static inline int mmio_diff_handler(struct intel_gvt *gvt,
>  				    u32 offset, void *data)
>  {
> -	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	struct drm_i915_private *i915 = gvt->dev_priv;
>  	struct mmio_diff_param *param = data;
>  	struct diff_mmio *node;
>  	u32 preg, vreg;
>  
> -	preg = I915_READ_NOTRACE(_MMIO(offset));
> +	preg = intel_uncore_read_notrace(&i915->uncore, _MMIO(offset));
>  	vreg = vgpu_vreg(param->vgpu, offset);
>  
>  	if (preg != vreg) {
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
> index 4ac18b447247..049775e8e350 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -68,9 +68,10 @@ static struct bin_attribute firmware_attr = {
>  
>  static int mmio_snapshot_handler(struct intel_gvt *gvt, u32 offset, void *data)
>  {
> -	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	struct drm_i915_private *i915 = gvt->dev_priv;
>  
> -	*(u32 *)(data + offset) = I915_READ_NOTRACE(_MMIO(offset));
> +	*(u32 *)(data + offset) = intel_uncore_read_notrace(&i915->uncore,
> +							    _MMIO(offset));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1af6751e1b36..81ff2c78fd55 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2708,7 +2708,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  	I915_WRITE(VLV_GUNIT_CLOCK_GATE2,	s->clock_gate_dis2);
>  }
>  
> -static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
> +static int vlv_wait_for_pw_status(struct drm_i915_private *i915,
>  				  u32 mask, u32 val)
>  {
>  	i915_reg_t reg = VLV_GTLC_PW_STATUS;
> @@ -2722,7 +2722,9 @@ static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
>  	 * Transitioning between RC6 states should be at most 2ms (see
>  	 * valleyview_enable_rps) so use a 3ms timeout.
>  	 */
> -	ret = wait_for(((reg_value = I915_READ_NOTRACE(reg)) & mask) == val, 3);
> +	ret = wait_for(((reg_value =
> +			 intel_uncore_read_notrace(&i915->uncore, reg)) & mask)
> +		       == val, 3);
>  
>  	/* just trace the final value */
>  	trace_i915_reg_rw(false, reg, reg_value, sizeof(reg_value), true);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1ea2d5f42e52..9f0208e410ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2857,7 +2857,6 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>  
>  #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
>  #define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
> -#define I915_READ_NOTRACE(reg__)	 __I915_REG_OP(read_notrace, dev_priv, (reg__))
>  
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
>  
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 1ccda0ee4ff5..eb9c0e0e545c 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -227,9 +227,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
>  		if (dev_priv->gt.awake) {
>  			intel_wakeref_t wakeref;
>  
> -			with_intel_runtime_pm_if_in_use(dev_priv, wakeref)
> -				val = intel_get_cagf(dev_priv,
> -						     I915_READ_NOTRACE(GEN6_RPSTAT1));
> +			with_intel_runtime_pm_if_in_use(dev_priv, wakeref) {
> +				val = intel_uncore_read_notrace(&dev_priv->uncore,
> +								GEN6_RPSTAT1);
> +				val = intel_get_cagf(dev_priv, val);
> +			}
>  		}
>  
>  		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT],
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b099a9dc28fd..0862678e68b9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1082,13 +1082,13 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
>  static u32
>  intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>  {
> -	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
>  	u32 status;
>  	bool done;
>  
> -#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> -	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> +#define C (((status = intel_uncore_read_notrace(&i915->uncore, ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> +	done = wait_event_timeout(i915->gmbus_wait_queue, C,
>  				  msecs_to_jiffies_timeout(10));
>  
>  	/* just trace the final value */
> @@ -1221,8 +1221,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		  u32 aux_send_ctl_flags)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_i915_private *dev_priv =
> +	struct drm_i915_private *i915 =
>  			to_i915(intel_dig_port->base.base.dev);
> +	struct intel_uncore *uncore = &i915->uncore;
>  	i915_reg_t ch_ctl, ch_data[5];
>  	u32 aux_clock_divider;
>  	enum intel_display_power_domain aux_domain =
> @@ -1238,7 +1239,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	for (i = 0; i < ARRAY_SIZE(ch_data); i++)
>  		ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
>  
> -	aux_wakeref = intel_display_power_get(dev_priv, aux_domain);
> +	aux_wakeref = intel_display_power_get(i915, aux_domain);
>  	pps_wakeref = pps_lock(intel_dp);
>  
>  	/*
> @@ -1253,13 +1254,13 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  	 * lowest possible wakeup latency and so prevent the cpu from going into
>  	 * deep sleep states.
>  	 */
> -	pm_qos_update_request(&dev_priv->pm_qos, 0);
> +	pm_qos_update_request(&i915->pm_qos, 0);
>  
>  	intel_dp_check_edp(intel_dp);
>  
>  	/* Try to wait for any previous AUX channel activity */
>  	for (try = 0; try < 3; try++) {
> -		status = I915_READ_NOTRACE(ch_ctl);
> +		status = intel_uncore_read_notrace(uncore, ch_ctl);
>  		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>  			break;
>  		msleep(1);
> @@ -1269,7 +1270,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  
>  	if (try == 3) {
>  		static u32 last_status = -1;
> -		const u32 status = I915_READ(ch_ctl);
> +		const u32 status = intel_uncore_read(uncore, ch_ctl);
>  
>  		if (status != last_status) {
>  			WARN(1, "dp_aux_ch not started status 0x%08x\n",
> @@ -1298,21 +1299,23 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		for (try = 0; try < 5; try++) {
>  			/* Load the send data into the aux channel data registers */
>  			for (i = 0; i < send_bytes; i += 4)
> -				I915_WRITE(ch_data[i >> 2],
> -					   intel_dp_pack_aux(send + i,
> -							     send_bytes - i));
> +				intel_uncore_write(uncore,
> +						   ch_data[i >> 2],
> +						   intel_dp_pack_aux(send + i,
> +								     send_bytes - i));
>  
>  			/* Send the command and wait for it to complete */
> -			I915_WRITE(ch_ctl, send_ctl);
> +			intel_uncore_write(uncore, ch_ctl, send_ctl);
>  
>  			status = intel_dp_aux_wait_done(intel_dp);
>  
>  			/* Clear done status and any errors */
> -			I915_WRITE(ch_ctl,
> -				   status |
> -				   DP_AUX_CH_CTL_DONE |
> -				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -				   DP_AUX_CH_CTL_RECEIVE_ERROR);
> +			intel_uncore_write(uncore,
> +					   ch_ctl,
> +					   status |
> +					   DP_AUX_CH_CTL_DONE |
> +					   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +					   DP_AUX_CH_CTL_RECEIVE_ERROR);
>  
>  			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
>  			 *   400us delay required for errors and timeouts
> @@ -1375,18 +1378,18 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		recv_bytes = recv_size;
>  
>  	for (i = 0; i < recv_bytes; i += 4)
> -		intel_dp_unpack_aux(I915_READ(ch_data[i >> 2]),
> +		intel_dp_unpack_aux(intel_uncore_read(uncore, ch_data[i >> 2]),
>  				    recv + i, recv_bytes - i);
>  
>  	ret = recv_bytes;
>  out:
> -	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
> +	pm_qos_update_request(&i915->pm_qos, PM_QOS_DEFAULT_VALUE);
>  
>  	if (vdd)
>  		edp_panel_vdd_off(intel_dp, false);
>  
>  	pps_unlock(intel_dp, pps_wakeref);
> -	intel_display_power_put_async(dev_priv, aux_domain, aux_wakeref);
> +	intel_display_power_put_async(i915, aux_domain, aux_wakeref);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_gmbus.c b/drivers/gpu/drm/i915/intel_gmbus.c
> index 5e4c543e82e8..aa88e6e7cc65 100644
> --- a/drivers/gpu/drm/i915/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/intel_gmbus.c
> @@ -186,14 +186,15 @@ static void bxt_gmbus_clock_gating(struct drm_i915_private *dev_priv,
>  
>  static u32 get_reserved(struct intel_gmbus *bus)
>  {
> -	struct drm_i915_private *dev_priv = bus->dev_priv;
> +	struct drm_i915_private *i915 = bus->dev_priv;
> +	struct intel_uncore *uncore = &i915->uncore;
>  	u32 reserved = 0;
>  
>  	/* On most chips, these bits must be preserved in software. */
> -	if (!IS_I830(dev_priv) && !IS_I845G(dev_priv))
> -		reserved = I915_READ_NOTRACE(bus->gpio_reg) &
> -					     (GPIO_DATA_PULLUP_DISABLE |
> -					      GPIO_CLOCK_PULLUP_DISABLE);
> +	if (!IS_I830(i915) && !IS_I845G(i915))
> +		reserved = intel_uncore_read_notrace(uncore, bus->gpio_reg) &
> +			   (GPIO_DATA_PULLUP_DISABLE |
> +			    GPIO_CLOCK_PULLUP_DISABLE);
>  
>  	return reserved;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx