LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <christian.koenig@amd.com>
Cc: Zhenneng Li <lizhenneng@kylinos.cn>,
	Alex Deucher <alexander.deucher@amd.com>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/radeon: Update pitch for page flip
Date: Mon, 2 Aug 2021 10:31:24 +0200	[thread overview]
Message-ID: <YQetXMaASz/F2EyS@phenom.ffwll.local> (raw)
In-Reply-To: <e6e77cfb-4e6b-c30e-ae7c-ac84b82c9a75@amd.com>

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

  parent reply	other threads:[~2021-08-02  8:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02  7:43 Zhenneng Li
     [not found] ` <e6e77cfb-4e6b-c30e-ae7c-ac84b82c9a75@amd.com>
2021-08-02  8:31   ` Daniel Vetter [this message]
2021-08-02 14:51     ` Alex Deucher
2021-08-03  8:00       ` 李真能
2021-08-03  8:34       ` Michel Dänzer
2021-08-03 14:49         ` Alex Deucher
2021-08-04 15:46           ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YQetXMaASz/F2EyS@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Xinhui.Pan@amd.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizhenneng@kylinos.cn \
    --subject='Re: [PATCH] drm/radeon: Update pitch for page flip' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).