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

From: Alex Deucher
Date: Thu Aug 05 2021 - 00:24:49 EST

On Tue, Aug 3, 2021 at 10:39 PM Zhenneng Li <lizhenneng@xxxxxxxxxx> wrote:
>
>
> 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.
>
> v1->v2:
> Update all of the pitch in all of the page_flip functions
> in radeon rather than just the evergreen one.
>
> 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 +++++++-
> drivers/gpu/drm/radeon/r100.c | 5 +++++
> drivers/gpu/drm/radeon/rs600.c | 8 +++++++-
> drivers/gpu/drm/radeon/rv770.c | 8 +++++++-
> 4 files changed, 26 insertions(+), 3 deletions(-)
>
> 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,
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index ba724198b72e..1268854552ff 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -162,6 +162,7 @@ void r100_wait_for_vblank(struct radeon_device *rdev, int crtc)
> void r100_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;
> u32 tmp = ((u32)crtc_base) | RADEON_CRTC_OFFSET__OFFSET_LOCK;
> int i;
>
> @@ -169,6 +170,10 @@ void r100_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base, bool
> /* update the scanout addresses */
> WREG32(RADEON_CRTC_OFFSET + radeon_crtc->crtc_offset, tmp);
>
> + /* update pitch */
> + WREG32(RADEON_CRTC_PITCH + radeon_crtc->crtc_offset,
> + fb->pitches[0] / fb->format->cpp[0]);
> +

This needs the follow formatting (from radeon_legacy_crtc.c):
pitch_pixels = fb->pitches[0] / fb->format->cpp[0];
crtc_pitch = DIV_ROUND_UP(pitch_pixels * fb->format->cpp[0] * 8,
fb->format->cpp[0] * 8 * 8);
crtc_pitch |= crtc_pitch << 16;
WREG32(RADEON_CRTC_PITCH + radeon_crtc->crtc_offset, crtc_pitch);

With that fixed,
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>

> /* Wait for update_pending to go high. */
> for (i = 0; i < rdev->usec_timeout; i++) {
> if (RREG32(RADEON_CRTC_OFFSET + radeon_crtc->crtc_offset) & RADEON_CRTC_OFFSET__GUI_TRIG_OFFSET)
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index b2d22e25eee1..b87dd551e939 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -41,6 +41,7 @@
>
> #include <drm/drm_device.h>
> #include <drm/drm_vblank.h>
> +#include <drm/drm_fourcc.h>
>
> #include "atom.h"
> #include "radeon.h"
> @@ -118,6 +119,7 @@ void avivo_wait_for_vblank(struct radeon_device *rdev, int crtc)
> void rs600_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;
> u32 tmp = RREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset);
> int i;
>
> @@ -125,9 +127,13 @@ void rs600_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base, boo
> tmp |= AVIVO_D1GRPH_UPDATE_LOCK;
> WREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset, tmp);
>
> - /* update the scanout addresses */
> + /* flip at hsync for async, default is vsync */
> WREG32(AVIVO_D1GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
> async ? AVIVO_D1GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
> + /* update pitch */
> + WREG32(AVIVO_D1GRPH_PITCH + radeon_crtc->crtc_offset,
> + fb->pitches[0] / fb->format->cpp[0]);
> + /* update the scanout addresses */
> WREG32(AVIVO_D1GRPH_SECONDARY_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
> (u32)crtc_base);
> WREG32(AVIVO_D1GRPH_PRIMARY_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
> diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
> index 74499307285b..e592e57be1bb 100644
> --- a/drivers/gpu/drm/radeon/rv770.c
> +++ b/drivers/gpu/drm/radeon/rv770.c
> @@ -32,6 +32,7 @@
>
> #include <drm/drm_device.h>
> #include <drm/radeon_drm.h>
> +#include <drm/drm_fourcc.h>
>
> #include "atom.h"
> #include "avivod.h"
> @@ -809,6 +810,7 @@ u32 rv770_get_xclk(struct radeon_device *rdev)
> void rv770_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;
> u32 tmp = RREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset);
> int i;
>
> @@ -816,9 +818,13 @@ void rv770_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base, boo
> tmp |= AVIVO_D1GRPH_UPDATE_LOCK;
> WREG32(AVIVO_D1GRPH_UPDATE + radeon_crtc->crtc_offset, tmp);
>
> - /* update the scanout addresses */
> + /* flip at hsync for async, default is vsync */
> WREG32(AVIVO_D1GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
> async ? AVIVO_D1GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
> + /* update pitch */
> + WREG32(AVIVO_D1GRPH_PITCH + radeon_crtc->crtc_offset,
> + fb->pitches[0] / fb->format->cpp[0]);
> + /* update the scanout addresses */
> if (radeon_crtc->crtc_id) {
> WREG32(D2GRPH_SECONDARY_SURFACE_ADDRESS_HIGH, upper_32_bits(crtc_base));
> WREG32(D2GRPH_PRIMARY_SURFACE_ADDRESS_HIGH, upper_32_bits(crtc_base));
> --
> 2.25.1
>
> Content-type: Text/plain
>
> No virus found
> Checked by Hillstone Network AntiVirus