On Thu, Jan 05, 2023 at 02:10:43PM +0100, Andrzej Hajda wrote: > The helper makes the code more compact and readable. > > Signed-off-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/g4x_hdmi.c | 8 ++--- > drivers/gpu/drm/i915/display/intel_hdcp.c | 15 ++++----- > drivers/gpu/drm/i915/display/intel_hdmi.c | 40 +++++++---------------- > 3 files changed, 22 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c > index c3580d96765c6c..f58849b416ea89 100644 > --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c > +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c > @@ -271,8 +271,8 @@ static void cpt_enable_hdmi(struct intel_atomic_state *state, > */ > > if (pipe_config->pipe_bpp > 24) { > - intel_de_write(dev_priv, TRANS_CHICKEN1(pipe), > - intel_de_read(dev_priv, TRANS_CHICKEN1(pipe)) | TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE); > + intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe), > + 0, TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE); > > temp &= ~SDVO_COLOR_FORMAT_MASK; > temp |= SDVO_COLOR_FORMAT_8bpc; > @@ -288,8 +288,8 @@ static void cpt_enable_hdmi(struct intel_atomic_state *state, > intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp); > intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg); > > - intel_de_write(dev_priv, TRANS_CHICKEN1(pipe), > - intel_de_read(dev_priv, TRANS_CHICKEN1(pipe)) & ~TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE); > + intel_de_rmw(dev_priv, TRANS_CHICKEN1(pipe), > + TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE, 0); > } > > drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio && > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 6406fd487ee524..2984d2810e42cc 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -943,8 +943,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector) > > repeater_ctl = intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder, > port); > - intel_de_write(dev_priv, HDCP_REP_CTL, > - intel_de_read(dev_priv, HDCP_REP_CTL) & ~repeater_ctl); > + intel_de_rmw(dev_priv, HDCP_REP_CTL, repeater_ctl, 0); > > ret = hdcp->shim->toggle_signalling(dig_port, cpu_transcoder, false); > if (ret) { > @@ -1819,12 +1818,10 @@ static int hdcp2_enable_encryption(struct intel_connector *connector) > } > > if (intel_de_read(dev_priv, HDCP2_STATUS(dev_priv, cpu_transcoder, port)) & > - LINK_AUTH_STATUS) { > + LINK_AUTH_STATUS) > /* Link is Authenticated. Now set for Encryption */ > - intel_de_write(dev_priv, > - HDCP2_CTL(dev_priv, cpu_transcoder, port), > - intel_de_read(dev_priv, HDCP2_CTL(dev_priv, cpu_transcoder, port)) | CTL_LINK_ENCRYPTION_REQ); > - } > + intel_de_rmw(dev_priv, HDCP2_CTL(dev_priv, cpu_transcoder, port), > + 0, CTL_LINK_ENCRYPTION_REQ); > > ret = intel_de_wait_for_set(dev_priv, > HDCP2_STATUS(dev_priv, cpu_transcoder, > @@ -1848,8 +1845,8 @@ static int hdcp2_disable_encryption(struct intel_connector *connector) > drm_WARN_ON(&dev_priv->drm, !(intel_de_read(dev_priv, HDCP2_STATUS(dev_priv, cpu_transcoder, port)) & > LINK_ENCRYPTION_STATUS)); > > - intel_de_write(dev_priv, HDCP2_CTL(dev_priv, cpu_transcoder, port), > - intel_de_read(dev_priv, HDCP2_CTL(dev_priv, cpu_transcoder, port)) & ~CTL_LINK_ENCRYPTION_REQ); > + intel_de_rmw(dev_priv, HDCP2_CTL(dev_priv, cpu_transcoder, port), > + CTL_LINK_ENCRYPTION_REQ, 0); > > ret = intel_de_wait_for_clear(dev_priv, > HDCP2_STATUS(dev_priv, cpu_transcoder, > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c > index efa2da080f62d4..4b09f17aa4b23b 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -237,15 +237,11 @@ static void g4x_read_infoframe(struct intel_encoder *encoder, > void *frame, ssize_t len) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > - u32 val, *data = frame; > + u32 *data = frame; > int i; > > - val = intel_de_read(dev_priv, VIDEO_DIP_CTL); > - > - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ A probably good follow-up clean up would be to define the missing masks and remove the hardcoded things like this 0xf. And also something that I had noticed on the previous patches but I forgot to mention: it would be good as a followup to replace the local value << shift per FIELD_PREP() helpers and remove the shift definitions... But really nothing related directly with this patch. For this: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> Oh, and I also noticed that CI didn't return yet for these patches... https://patchwork.freedesktop.org/series/112438/ a strange delay... I will probably hit the retest if we don't get anything by the end of the day today. > - val |= g4x_infoframe_index(type); > - > - intel_de_write(dev_priv, VIDEO_DIP_CTL, val); > + intel_de_rmw(dev_priv, VIDEO_DIP_CTL, > + VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); > > for (i = 0; i < len; i += 4) > *data++ = intel_de_read(dev_priv, VIDEO_DIP_DATA); > @@ -313,15 +309,11 @@ static void ibx_read_infoframe(struct intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - u32 val, *data = frame; > + u32 *data = frame; > int i; > > - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); > - > - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ > - val |= g4x_infoframe_index(type); > - > - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); > + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), > + VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); > > for (i = 0; i < len; i += 4) > *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); > @@ -395,15 +387,11 @@ static void cpt_read_infoframe(struct intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - u32 val, *data = frame; > + u32 *data = frame; > int i; > > - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); > - > - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ > - val |= g4x_infoframe_index(type); > - > - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); > + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), > + VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); > > for (i = 0; i < len; i += 4) > *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); > @@ -471,15 +459,11 @@ static void vlv_read_infoframe(struct intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - u32 val, *data = frame; > + u32 *data = frame; > int i; > > - val = intel_de_read(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe)); > - > - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ > - val |= g4x_infoframe_index(type); > - > - intel_de_write(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), val); > + intel_de_rmw(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), > + VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); > > for (i = 0; i < len; i += 4) > *data++ = intel_de_read(dev_priv, > -- > 2.34.1 >