Re: xsrc/54167: Xorg wsfb server "Rotate" does not work on NetBSD8.0

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

The following reply was made to PR xsrc/54167; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: macallan%netbsd.org@localhost
Cc: gnats-bugs%netbsd.org@localhost, tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: xsrc/54167: Xorg wsfb server "Rotate" does not work on NetBSD8.0
Date: Sun, 22 Dec 2019 09:46:45 +0900

 Hi,
 
 > > The following patch (use pScrn->virtualX instead of hardware fbi_stride
 > > in "rotate" cases) fixes CW case on Zaurus SL-C1000 and SL-C3000.
 > > 
 > > Michael, could you confirm?
 > 
 > I'll dig up the wildcat and test. If I can't get to it please go ahead
 > and commit, I think your use case is far more common than mine.
 
 Thanks for your comment.
 
 I noticed "Rotate" and "ShadowFB" are valid only on depth >= 8,
 so I updated my patch to remove unnecessary cases.
 
 > > -		    /* apparently fb wants stride in pixels, not bytes */
 > 
 > There has to be a reason why I left that comment in there, I'll investigate.
 
 According to fbScreenInit() function in fbscreen.c,
 it seems to require "stride in pixel":
  https://cgit.freedesktop.org/xorg/xserver/tree/fb/fbscreen.c?h=xorg-server-1.10.6#n100
  https://cgit.freedesktop.org/xorg/xserver/tree/fb/fbscreen.c?h=xorg-server-1.10.6#n175
 
 I agree the "stride in pixels" is really weird, because
 stride is typically used to calculate VRAM size or address offsets
 (i.e. in bytes).  I also put a comment back for it.
 
 I'll commit the attached patch soon.
 
 ---
 Index: wsfb_driver.c
 ===================================================================
 RCS file: /cvsroot/xsrc/external/mit/xf86-video-wsfb/dist/src/wsfb_driver.c,v
 retrieving revision 1.35
 diff -u -p -d -r1.35 wsfb_driver.c
 --- wsfb_driver.c	23 Jul 2019 12:01:53 -0000	1.35
 +++ wsfb_driver.c	22 Dec 2019 00:34:28 -0000
 @@ -875,6 +875,7 @@ WsfbScreenInit(SCREEN_INIT_ARGS_DECL)
  	int ret, flags, ncolors;
  	int wsmode = WSDISPLAYIO_MODE_DUMBFB;
  	int wstype;
 +	int width;
  	size_t len;
  
  	TRACE_ENTER("WsfbScreenInit");
 @@ -979,7 +980,17 @@ WsfbScreenInit(SCREEN_INIT_ARGS_DECL)
  	fPtr->fbstart = fPtr->fbmem + fPtr->fbi.fbi_fboffset;
  
  	if (fPtr->shadowFB) {
 -		fPtr->shadow = calloc(1, fPtr->fbi.fbi_stride * pScrn->virtualY);
 +		if (fPtr->rotate) {
 +			/*
 +			 * Note Rotate and Shadow FB options are valid
 +			 * only on depth >= 8.
 +			 */
 +			len = pScrn->virtualX * pScrn->virtualY *
 +			    (pScrn->bitsPerPixel >> 3);
 +		} else {
 +			len = fPtr->fbi.fbi_stride * pScrn->virtualY;
 +		}
 +		fPtr->shadow = calloc(1, len);
  
  		if (!fPtr->shadow) {
  			xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 @@ -988,13 +999,29 @@ WsfbScreenInit(SCREEN_INIT_ARGS_DECL)
  		}
  	}
  
 +	/*
 +	 * fbScreenInit() seems to require "pixel width of frame buffer"
 +	 * but it is actually "stride in pixel" of frame buffer,
 +	 * per xorg/xserver/tree/fb/fbscreen.c.
 +	 */
 +	if (fPtr->rotate) {
 +		width = pScrn->displayWidth;
 +	} else {
 +		if (pScrn->bitsPerPixel > 8) {
 +			width =
 +			    fPtr->fbi.fbi_stride / (pScrn->bitsPerPixel >> 3);
 +		} else {
 +			width =
 +			    fPtr->fbi.fbi_stride * (8 / pScrn->bitsPerPixel);
 +		}
 +	}
  	switch (pScrn->bitsPerPixel) {
  	case 1:
  		ret = fbScreenInit(pScreen,
  		    fPtr->fbstart,
  		    pScrn->virtualX, pScrn->virtualY,
  		    pScrn->xDpi, pScrn->yDpi,
 -		    fPtr->fbi.fbi_stride * 8, pScrn->bitsPerPixel);
 +		    width, pScrn->bitsPerPixel);
  		break;
  	case 4:
  	case 8:
 @@ -1005,8 +1032,7 @@ WsfbScreenInit(SCREEN_INIT_ARGS_DECL)
  		    fPtr->shadowFB ? fPtr->shadow : fPtr->fbstart,
  		    pScrn->virtualX, pScrn->virtualY,
  		    pScrn->xDpi, pScrn->yDpi,
 -		    /* apparently fb wants stride in pixels, not bytes */
 -		    fPtr->fbi.fbi_stride / (pScrn->bitsPerPixel >> 3),
 +		    width,
  		    pScrn->bitsPerPixel);
  		break;
  	default:
 
 ---
 Izumi Tsutsui