Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible

2023-01-09 Thread Jani Nikula 
On Mon, 09 Jan 2023, Andrzej Hajda  wrote:
> On 06.01.2023 16:35, Rodrigo Vivi wrote:
>> 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 
>>> ---
>>>   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/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 
>> 
>> 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.
>
> Thx for reviews.
> Apparently CI missed this series, I have just hit retest.

Cc: Michal and Mateusz

This has been happening a lot lately.

BR,
Jani.


>
> Regards
> Andrzej
>
>
>> 
>>> -   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,

Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible

2023-01-09 Thread Andrzej Hajda 

On 06.01.2023 16:35, Rodrigo Vivi wrote:

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 
---
  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/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 

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.


Thx for reviews.
Apparently CI missed this series, I have just hit retest.

Regards
Andrzej





-   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

Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible

2023-01-06 Thread Rodrigo Vivi 
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 
> ---
>  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(_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(_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

[Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible

2023-01-05 Thread Andrzej Hajda 
The helper makes the code more compact and readable.

Signed-off-by: Andrzej Hajda 
---
 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(_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(_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 */
-   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,