Re: [PATCH] drm/radeon: Update pitch for page flip

[李真能 <lizhenneng@kylinos.cn>]

[-- Attachment #1: Type: text/plain, Size: 10354 bytes --]


在 2021/8/2 下午10:51, Alex Deucher 写道:
> On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote:
>>> Am 02.08.21 um 09:43 schrieb Zhenneng Li:
>>>> When primary bo is updated, crtc's pitch may
>>>> have not been updated, this will lead to show
>>>> disorder content when user changes display mode,
>>>> we update crtc's pitch in page flip to avoid
>>>> this bug.
>>>> This refers to amdgpu's pageflip.
>>> Alex is the expert to ask about that code, but I'm not sure if that is
>>> really correct for the old hardware.
>>>
>>> As far as I know the crtc's pitch should not change during a page flip, but
>>> only during a full mode set.
>>>
>>> So could you elaborate a bit more how you trigger this?
>> legacy page_flip ioctl only verifies that the fb->format stays the same.
>> It doesn't check anything else (afair never has), this is all up to
>> drivers to verify.
>>
>> Personally I'd say add a check to reject this, since testing this and
>> making sure it really works everywhere is probably a bit much on this old
>> hw.
> If just the pitch changed, that probably wouldn't be much of a
> problem, but if the pitch is changing, that probably implies other
> stuff has changed as well and we'll just be chasing changes.  I agree
> it would be best to just reject anything other than updating the
> scanout address.
Thanks for your reply!

  In theory , pitch is updated only when the scanout address is 
updated,  but we can trigger this bug on r5230 when using two screens 
and changing modeset,  you can execute shell:


#!/bin/bash

cnt=0;
while [ 1 ]
do
xrandr --output HDMI-0 --right-of VGA-0 --auto
echo "hdmi----vga"
sleep  8
xrandr --output VGA-0 --auto --output HDMI-0 --off
sleep 3
let cnt+=1;
echo $cnt
done

at the same time, open a new terminal, press F11 to fast switch this 
window between maximize size and normal size to improve the 
possibility，attachement picture shows the phenomenon.

I add some debug messages:

[   89.065206] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0xffffffa0e5956100, set->fb: 0xffffffa0e701e400
[   89.065209] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:601
[   89.065213] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 1, mode_changed: 0
[   89.065216] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1148, x: 0: 
y: 0, crtc: 0xffffffa0e62dc000, atomic: 0
[   89.065222] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1426, 
fb_location: 0x117f2000, target_fb->width: 1920, target_fb->height: 1080
[   89.065223] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1430, 
fb_pitch_pixels: 1920, target_fb->pitches[0]: 7680, 
target_fb->format->cpp[0]: 4
[   89.065240] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************
[   92.079288] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0xffffffa0e701e400, set->fb: 0xffffffa0eafced00
[   92.079291] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:601
[   92.079295] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 1, mode_changed: 0
[   92.079298] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1148, x: 0: 
y: 0, crtc: 0xffffffa0e62dc000, atomic: 0
[   92.079304] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1426, 
fb_location: 0x10000000, target_fb->width: 3840, target_fb->height: 1080
[   92.079306] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1430, 
fb_pitch_pixels: 3840, target_fb->pitches[0]: 15360, 
target_fb->format->cpp[0]: 4
[   92.079323] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************
[   92.080382] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0x0, set->fb: 0xffffffa0eafced00
[   92.080384] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:595
[   92.080387] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 0, mode_changed: 1
[   92.080389] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:715
[   92.083143] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1148, x: 
1920: y: 0, crtc: 0xffffffa0e62db000, atomic: 0
[   92.083149] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1426, 
fb_location: 0x10000000, target_fb->width: 3840, target_fb->height: 1080
[   92.083164] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1430, 
fb_pitch_pixels: 3840, target_fb->pitches[0]: 15360, 
target_fb->format->cpp[0]: 4
[   92.120595] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************
[  100.157744] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0xffffffa0eafced00, set->fb: 0xffffffa0f3a5bb00
[  100.157746] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:601
[  100.157749] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 1, mode_changed: 0
[  100.157752] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1148, x: 0: 
y: 0, crtc: 0xffffffa0e62dc000, atomic: 0
[  100.157758] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1426, 
fb_location: 0x1790c000, target_fb->width: 1920, target_fb->height: 1080
[  100.157760] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1430, 
fb_pitch_pixels: 1920, target_fb->pitches[0]: 7680, 
target_fb->format->cpp[0]: 4
[  100.157777] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************
[  103.166751] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0xffffffa0e9a0cd00, set->fb: 0xffffffa0e9a0cd00
[  103.166757] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 0, mode_changed: 0
[  103.166758] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************
[  103.169353] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:589, 
set->crtc->primary->fb: 0x0, set->fb: 0xffffffa0e9a0cd00
[  103.169355] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:595
[  103.169359] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:713, 
fb_changed: 0, mode_changed: 1
[  103.169360] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:715
[  103.170396] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1148, x: 
1920: y: 0, crtc: 0xffffffa0e62db000, atomic: 0
[  103.170403] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1426, 
fb_location: 0x11fea000, target_fb->width: 3840, target_fb->height: 1080
[  103.170404] lzn debug, 
drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base:1430, 
fb_pitch_pixels: 3840, target_fb->pitches[0]: 15360, 
target_fb->format->cpp[0]: 4
[  103.224592] lzn debug, 
drivers/gpu/drm/drm_crtc_helper.c:drm_crtc_helper_set_config:752*****************************************************************

After 103s of os started,  bug is triggerd, fb_changed and mode_changed 
are zero, and crtc's pitch has not been updated.

Amdgpu has the same problem,  but amdgpu's page_flip will update pitch, 
so this bug can't trigger on amdgpu.

you can refer to dce_v6_0_page_flip in amdgpu，all amdgpu drivers update 
pitch in page_flip.

test os: focal-desktop-arm64.iso

Zhenneng Li
>
> Alex
>
>> -Daniel
>>
>>> Thanks,
>>> Christian.
>>>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Cc: "Christian König" <christian.koenig@amd.com>
>>>> Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
>>>> Cc: David Airlie <airlied@linux.ie>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>> Cc: amd-gfx@lists.freedesktop.org
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Zhenneng Li <lizhenneng@kylinos.cn>
>>>> ---
>>>>    drivers/gpu/drm/radeon/evergreen.c | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
>>>> index 36a888e1b179..eeb590d2dec2 100644
>>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>>> @@ -28,6 +28,7 @@
>>>>    #include <drm/drm_vblank.h>
>>>>    #include <drm/radeon_drm.h>
>>>> +#include <drm/drm_fourcc.h>
>>>>    #include "atom.h"
>>>>    #include "avivod.h"
>>>> @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base,
>>>>                       bool async)
>>>>    {
>>>>      struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
>>>> +   struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
>>>> -   /* update the scanout addresses */
>>>> +   /* flip at hsync for async, default is vsync */
>>>>      WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
>>>>             async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
>>>> +   /* update pitch */
>>>> +   WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset,
>>>> +          fb->pitches[0] / fb->format->cpp[0]);
>>>> +   /* update the scanout addresses */
>>>>      WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset,
>>>>             upper_32_bits(crtc_base));
>>>>      WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
>>>>
>>>> No virus found
>>>>              Checked by Hillstone Network AntiVirus
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

[-- Attachment #2: r5230.jpg --]
[-- Type: image/jpeg, Size: 2956751 bytes --]