On Mon, 09 Jan 2023, Andrzej Hajda <andrzej.hajda@xxxxxxxxx> 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 <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/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.
>
> 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,
>>> --
>>> 2.34.1
>>>
>
--
Jani Nikula, Intel Open Source Graphics Center
