On Thu, Jan 05, 2023 at 02:10:45PM +0100, Andrzej Hajda wrote:
> The helper makes the code more compact and readable.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
more cases in this patch where we are now always cleaning the bits,
but as every other place I believe this is the right thing to do.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 49 +++++++---------------
> drivers/gpu/drm/i915/display/intel_fdi.c | 3 +-
> drivers/gpu/drm/i915/display/intel_gmbus.c | 30 +++----------
> 3 files changed, 22 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 1f5a471a0adf27..500dac59a14157 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -644,19 +644,14 @@ int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> struct drm_i915_private *dev_priv = to_i915(dev);
> intel_wakeref_t wakeref;
> int ret = 0;
> - u32 tmp;
>
> wakeref = intel_display_power_get_if_enabled(dev_priv,
> intel_encoder->power_domain);
> if (drm_WARN_ON(dev, !wakeref))
> return -ENXIO;
>
> - tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> - if (enable)
> - tmp |= hdcp_mask;
> - else
> - tmp &= ~hdcp_mask;
> - intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
> + intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder),
> + hdcp_mask, enable ? hdcp_mask : 0);
> intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
> return ret;
> }
> @@ -2200,15 +2195,13 @@ static void intel_ddi_enable_fec(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dp *intel_dp;
> - u32 val;
>
> if (!crtc_state->fec_enable)
> return;
>
> intel_dp = enc_to_intel_dp(encoder);
> - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
> - val |= DP_TP_CTL_FEC_ENABLE;
> - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val);
> + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> + 0, DP_TP_CTL_FEC_ENABLE);
> }
>
> static void intel_ddi_disable_fec_state(struct intel_encoder *encoder,
> @@ -2216,15 +2209,13 @@ static void intel_ddi_disable_fec_state(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dp *intel_dp;
> - u32 val;
>
> if (!crtc_state->fec_enable)
> return;
>
> intel_dp = enc_to_intel_dp(encoder);
> - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
> - val &= ~DP_TP_CTL_FEC_ENABLE;
> - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val);
> + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> + DP_TP_CTL_FEC_ENABLE, 0);
> intel_de_posting_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
> }
>
> @@ -2622,12 +2613,10 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder,
> wait = true;
> }
>
> - if (intel_crtc_has_dp_encoder(crtc_state)) {
> - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
> - val &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> - val |= DP_TP_CTL_LINK_TRAIN_PAT1;
> - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val);
> - }
> + if (intel_crtc_has_dp_encoder(crtc_state))
> + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> + DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK,
> + DP_TP_CTL_LINK_TRAIN_PAT1);
>
> /* Disable FEC in DP Sink */
> intel_ddi_disable_fec_state(encoder, crtc_state);
> @@ -2660,15 +2649,10 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
> if (DISPLAY_VER(dev_priv) >= 12) {
> if (is_mst) {
> enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
> - u32 val;
>
> - val = intel_de_read(dev_priv,
> - TRANS_DDI_FUNC_CTL(cpu_transcoder));
> - val &= ~(TGL_TRANS_DDI_PORT_MASK |
> - TRANS_DDI_MODE_SELECT_MASK);
> - intel_de_write(dev_priv,
> - TRANS_DDI_FUNC_CTL(cpu_transcoder),
> - val);
> + intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder),
> + TGL_TRANS_DDI_PORT_MASK | TRANS_DDI_MODE_SELECT_MASK,
> + 0);
> }
> } else {
> if (!is_mst)
> @@ -3222,12 +3206,9 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp,
> struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> enum port port = encoder->port;
> - u32 val;
>
> - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
> - val &= ~DP_TP_CTL_LINK_TRAIN_MASK;
> - val |= DP_TP_CTL_LINK_TRAIN_IDLE;
> - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val);
> + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> + DP_TP_CTL_LINK_TRAIN_MASK, DP_TP_CTL_LINK_TRAIN_IDLE);
>
> /*
> * Until TGL on PORT_A we can have only eDP in SST mode. There the only
> diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c
> index f62d9a9313498c..02bba5bcc00afc 100644
> --- a/drivers/gpu/drm/i915/display/intel_fdi.c
> +++ b/drivers/gpu/drm/i915/display/intel_fdi.c
> @@ -366,8 +366,7 @@ void intel_fdi_normal_train(struct intel_crtc *crtc)
>
> /* IVB wants error correction enabled */
> if (IS_IVYBRIDGE(dev_priv))
> - intel_de_write(dev_priv, reg,
> - intel_de_read(dev_priv, reg) | FDI_FS_ERRC_ENABLE | FDI_FE_ERRC_ENABLE);
> + intel_de_rmw(dev_priv, reg, 0, FDI_FS_ERRC_ENABLE | FDI_FE_ERRC_ENABLE);
> }
>
> /* The FDI link training functions for ILK/Ibexpeak. */
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 0bc4f6b48e80ae..3ddfc8080ee890 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -215,41 +215,23 @@ intel_gmbus_reset(struct drm_i915_private *i915)
> static void pnv_gmbus_clock_gating(struct drm_i915_private *i915,
> bool enable)
> {
> - u32 val;
> -
> /* When using bit bashing for I2C, this bit needs to be set to 1 */
> - val = intel_de_read(i915, DSPCLK_GATE_D(i915));
> - if (!enable)
> - val |= PNV_GMBUSUNIT_CLOCK_GATE_DISABLE;
> - else
> - val &= ~PNV_GMBUSUNIT_CLOCK_GATE_DISABLE;
> - intel_de_write(i915, DSPCLK_GATE_D(i915), val);
> + intel_de_rmw(i915, DSPCLK_GATE_D(i915), PNV_GMBUSUNIT_CLOCK_GATE_DISABLE,
> + !enable ? PNV_GMBUSUNIT_CLOCK_GATE_DISABLE : 0);
> }
>
> static void pch_gmbus_clock_gating(struct drm_i915_private *i915,
> bool enable)
> {
> - u32 val;
> -
> - val = intel_de_read(i915, SOUTH_DSPCLK_GATE_D);
> - if (!enable)
> - val |= PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;
> - else
> - val &= ~PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;
> - intel_de_write(i915, SOUTH_DSPCLK_GATE_D, val);
> + intel_de_rmw(i915, SOUTH_DSPCLK_GATE_D, PCH_GMBUSUNIT_CLOCK_GATE_DISABLE,
> + !enable ? PCH_GMBUSUNIT_CLOCK_GATE_DISABLE : 0);
> }
>
> static void bxt_gmbus_clock_gating(struct drm_i915_private *i915,
> bool enable)
> {
> - u32 val;
> -
> - val = intel_de_read(i915, GEN9_CLKGATE_DIS_4);
> - if (!enable)
> - val |= BXT_GMBUS_GATING_DIS;
> - else
> - val &= ~BXT_GMBUS_GATING_DIS;
> - intel_de_write(i915, GEN9_CLKGATE_DIS_4, val);
> + intel_de_rmw(i915, GEN9_CLKGATE_DIS_4, BXT_GMBUS_GATING_DIS,
> + !enable ? BXT_GMBUS_GATING_DIS : 0);
> }
>
> static u32 get_reserved(struct intel_gmbus *bus)
> --
> 2.34.1
>
