From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Date: Mon, 27 Aug 2018 12:55:14 +0000 Subject: Re: [PATCH 2/3] mach64: fix image corruption due to reading accelerator registers Message-Id: <20180827125514.GE11867@sci.fi> List-Id: References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Mikulas Patocka Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Bartlomiej Zolnierkiewicz On Sat, Aug 25, 2018 at 03:51:52PM -0400, Mikulas Patocka wrote: > Reading the registers without waiting for engine idle returns > unpredictable values. These unpredictable values result in display > corruption - if atyfb_imageblit reads the content of DP_PIX_WIDTH with the > bit DP_HOST_TRIPLE_EN set (from previous invocation), the driver would > never ever clear the bit, resulting in display corruption. >=20 > We don't want to wait for idle because it would degrade performance, so > this patch modifies the driver so that it never reads accelerator > registers. >=20 > HOST_CNTL doesn't have to be read, we can just write it with > HOST_BYTE_ALIGN because no other part of the driver cares if > HOST_BYTE_ALIGN is set. >=20 > DP_PIX_WIDTH is written in the functions atyfb_copyarea and atyfb_fillrect > with the default value and in atyfb_imageblit with the value set according > to the source image data. >=20 > Signed-off-by: Mikulas Patocka > Cc: stable@vger.kernel.org >=20 > --- > drivers/video/fbdev/aty/mach64_accel.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) >=20 > Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D> --- linux-stable.orig/drivers/video/fbdev/aty/= mach64_accel.c 2018-08-24 19:51:34.000000000 +0200 > +++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-24 20:28:= 55.000000000 +0200 > @@ -127,7 +127,7 @@ void aty_init_engine(struct atyfb_par *p > =20 > /* set host attributes */ > wait_for_fifo(13, par); > - aty_st_le32(HOST_CNTL, 0, par); > + aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par); > =20 > /* set pattern attributes */ > aty_st_le32(PAT_REG0, 0, par); > @@ -233,7 +233,8 @@ void atyfb_copyarea(struct fb_info *info > rotation =3D rotation24bpp(dx, direction); > } > =20 > - wait_for_fifo(4, par); > + wait_for_fifo(5, par); > + aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par); > aty_st_le32(DP_SRC, FRGD_SRC_BLIT, par); > aty_st_le32(SRC_Y_X, (sx << 16) | sy, par); > aty_st_le32(SRC_HEIGHT1_WIDTH1, (width << 16) | area->height, par); > @@ -269,7 +270,8 @@ void atyfb_fillrect(struct fb_info *info > rotation =3D rotation24bpp(dx, DST_X_LEFT_TO_RIGHT); > } > =20 > - wait_for_fifo(3, par); > + wait_for_fifo(4, par); > + aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par); > aty_st_le32(DP_FRGD_CLR, color, par); > aty_st_le32(DP_SRC, > BKGD_SRC_BKGD_CLR | FRGD_SRC_FRGD_CLR | MONO_SRC_ONE, > @@ -284,7 +286,7 @@ void atyfb_imageblit(struct fb_info *inf > { > struct atyfb_par *par =3D (struct atyfb_par *) info->par; > u32 src_bytes, dx =3D image->dx, dy =3D image->dy, width =3D image->wid= th; > - u32 pix_width_save, pix_width, host_cntl, rotation =3D 0, src, mix; > + u32 pix_width, rotation =3D 0, src, mix; > =20 > if (par->asleep) > return; > @@ -296,8 +298,7 @@ void atyfb_imageblit(struct fb_info *inf > return; > } > =20 > - pix_width =3D pix_width_save =3D aty_ld_le32(DP_PIX_WIDTH, par); > - host_cntl =3D aty_ld_le32(HOST_CNTL, par) | HOST_BYTE_ALIGN; > + pix_width =3D par->crtc.dp_pix_width; > =20 > switch (image->depth) { > case 1: > @@ -370,12 +371,11 @@ void atyfb_imageblit(struct fb_info *inf > mix =3D FRGD_MIX_D_XOR_S | BKGD_MIX_D; > } > =20 > - wait_for_fifo(6, par); > - aty_st_le32(DP_WRITE_MASK, 0xFFFFFFFF, par); Looks like init_engine() sets this one for us. So dropping should be ok. > + wait_for_fifo(5, par); > aty_st_le32(DP_PIX_WIDTH, pix_width, par); > aty_st_le32(DP_MIX, mix, par); > aty_st_le32(DP_SRC, src, par); > - aty_st_le32(HOST_CNTL, host_cntl, par); > + aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par); Presumably we could drop this as well since it never changes. Either way Reviewed-by: Ville Syrj=E4l=E4 > aty_st_le32(DST_CNTL, DST_Y_TOP_TO_BOTTOM | DST_X_LEFT_TO_RIGHT | rotat= ion, par); > =20 > draw_rect(dx, dy, width, image->height, par); > @@ -424,8 +424,4 @@ void atyfb_imageblit(struct fb_info *inf > aty_st_le32(HOST_DATA0, get_unaligned_le32(pbitmap), par); > } > } > - > - /* restore pix_width */ > - wait_for_fifo(1, par); > - aty_st_le32(DP_PIX_WIDTH, pix_width_save, par); > } --=20 Ville Syrj=E4l=E4 syrjala@sci.fi http://www.sci.fi/~syrjala/ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/3] mach64: fix image corruption due to reading accelerator registers Date: Mon, 27 Aug 2018 15:55:14 +0300 Message-ID: <20180827125514.GE11867@sci.fi> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from welho-filter3.welho.com (welho-filter3.welho.com [83.102.41.25]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8DEC96E250 for ; Mon, 27 Aug 2018 12:55:24 +0000 (UTC) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Mikulas Patocka Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Bartlomiej Zolnierkiewicz List-Id: dri-devel@lists.freedesktop.org T24gU2F0LCBBdWcgMjUsIDIwMTggYXQgMDM6NTE6NTJQTSAtMDQwMCwgTWlrdWxhcyBQYXRvY2th IHdyb3RlOgo+IFJlYWRpbmcgdGhlIHJlZ2lzdGVycyB3aXRob3V0IHdhaXRpbmcgZm9yIGVuZ2lu ZSBpZGxlIHJldHVybnMKPiB1bnByZWRpY3RhYmxlIHZhbHVlcy4gVGhlc2UgdW5wcmVkaWN0YWJs ZSB2YWx1ZXMgcmVzdWx0IGluIGRpc3BsYXkKPiBjb3JydXB0aW9uIC0gaWYgYXR5ZmJfaW1hZ2Vi bGl0IHJlYWRzIHRoZSBjb250ZW50IG9mIERQX1BJWF9XSURUSCB3aXRoIHRoZQo+IGJpdCBEUF9I T1NUX1RSSVBMRV9FTiBzZXQgKGZyb20gcHJldmlvdXMgaW52b2NhdGlvbiksIHRoZSBkcml2ZXIg d291bGQKPiBuZXZlciBldmVyIGNsZWFyIHRoZSBiaXQsIHJlc3VsdGluZyBpbiBkaXNwbGF5IGNv cnJ1cHRpb24uCj4gCj4gV2UgZG9uJ3Qgd2FudCB0byB3YWl0IGZvciBpZGxlIGJlY2F1c2UgaXQg d291bGQgZGVncmFkZSBwZXJmb3JtYW5jZSwgc28KPiB0aGlzIHBhdGNoIG1vZGlmaWVzIHRoZSBk cml2ZXIgc28gdGhhdCBpdCBuZXZlciByZWFkcyBhY2NlbGVyYXRvcgo+IHJlZ2lzdGVycy4KPiAK PiBIT1NUX0NOVEwgZG9lc24ndCBoYXZlIHRvIGJlIHJlYWQsIHdlIGNhbiBqdXN0IHdyaXRlIGl0 IHdpdGgKPiBIT1NUX0JZVEVfQUxJR04gYmVjYXVzZSBubyBvdGhlciBwYXJ0IG9mIHRoZSBkcml2 ZXIgY2FyZXMgaWYKPiBIT1NUX0JZVEVfQUxJR04gaXMgc2V0Lgo+IAo+IERQX1BJWF9XSURUSCBp cyB3cml0dGVuIGluIHRoZSBmdW5jdGlvbnMgYXR5ZmJfY29weWFyZWEgYW5kIGF0eWZiX2ZpbGxy ZWN0Cj4gd2l0aCB0aGUgZGVmYXVsdCB2YWx1ZSBhbmQgaW4gYXR5ZmJfaW1hZ2VibGl0IHdpdGgg dGhlIHZhbHVlIHNldCBhY2NvcmRpbmcKPiB0byB0aGUgc291cmNlIGltYWdlIGRhdGEuCj4gCj4g U2lnbmVkLW9mZi1ieTogTWlrdWxhcyBQYXRvY2thIDxtcGF0b2NrYUByZWRoYXQuY29tPgo+IENj OiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4gCj4gLS0tCj4gIGRyaXZlcnMvdmlkZW8vZmJkZXYv YXR5L21hY2g2NF9hY2NlbC5jIHwgICAyMiArKysrKysrKystLS0tLS0tLS0tLS0tCj4gIDEgZmls ZSBjaGFuZ2VkLCA5IGluc2VydGlvbnMoKyksIDEzIGRlbGV0aW9ucygtKQo+IAo+IEluZGV4OiBs aW51eC1zdGFibGUvZHJpdmVycy92aWRlby9mYmRldi9hdHkvbWFjaDY0X2FjY2VsLmMKPiA9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Cj4gLS0tIGxpbnV4LXN0YWJsZS5vcmlnL2RyaXZlcnMvdmlkZW8vZmJkZXYvYXR5L21h Y2g2NF9hY2NlbC5jCTIwMTgtMDgtMjQgMTk6NTE6MzQuMDAwMDAwMDAwICswMjAwCj4gKysrIGxp bnV4LXN0YWJsZS9kcml2ZXJzL3ZpZGVvL2ZiZGV2L2F0eS9tYWNoNjRfYWNjZWwuYwkyMDE4LTA4 LTI0IDIwOjI4OjU1LjAwMDAwMDAwMCArMDIwMAo+IEBAIC0xMjcsNyArMTI3LDcgQEAgdm9pZCBh dHlfaW5pdF9lbmdpbmUoc3RydWN0IGF0eWZiX3BhciAqcAo+ICAKPiAgCS8qIHNldCBob3N0IGF0 dHJpYnV0ZXMgKi8KPiAgCXdhaXRfZm9yX2ZpZm8oMTMsIHBhcik7Cj4gLQlhdHlfc3RfbGUzMihI T1NUX0NOVEwsIDAsIHBhcik7Cj4gKwlhdHlfc3RfbGUzMihIT1NUX0NOVEwsIEhPU1RfQllURV9B TElHTiwgcGFyKTsKPiAgCj4gIAkvKiBzZXQgcGF0dGVybiBhdHRyaWJ1dGVzICovCj4gIAlhdHlf c3RfbGUzMihQQVRfUkVHMCwgMCwgcGFyKTsKPiBAQCAtMjMzLDcgKzIzMyw4IEBAIHZvaWQgYXR5 ZmJfY29weWFyZWEoc3RydWN0IGZiX2luZm8gKmluZm8KPiAgCQlyb3RhdGlvbiA9IHJvdGF0aW9u MjRicHAoZHgsIGRpcmVjdGlvbik7Cj4gIAl9Cj4gIAo+IC0Jd2FpdF9mb3JfZmlmbyg0LCBwYXIp Owo+ICsJd2FpdF9mb3JfZmlmbyg1LCBwYXIpOwo+ICsJYXR5X3N0X2xlMzIoRFBfUElYX1dJRFRI LCBwYXItPmNydGMuZHBfcGl4X3dpZHRoLCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIoRFBfU1JDLCBG UkdEX1NSQ19CTElULCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIoU1JDX1lfWCwgKHN4IDw8IDE2KSB8 IHN5LCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIoU1JDX0hFSUdIVDFfV0lEVEgxLCAod2lkdGggPDwg MTYpIHwgYXJlYS0+aGVpZ2h0LCBwYXIpOwo+IEBAIC0yNjksNyArMjcwLDggQEAgdm9pZCBhdHlm Yl9maWxscmVjdChzdHJ1Y3QgZmJfaW5mbyAqaW5mbwo+ICAJCXJvdGF0aW9uID0gcm90YXRpb24y NGJwcChkeCwgRFNUX1hfTEVGVF9UT19SSUdIVCk7Cj4gIAl9Cj4gIAo+IC0Jd2FpdF9mb3JfZmlm bygzLCBwYXIpOwo+ICsJd2FpdF9mb3JfZmlmbyg0LCBwYXIpOwo+ICsJYXR5X3N0X2xlMzIoRFBf UElYX1dJRFRILCBwYXItPmNydGMuZHBfcGl4X3dpZHRoLCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIo RFBfRlJHRF9DTFIsIGNvbG9yLCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIoRFBfU1JDLAo+ICAJCSAg ICBCS0dEX1NSQ19CS0dEX0NMUiB8IEZSR0RfU1JDX0ZSR0RfQ0xSIHwgTU9OT19TUkNfT05FLAo+ IEBAIC0yODQsNyArMjg2LDcgQEAgdm9pZCBhdHlmYl9pbWFnZWJsaXQoc3RydWN0IGZiX2luZm8g KmluZgo+ICB7Cj4gIAlzdHJ1Y3QgYXR5ZmJfcGFyICpwYXIgPSAoc3RydWN0IGF0eWZiX3BhciAq KSBpbmZvLT5wYXI7Cj4gIAl1MzIgc3JjX2J5dGVzLCBkeCA9IGltYWdlLT5keCwgZHkgPSBpbWFn ZS0+ZHksIHdpZHRoID0gaW1hZ2UtPndpZHRoOwo+IC0JdTMyIHBpeF93aWR0aF9zYXZlLCBwaXhf d2lkdGgsIGhvc3RfY250bCwgcm90YXRpb24gPSAwLCBzcmMsIG1peDsKPiArCXUzMiBwaXhfd2lk dGgsIHJvdGF0aW9uID0gMCwgc3JjLCBtaXg7Cj4gIAo+ICAJaWYgKHBhci0+YXNsZWVwKQo+ICAJ CXJldHVybjsKPiBAQCAtMjk2LDggKzI5OCw3IEBAIHZvaWQgYXR5ZmJfaW1hZ2VibGl0KHN0cnVj dCBmYl9pbmZvICppbmYKPiAgCQlyZXR1cm47Cj4gIAl9Cj4gIAo+IC0JcGl4X3dpZHRoID0gcGl4 X3dpZHRoX3NhdmUgPSBhdHlfbGRfbGUzMihEUF9QSVhfV0lEVEgsIHBhcik7Cj4gLQlob3N0X2Nu dGwgPSBhdHlfbGRfbGUzMihIT1NUX0NOVEwsIHBhcikgfCBIT1NUX0JZVEVfQUxJR047Cj4gKwlw aXhfd2lkdGggPSBwYXItPmNydGMuZHBfcGl4X3dpZHRoOwo+ICAKPiAgCXN3aXRjaCAoaW1hZ2Ut PmRlcHRoKSB7Cj4gIAljYXNlIDE6Cj4gQEAgLTM3MCwxMiArMzcxLDExIEBAIHZvaWQgYXR5ZmJf aW1hZ2VibGl0KHN0cnVjdCBmYl9pbmZvICppbmYKPiAgCQltaXggPSBGUkdEX01JWF9EX1hPUl9T IHwgQktHRF9NSVhfRDsKPiAgCX0KPiAgCj4gLQl3YWl0X2Zvcl9maWZvKDYsIHBhcik7Cj4gLQlh dHlfc3RfbGUzMihEUF9XUklURV9NQVNLLCAweEZGRkZGRkZGLCBwYXIpOwoKTG9va3MgbGlrZSBp bml0X2VuZ2luZSgpIHNldHMgdGhpcyBvbmUgZm9yIHVzLiBTbyBkcm9wcGluZyBzaG91bGQgYmUg b2suCgo+ICsJd2FpdF9mb3JfZmlmbyg1LCBwYXIpOwo+ICAJYXR5X3N0X2xlMzIoRFBfUElYX1dJ RFRILCBwaXhfd2lkdGgsIHBhcik7Cj4gIAlhdHlfc3RfbGUzMihEUF9NSVgsIG1peCwgcGFyKTsK PiAgCWF0eV9zdF9sZTMyKERQX1NSQywgc3JjLCBwYXIpOwo+IC0JYXR5X3N0X2xlMzIoSE9TVF9D TlRMLCBob3N0X2NudGwsIHBhcik7Cj4gKwlhdHlfc3RfbGUzMihIT1NUX0NOVEwsIEhPU1RfQllU RV9BTElHTiwgcGFyKTsKClByZXN1bWFibHkgd2UgY291bGQgZHJvcCB0aGlzIGFzIHdlbGwgc2lu Y2UgaXQgbmV2ZXIgY2hhbmdlcy4KCkVpdGhlciB3YXkKUmV2aWV3ZWQtYnk6IFZpbGxlIFN5cmrD pGzDpCA8c3lyamFsYUBzY2kuZmk+Cgo+ICAJYXR5X3N0X2xlMzIoRFNUX0NOVEwsIERTVF9ZX1RP UF9UT19CT1RUT00gfCBEU1RfWF9MRUZUX1RPX1JJR0hUIHwgcm90YXRpb24sIHBhcik7Cj4gIAo+ ICAJZHJhd19yZWN0KGR4LCBkeSwgd2lkdGgsIGltYWdlLT5oZWlnaHQsIHBhcik7Cj4gQEAgLTQy NCw4ICs0MjQsNCBAQCB2b2lkIGF0eWZiX2ltYWdlYmxpdChzdHJ1Y3QgZmJfaW5mbyAqaW5mCj4g IAkJCWF0eV9zdF9sZTMyKEhPU1RfREFUQTAsIGdldF91bmFsaWduZWRfbGUzMihwYml0bWFwKSwg cGFyKTsKPiAgCQl9Cj4gIAl9Cj4gLQo+IC0JLyogcmVzdG9yZSBwaXhfd2lkdGggKi8KPiAtCXdh aXRfZm9yX2ZpZm8oMSwgcGFyKTsKPiAtCWF0eV9zdF9sZTMyKERQX1BJWF9XSURUSCwgcGl4X3dp ZHRoX3NhdmUsIHBhcik7Cj4gIH0KCi0tIApWaWxsZSBTeXJqw6Rsw6QKc3lyamFsYUBzY2kuZmkK aHR0cDovL3d3dy5zY2kuZmkvfnN5cmphbGEvCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3Rz LmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2RyaS1kZXZlbAo=