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. -Daniel > > Thanks, > Christian. > > > > > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > > Cc: "Christian König" <christian.koenig@xxxxxxx> > > Cc: "Pan, Xinhui" <Xinhui.Pan@xxxxxxx> > > Cc: David Airlie <airlied@xxxxxxxx> > > Cc: Daniel Vetter <daniel@xxxxxxxx> > > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > > Cc: linux-kernel@xxxxxxxxxxxxxxx > > Signed-off-by: Zhenneng Li <lizhenneng@xxxxxxxxxx> > > --- > > 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