[PATCH] Only align screen / scanout pixmap height where necessary

Stephen Chandler Paul cpaul at redhat.com
Fri Oct 2 07:04:16 PDT 2015


Yep, patch works fine here in both EXA and glamor. Tested with a Radeon
7750 and 6850.

Cheers,
	Lyude

On Fri, 2015-10-02 at 18:11 +0900, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> When using glamor acceleration, the pixmap's header has to have a
> height
> that matches exactly what the actual height is minus the GPU memory
> alignment. Otherwise CRTCs scanning out from the main scanout buffer
> (e.g. every CRTC that isn't rotated or transformed in some way) won't
> always work. This results in a bug where rotating one monitor in a
> multi-monitor setup won't always work properly. Easiest way to
> reproduce
> this:
> 
> - Have two monitors (I've gotten this working with a 1920x1080 and
>   1280x1024, along with two 1920x1080s)
> - Rotate one of them from 0° to 90°, then rotate the same monitor
> from
>   90° to 180°. The monitor that hasn't been rotated won't properly
>   update, and will stay on a blank screen
> 
> This doesn't seem to make any difference when using EXA for
> acceleration.
> 
> Compared to Stephen Chandler's patch, this drops the height alignment
> in most places and only keeps it where it's really necessary.
> 
> Reported-by: Stephen Chandler Paul <cpaul at redhat.com>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
> 
> Stephen Chandler,
> 
> thanks for the good catch, but I think we can take this even further.
> Does this fix the problem for you as well?
> 
>  src/drmmode_display.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index efc35f0..623124e 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -525,6 +525,7 @@ drmmode_crtc_scanout_allocate(xf86CrtcPtr crtc,
>  	RADEONInfoPtr info = RADEONPTR(pScrn);
>  	drmmode_crtc_private_ptr drmmode_crtc = crtc
> ->driver_private;
>  	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> +	int aligned_height;
>  	int size;
>  	int ret;
>  	unsigned long rotate_pitch;
> @@ -547,9 +548,9 @@ drmmode_crtc_scanout_allocate(xf86CrtcPtr crtc,
>  	rotate_pitch =
>  		RADEON_ALIGN(width, drmmode_get_pitch_align(pScrn,
> drmmode->cpp, 0))
>  		* drmmode->cpp;
> -	height = RADEON_ALIGN(height,
> drmmode_get_height_align(pScrn, 0));
> +	aligned_height = RADEON_ALIGN(height,
> drmmode_get_height_align(pScrn, 0));
>  	base_align = drmmode_get_base_align(pScrn, drmmode->cpp, 0);
> -	size = RADEON_ALIGN(rotate_pitch * height,
> RADEON_GPU_PAGE_SIZE);
> +	size = RADEON_ALIGN(rotate_pitch * aligned_height,
> RADEON_GPU_PAGE_SIZE);
>  
>  	scanout->bo = radeon_bo_open(drmmode->bufmgr, 0, size,
> base_align,
>  				     RADEON_GEM_DOMAIN_VRAM, 0);
> @@ -633,7 +634,6 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
>  	drmModeModeInfo kmode;
>  	int pitch;
>  	uint32_t tiling_flags = 0;
> -	int height;
>  
>  	if (info->allowColorTiling) {
>  		if (info->ChipFamily >= CHIP_FAMILY_R600)
> @@ -644,14 +644,13 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
>  
>  	pitch = RADEON_ALIGN(pScrn->displayWidth,
> drmmode_get_pitch_align(pScrn, info->pixel_bytes, tiling_flags)) *
>  		info->pixel_bytes;
> -	height = RADEON_ALIGN(pScrn->virtualY,
> drmmode_get_height_align(pScrn, tiling_flags));
>  	if (info->ChipFamily >= CHIP_FAMILY_R600) {
>  		pitch = info->front_surface.level[0].pitch_bytes;
>  	}
>  
>  	if (drmmode->fb_id == 0) {
>  		ret = drmModeAddFB(drmmode->fd,
> -				   pScrn->virtualX, height,
> +				   pScrn->virtualX, pScrn->virtualY,
>                                     pScrn->depth, pScrn
> ->bitsPerPixel,
>  				   pitch,
>  				   info->front_bo->handle,
> @@ -1789,6 +1788,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int
> width, int height)
>  	ScreenPtr   screen = xf86ScrnToScreen(scrn);
>  	uint32_t    old_fb_id;
>  	int	    i, pitch, old_width, old_height, old_pitch;
> +	int aligned_height;
>  	uint32_t screen_size;
>  	int cpp = info->pixel_bytes;
>  	struct radeon_bo *front_bo;
> @@ -1822,8 +1822,8 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int
> width, int height)
>  	}
>  
>  	pitch = RADEON_ALIGN(width, drmmode_get_pitch_align(scrn,
> cpp, tiling_flags)) * cpp;
> -	height = RADEON_ALIGN(height, drmmode_get_height_align(scrn,
> tiling_flags));
> -	screen_size = RADEON_ALIGN(pitch * height,
> RADEON_GPU_PAGE_SIZE);
> +	aligned_height = RADEON_ALIGN(height,
> drmmode_get_height_align(scrn, tiling_flags));
> +	screen_size = RADEON_ALIGN(pitch * aligned_height,
> RADEON_GPU_PAGE_SIZE);
>  	base_align = 4096;
>  	if (info->ChipFamily >= CHIP_FAMILY_R600) {
>  		memset(&surface, 0, sizeof(struct radeon_surface));
> @@ -2473,7 +2473,6 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>  	unsigned int pitch;
>  	int i;
>  	uint32_t tiling_flags = 0;
> -	int height;
>  	drmmode_flipdata_ptr flipdata;
>  	drmmode_flipevtcarrier_ptr flipcarrier = NULL;
>  	struct radeon_drm_queue_entry *drm_queue = NULL;
> @@ -2487,7 +2486,6 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>  
>  	pitch = RADEON_ALIGN(scrn->displayWidth,
> drmmode_get_pitch_align(scrn, info->pixel_bytes, tiling_flags)) *
>  		info->pixel_bytes;
> -	height = RADEON_ALIGN(scrn->virtualY,
> drmmode_get_height_align(scrn, tiling_flags));
>  	if (info->ChipFamily >= CHIP_FAMILY_R600 && info->surf_man)
> {
>  		pitch = info->front_surface.level[0].pitch_bytes;
>  	}
> @@ -2503,7 +2501,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>  	 * Create a new handle for the back buffer
>  	 */
>  	flipdata->old_fb_id = drmmode->fb_id;
> -	if (drmModeAddFB(drmmode->fd, scrn->virtualX, height,
> +	if (drmModeAddFB(drmmode->fd, scrn->virtualX, scrn
> ->virtualY,
>  			 scrn->depth, scrn->bitsPerPixel, pitch,
>  			 new_front_handle, &drmmode->fb_id))
>  		goto error;


More information about the xorg-devel mailing list